Skip to content

Commit 2b4364b

Browse files
allevatoallevato
authored
Merge pull request #208 from allevato/one-var-per-line-fixes
Rewrite and fix `OneVariableDeclarationPerLine`.
2 parents 470b634 + 6104398 commit 2b4364b

File tree

2 files changed

+379
-116
lines changed

2 files changed

+379
-116
lines changed

‎Sources/SwiftFormatRules/OneVariableDeclarationPerLine.swift

+165-65
Original file line numberDiff line numberDiff line change
@@ -13,96 +13,196 @@
1313
import SwiftFormatCore
1414
import SwiftSyntax
1515

16-
/// Each variable declaration, with the exception of tuple destructuring, should declare 1 variable.
16+
/// Each variable declaration, with the exception of tuple destructuring, should
17+
/// declare 1 variable.
1718
///
18-
/// Lint: If a variable declaration declares multiple variables, a lint error is raised.
19+
/// Lint: If a variable declaration declares multiple variables, a lint error is
20+
/// raised.
1921
///
20-
/// Format: If a variable declaration declares multiple variables, it will be split into multiple
21-
/// declarations, each declaring one of the variables.
22+
/// Format: If a variable declaration declares multiple variables, it will be
23+
/// split into multiple declarations, each declaring one of the variables, as
24+
/// long as the result would still be syntactically valid.
2225
public final class OneVariableDeclarationPerLine: SyntaxFormatRule {
23-
private func splitVariableDecls(
24-
_ items: CodeBlockItemListSyntax
25-
) -> CodeBlockItemListSyntax? {
26-
// If we're here, then there's at least one VariableDeclSyntax that
27-
// needs to be split.
28-
var needsWork = false
29-
for codeBlockItem in items {
30-
if let varDecl = codeBlockItem.item.as(VariableDeclSyntax.self), varDecl.bindings.count > 1 {
31-
needsWork = true
32-
}
26+
public override func visit(_ node: CodeBlockItemListSyntax) -> Syntax {
27+
guard node.contains(where: codeBlockItemHasMultipleVariableBindings) else {
28+
return super.visit(node)
3329
}
34-
if !needsWork { return nil }
3530

3631
var newItems = [CodeBlockItemSyntax]()
37-
for codeBlockItem in items {
38-
// If we're not looking at a VariableDecl with more than 1 binding, visit the item and
39-
// skip it.
32+
for codeBlockItem in node {
4033
guard let varDecl = codeBlockItem.item.as(VariableDeclSyntax.self),
4134
varDecl.bindings.count > 1
4235
else {
43-
newItems.append(codeBlockItem)
36+
// It's not a variable declaration with multiple bindings, so visit it
37+
// recursively (in case it's something that contains bindings that need
38+
// to be split) but otherwise do nothing.
39+
let newItem = super.visit(codeBlockItem).as(CodeBlockItemSyntax.self)!
40+
newItems.append(newItem)
4441
continue
4542
}
4643

4744
diagnose(.onlyOneVariableDeclaration, on: varDecl)
4845

49-
// The first binding corresponds to the original `var`/`let`
50-
// declaration, so it should not have its trivia replaced.
51-
var isFirst = true
52-
// The last binding only has type annotation when multiple
53-
// declarations in one line, so make a new binding with its type
54-
let typeAnnotation = varDecl.bindings.last?.typeAnnotation
55-
for binding in varDecl.bindings {
56-
var newBinding = binding.withTrailingComma(nil)
57-
if typeAnnotation != nil && binding.typeAnnotation == nil {
58-
newBinding = newBinding.withTypeAnnotation(typeAnnotation)
59-
}
60-
let newDecl = varDecl.withBindings(
61-
SyntaxFactory.makePatternBindingList([newBinding]))
62-
var finalDecl = Syntax(newDecl)
63-
// Only add a newline if this is a brand new binding.
64-
if !isFirst {
65-
let firstTok = newDecl.firstToken
66-
let origLeading = firstTok?.leadingTrivia.withoutNewlines() ?? []
67-
finalDecl = replaceTrivia(
68-
on: finalDecl,
69-
token: newDecl.firstToken,
70-
leadingTrivia: .newlines(1) + origLeading
71-
)
72-
}
73-
let newCodeBlockItem = codeBlockItem.withItem(finalDecl)
74-
newItems.append(newCodeBlockItem)
75-
isFirst = false
46+
// Visit the decl recursively to make sure nested code block items in the
47+
// bindings (for example, an initializer expression that contains a
48+
// closure expression) are transformed first before we rewrite the decl
49+
// itself.
50+
let visitedDecl = super.visit(varDecl).as(VariableDeclSyntax.self)!
51+
var splitter = VariableDeclSplitter {
52+
SyntaxFactory.makeCodeBlockItem(
53+
item: Syntax($0),
54+
semicolon: nil,
55+
errorTokens: nil)
7656
}
57+
newItems.append(contentsOf: splitter.nodes(bySplitting: visitedDecl))
7758
}
78-
return SyntaxFactory.makeCodeBlockItemList(newItems)
79-
}
8059

81-
public override func visit(_ node: CodeBlockSyntax) -> Syntax {
82-
guard let newStmts = splitVariableDecls(node.statements) else {
83-
return super.visit(node)
84-
}
85-
return Syntax(node.withStatements(newStmts))
60+
return Syntax(SyntaxFactory.makeCodeBlockItemList(newItems))
8661
}
8762

88-
public override func visit(_ node: ClosureExprSyntax) -> ExprSyntax {
89-
guard let newStmts = splitVariableDecls(node.statements) else {
90-
return super.visit(node)
63+
/// Returns true if the given `CodeBlockItemSyntax` contains a `let` or `var`
64+
/// declaration with multiple bindings.
65+
private func codeBlockItemHasMultipleVariableBindings(
66+
_ node: CodeBlockItemSyntax
67+
) -> Bool {
68+
if let varDecl = node.item.as(VariableDeclSyntax.self),
69+
varDecl.bindings.count > 1
70+
{
71+
return true
9172
}
92-
return ExprSyntax(node.withStatements(newStmts))
93-
}
94-
95-
public override func visit(_ node: SourceFileSyntax) -> Syntax {
96-
guard let newStmts = splitVariableDecls(node.statements) else {
97-
return super.visit(node)
98-
}
99-
return Syntax(node.withStatements(newStmts))
73+
return false
10074
}
10175
}
10276

10377
extension Diagnostic.Message {
10478
public static let onlyOneVariableDeclaration = Diagnostic.Message(
10579
.warning,
106-
"split variable binding into multiple declarations"
80+
"split this variable declaration to have one variable per declaration"
10781
)
10882
}
83+
84+
/// Splits a variable declaration with multiple bindings into individual
85+
/// declarations.
86+
///
87+
/// Swift's grammar allows each identifier in a variable declaration to have a
88+
/// type annotation, an initializer expression, both, or neither. Stricter
89+
/// checks occur after parsing, however; a lone identifier may only be followed
90+
/// by zero or more other lone identifiers and then an identifier with *only* a
91+
/// type annotation (and the type annotation is applied to all of them). If we
92+
/// have something else, we should handle them gracefully (i.e., not destroy
93+
/// them) but we don't need to try to fix them since they didn't compile in the
94+
/// first place so we can't guess what the user intended.
95+
///
96+
/// So, this algorithm works by scanning forward and collecting lone identifiers
97+
/// in a queue until we reach a binding that has an initializer or a type
98+
/// annotation. If we see a type annotation (without an initializer), we can
99+
/// create individual variable declarations for each entry in the queue by
100+
/// projecting that type annotation onto each of them. If we reach a case that
101+
/// isn't valid, we just flush the queue contents as a single declaration, to
102+
/// effectively preserve what the user originally had.
103+
private struct VariableDeclSplitter<Node: SyntaxProtocol> {
104+
/// A function that takes a `VariableDeclSyntax` and returns a new node, such
105+
/// as a `CodeBlockItemSyntax`, that wraps it.
106+
private let generator: (VariableDeclSyntax) -> Node
107+
108+
/// Bindings that have been collected so far.
109+
private var bindingQueue = [PatternBindingSyntax]()
110+
111+
/// The variable declaration being split.
112+
///
113+
/// This is an implicitly-unwrapped optional because it isn't initialized
114+
/// until `nodes(bySplitting:)` is called.
115+
private var varDecl: VariableDeclSyntax!
116+
117+
/// The list of nodes generated by splitting the variable declaration into
118+
/// individual bindings.
119+
private var nodes = [Node]()
120+
121+
/// Tracks whether the trivia of `varDecl` has already been fixed up for nodes
122+
/// after the first.
123+
private var fixedUpTrivia = false
124+
125+
/// Creates a new variable declaration splitter.
126+
///
127+
/// - Parameter generator: A function that takes a `VariableDeclSyntax` and
128+
/// returns a new node, such as a `CodeBlockItemSyntax`, that wraps it.
129+
init(generator: @escaping (VariableDeclSyntax) -> Node) {
130+
self.generator = generator
131+
}
132+
133+
/// Returns an array of nodes generated by splitting the given variable
134+
/// declaration into individual bindings.
135+
mutating func nodes(bySplitting varDecl: VariableDeclSyntax) -> [Node] {
136+
self.varDecl = varDecl
137+
self.nodes = []
138+
139+
for binding in varDecl.bindings {
140+
if binding.initializer != nil {
141+
// If this is the only initializer in the queue so far, that's ok. If
142+
// it's an initializer following other un-flushed lone identifier
143+
// bindings, that's not valid Swift. But in either case, we'll flush
144+
// them as a single decl.
145+
bindingQueue.append(binding.withTrailingComma(nil))
146+
flushRemaining()
147+
} else if let typeAnnotation = binding.typeAnnotation {
148+
bindingQueue.append(binding)
149+
flushIndividually(typeAnnotation: typeAnnotation)
150+
} else {
151+
bindingQueue.append(binding)
152+
}
153+
}
154+
flushRemaining()
155+
156+
return nodes
157+
}
158+
159+
/// Replaces the original variable declaration with a copy of itself with
160+
/// updates trivia appropriate for subsequent declarations inserted by the
161+
/// rule.
162+
private mutating func fixOriginalVarDeclTrivia() {
163+
guard !fixedUpTrivia else { return }
164+
165+
// We intentionally don't try to infer the indentation for subsequent
166+
// lines because the pretty printer will re-indent them correctly; we just
167+
// need to ensure that a newline is inserted before new decls.
168+
varDecl = replaceTrivia(
169+
on: varDecl, token: varDecl.firstToken, leadingTrivia: .newlines(1))
170+
fixedUpTrivia = true
171+
}
172+
173+
/// Flushes any remaining bindings as a single variable declaration.
174+
private mutating func flushRemaining() {
175+
guard !bindingQueue.isEmpty else { return }
176+
177+
let newDecl =
178+
varDecl.withBindings(SyntaxFactory.makePatternBindingList(bindingQueue))
179+
nodes.append(generator(newDecl))
180+
181+
fixOriginalVarDeclTrivia()
182+
183+
bindingQueue = []
184+
}
185+
186+
/// Flushes any remaining bindings as individual variable declarations where
187+
/// each has the given type annotation.
188+
private mutating func flushIndividually(
189+
typeAnnotation: TypeAnnotationSyntax
190+
) {
191+
assert(!bindingQueue.isEmpty)
192+
193+
for binding in bindingQueue {
194+
assert(binding.initializer == nil)
195+
196+
let newBinding =
197+
binding.withTrailingComma(nil).withTypeAnnotation(typeAnnotation)
198+
let newDecl =
199+
varDecl.withBindings(SyntaxFactory.makePatternBindingList([newBinding]))
200+
nodes.append(generator(newDecl))
201+
202+
fixOriginalVarDeclTrivia()
203+
}
204+
205+
bindingQueue = []
206+
}
207+
}
208+

0 commit comments

Comments
 (0)