From b6f45b7b48a0f0e332fb2f827308639178255736 Mon Sep 17 00:00:00 2001 From: Felix Wehnert Date: Tue, 28 Feb 2023 22:57:10 +0100 Subject: [PATCH] Improved diagnostics for misplaced AttributeList in variable declaration --- .../swiftsyntax/SyntaxRewriterFile.swift | 23 ++++++++++++++++- Sources/SwiftParser/Declarations.swift | 23 +++++++++++++++++ .../ParseDiagnosticsGenerator.swift | 25 +++++++++++++++++-- .../ParserDiagnosticMessages.swift | 13 ++++++---- .../generated/SyntaxRewriter.swift | 16 +++++++++++- Tests/SwiftParserTest/AttributeTests.swift | 22 ++++++++++++++++ Tests/SwiftParserTest/DeclarationTests.swift | 17 +++++++++++++ 7 files changed, 130 insertions(+), 9 deletions(-) diff --git a/CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift b/CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift index 799c8771413..e45b6c3b8b0 100644 --- a/CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift +++ b/CodeGeneration/Sources/generate-swift-syntax/templates/swiftsyntax/SyntaxRewriterFile.swift @@ -31,11 +31,32 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) { """ ) { DeclSyntax("public let viewMode: SyntaxTreeViewMode") + DeclSyntax( + """ + /// The arena in which the parents of rewritten nodes should be allocated. + /// + /// The `SyntaxRewriter` subclass is responsible for generating the rewritten nodes. To incorporate them into the + /// tree, all of the rewritten node's parents also need to be re-created. This is the arena in which those + /// intermediate nodes should be allocated. + private let arena: SyntaxArena? + """ + ) DeclSyntax( """ public init(viewMode: SyntaxTreeViewMode = .sourceAccurate) { self.viewMode = viewMode + self.arena = nil + } + """ + ) + + DeclSyntax( + """ + @_spi(RawSyntax) + public init(viewMode: SyntaxTreeViewMode = .sourceAccurate, arena: SyntaxArena? = nil) { + self.viewMode = viewMode + self.arena = arena } """ ) @@ -346,7 +367,7 @@ let syntaxRewriterFile = SourceFileSyntax(leadingTrivia: copyrightHeader) { // Sanity check, ensure the new children are the same length. precondition(newLayout.count == node.raw.layoutView!.children.count) - let arena = SyntaxArena() + let arena = self.arena ?? SyntaxArena() let newRaw = node.raw.layoutView!.replacingLayout(with: Array(newLayout), arena: arena) // 'withExtendedLifetime' to keep 'SyntaxArena's of them alive until here. return withExtendedLifetime(rewrittens) { diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift index 7900150be3a..b55038af2b7 100644 --- a/Sources/SwiftParser/Declarations.swift +++ b/Sources/SwiftParser/Declarations.swift @@ -1262,11 +1262,33 @@ extension Parser { let (unexpectedBeforeIntroducer, introducer) = self.eat(handle) let hasTryBeforeIntroducer = unexpectedBeforeIntroducer?.containsToken(where: { TokenSpec(.try) ~= $0 }) ?? false + var attrs = attrs var elements = [RawPatternBindingSyntax]() do { var keepGoing: RawTokenSyntax? = nil var loopProgress = LoopProgressCondition() repeat { + var unexpectedBeforePattern: RawUnexpectedNodesSyntax? + + if self.at(.atSign), attrs.attributes.isEmpty { + let recoveredAttributes = self.parseAttributeList() + unexpectedBeforePattern = RawUnexpectedNodesSyntax( + [recoveredAttributes], + arena: self.arena + ) + + /// Create a version of the same attribute with all tokens missing. + class TokenMissingMaker: SyntaxRewriter { + override func visit(_ token: TokenSyntax) -> TokenSyntax { + return .init(token.tokenKind, presence: .missing) + } + } + let tokenMissingMaker = TokenMissingMaker(arena: self.arena) + let missingAttributes = tokenMissingMaker.rewrite( + Syntax(raw: RawSyntax(recoveredAttributes), rawNodeArena: arena) + ).raw + attrs.attributes = missingAttributes.cast(RawAttributeListSyntax.self) + } var (pattern, typeAnnotation) = self.parseTypedPattern() @@ -1344,6 +1366,7 @@ extension Parser { keepGoing = self.consume(if: .comma) elements.append( RawPatternBindingSyntax( + unexpectedBeforePattern, pattern: pattern, typeAnnotation: typeAnnotation, initializer: initializer, diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index e5fa61308da..a7579573dd0 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -262,7 +262,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { unexpectedTokenCondition: { EffectSpecifier(token: $0) != nil }, correctTokens: [effectSpecifiers?.asyncSpecifier, effectSpecifiers?.throwsClause?.throwsSpecifier], message: { EffectsSpecifierAfterArrow(effectsSpecifiersAfterArrow: $0) }, - moveFixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: .arrow) }, + moveFixIt: { MoveNodesInFrontOfFixIt(movedNodes: $0, inFrontOf: .arrow) }, removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) } ) } @@ -324,7 +324,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { unexpectedTokenCondition: { AsyncEffectSpecifier(token: $0) != nil }, correctTokens: [node.asyncSpecifier], message: { AsyncMustPrecedeThrows(asyncKeywords: $0, throwsKeyword: throwsClause.throwsSpecifier) }, - moveFixIt: { MoveTokensInFrontOfFixIt(movedTokens: $0, inFrontOf: throwsClause.throwsSpecifier.tokenKind) }, + moveFixIt: { MoveNodesInFrontOfFixIt(movedNodes: $0, inFrontOf: throwsClause.throwsSpecifier.tokenKind) }, removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) } ) } @@ -2052,6 +2052,27 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { moveFixIt: { MoveTokensAfterFixIt(movedTokens: $0, after: .equal) }, removeRedundantFixIt: { RemoveRedundantFixIt(removeTokens: $0) } ) + + if node.attributes.isMissingAllTokens, + let unexpected = node.bindings.compactMap({ $0.unexpectedBeforePattern }).first, + unexpected.only?.is(AttributeListSyntax.self) ?? false + { + let fixIt = FixIt( + message: MoveNodesInFrontOfFixIt(movedNodes: [unexpected], inFrontOf: node.bindingSpecifier.tokenKind), + changes: [ + .makeMissing(unexpected), + .makePresent(node.attributes, trailingTrivia: .space), + ] + ) + + addDiagnostic( + unexpected, + .misplacedAttributeInVarDecl, + fixIts: [fixIt], + handledNodes: [node.attributes.id, unexpected.id] + ) + } + return .visitChildren } diff --git a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift index 4e11b4671d9..e7233d7610f 100644 --- a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift +++ b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift @@ -191,6 +191,9 @@ extension DiagnosticMessage where Self == StaticParserError { public static var maximumNestingLevelOverflow: Self { .init("parsing has exceeded the maximum nesting level") } + public static var misplacedAttributeInVarDecl: Self { + .init("misplaced attribute in variable declaration") + } public static var missingColonAndExprInTernaryExpr: Self { .init("expected ':' and expression after '? ...' in ternary expression") } @@ -710,15 +713,15 @@ public struct MoveTokensAfterFixIt: ParserFixIt { } } -public struct MoveTokensInFrontOfFixIt: ParserFixIt { - /// The token that should be moved - public let movedTokens: [TokenSyntax] +public struct MoveNodesInFrontOfFixIt: ParserFixIt { + /// The nodes that should be moved + public let movedNodes: [Node] - /// The token before which 'movedTokens' should be moved + /// The token before which `movedNodes` should be moved public let inFrontOf: TokenKind public var message: String { - "move \(nodesDescription(movedTokens, format: false)) in front of '\(inFrontOf.nameForDiagnostics)'" + "move \(nodesDescription(movedNodes, format: false)) in front of '\(inFrontOf.nameForDiagnostics)'" } } diff --git a/Sources/SwiftSyntax/generated/SyntaxRewriter.swift b/Sources/SwiftSyntax/generated/SyntaxRewriter.swift index e3b3b26d480..137773c042c 100644 --- a/Sources/SwiftSyntax/generated/SyntaxRewriter.swift +++ b/Sources/SwiftSyntax/generated/SyntaxRewriter.swift @@ -24,8 +24,22 @@ open class SyntaxRewriter { public let viewMode: SyntaxTreeViewMode + /// The arena in which the parents of rewritten nodes should be allocated. + /// + /// The `SyntaxRewriter` subclass is responsible for generating the rewritten nodes. To incorporate them into the + /// tree, all of the rewritten node's parents also need to be re-created. This is the arena in which those + /// intermediate nodes should be allocated. + private let arena: SyntaxArena? + public init(viewMode: SyntaxTreeViewMode = .sourceAccurate) { self.viewMode = viewMode + self.arena = nil + } + + @_spi(RawSyntax) + public init(viewMode: SyntaxTreeViewMode = .sourceAccurate, arena: SyntaxArena? = nil) { + self.viewMode = viewMode + self.arena = arena } /// Rewrite `node`, keeping its parent unless `detach` is `true`. @@ -3966,7 +3980,7 @@ open class SyntaxRewriter { // Sanity check, ensure the new children are the same length. precondition(newLayout.count == node.raw.layoutView!.children.count) - let arena = SyntaxArena() + let arena = self.arena ?? SyntaxArena() let newRaw = node.raw.layoutView!.replacingLayout(with: Array(newLayout), arena: arena) // 'withExtendedLifetime' to keep 'SyntaxArena's of them alive until here. return withExtendedLifetime(rewrittens) { diff --git a/Tests/SwiftParserTest/AttributeTests.swift b/Tests/SwiftParserTest/AttributeTests.swift index c1da7778ff2..c514b029498 100644 --- a/Tests/SwiftParserTest/AttributeTests.swift +++ b/Tests/SwiftParserTest/AttributeTests.swift @@ -1022,4 +1022,26 @@ final class AttributeTests: ParserTestCase { ] ) } + + func testMisplacedAttributeInVariableDecl() { + assertParse( + """ + struct A { + var 1️⃣@State name: String + } + """, + diagnostics: [ + DiagnosticSpec( + message: "misplaced attribute in variable declaration", + fixIts: ["move attributes in front of 'var'"] + ) + ], + fixedSource: + """ + struct A { + @State var name: String + } + """ + ) + } } diff --git a/Tests/SwiftParserTest/DeclarationTests.swift b/Tests/SwiftParserTest/DeclarationTests.swift index 30ed2f2e096..b570f943f24 100644 --- a/Tests/SwiftParserTest/DeclarationTests.swift +++ b/Tests/SwiftParserTest/DeclarationTests.swift @@ -3308,4 +3308,21 @@ final class DeclarationTests: ParserTestCase { experimentalFeatures: .transferringArgsAndResults ) } + + func testMisplacedAttributeInVarDeclWithMultipleBindings() { + assertParse( + """ + let a = "a", 1️⃣@foo b = "b" + """, + diagnostics: [ + DiagnosticSpec( + message: "misplaced attribute in variable declaration", + fixIts: ["move attributes in front of 'let'"] + ) + ], + fixedSource: """ + @foo let a = "a", b = "b" + """ + ) + } }