Skip to content

Commit 45ee19f

Browse files
ahoppenahoppen
authored
Merge pull request #2492 from vinu-vanjari/SyntaxProtocol.shouldBeInsertedBeforePreviousTokenTrivia
Fix: Incorrect insertion location in fix-it for `)`
2 parents 38ff630 + 30b891c commit 45ee19f

21 files changed

+136
-91
lines changed

‎Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,8 @@ extension FixIt.MultiNodeChange {
198198
}
199199
}
200200

201-
if let previousToken = node.previousToken(viewMode: .fixedUp),
202-
previousToken.isPresent,
203-
let firstToken = node.firstToken(viewMode: .all),
204-
previousToken.trailingTrivia.allSatisfy({ $0.isWhitespace }),
205-
!BasicFormat().requiresWhitespace(between: previousToken, and: firstToken),
206-
!BasicFormat().requiresNewline(between: previousToken, and: firstToken)
201+
if node.shouldBeInsertedBeforePreviousTokenTrivia,
202+
let previousToken = node.previousToken(viewMode: .fixedUp)
207203
{
208204
// If the previous token and this node don't need to be separated, remove
209205
// the separation.

‎Sources/SwiftParserDiagnostics/MissingNodesError.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,8 @@ extension ParseDiagnosticsGenerator {
445445
position = overridePosition
446446
} else if node.shouldBeInsertedAfterNextTokenTrivia, let nextToken = node.nextToken(viewMode: .sourceAccurate) {
447447
position = nextToken.positionAfterSkippingLeadingTrivia
448+
} else if node.shouldBeInsertedBeforePreviousTokenTrivia, let previousToken = node.previousToken(viewMode: .sourceAccurate) {
449+
position = previousToken.endPositionBeforeTrailingTrivia
448450
} else {
449451
position = node.endPosition
450452
}

‎Sources/SwiftParserDiagnostics/SyntaxExtensions.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import SwiftBasicFormat
1314
@_spi(Diagnostics) import SwiftParser
1415
@_spi(RawSyntax) import SwiftSyntax
1516

@@ -134,6 +135,22 @@ extension SyntaxProtocol {
134135
return false
135136
}
136137
}
138+
139+
/// Returns `true` if the previous token and this node don't need to be separated,
140+
/// when it is switched from being missing to present.
141+
var shouldBeInsertedBeforePreviousTokenTrivia: Bool {
142+
if let previousToken = self.previousToken(viewMode: .fixedUp),
143+
previousToken.isPresent,
144+
let firstToken = self.firstToken(viewMode: .all),
145+
previousToken.trailingTrivia.allSatisfy({ $0.isWhitespace }),
146+
!BasicFormat().requiresWhitespace(between: previousToken, and: firstToken),
147+
!BasicFormat().requiresNewline(between: previousToken, and: firstToken)
148+
{
149+
return true
150+
} else {
151+
return false
152+
}
153+
}
137154
}
138155

139156
extension TokenKind {

‎Tests/SwiftDiagnosticsTest/ParserDiagnosticsFormatterIntegrationTests.swift

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,11 +128,26 @@ final class ParserDiagnosticsFormatterIntegrationTests: XCTestCase {
128128

129129
let expectedOutput = """
130130
\u{001B}[0;36m1 │\u{001B}[0;0m for \u{001B}[4;39m(i\u{001B}[0;0m \u{001B}[4;39m= 🐮; i != 👩‍👩‍👦‍👦; i += 1)\u{001B}[0;0m { }
131-
\u{001B}[0;36m│\u{001B}[0;0m │ ╰─ \u{001B}[1;31merror: \u{001B}[1;39mexpected ')' to end tuple pattern\u{001B}[0;0m
131+
\u{001B}[0;36m│\u{001B}[0;0m │ ╰─ \u{001B}[1;31merror: \u{001B}[1;39mexpected ')' to end tuple pattern\u{001B}[0;0m
132132
\u{001B}[0;36m│\u{001B}[0;0m ╰─ \u{001B}[1;31merror: \u{001B}[1;39mC-style for statement has been removed in Swift 3\u{001B}[0;0m
133133
134134
"""
135135

136136
assertStringsEqualWithDiff(annotate(source: source, colorize: true), expectedOutput)
137137
}
138+
139+
func testRighParenLocation() {
140+
let source = """
141+
let _ : Float -> Int
142+
"""
143+
144+
let expectedOutput = """
145+
1 │ let _ : Float -> Int
146+
│ │ ╰─ error: expected ')' in function type
147+
│ ╰─ error: expected '(' to start function type
148+
149+
"""
150+
151+
assertStringsEqualWithDiff(annotate(source: source), expectedOutput)
152+
}
138153
}

