Skip to content

Commit 1731e82

Browse files
kimdvkimdv
committed
Add newline fix-it after consecutive declarations
1 parent 3d35a80 commit 1731e82

File tree

7 files changed

+359
-29
lines changed

7 files changed

+359
-29
lines changed

‎Sources/SwiftBasicFormat/Trivia+FormatExtensions.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ extension Trivia {
4545

4646
/// Returns the indentation of the last trivia piece in this trivia that is
4747
/// not a whitespace.
48-
func indentation(isOnNewline: Bool) -> Trivia? {
48+
/// - Parameter isOnNewline: Specifies if the character before this trivia is a newline character, i.e. if this trivia already starts on a new line.
49+
/// - Returns: An optional ``Trivia`` with indentation of the last trivia piece.
50+
public func indentation(isOnNewline: Bool) -> Trivia? {
4951
let lastNonWhitespaceTriviaPieceIndex = self.pieces.lastIndex(where: { !$0.isWhitespace }) ?? self.pieces.endIndex
5052
let piecesBeforeLastNonWhitespace = self.pieces[..<lastNonWhitespaceTriviaPieceIndex]
5153
let indentation: ArraySlice<TriviaPiece>

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,31 @@ fileprivate func getTokens(between first: TokenSyntax, and second: TokenSyntax)
5151
}
5252

5353
fileprivate extension TokenSyntax {
54+
/// The indentation of this token
55+
///
56+
/// In contrast to `indentation`, this does not walk to previous tokens to
57+
/// find the indentation of the line this token occurs on.
58+
private var indentation: Trivia? {
59+
let previous = self.previousToken(viewMode: .sourceAccurate)
60+
return ((previous?.trailingTrivia ?? []) + leadingTrivia).indentation(isOnNewline: false)
61+
}
62+
63+
/// Returns the indentation of the line this token occurs on
64+
var indentationOfLine: Trivia {
65+
var token: TokenSyntax = self
66+
if let indentation = token.indentation {
67+
return indentation
68+
}
69+
while let previous = token.previousToken(viewMode: .sourceAccurate) {
70+
token = previous
71+
if let indentation = token.indentation {
72+
return indentation
73+
}
74+
}
75+
76+
return []
77+
}
78+
5479
/// Assuming this token is a `poundAvailableKeyword` or `poundUnavailableKeyword`
5580
/// returns the opposite keyword.
5681
var negatedAvailabilityKeyword: TokenSyntax {
@@ -1155,13 +1180,28 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
11551180
// Only diagnose the missing semicolon if the decl doesn't contain any errors.
11561181
// If the decl contains errors, the root cause is most likely something different and not the missing semicolon.
11571182
let position = semicolon.previousToken(viewMode: .sourceAccurate)?.endPositionBeforeTrailingTrivia
1183+
var fixIts: [FixIt] = [
1184+
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
1185+
]
1186+
if let firstToken = node.firstToken(viewMode: .sourceAccurate),
1187+
let lastToken = node.lastToken(viewMode: .sourceAccurate)
1188+
{
1189+
fixIts.insert(
1190+
FixIt(
1191+
message: .insertNewline,
1192+
changes: [
1193+
.replaceTrailingTrivia(token: lastToken, newTrivia: lastToken.trailingTrivia + .newlines(1) + firstToken.indentationOfLine)
1194+
]
1195+
),
1196+
at: 0
1197+
)
1198+
}
1199+
11581200
addDiagnostic(
11591201
semicolon,
11601202
position: position,
11611203
.consecutiveDeclarationsOnSameLine,
1162-
fixIts: [
1163-
FixIt(message: .insertSemicolon, changes: .makePresent(semicolon))
1164-
],
1204+
fixIts: fixIts,
11651205
handledNodes: [semicolon.id]
11661206
)
11671207
} else {

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ extension DiagnosticMessage where Self == StaticParserError {
106106
.init("'class' constraint can only appear on protocol declarations")
107107
}
108108
public static var consecutiveDeclarationsOnSameLine: Self {
109-
.init("consecutive declarations on a line must be separated by ';'")
109+
.init("consecutive declarations on a line must be separated by newline or ';'")
110110
}
111111
public static var consecutiveStatementsOnSameLine: Self {
112112
.init("consecutive statements on a line must be separated by ';'")

‎Tests/SwiftParserTest/Assertions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ class FixItApplier: SyntaxRewriter {
275275
var changes: [FixIt.Change]
276276

277277
init(diagnostics: [Diagnostic], withMessages messages: [String]?) {
278-
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
278+
let messages = messages ?? diagnostics.compactMap { $0.fixIts.first?.message.message }
279279

280280
self.changes =
281281
diagnostics

‎Tests/SwiftParserTest/translated/ConsecutiveStatementsTests.swift

Lines changed: 134 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,16 @@ final class ConsecutiveStatementsTests: XCTestCase {
102102
}
103103
""",
104104
diagnostics: [
105-
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
106-
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
105+
DiagnosticSpec(
106+
locationMarker: "1️⃣",
107+
message: "consecutive declarations on a line must be separated by newline or ';'",
108+
fixIts: ["insert newline", "insert ';'"]
109+
),
110+
DiagnosticSpec(
111+
locationMarker: "2️⃣",
112+
message: "consecutive declarations on a line must be separated by newline or ';'",
113+
fixIts: ["insert newline", "insert ';'"]
114+
),
107115
DiagnosticSpec(locationMarker: "3️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
108116
DiagnosticSpec(locationMarker: "4️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
109117
DiagnosticSpec(locationMarker: "5️⃣", message: "consecutive statements on a line must be separated by ';'", fixIts: ["insert ';'"]),
@@ -129,7 +137,7 @@ final class ConsecutiveStatementsTests: XCTestCase {
129137
)
130138
}
131139

132-
func testConsecutiveStatements4() {
140+
func testConsecutiveStatements4a() {
133141
assertParse(
134142
"""
135143
class C {
@@ -142,8 +150,39 @@ final class ConsecutiveStatementsTests: XCTestCase {
142150
}
143151
""",
144152
diagnostics: [
145-
DiagnosticSpec(message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"])
153+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
146154
],
155+
applyFixIts: ["insert newline"],
156+
fixedSource: """
157+
class C {
158+
// In a sequence of declarations.
159+
var a, b : Int
160+
func d() -> Int {}
161+
init() {
162+
a = 0
163+
b = 0
164+
}
165+
}
166+
"""
167+
)
168+
}
169+
170+
func testConsecutiveStatements4b() {
171+
assertParse(
172+
"""
173+
class C {
174+
// In a sequence of declarations.
175+
var a, b : Int1️⃣ func d() -> Int {}
176+
init() {
177+
a = 0
178+
b = 0
179+
}
180+
}
181+
""",
182+
diagnostics: [
183+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
184+
],
185+
applyFixIts: ["insert ';'"],
147186
fixedSource: """
148187
class C {
149188
// In a sequence of declarations.
@@ -157,16 +196,37 @@ final class ConsecutiveStatementsTests: XCTestCase {
157196
)
158197
}
159198

160-
func testConsecutiveStatements5() {
199+
func testConsecutiveStatements5a() {
200+
assertParse(
201+
"""
202+
protocol P {
203+
func a()1️⃣ func b()
204+
}
205+
""",
206+
diagnostics: [
207+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
208+
],
209+
applyFixIts: ["insert newline"],
210+
fixedSource: """
211+
protocol P {
212+
func a()
213+
func b()
214+
}
215+
"""
216+
)
217+
}
218+
219+
func testConsecutiveStatements5b() {
161220
assertParse(
162221
"""
163222
protocol P {
164223
func a()1️⃣ func b()
165224
}
166225
""",
167226
diagnostics: [
168-
DiagnosticSpec(message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"])
227+
DiagnosticSpec(message: "consecutive declarations on a line must be separated by newline or ';'", fixIts: ["insert newline", "insert ';'"])
169228
],
229+
applyFixIts: ["insert ';'"],
170230
fixedSource: """
171231
protocol P {
172232
func a(); func b()
@@ -175,7 +235,39 @@ final class ConsecutiveStatementsTests: XCTestCase {
175235
)
176236
}
177237

178-
func testConsecutiveStatements6() {
238+
func testConsecutiveStatements6a() {
239+
assertParse(
240+
"""
241+
enum Color {
242+
case Red1️⃣ case Blue
243+
func a() {}2️⃣ func b() {}
244+
}
245+
""",
246+
diagnostics: [
247+
DiagnosticSpec(
248+
locationMarker: "1️⃣",
249+
message: "consecutive declarations on a line must be separated by newline or ';'",
250+
fixIts: ["insert newline", "insert ';'"]
251+
),
252+
DiagnosticSpec(
253+
locationMarker: "2️⃣",
254+
message: "consecutive declarations on a line must be separated by newline or ';'",
255+
fixIts: ["insert newline", "insert ';'"]
256+
),
257+
],
258+
applyFixIts: ["insert newline"],
259+
fixedSource: """
260+
enum Color {
261+
case Red
262+
case Blue
263+
func a() {}
264+
func b() {}
265+
}
266+
"""
267+
)
268+
}
269+
270+
func testConsecutiveStatements6b() {
179271
assertParse(
180272
"""
181273
enum Color {
@@ -184,9 +276,18 @@ final class ConsecutiveStatementsTests: XCTestCase {
184276
}
185277
""",
186278
diagnostics: [
187-
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
188-
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'", fixIts: ["insert ';'"]),
279+
DiagnosticSpec(
280+
locationMarker: "1️⃣",
281+
message: "consecutive declarations on a line must be separated by newline or ';'",
282+
fixIts: ["insert newline", "insert ';'"]
283+
),
284+
DiagnosticSpec(
285+
locationMarker: "2️⃣",
286+
message: "consecutive declarations on a line must be separated by newline or ';'",
287+
fixIts: ["insert newline", "insert ';'"]
288+
),
189289
],
290+
applyFixIts: ["insert ';'"],
190291
fixedSource: """
191292
enum Color {
192293
case Red; case Blue
@@ -212,4 +313,28 @@ final class ConsecutiveStatementsTests: XCTestCase {
212313
"""
213314
)
214315
}
316+
317+
func testConsecutiveStatements8() {
318+
assertParse(
319+
"""
320+
class Foo {
321+
func a() {}1️⃣/* some comment */ func b() {}
322+
}
323+
""",
324+
diagnostics: [
325+
DiagnosticSpec(
326+
locationMarker: "1️⃣",
327+
message: "consecutive declarations on a line must be separated by newline or ';'",
328+
fixIts: ["insert newline", "insert ';'"]
329+
)
330+
],
331+
applyFixIts: ["insert newline"],
332+
fixedSource: """
333+
class Foo {
334+
func a() {}/* some comment */
335+
func b() {}
336+
}
337+
"""
338+
)
339+
}
215340
}

‎Tests/SwiftParserTest/translated/RecoveryTests.swift

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ final class RecoveryTests: XCTestCase {
938938
)
939939
}
940940

941-
func testRecovery62() {
941+
func testRecovery62a() {
942942
assertParse(
943943
"""
944944
enum EE 1️⃣EE<T> where T : Multi {
@@ -954,11 +954,44 @@ final class RecoveryTests: XCTestCase {
954954
),
955955
DiagnosticSpec(
956956
locationMarker: "2️⃣",
957-
message: "consecutive declarations on a line must be separated by ';'",
958-
fixIts: ["insert ';'"]
957+
message: "consecutive declarations on a line must be separated by newline or ';'",
958+
fixIts: ["insert newline", "insert ';'"]
959+
),
960+
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
961+
],
962+
applyFixIts: ["join the identifiers together", "insert newline"],
963+
fixedSource: """
964+
enum EEEE<T> where T : Multi {
965+
case a
966+
a
967+
case b
968+
}
969+
"""
970+
)
971+
}
972+
973+
func testRecovery62b() {
974+
assertParse(
975+
"""
976+
enum EE 1️⃣EE<T> where T : Multi {
977+
case a2️⃣ 3️⃣a
978+
case b
979+
}
980+
""",
981+
diagnostics: [
982+
DiagnosticSpec(
983+
locationMarker: "1️⃣",
984+
message: "found an unexpected second identifier in enum; is there an accidental break?",
985+
fixIts: ["join the identifiers together"]
986+
),
987+
DiagnosticSpec(
988+
locationMarker: "2️⃣",
989+
message: "consecutive declarations on a line must be separated by newline or ';'",
990+
fixIts: ["insert newline", "insert ';'"]
959991
),
960992
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
961993
],
994+
applyFixIts: ["join the identifiers together", "insert ';'"],
962995
fixedSource: """
963996
enum EEEE<T> where T : Multi {
964997
case a;a
@@ -1565,8 +1598,8 @@ final class RecoveryTests: XCTestCase {
15651598
diagnostics: [
15661599
DiagnosticSpec(
15671600
locationMarker: "1️⃣",
1568-
message: "consecutive declarations on a line must be separated by ';'",
1569-
fixIts: ["insert ';'"]
1601+
message: "consecutive declarations on a line must be separated by newline or ';'",
1602+
fixIts: ["insert newline", "insert ';'"]
15701603
),
15711604
DiagnosticSpec(
15721605
locationMarker: "1️⃣",
@@ -1605,7 +1638,8 @@ final class RecoveryTests: XCTestCase {
16051638
],
16061639
fixedSource: """
16071640
struct ErrorTypeInVarDeclDictionaryType {
1608-
let a1: String;:
1641+
let a1: String
1642+
:
16091643
let a2: [String: Int]
16101644
let a3: [String: [Int]]
16111645
let a4: [String: Int]

0 commit comments

Comments
 (0)