From 6ff8de48610cb67a0320d67d1184eb2f3c657eb3 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 26 Apr 2023 17:35:51 +0200 Subject: [PATCH 1/3] Merge pull request #1580 from kimdv/kimdv/fix-wrong-diagnostic-for-wrong-pattern --- Sources/SwiftParser/Patterns.swift | 8 +-- .../ParseDiagnosticsGenerator.swift | 2 +- .../translated/RecoveryTests.swift | 53 ++++++++++++++++--- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/Sources/SwiftParser/Patterns.swift b/Sources/SwiftParser/Patterns.swift index d4fce79e88f..d73f3e3f2b4 100644 --- a/Sources/SwiftParser/Patterns.swift +++ b/Sources/SwiftParser/Patterns.swift @@ -175,10 +175,12 @@ extension Parser { && !self.currentToken.isAtStartOfLine && lookahead.canParseType() { - // Recovery if the user forgot to add ':' - let result = self.parseResultType() + let (unexpectedBeforeColon, colon) = self.expect(.colon) + let result = self.parseType() + type = RawTypeAnnotationSyntax( - colon: self.missingToken(.colon), + unexpectedBeforeColon, + colon: colon, type: result, arena: self.arena ) diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 072a22ccde1..d9c60e28de9 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -315,7 +315,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { } else if node.first?.as(TokenSyntax.self)?.tokenKind.isIdentifier == true, let previousToken = node.previousToken(viewMode: .sourceAccurate), previousToken.tokenKind.isIdentifier, - previousToken.parent?.is(DeclSyntax.self) == true + previousToken.parent?.is(DeclSyntax.self) == true || previousToken.parent?.is(IdentifierPatternSyntax.self) == true { // If multiple identifiers are used for a declaration name, offer to join them together. let tokens = diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift index 31889b689c7..4c2e9983d9c 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift @@ -786,20 +786,39 @@ final class RecoveryTests: XCTestCase { assertParse( #""" struct SS 1️⃣SS : Multi { - private var a 2️⃣b 3️⃣: Int = "" + private var a 2️⃣b : Int = "" func f() { - var c 4️⃣d = 5 + var c 3️⃣d = 5 let _ = 0 } } """#, diagnostics: [ - DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct"), - DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' in type annotation"), - DiagnosticSpec(locationMarker: "3️⃣", message: #"unexpected code ': Int = ""' before function"#), - // TODO: (good first issue) Old parser expected error on line 4: found an unexpected second identifier in variable declaration; is there an accidental break? - DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' in type annotation"), - ] + DiagnosticSpec( + locationMarker: "1️⃣", + message: "found an unexpected second identifier in struct; is there an accidental break?", + fixIts: ["join the identifiers together"] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: "found an unexpected second identifier in pattern; is there an accidental break?", + fixIts: ["join the identifiers together", "join the identifiers together with camel-case"] + ), + DiagnosticSpec( + locationMarker: "3️⃣", + message: "expected ':' in type annotation", + fixIts: ["insert ':'"] + ), + ], + fixedSource: #""" + struct SSSS : Multi { + private var ab : Int = "" + func f() { + var c: d = 5 + let _ = 0 + } + } + """# ) } @@ -815,6 +834,24 @@ final class RecoveryTests: XCTestCase { ) } + func testRecovery64c() { + assertParse( + """ + private var a 1️⃣b : Int = "" + """, + diagnostics: [ + DiagnosticSpec( + locationMarker: "1️⃣", + message: "found an unexpected second identifier in pattern; is there an accidental break?", + fixIts: ["join the identifiers together", "join the identifiers together with camel-case"] + ) + ], + fixedSource: """ + private var ab : Int = "" + """ + ) + } + func testRecovery65() { assertParse( """ From e9d3460e906a53e20d9299a8b5d116fd7efc03de Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Wed, 26 Apr 2023 20:14:20 +0200 Subject: [PATCH 2/3] Merge pull request #1566 from kimdv/kimdv/fix-wrong-wrapping-of-quote-tokens Fix for some wrong attribute fix its --- Sources/SwiftParser/StringLiterals.swift | 2 +- .../ParseDiagnosticsGenerator.swift | 59 +++++++++++++++++++ Tests/SwiftParserTest/AttributeTests.swift | 48 +++++++++++---- .../OriginalDefinedInAttrTests.swift | 52 +++++++++++++--- 4 files changed, 141 insertions(+), 20 deletions(-) diff --git a/Sources/SwiftParser/StringLiterals.swift b/Sources/SwiftParser/StringLiterals.swift index ffd756e6b49..6b954ce8e56 100644 --- a/Sources/SwiftParser/StringLiterals.swift +++ b/Sources/SwiftParser/StringLiterals.swift @@ -483,7 +483,7 @@ extension Parser { // string literal. guard currentToken.leadingTriviaText.isEmpty else { break } - if let stringSegment = self.consume(if: .stringSegment) { + if let stringSegment = self.consume(if: .stringSegment, TokenSpec(.identifier, remapping: .stringSegment)) { segments.append(.stringSegment(RawStringSegmentSyntax(content: stringSegment, arena: self.arena))) } else if let backslash = self.consume(if: .backslash) { let (unexpectedBeforeDelimiter, delimiter) = self.parsePoundDelimiter(.rawStringDelimiter, matching: openDelimiter) diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index d9c60e28de9..decf47fc0e8 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -893,6 +893,35 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { return handleMissingSyntax(node, additionalHandledNodes: [node.placeholder.id]) } + override open func visit(_ node: OriginallyDefinedInArgumentsSyntax) -> SyntaxVisitorContinueKind { + if shouldSkip(node) { + return .skipChildren + } + if let token = node.unexpectedBetweenModuleLabelAndColon?.onlyToken(where: { $0.tokenKind.isIdentifier }), + node.moduleLabel.presence == .missing + { + addDiagnostic( + node, + MissingNodesError(missingNodes: [Syntax(node.moduleLabel)]), + fixIts: [ + FixIt( + message: ReplaceTokensFixIt( + replaceTokens: [token], + replacements: [node.moduleLabel] + ), + changes: [ + FixIt.MultiNodeChange.makeMissing(token), + FixIt.MultiNodeChange.makePresent(node.moduleLabel), + ] + ) + ], + handledNodes: [node.moduleLabel.id, token.id] + ) + } + + return .visitChildren + } + public override func visit(_ node: OperatorDeclSyntax) -> SyntaxVisitorContinueKind { if shouldSkip(node) { return .skipChildren @@ -1191,6 +1220,36 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { return .visitChildren } + public override func visit(_ node: UnavailableFromAsyncArgumentsSyntax) -> SyntaxVisitorContinueKind { + if shouldSkip(node) { + return .skipChildren + } + + if let token = node.unexpectedBetweenMessageLabelAndColon?.onlyToken(where: { $0.tokenKind.isIdentifier }), + node.messageLabel.presence == .missing + { + addDiagnostic( + node, + MissingNodesError(missingNodes: [Syntax(node.messageLabel)]), + fixIts: [ + FixIt( + message: ReplaceTokensFixIt( + replaceTokens: [token], + replacements: [node.messageLabel] + ), + changes: [ + FixIt.MultiNodeChange.makeMissing(token), + FixIt.MultiNodeChange.makePresent(node.messageLabel), + ] + ) + ], + handledNodes: [node.messageLabel.id, token.id] + ) + } + + return .visitChildren + } + public override func visit(_ node: UnresolvedTernaryExprSyntax) -> SyntaxVisitorContinueKind { if shouldSkip(node) { return .skipChildren diff --git a/Tests/SwiftParserTest/AttributeTests.swift b/Tests/SwiftParserTest/AttributeTests.swift index 900775342ad..ffbbc651199 100644 --- a/Tests/SwiftParserTest/AttributeTests.swift +++ b/Tests/SwiftParserTest/AttributeTests.swift @@ -422,12 +422,23 @@ final class AttributeTests: XCTestCase { assertParse( """ - @_expose(Cxx, 1️⃣baz) func foo() {} + @_expose(Cxx, 1️⃣baz2️⃣) func foo() {} """, diagnostics: [ - DiagnosticSpec(message: "expected string literal to end @_expose arguments"), - DiagnosticSpec(message: "unexpected code 'baz' in attribute"), - ] + DiagnosticSpec( + locationMarker: "1️⃣", + message: #"expected '"' in string literal"#, + fixIts: [#"insert '"'"#] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: #"expected '"' to end string literal"#, + fixIts: [#"insert '"'"#] + ), + ], + fixedSource: """ + @_expose(Cxx, "baz") func foo() {} + """ ) } @@ -475,9 +486,12 @@ final class AttributeTests: XCTestCase { func foo() {} """, diagnostics: [ - DiagnosticSpec(message: "expected 'message' in @_unavailableFromAsync argument"), - DiagnosticSpec(message: "unexpected code 'nope' before @_unavailableFromAsync argument"), - ] + DiagnosticSpec(message: "expected 'message' in @_unavailableFromAsync argument", fixIts: ["replace 'nope' with 'message'"]) + ], + fixedSource: """ + @_unavailableFromAsync(message: "abc") + func foo() {} + """ ) assertParse( @@ -493,13 +507,25 @@ final class AttributeTests: XCTestCase { assertParse( """ - @_unavailableFromAsync(message: 1️⃣abc) + @_unavailableFromAsync(message: 1️⃣abc2️⃣) func foo() {} """, diagnostics: [ - DiagnosticSpec(message: "expected string literal to end @_unavailableFromAsync argument"), - DiagnosticSpec(message: "unexpected code 'abc' in attribute"), - ] + DiagnosticSpec( + locationMarker: "1️⃣", + message: #"expected '"' in string literal"#, + fixIts: [#"insert '"'"#] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: #"expected '"' to end string literal"#, + fixIts: [#"insert '"'"#] + ), + ], + fixedSource: """ + @_unavailableFromAsync(message: "abc") + func foo() {} + """ ) } diff --git a/Tests/SwiftParserTest/translated/OriginalDefinedInAttrTests.swift b/Tests/SwiftParserTest/translated/OriginalDefinedInAttrTests.swift index ef7bfffdd76..ef7b985cc13 100644 --- a/Tests/SwiftParserTest/translated/OriginalDefinedInAttrTests.swift +++ b/Tests/SwiftParserTest/translated/OriginalDefinedInAttrTests.swift @@ -31,9 +31,15 @@ final class OriginalDefinedInAttrTests: XCTestCase { public func foo1() {} """#, diagnostics: [ - DiagnosticSpec(message: "expected 'module' in @_originallyDefinedIn arguments"), - DiagnosticSpec(message: "unexpected code 'modulename' before @_originallyDefinedIn arguments"), - ] + DiagnosticSpec( + message: "expected 'module' in @_originallyDefinedIn arguments", + fixIts: ["replace 'modulename' with 'module'"] + ) + ], + fixedSource: #""" + @_originallyDefinedIn(module: "foo", OSX 13.13) + public func foo1() {} + """# ) } @@ -53,7 +59,10 @@ final class OriginalDefinedInAttrTests: XCTestCase { public class ToplevelClass1 {} """#, diagnostics: [ - DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' and version list in @_originallyDefinedIn arguments") + DiagnosticSpec( + message: "expected ',' and version list in @_originallyDefinedIn arguments", + fixIts: ["insert ',' and version list"] + ) ] ) } @@ -61,12 +70,40 @@ final class OriginalDefinedInAttrTests: XCTestCase { func testOriginalDefinedInAttr5() { assertParse( """ - @_originallyDefinedIn(1️⃣OSX 13.13.3) + @_originallyDefinedIn(1️⃣OSX 2️⃣13.13.3) public class ToplevelClass2 {} """, diagnostics: [ - DiagnosticSpec(message: "expected 'module:', string literal, and ',' in @_originallyDefinedIn arguments") - ] + DiagnosticSpec( + locationMarker: "1️⃣", + message: "expected 'module:' in @_originallyDefinedIn arguments", + fixIts: ["insert 'module:'"] + ), + DiagnosticSpec( + locationMarker: "1️⃣", + message: #"expected '"' in string literal"#, + fixIts: [#"insert '"'"#] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: #"expected '"' to end string literal"#, + fixIts: [#"insert '"'"#] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: "expected ',' in @_originallyDefinedIn arguments", + fixIts: ["insert ','"] + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: "expected platform in version restriction", + fixIts: ["insert platform"] + ), + ], + fixedSource: """ + @_originallyDefinedIn(module: "OSX", <#identifier#> 13.13.3) + public class ToplevelClass2 {} + """ ) } @@ -172,5 +209,4 @@ final class OriginalDefinedInAttrTests: XCTestCase { """ ) } - } From 7f41da9deaacfacb5407c0810df307dcb59cdeb9 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Fri, 28 Apr 2023 19:48:37 +0200 Subject: [PATCH 3/3] Merge pull request #1484 from kimdv/kimdv/add-diagnostic-for-dictionary Add diagnostic for dictionary --- Sources/SwiftParser/Types.swift | 56 +++++++++++++- .../translated/RecoveryTests.swift | 77 +++++++++++++++---- 2 files changed, 119 insertions(+), 14 deletions(-) diff --git a/Sources/SwiftParser/Types.swift b/Sources/SwiftParser/Types.swift index 8ddab9caa8b..0c6e2ff19f6 100644 --- a/Sources/SwiftParser/Types.swift +++ b/Sources/SwiftParser/Types.swift @@ -1052,7 +1052,61 @@ extension Parser { ) ) } else { - return self.parseType() + var result = self.parseType() + + guard !result.hasError else { + return result + } + + if self.at(.rightSquareBracket) { + let (unexpectedBeforeRSquareBracket, rightSquareBracket) = self.expect(.rightSquareBracket) + result = RawTypeSyntax( + RawArrayTypeSyntax( + leftSquareBracket: missingToken(.leftSquareBracket), + elementType: result, + unexpectedBeforeRSquareBracket, + rightSquareBracket: rightSquareBracket, + arena: self.arena + ) + ) + } else if self.at(.colon) { + var lookahead = self.lookahead() + + // We only want to continue with a dictionary if we can parse a colon and a simpletype. + // Otherwise we can get a wrong diagnostic if we get a Python-style function declaration. + guard lookahead.consume(if: .colon) != nil && lookahead.canParseSimpleType() else { + return result + } + + let (unexpectedBeforeColon, colon) = self.expect(.colon) + let secondType = self.parseSimpleType() + let rightSquareBracket = self.consume(if: .rightSquareBracket) ?? self.missingToken(.rightSquareBracket) + + result = RawTypeSyntax( + RawDictionaryTypeSyntax( + leftSquareBracket: self.missingToken(.leftSquareBracket), + keyType: result, + unexpectedBeforeColon, + colon: colon, + valueType: secondType, + rightSquareBracket: rightSquareBracket, + arena: self.arena + ) + ) + } + + var loopProgress = LoopProgressCondition() + while loopProgress.evaluate(currentToken) { + if self.at(TokenSpec(.postfixQuestionMark, allowAtStartOfLine: false)) { + result = RawTypeSyntax(self.parseOptionalType(result)) + } else if self.at(TokenSpec(.exclamationMark, allowAtStartOfLine: false)) { + result = RawTypeSyntax(self.parseImplicitlyUnwrappedOptionalType(result)) + } else { + break + } + } + + return result } } } diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift index 4c2e9983d9c..9dd82f6f73e 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift @@ -1209,22 +1209,63 @@ final class RecoveryTests: XCTestCase { """ struct ErrorTypeInVarDeclDictionaryType { let a1: String1️⃣: - let a2: String2️⃣: Int] - let a3: String3️⃣: [Int] - let a4: String4️⃣: Int + let a2: 2️⃣String: Int] + let a3: 3️⃣String: [Int]4️⃣ + let a4: 5️⃣String: Int6️⃣ + let a4: 7️⃣String: Int]?? } """, diagnostics: [ - // TODO: Old parser expected error on line 2: unexpected ':' in type; did you mean to write a dictionary type?, Fix-It replacements: 11 - 11 = '[' - DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive declarations on a line must be separated by ';'"), - DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code ':' before variable"), - // TODO: Old parser expected error on line 3: unexpected ':' in type; did you mean to write a dictionary type?, Fix-It replacements: 11 - 11 = '[' - DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code ': Int]' before variable"), - // TODO: Old parser expected error on line 4: unexpected ':' in type; did you mean to write a dictionary type?, Fix-It replacements: 11 - 11 = '[', 24 - 24 = ']' - DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code ': [Int]' before variable"), - // TODO: Old parser expected error on line 5: unexpected ':' in type; did you mean to write a dictionary type?, Fix-It replacements: 11 - 11 = '[', 22 - 22 = ']' - DiagnosticSpec(locationMarker: "4️⃣", message: "unexpected code ': Int' in struct"), - ] + DiagnosticSpec( + locationMarker: "1️⃣", + message: "consecutive declarations on a line must be separated by ';'", + fixIts: ["insert ';'"] + ), + DiagnosticSpec( + locationMarker: "1️⃣", + message: "unexpected code ':' before variable" + ), + DiagnosticSpec( + locationMarker: "2️⃣", + message: "expected '[' to start dictionary type", + fixIts: ["insert '['"] + ), + DiagnosticSpec( + locationMarker: "3️⃣", + message: "expected '[' to start dictionary type", + fixIts: ["insert '['"] + ), + DiagnosticSpec( + locationMarker: "4️⃣", + message: "expected ']' to end dictionary type", + fixIts: ["insert ']'"] + ), + DiagnosticSpec( + locationMarker: "5️⃣", + message: "expected '[' to start dictionary type", + fixIts: ["insert '['"] + ), + DiagnosticSpec( + locationMarker: "6️⃣", + message: "expected ']' to end dictionary type", + fixIts: ["insert ']'"] + ), + DiagnosticSpec( + locationMarker: "7️⃣", + message: "expected '[' to start dictionary type", + fixIts: ["insert '['"] + ), + + ], + fixedSource: """ + struct ErrorTypeInVarDeclDictionaryType { + let a1: String;: + let a2: [String: Int] + let a3: [String: [Int]] + let a4: [String: Int] + let a4: [String: Int]?? + } + """ ) } @@ -2312,4 +2353,14 @@ final class RecoveryTests: XCTestCase { """ ) } + + // https://github.com/apple/swift-syntax/pull/1484/files#r1176740738 + func testRecovery184() { + assertParse( + "func foo() -> Int1️⃣:", + diagnostics: [ + DiagnosticSpec(message: "extraneous code ':' at top level") + ] + ) + } }