From 86dd04138fcfcfecd0725ee5eb474dffca17832d Mon Sep 17 00:00:00 2001 From: stevenwong Date: Thu, 9 Mar 2023 16:20:15 +0800 Subject: [PATCH 1/6] Add diagnostics for init declarations --- Sources/SwiftParser/Declarations.swift | 9 +++++++ .../ParseDiagnosticsGenerator.swift | 27 +++++++++++++++++++ .../ParserDiagnosticMessages.swift | 6 +++++ 3 files changed, 42 insertions(+) diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift index 524bc803129..630a625cfdd 100644 --- a/Sources/SwiftParser/Declarations.swift +++ b/Sources/SwiftParser/Declarations.swift @@ -1061,6 +1061,14 @@ extension Parser { // Parse the signature. let signature = self.parseFunctionSignature() + // mark the unexpected result type of initializer + // and replace the ‘output‘ of signature with nil + var unexpectedResultType: RawUnexpectedNodesSyntax? + if let output = signature.output { + signature.raw = signature.layoutView.replacingChild(at: 5, with: nil, arena: self.arena) + unexpectedResultType = RawUnexpectedNodesSyntax([output.raw], arena: self.arena) + } + let whereClause: RawGenericWhereClauseSyntax? if self.at(.keyword(.where)) { whereClause = self.parseGenericWhereClause() @@ -1078,6 +1086,7 @@ extension Parser { optionalMark: failable, genericParameterClause: generics, signature: signature, + unexpectedResultType, genericWhereClause: whereClause, body: items, arena: self.arena diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index f477a29434e..4efe2e6bae4 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -748,6 +748,33 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { return .visitChildren } + public override func visit(_ node: InitializerDeclSyntax) -> SyntaxVisitorContinueKind { + if shouldSkip(node) { + return .skipChildren + } + + if let unexpectedName = node.signature.input.unexpectedBeforeLeftParen { + addDiagnostic( + unexpectedName, + .initializerCannotHaveName, + fixIts: [ + FixIt(message: RemoveNodesFixIt(unexpectedName), changes: .makeMissing(unexpectedName)) + ], + handledNodes: [unexpectedName.id] + ) + } + + if let unexpectedOutput = node.unexpectedBetweenSignatureAndGenericWhereClause { + addDiagnostic( + unexpectedOutput, + .initializerCannotHaveResultType, + handledNodes: [unexpectedOutput.id] + ) + } + + return .visitChildren + } + public override func visit(_ node: MemberDeclListItemSyntax) -> SyntaxVisitorContinueKind { if shouldSkip(node) { return .skipChildren diff --git a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift index d603c1e5b4a..d0907ca85da 100644 --- a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift +++ b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift @@ -131,6 +131,12 @@ extension DiagnosticMessage where Self == StaticParserError { public static var initializerInPattern: Self { .init("unexpected initializer in pattern; did you mean to use '='?") } + public static var initializerCannotHaveName: Self { + .init("initializers cannot have a name") + } + public static var initializerCannotHaveResultType: Self { + .init("initializers cannot have a result type") + } public static var invalidFlagAfterPrecedenceGroupAssignment: Self { .init("expected 'true' or 'false' after 'assignment'") } From 9c889489d5bdb7b9104ea208ef5eda0e2df67cf6 Mon Sep 17 00:00:00 2001 From: stevenwong Date: Thu, 9 Mar 2023 16:20:28 +0800 Subject: [PATCH 2/6] Update testcases & Remove TODOs --- .../translated/InitDeinitTests.swift | 4 +-- .../translated/RecoveryTests.swift | 34 ++++++++++++------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/Tests/SwiftParserTest/translated/InitDeinitTests.swift b/Tests/SwiftParserTest/translated/InitDeinitTests.swift index 93a072ba194..2e0dba10e72 100644 --- a/Tests/SwiftParserTest/translated/InitDeinitTests.swift +++ b/Tests/SwiftParserTest/translated/InitDeinitTests.swift @@ -83,11 +83,11 @@ final class InitDeinitTests: XCTestCase { AssertParse( """ struct FooStructConstructorD { - init() -> FooStructConstructorD { } + init() 1️⃣-> FooStructConstructorD { } } """, diagnostics: [ - // TODO: Old parser expected error on line 2: initializers cannot have a result type + DiagnosticSpec(message: "initializers cannot have a result type") ] ) } diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift index 97980e42cbc..5818ef7ffc1 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift @@ -1741,7 +1741,7 @@ final class RecoveryTests: XCTestCase { DiagnosticSpec(locationMarker: "1️⃣", message: "expected '>' to end generic parameter clause"), DiagnosticSpec(locationMarker: "2️⃣", message: "expected type and ')' to end parameter clause"), DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code ')}' before struct"), - DiagnosticSpec(locationMarker: "4️⃣", message: "unexpected code 'x' before parameter clause"), + DiagnosticSpec(locationMarker: "4️⃣", message: "initializers cannot have a name", fixIts: ["remove 'x'"]), ] ) } @@ -1752,9 +1752,11 @@ final class RecoveryTests: XCTestCase { init 1️⃣a(b: Int) {} """, diagnostics: [ - // TODO: (good first issue) Old parser expected error on line 3: initializers cannot have a name, Fix-It replacements: 8 - 9 = '' - DiagnosticSpec(message: "unexpected code 'a' before parameter clause") - ] + DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'a'"]) + ], + fixedSource: """ + init (b: Int) {} + """ ) } @@ -1764,9 +1766,11 @@ final class RecoveryTests: XCTestCase { init? 1️⃣c(_ d: Int) {} """, diagnostics: [ - // TODO: (good first issue) Old parser expected error on line 1: initializers cannot have a name, Fix-It replacements: 9 - 10 = '' - DiagnosticSpec(message: "unexpected code 'c' before parameter clause") - ] + DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'c'"]) + ], + fixedSource: """ + init? (_ d: Int) {} + """ ) } @@ -1776,9 +1780,11 @@ final class RecoveryTests: XCTestCase { init 1️⃣e(f: T) {} """, diagnostics: [ - // TODO: (good first issue) Old parser expected error on line 1: initializers cannot have a name, Fix-It replacements: 8 - 9 = '' - DiagnosticSpec(message: "unexpected code 'e' before parameter clause") - ] + DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'e'"]) + ], + fixedSource: """ + init (f: T) {} + """ ) } @@ -1788,9 +1794,11 @@ final class RecoveryTests: XCTestCase { init? 1️⃣g(_: T) {} """, diagnostics: [ - // TODO: (good first issue) Old parser expected error on line 1: initializers cannot have a name, Fix-It replacements: 9 - 10 = '' - DiagnosticSpec(message: "unexpected code 'g' before parameter clause") - ] + DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'g'"]) + ], + fixedSource: """ + init? (_: T) {} + """ ) } From a72020106623a90dd33755a50f11764a84f23ed4 Mon Sep 17 00:00:00 2001 From: stevenwong Date: Fri, 10 Mar 2023 18:09:26 +0800 Subject: [PATCH 3/6] Code refactor, not to replace a child by its index --- Sources/SwiftParser/Declarations.swift | 24 +++++++++---------- .../ParseDiagnosticsGenerator.swift | 14 ++++++++--- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift index 630a625cfdd..41532cd5ead 100644 --- a/Sources/SwiftParser/Declarations.swift +++ b/Sources/SwiftParser/Declarations.swift @@ -1059,15 +1059,7 @@ extension Parser { } // Parse the signature. - let signature = self.parseFunctionSignature() - - // mark the unexpected result type of initializer - // and replace the ‘output‘ of signature with nil - var unexpectedResultType: RawUnexpectedNodesSyntax? - if let output = signature.output { - signature.raw = signature.layoutView.replacingChild(at: 5, with: nil, arena: self.arena) - unexpectedResultType = RawUnexpectedNodesSyntax([output.raw], arena: self.arena) - } + let signature = self.parseFunctionSignature(allowOutput: false) let whereClause: RawGenericWhereClauseSyntax? if self.at(.keyword(.where)) { @@ -1086,7 +1078,6 @@ extension Parser { optionalMark: failable, genericParameterClause: generics, signature: signature, - unexpectedResultType, genericWhereClause: whereClause, body: items, arena: self.arena @@ -1398,12 +1389,12 @@ extension Parser { } @_spi(RawSyntax) - public mutating func parseFunctionSignature() -> RawFunctionSignatureSyntax { + public mutating func parseFunctionSignature(allowOutput: Bool = true) -> RawFunctionSignatureSyntax { let input = self.parseParameterClause(for: .functionParameters) var effectSpecifiers = self.parseDeclEffectSpecifiers() - let output: RawReturnClauseSyntax? + var output: RawReturnClauseSyntax? /// Only allow recovery to the arrow with exprKeyword precedence so we only /// skip over misplaced identifiers and don't e.g. recover to an arrow in a 'where' clause. @@ -1413,10 +1404,19 @@ extension Parser { output = nil } + var unexpectedAfterOutput: RawUnexpectedNodesSyntax? + if !allowOutput, + let unexpectedOutput = output + { + output = nil + unexpectedAfterOutput = RawUnexpectedNodesSyntax([unexpectedOutput.raw], arena: self.arena) + } + return RawFunctionSignatureSyntax( input: input, effectSpecifiers: effectSpecifiers, output: output, + unexpectedAfterOutput, arena: self.arena ) } diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 4efe2e6bae4..f08598f6eff 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -753,18 +753,26 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { return .skipChildren } - if let unexpectedName = node.signature.input.unexpectedBeforeLeftParen { + if let unexpectedName = node.signature.input.unexpectedBeforeLeftParen, + let previous = unexpectedName.previousToken(viewMode: .sourceAccurate) + { addDiagnostic( unexpectedName, .initializerCannotHaveName, fixIts: [ - FixIt(message: RemoveNodesFixIt(unexpectedName), changes: .makeMissing(unexpectedName)) + FixIt( + message: RemoveNodesFixIt(unexpectedName), + changes: [ + .makeMissing(unexpectedName), + FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: .zero)]) + ] + ) ], handledNodes: [unexpectedName.id] ) } - if let unexpectedOutput = node.unexpectedBetweenSignatureAndGenericWhereClause { + if let unexpectedOutput = node.signature.unexpectedAfterOutput { addDiagnostic( unexpectedOutput, .initializerCannotHaveResultType, From 916af7a51cbfbf63c9aa4207a7d6d163f0e3e608 Mon Sep 17 00:00:00 2001 From: stevenwong Date: Fri, 10 Mar 2023 18:09:34 +0800 Subject: [PATCH 4/6] Update testcases --- .../SwiftParserTest/translated/InitDeinitTests.swift | 11 +++++++++++ Tests/SwiftParserTest/translated/RecoveryTests.swift | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/Tests/SwiftParserTest/translated/InitDeinitTests.swift b/Tests/SwiftParserTest/translated/InitDeinitTests.swift index 2e0dba10e72..5d23185bc3c 100644 --- a/Tests/SwiftParserTest/translated/InitDeinitTests.swift +++ b/Tests/SwiftParserTest/translated/InitDeinitTests.swift @@ -391,6 +391,17 @@ final class InitDeinitTests: XCTestCase { ) } + func testInitDeinit28() { + AssertParse( + """ + init(_ foo: T) 1️⃣-> Int where T: Comparable {} + """, + diagnostics: [ + DiagnosticSpec(message: "initializers cannot have a result type") + ] + ) + } + func testDeinitInSwiftinterfaceIsFollowedByFinalFunc() { AssertParse( """ diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift index 5818ef7ffc1..b251ac9bbc0 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift @@ -1755,7 +1755,7 @@ final class RecoveryTests: XCTestCase { DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'a'"]) ], fixedSource: """ - init (b: Int) {} + init(b: Int) {} """ ) } @@ -1769,7 +1769,7 @@ final class RecoveryTests: XCTestCase { DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'c'"]) ], fixedSource: """ - init? (_ d: Int) {} + init?(_ d: Int) {} """ ) } @@ -1783,7 +1783,7 @@ final class RecoveryTests: XCTestCase { DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'e'"]) ], fixedSource: """ - init (f: T) {} + init(f: T) {} """ ) } @@ -1797,7 +1797,7 @@ final class RecoveryTests: XCTestCase { DiagnosticSpec(message: "initializers cannot have a name", fixIts: ["remove 'g'"]) ], fixedSource: """ - init? (_: T) {} + init?(_: T) {} """ ) } From ddc0447fb4b0eabe81c11d2a4e95371c1446af35 Mon Sep 17 00:00:00 2001 From: stevenwong Date: Fri, 10 Mar 2023 18:22:11 +0800 Subject: [PATCH 5/6] Run swift-format --- Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index f08598f6eff..21f0e7ef28b 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -764,7 +764,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { message: RemoveNodesFixIt(unexpectedName), changes: [ .makeMissing(unexpectedName), - FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: .zero)]) + FixIt.Changes(changes: [.replaceTrailingTrivia(token: previous, newTrivia: .zero)]), ] ) ], From 2d02851b577f0d554cfa1914f82f673e17d20499 Mon Sep 17 00:00:00 2001 From: stevenwong Date: Sat, 11 Mar 2023 08:12:12 +0800 Subject: [PATCH 6/6] Remove unnecessary .raw --- Sources/SwiftParser/Declarations.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift index 41532cd5ead..3d6e859799e 100644 --- a/Sources/SwiftParser/Declarations.swift +++ b/Sources/SwiftParser/Declarations.swift @@ -1409,7 +1409,7 @@ extension Parser { let unexpectedOutput = output { output = nil - unexpectedAfterOutput = RawUnexpectedNodesSyntax([unexpectedOutput.raw], arena: self.arena) + unexpectedAfterOutput = RawUnexpectedNodesSyntax([unexpectedOutput], arena: self.arena) } return RawFunctionSignatureSyntax(