‎Tests/SwiftParserTest/DeclarationTests.swift

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,7 @@ final class DeclarationTests: ParserTestCase {
850850

851851
func testMissingColonInFunctionSignature() {
852852
assertParse(
853-
"func test(first second 1️⃣Int)",
853+
"func test(first second1️⃣ Int)",
854854
diagnostics: [
855855
DiagnosticSpec(message: "expected ':' in parameter", fixIts: ["insert ':'"])
856856
],
@@ -887,7 +887,7 @@ final class DeclarationTests: ParserTestCase {
887887

888888
func testMissingOpeningParenInFunctionSignature() {
889889
assertParse(
890-
"func test 1️⃣first second: Int)",
890+
"func test1️⃣ first second: Int)",
891891
diagnostics: [
892892
DiagnosticSpec(message: "expected '(' to start parameter clause", fixIts: ["insert '('"])
893893
],
@@ -1368,7 +1368,7 @@ final class DeclarationTests: ParserTestCase {
13681368

13691369
func testDontRecoverFromDeclKeyword() {
13701370
assertParse(
1371-
"func fooℹ️(first second 1️⃣third 2️⃣struct3️⃣: Int4️⃣) {}",
1371+
"func fooℹ️(first second1️⃣ third2️⃣ struct3️⃣: Int4️⃣) {}",
13721372
substructure: FunctionParameterSyntax(
13731373
firstName: .identifier("first"),
13741374
secondName: .identifier("second"),
@@ -1426,7 +1426,7 @@ final class DeclarationTests: ParserTestCase {
14261426

14271427
func testDontRecoverFromUnbalancedParens() {
14281428
assertParse(
1429-
"func foo(first second 1️⃣[third 2️⃣fourth: Int) {}",
1429+
"func foo(first second1️⃣ 2️⃣[third3️⃣ 4️⃣fourth: Int) {}",
14301430
substructure: FunctionParameterSyntax(
14311431
firstName: .identifier("first"),
14321432
secondName: .identifier("second"),
@@ -1444,13 +1444,13 @@ final class DeclarationTests: ParserTestCase {
14441444
fixIts: ["insert ':'"]
14451445
),
14461446
DiagnosticSpec(
1447-
locationMarker: "2️⃣",
1447+
locationMarker: "3️⃣",
14481448
message: "expected ']' to end array type",
1449-
notes: [NoteSpec(locationMarker: "1️⃣", message: "to match this opening '['")],
1449+
notes: [NoteSpec(locationMarker: "2️⃣", message: "to match this opening '['")],
14501450
fixIts: ["insert ']'"]
14511451
),
14521452
DiagnosticSpec(
1453-
locationMarker: "2️⃣",
1453+
locationMarker: "4️⃣",
14541454
message: "unexpected code 'fourth: Int' in parameter clause"
14551455
),
14561456
],
@@ -1463,7 +1463,7 @@ final class DeclarationTests: ParserTestCase {
14631463
func testDontRecoverIfNewlineIsBeforeColon() {
14641464
assertParse(
14651465
"""
1466-
func fooℹ️(first second 1️⃣third2️⃣
1466+
func fooℹ️(first second1️⃣ third2️⃣
14671467
3️⃣: Int) {}
14681468
""",
14691469
substructure: FunctionParameterSyntax(
@@ -2273,7 +2273,7 @@ final class DeclarationTests: ParserTestCase {
22732273

22742274
assertParse(
22752275
"""
2276-
macro m1 1️⃣= A
2276+
macro m11️⃣ = A
22772277
""",
22782278
diagnostics: [
22792279
DiagnosticSpec(locationMarker: "1️⃣", message: "expected parameter clause in function signature", fixIts: ["insert parameter clause"])
@@ -2462,7 +2462,7 @@ final class DeclarationTests: ParserTestCase {
24622462

24632463
func testMisplaceSpecifierInTupleTypeBody() {
24642464
assertParse(
2465-
"func test(a: (1️⃣borrowing F 2️⃣o))",
2465+
"func test(a: (1️⃣borrowing F2️⃣ o))",
24662466
diagnostics: [
24672467
DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code 'borrowing' in tuple type"),
24682468
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ',' in tuple type", fixIts: ["insert ','"]),
@@ -2628,7 +2628,7 @@ final class DeclarationTests: ParserTestCase {
26282628

26292629
assertParse(
26302630
"""
2631-
typealias T = 1️⃣~Int 2️⃣-> Bool
2631+
typealias T = 1️⃣~Int2️⃣ -> Bool
26322632
""",
26332633
diagnostics: [
26342634
DiagnosticSpec(

‎Tests/SwiftParserTest/ExpressionTests.swift

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2717,7 +2717,7 @@ final class StatementExpressionTests: ParserTestCase {
27172717
func testClosureWithMissingParentheses() {
27182718
assertParse(
27192719
"""
2720-
_ = { 1️⃣a: Int, b: Int 2️⃣in
2720+
_ = { 1️⃣a: Int, b: Int2️⃣ in
27212721
}
27222722
""",
27232723
diagnostics: [
@@ -2742,7 +2742,7 @@ final class StatementExpressionTests: ParserTestCase {
27422742
func testClosureWithReturnArrowAndMissingParentheses() {
27432743
assertParse(
27442744
"""
2745-
_ = { 1️⃣a: Int, b: 2️⃣Int 3️⃣-> Int 4️⃣in
2745+
_ = { 1️⃣a: Int, b: 2️⃣Int3️⃣ -> Int4️⃣ in
27462746
}
27472747
""",
27482748
diagnostics: [
@@ -2834,12 +2834,12 @@ final class StatementExpressionTests: ParserTestCase {
28342834
fixedSource: "[() async throws (MyError) -> Void]()"
28352835
)
28362836
assertParse(
2837-
"[() 1️⃣try(MyError) async -> Void]()",
2837+
"[()1️⃣ try(MyError) async -> Void]()",
28382838
diagnostics: [DiagnosticSpec(message: "expected ',' in array element", fixIts: ["insert ','"])],
28392839
fixedSource: "[(), try(MyError) async -> Void]()"
28402840
)
28412841
assertParse(
2842-
"[() 1️⃣try async(MyError) -> Void]()",
2842+
"[()1️⃣ try async(MyError) -> Void]()",
28432843
diagnostics: [DiagnosticSpec(message: "expected ',' in array element", fixIts: ["insert ','"])],
28442844
fixedSource: "[(), try async(MyError) -> Void]()"
28452845
)
@@ -2859,7 +2859,7 @@ final class StatementExpressionTests: ParserTestCase {
28592859
fixedSource: "[() async throws (MyError) -> Void]()"
28602860
)
28612861
assertParse(
2862-
"[() 1️⃣try(MyError) 2️⃣await 3️⃣-> Void]()",
2862+
"[()1️⃣ try(MyError)2️⃣ await 3️⃣-> Void]()",
28632863
diagnostics: [
28642864
DiagnosticSpec(
28652865
locationMarker: "1️⃣",
@@ -2880,22 +2880,22 @@ final class StatementExpressionTests: ParserTestCase {
28802880
fixedSource: "[(), try(MyError), await <#expression#> -> Void]()"
28812881
)
28822882
assertParse(
2883-
"[() 1️⃣try await(MyError) -> Void]()",
2883+
"[()1️⃣ try await(MyError) -> Void]()",
28842884
diagnostics: [DiagnosticSpec(message: "expected ',' in array element", fixIts: ["insert ','"])],
28852885
fixedSource: "[(), try await(MyError) -> Void]()"
28862886
)
28872887
assertParse(
2888-
"[() 1️⃣async(MyError) -> Void]()",
2888+
"[()1️⃣ async(MyError) -> Void]()",
28892889
diagnostics: [DiagnosticSpec(message: "expected ',' in array element", fixIts: ["insert ','"])],
28902890
fixedSource: "[(), async(MyError) -> Void]()"
28912891
)
28922892
assertParse(
2893-
"[() 1️⃣await(MyError) -> Void]()",
2893+
"[()1️⃣ await(MyError) -> Void]()",
28942894
diagnostics: [DiagnosticSpec(message: "expected ',' in array element", fixIts: ["insert ','"])],
28952895
fixedSource: "[(), await(MyError) -> Void]()"
28962896
)
28972897
assertParse(
2898-
"[() 1️⃣try(MyError) -> Void]()",
2898+
"[()1️⃣ try(MyError) -> Void]()",
28992899
diagnostics: [DiagnosticSpec(message: "expected ',' in array element", fixIts: ["insert ','"])],
29002900
fixedSource: "[(), try(MyError) -> Void]()"
29012901
)
@@ -2914,7 +2914,7 @@ final class StatementExpressionTests: ParserTestCase {
29142914
assertParse("[() ()]")
29152915
29162916
assertParse(
2917-
"[1 1️⃣2]",
2917+
"[11️⃣ 2]",
29182918
diagnostics: [
29192919
DiagnosticSpec(
29202920
message: "expected ',' in array element",
@@ -2925,7 +2925,7 @@ final class StatementExpressionTests: ParserTestCase {
29252925
)
29262926
29272927
assertParse(
2928-
#"["hello" 1️⃣"world"]"#,
2928+
#"["hello"1️⃣ "world"]"#,
29292929
diagnostics: [
29302930
DiagnosticSpec(
29312931
message: "expected ',' in array element",
@@ -2938,7 +2938,7 @@ final class StatementExpressionTests: ParserTestCase {
29382938
29392939
func testDictionaryExprWithNoCommas() {
29402940
assertParse(
2941-
"[1: () 1️⃣2: ()]",
2941+
"[1: ()1️⃣ 2: ()]",
29422942
diagnostics: [
29432943
DiagnosticSpec(
29442944
message: "expected ',' in dictionary element",
@@ -2949,7 +2949,7 @@ final class StatementExpressionTests: ParserTestCase {
29492949
)
29502950
29512951
assertParse(
2952-
#"["foo": 1 1️⃣"bar": 2]"#,
2952+
#"["foo": 11️⃣ "bar": 2]"#,
29532953
diagnostics: [
29542954
DiagnosticSpec(
29552955
message: "expected ',' in dictionary element",
@@ -2960,7 +2960,7 @@ final class StatementExpressionTests: ParserTestCase {
29602960
)
29612961
29622962
assertParse(
2963-
#"[1: "hello" 1️⃣2: "world"]"#,
2963+
#"[1: "hello"1️⃣ 2: "world"]"#,
29642964
diagnostics: [
29652965
DiagnosticSpec(
29662966
message: "expected ',' in dictionary element",

‎Tests/SwiftParserTest/RegexLiteralTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ final class RegexLiteralTests: ParserTestCase {
776776
// FIXME: The diagnostic should be one character back
777777
assertParse(
778778
#"""
779-
\ 1️⃣/^ x/
779+
\1️⃣ /^ x/
780780
"""#,
781781
diagnostics: [
782782
DiagnosticSpec(message: "expected root in key path", fixIts: ["insert root"])

‎Tests/SwiftParserTest/TypeTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ final class TypeTests: ParserTestCase {
1818
func testMissingColonInType() {
1919
assertParse(
2020
"""
21-
var foo 1️⃣Bar = 1
21+
var foo1️⃣ Bar = 1
2222
""",
2323
diagnostics: [
2424
DiagnosticSpec(message: "expected ':' in type annotation", fixIts: ["insert ':'"])

‎Tests/SwiftParserTest/VariadicGenericsTests.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ final class VariadicGenericsTests: ParserTestCase {
5555
func testInvalidPackElement() {
5656
assertParse(
5757
"""
58-
func invalid<each T>() -> (each any 1️⃣T) {}
58+
func invalid<each T>() -> (each any1️⃣ T) {}
5959
""",
6060
diagnostics: [
6161
DiagnosticSpec(message: "expected ',' in tuple type", fixIts: ["insert ','"])
@@ -564,7 +564,7 @@ final class TypeParameterPackTests: ParserTestCase {
564564

565565
assertParse(
566566
"""
567-
var foo: (bar: Int 1️⃣bar2: Int)
567+
var foo: (bar: Int1️⃣ bar2: Int)
568568
""",
569569
diagnostics: [
570570
DiagnosticSpec(message: "expected ',' in tuple type", fixIts: ["insert ','"])
@@ -576,7 +576,7 @@ final class TypeParameterPackTests: ParserTestCase {
576576

577577
assertParse(
578578
"""
579-
var foo: (bar: Int 1️⃣Int)
579+
var foo: (bar: Int1️⃣ Int)
580580
""",
581581
diagnostics: [
582582
DiagnosticSpec(message: "expected ',' in tuple type", fixIts: ["insert ','"])
@@ -588,7 +588,7 @@ final class TypeParameterPackTests: ParserTestCase {
588588

589589
assertParse(
590590
"""
591-
var foo: (a 1️⃣Int)
591+
var foo: (a1️⃣ Int)
592592
""",
593593
diagnostics: [
594594
DiagnosticSpec(message: "expected ':' in tuple type", fixIts: ["insert ':'"])
@@ -600,7 +600,7 @@ final class TypeParameterPackTests: ParserTestCase {
600600

601601
assertParse(
602602
"""
603-
var foo: (A 1️⃣Int)
603+
var foo: (A1️⃣ Int)
604604
""",
605605
diagnostics: [
606606
DiagnosticSpec(message: "expected ',' in tuple type", fixIts: ["insert ','"])
@@ -612,7 +612,7 @@ final class TypeParameterPackTests: ParserTestCase {
612612

613613
assertParse(
614614
"""
615-
var foo: (_ 1️⃣a 2️⃣Int)
615+
var foo: (_1️⃣ a2️⃣ Int)
616616
""",
617617
diagnostics: [
618618
DiagnosticSpec(locationMarker: "1️⃣", message: "expected ':' in tuple type", fixIts: ["insert ':'"]),
@@ -625,7 +625,7 @@ final class TypeParameterPackTests: ParserTestCase {
625625

626626
assertParse(
627627
"""
628-
var foo: (Array<Foo> 1️⃣Array<Bar>)
628+
var foo: (Array<Foo>1️⃣ Array<Bar>)
629629
""",
630630
diagnostics: [
631631
DiagnosticSpec(message: "expected ',' in tuple type", fixIts: ["insert ','"])
@@ -637,7 +637,7 @@ final class TypeParameterPackTests: ParserTestCase {
637637

638638
assertParse(
639639
"""
640-
var foo: (a 1️⃣Array<Bar>)
640+
var foo: (a1️⃣ Array<Bar>)
641641
""",
642642
diagnostics: [
643643
DiagnosticSpec(message: "expected ':' in tuple type", fixIts: ["insert ':'"])
@@ -649,7 +649,7 @@ final class TypeParameterPackTests: ParserTestCase {
649649

650650
assertParse(
651651
"""
652-
var foo: (Array<Foo> 1️⃣a)
652+
var foo: (Array<Foo>1️⃣ a)
653653
""",
654654
diagnostics: [
655655
DiagnosticSpec(message: "expected ',' in tuple type", fixIts: ["insert ','"])

0 commit comments

Comments
 (0)