diff --git a/Sources/SwiftParser/Attributes.swift b/Sources/SwiftParser/Attributes.swift index 5946e76cdff..39e7d5a0241 100644 --- a/Sources/SwiftParser/Attributes.swift +++ b/Sources/SwiftParser/Attributes.swift @@ -17,18 +17,56 @@ #endif extension Parser { - mutating func parseAttributeList() -> RawAttributeListSyntax { - guard self.at(.atSign, .poundIf) else { - return self.emptyCollection(RawAttributeListSyntax.self) + private mutating func parseAttributeListElement() -> RawAttributeListSyntax.Element { + if self.at(.poundIf) { + // 'consumeIfConfigOfAttributes()' check in 'parseAttributeList()' already guarantees + // that this '#if' only contains attribute list elements. We don't check + // `consumeIfConfigOfAttributes()` here again because it's not only redundant, but it + // does *not* work for cases like: + // + // #if COND1 + // @attr + // #if COND2 + // #endif + // #endif + // func fn() {} + // + // In such cases, the second `#if` is not `consumeIfConfigOfAttributes()`. + return .ifConfigDecl( + self.parsePoundIfDirective { (parser, _) -> RawAttributeListSyntax.Element in + return parser.parseAttributeListElement() + } syntax: { parser, attributes in + return .attributes(RawAttributeListSyntax(elements: attributes, arena: parser.arena)) + } + ) + } else { + return .attribute(self.parseAttribute()) } + } + mutating func parseAttributeList() -> RawAttributeListSyntax { var elements = [RawAttributeListSyntax.Element]() + + func shouldContinue() -> Bool { + if self.at(.atSign) { + return true + } + if self.at(.poundIf) && self.withLookahead({ $0.consumeIfConfigOfAttributes() }) { + return true + } + return false + } + var loopProgress = LoopProgressCondition() - repeat { - let attribute = self.parseAttribute() + while self.hasProgressed(&loopProgress) && shouldContinue() { + let attribute = self.parseAttributeListElement() elements.append(attribute) - } while self.at(.atSign, .poundIf) && self.hasProgressed(&loopProgress) - return RawAttributeListSyntax(elements: elements, arena: self.arena) + } + if elements.isEmpty { + return self.emptyCollection(RawAttributeListSyntax.self) + } else { + return RawAttributeListSyntax(elements: elements, arena: self.arena) + } } } @@ -148,7 +186,7 @@ extension Parser { parseMissingArguments: ( (inout Parser) -> (unexpectedBefore: RawUnexpectedNodesSyntax?, arguments: RawAttributeSyntax.Arguments) )? = nil - ) -> RawAttributeListSyntax.Element { + ) -> RawAttributeSyntax { var (unexpectedBeforeAtSign, atSign) = self.expect(.atSign) if atSign.trailingTriviaByteLength > 0 || self.currentToken.leadingTriviaByteLength > 0 { let diagnostic = TokenDiagnostic( @@ -163,9 +201,7 @@ extension Parser { case .required: shouldParseArgument = true case .customAttribute: - shouldParseArgument = - self.withLookahead { $0.atAttributeOrSpecifierArgument() } - && self.at(TokenSpec(.leftParen, allowAtStartOfLine: false)) + shouldParseArgument = self.withLookahead { $0.atAttributeOrSpecifierArgument() } case .optional: shouldParseArgument = self.at(.leftParen) case .noArgument: @@ -190,46 +226,32 @@ extension Parser { (unexpectedBeforeArguments, argument) = parseArguments(&self) } let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen) - return .attribute( - RawAttributeSyntax( - unexpectedBeforeAtSign, - atSign: atSign, - attributeName: attributeName, - unexpectedBeforeLeftParen, - leftParen: leftParen, - unexpectedBeforeArguments, - arguments: argument, - unexpectedBeforeRightParen, - rightParen: rightParen, - arena: self.arena - ) + return RawAttributeSyntax( + unexpectedBeforeAtSign, + atSign: atSign, + attributeName: attributeName, + unexpectedBeforeLeftParen, + leftParen: leftParen, + unexpectedBeforeArguments, + arguments: argument, + unexpectedBeforeRightParen, + rightParen: rightParen, + arena: self.arena ) } else { - return .attribute( - RawAttributeSyntax( - unexpectedBeforeAtSign, - atSign: atSign, - attributeName: attributeName, - leftParen: nil, - arguments: nil, - rightParen: nil, - arena: self.arena - ) + return RawAttributeSyntax( + unexpectedBeforeAtSign, + atSign: atSign, + attributeName: attributeName, + leftParen: nil, + arguments: nil, + rightParen: nil, + arena: self.arena ) } } - mutating func parseAttribute() -> RawAttributeListSyntax.Element { - if self.at(.poundIf) { - return .ifConfigDecl( - self.parsePoundIfDirective { (parser, _) -> RawAttributeListSyntax.Element in - return parser.parseAttribute() - } syntax: { parser, attributes in - return .attributes(RawAttributeListSyntax(elements: attributes, arena: parser.arena)) - } - ) - } - + mutating func parseAttribute() -> RawAttributeSyntax { switch peek(isAtAnyIn: DeclarationAttributeWithSpecialSyntax.self) { case .abi: return parseAttribute(argumentMode: .required) { parser in @@ -299,17 +321,15 @@ extension Parser { case .rethrows: let (unexpectedBeforeAtSign, atSign) = self.expect(.atSign) let (unexpectedBeforeAttributeName, attributeName) = self.expect(TokenSpec(.rethrows, remapping: .identifier)) - return .attribute( - RawAttributeSyntax( - unexpectedBeforeAtSign, - atSign: atSign, - unexpectedBeforeAttributeName, - attributeName: RawIdentifierTypeSyntax(name: attributeName, genericArgumentClause: nil, arena: self.arena), - leftParen: nil, - arguments: nil, - rightParen: nil, - arena: self.arena - ) + return RawAttributeSyntax( + unexpectedBeforeAtSign, + atSign: atSign, + unexpectedBeforeAttributeName, + attributeName: RawIdentifierTypeSyntax(name: attributeName, genericArgumentClause: nil, arena: self.arena), + leftParen: nil, + arguments: nil, + rightParen: nil, + arena: self.arena ) case .Sendable: return parseAttribute(argumentMode: .noArgument) { parser in @@ -1023,6 +1043,10 @@ extension Parser { extension Parser.Lookahead { mutating func atAttributeOrSpecifierArgument() -> Bool { + if !self.at(TokenSpec(.leftParen, allowAtStartOfLine: false)) { + return false + } + var lookahead = self.lookahead() lookahead.skipSingle() @@ -1055,9 +1079,7 @@ extension Parser.Lookahead { return false } - if self.at(TokenSpec(.leftParen, allowAtStartOfLine: false)) - && self.withLookahead({ $0.atAttributeOrSpecifierArgument() }) - { + if self.withLookahead({ $0.atAttributeOrSpecifierArgument() }) { self.skipSingle() } diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift index 6ffffd81438..3f7cd4b39e2 100644 --- a/Sources/SwiftParser/Declarations.swift +++ b/Sources/SwiftParser/Declarations.swift @@ -65,9 +65,16 @@ extension TokenConsumer { var hasAttribute = false var attributeProgress = LoopProgressCondition() - while subparser.hasProgressed(&attributeProgress) && subparser.at(.atSign) { - hasAttribute = true - _ = subparser.consumeAttributeList() + while subparser.hasProgressed(&attributeProgress) { + if subparser.at(.atSign) { + _ = subparser.consumeAttributeList() + hasAttribute = true + } else if subparser.at(.poundIf) && subparser.consumeIfConfigOfAttributes() { + subparser.skipSingle() + hasAttribute = true + } else { + break + } } var hasModifier = false @@ -94,11 +101,6 @@ extension TokenConsumer { } } - if subparser.at(.poundIf) { - var attrLookahead = subparser.lookahead() - return attrLookahead.consumeIfConfigOfAttributes() - } - let declStartKeyword: DeclarationKeyword? if allowRecovery { declStartKeyword = @@ -175,6 +177,10 @@ extension TokenConsumer { allowRecovery: allowRecovery ) } + if allowRecovery && (hasAttribute || hasModifier) { + // If we found any attributes or modifiers, consider it's a missing decl. + return true + } return false } } @@ -492,11 +498,11 @@ extension Parser { arena: self.arena ) - if self.at(.atSign), case .attribute(let attribute) = self.parseAttribute() { + if self.at(.atSign) { return RawUsingDeclSyntax( unexpectedBeforeUsingKeyword, usingKeyword: usingKeyword, - specifier: .attribute(attribute), + specifier: .attribute(self.parseAttribute()), arena: self.arena ) } diff --git a/Sources/SwiftParser/Lookahead.swift b/Sources/SwiftParser/Lookahead.swift index 285fa2d72fc..55f9ab2978d 100644 --- a/Sources/SwiftParser/Lookahead.swift +++ b/Sources/SwiftParser/Lookahead.swift @@ -221,11 +221,8 @@ extension Parser.Lookahead { _ = self.consumeGenericArguments() } while self.consume(if: .period) != nil - if self.consume(if: .leftParen) != nil { - while !self.at(.endOfFile, .rightParen, .poundEndif) { - self.skipSingle() - } - self.consume(if: .rightParen) + if self.atAttributeOrSpecifierArgument() { + self.skipSingle() } } return true diff --git a/Sources/SwiftParser/TokenSpecSet.swift b/Sources/SwiftParser/TokenSpecSet.swift index b80c3794821..4f03301c344 100644 --- a/Sources/SwiftParser/TokenSpecSet.swift +++ b/Sources/SwiftParser/TokenSpecSet.swift @@ -267,9 +267,9 @@ enum ContextualDeclKeyword: TokenSpecSet { } } -/// A `DeclarationKeyword` that is not a `ValueBindingPatternSyntax.BindingSpecifierOptions`. +/// A `DeclarationKeyword` that is not a `VariableDeclSyntax.BindingSpecifierOptions`. /// -/// `ValueBindingPatternSyntax.BindingSpecifierOptions` are injected into +/// `VariableDeclSyntax.BindingSpecifierOptions` are injected into /// `DeclarationKeyword` via an `EitherTokenSpecSet`. enum PureDeclarationKeyword: TokenSpecSet { case actor @@ -344,7 +344,7 @@ enum PureDeclarationKeyword: TokenSpecSet { typealias DeclarationKeyword = EitherTokenSpecSet< PureDeclarationKeyword, - ValueBindingPatternSyntax.BindingSpecifierOptions + VariableDeclSyntax.BindingSpecifierOptions > enum DeclarationModifier: TokenSpecSet { diff --git a/Sources/SwiftParser/TopLevel.swift b/Sources/SwiftParser/TopLevel.swift index cdaa7cf34a0..0bbb7f4cfb9 100644 --- a/Sources/SwiftParser/TopLevel.swift +++ b/Sources/SwiftParser/TopLevel.swift @@ -251,10 +251,10 @@ extension Parser { return self.parseStatementItem() } else if self.atStartOfExpression() { return .expr(self.parseExpression(flavor: .basic, pattern: .none)) - } else if self.atStartOfDeclaration(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl, allowRecovery: true) { - return .decl(self.parseDeclaration()) } else if self.atStartOfStatement(allowRecovery: true, preferExpr: false) { return self.parseStatementItem() + } else if self.atStartOfDeclaration(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl, allowRecovery: true) { + return .decl(self.parseDeclaration()) } else { return .init(expr: RawMissingExprSyntax(arena: self.arena)) } diff --git a/Sources/SwiftParser/Types.swift b/Sources/SwiftParser/Types.swift index bbfd3ade5b1..df054e3a53b 100644 --- a/Sources/SwiftParser/Types.swift +++ b/Sources/SwiftParser/Types.swift @@ -1308,17 +1308,21 @@ extension Parser { return .attribute(self.parseDifferentiableAttribute()) case .isolated: - return parseAttribute(argumentMode: .required) { parser in - return (nil, .argumentList(parser.parseIsolatedAttributeArguments())) - } + return .attribute( + parseAttribute(argumentMode: .required) { parser in + return (nil, .argumentList(parser.parseIsolatedAttributeArguments())) + } + ) case .convention, ._opaqueReturnTypeOf, nil: // Custom attribute - return parseAttribute(argumentMode: .customAttribute) { parser in - let arguments = parser.parseArgumentListElements( - pattern: .none, - allowTrailingComma: true - ) - return (nil, .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena))) - } + return .attribute( + parseAttribute(argumentMode: .customAttribute) { parser in + let arguments = parser.parseArgumentListElements( + pattern: .none, + allowTrailingComma: true + ) + return (nil, .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena))) + } + ) } } diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 97a4705e431..87f919e1829 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -482,21 +482,36 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor { otherNode.lastToken(viewMode: .sourceAccurate)?.tokenKind == .poundEndif { let diagnoseOn = parent.parent ?? parent - addDiagnostic( - diagnoseOn, - IfConfigDeclNotAllowedInContext(context: diagnoseOn), - highlights: [Syntax(node), Syntax(otherNode)], - fixIts: [ - FixIt( - message: RemoveNodesFixIt([Syntax(node), Syntax(otherNode)]), - changes: [ - .makeMissing([Syntax(node)], transferTrivia: false), - .makeMissing([Syntax(otherNode)], transferTrivia: false), - ] - ) - ], - handledNodes: [node.id, otherNode.id] - ) + if node == otherNode { + addDiagnostic( + diagnoseOn, + IfConfigDeclNotAllowedInContext(context: diagnoseOn), + highlights: [Syntax(node)], + fixIts: [ + FixIt( + message: RemoveNodesFixIt([Syntax(node)]), + changes: .makeMissing([Syntax(node)], transferTrivia: false) + ) + ], + handledNodes: [node.id] + ) + } else { + addDiagnostic( + diagnoseOn, + IfConfigDeclNotAllowedInContext(context: diagnoseOn), + highlights: [Syntax(node), Syntax(otherNode)], + fixIts: [ + FixIt( + message: RemoveNodesFixIt([Syntax(node), Syntax(otherNode)]), + changes: [ + .makeMissing([Syntax(node)], transferTrivia: false), + .makeMissing([Syntax(otherNode)], transferTrivia: false), + ] + ) + ], + handledNodes: [node.id, otherNode.id] + ) + } } else { addDiagnostic(node, UnexpectedNodesError(unexpectedNodes: node), highlights: [Syntax(node)]) } diff --git a/Tests/SwiftParserTest/AttributeTests.swift b/Tests/SwiftParserTest/AttributeTests.swift index 695df2b4735..bdb1c728d0f 100644 --- a/Tests/SwiftParserTest/AttributeTests.swift +++ b/Tests/SwiftParserTest/AttributeTests.swift @@ -1400,4 +1400,42 @@ final class AttributeTests: ParserTestCase { """ ) } + + func testAttributeParsable() { + assertParse( + """ + 1️⃣#if true + @discardableResult + #endif + """, + { AttributeSyntax.parse(from: &$0) }, + substructure: AttributeSyntax( + atSign: .atSignToken(presence: .missing), + attributeName: TypeSyntax(MissingTypeSyntax(placeholder: .identifier("<#type#>", presence: .missing))), + UnexpectedNodesSyntax([ + TokenSyntax.poundIfToken(), + TokenSyntax.keyword(.true), + TokenSyntax.atSignToken(), + TokenSyntax.identifier("discardableResult"), + TokenSyntax.poundEndifToken(), + ]) + ), + diagnostics: [ + DiagnosticSpec(message: "expected '@' and name in attribute", fixIts: ["insert '@' and name"]), + DiagnosticSpec( + message: "conditional compilation not permitted in attribute", + fixIts: [ + """ + remove '#if true + @discardableResult + #endif' + """ + ] + ), + ], + fixedSource: """ + @<#type#> + """ + ) + } } diff --git a/Tests/SwiftParserTest/DeclarationTests.swift b/Tests/SwiftParserTest/DeclarationTests.swift index 6a7bac74f68..80f254ed402 100644 --- a/Tests/SwiftParserTest/DeclarationTests.swift +++ b/Tests/SwiftParserTest/DeclarationTests.swift @@ -567,11 +567,15 @@ final class DeclarationTests: ParserTestCase { assertParse( """ - 1️⃣private( + private(1️⃣ """, diagnostics: [ - DiagnosticSpec(message: "extraneous code 'private(' at top level") - ] + DiagnosticSpec(message: "expected 'set)' to end modifier", fixIts: ["insert 'set)'"]), + DiagnosticSpec(message: "expected declaration after 'private' modifier", fixIts: ["insert declaration"]), + ], + fixedSource: """ + private(set) <#declaration#> + """ ) assertParse( diff --git a/Tests/SwiftParserTest/DirectiveTests.swift b/Tests/SwiftParserTest/DirectiveTests.swift index a42e72c1059..b91965f308c 100644 --- a/Tests/SwiftParserTest/DirectiveTests.swift +++ b/Tests/SwiftParserTest/DirectiveTests.swift @@ -357,4 +357,83 @@ final class DirectiveTests: ParserTestCase { ] ) } + + func testIfConfigRRR() { + assertParse( + """ + @frozen1️⃣ + + #if true + func foo() {} + #endif + """, + substructure: CodeBlockItemListSyntax([ + CodeBlockItemSyntax( + item: .init( + MissingDeclSyntax( + attributes: [ + .attribute( + AttributeSyntax( + atSign: .atSignToken(), + attributeName: TypeSyntax(IdentifierTypeSyntax(name: .identifier("frozen"))) + ) + ) + ], + modifiers: [], + placeholder: .identifier("<#declaration#>", presence: .missing) + ) + ) + ), + CodeBlockItemSyntax( + item: .init( + IfConfigDeclSyntax( + clauses: IfConfigClauseListSyntax([ + IfConfigClauseSyntax( + poundKeyword: .poundIfToken(), + condition: ExprSyntax(BooleanLiteralExprSyntax(literal: .keyword(.true))), + elements: .statements( + CodeBlockItemListSyntax([ + CodeBlockItemSyntax( + item: CodeBlockItemSyntax.Item( + FunctionDeclSyntax( + attributes: [], + modifiers: [], + funcKeyword: .keyword(.func), + name: .identifier("foo"), + signature: FunctionSignatureSyntax( + parameterClause: FunctionParameterClauseSyntax( + leftParen: .leftParenToken(), + parameters: FunctionParameterListSyntax([]), + rightParen: .rightParenToken() + ) + ), + body: CodeBlockSyntax( + leftBrace: .leftBraceToken(), + statements: CodeBlockItemListSyntax([]), + rightBrace: .rightBraceToken() + ) + ) + ) + ) + ]) + ) + ) + ]), + poundEndif: .poundEndifToken() + ) + ) + ), + ]), + diagnostics: [ + DiagnosticSpec(message: "expected declaration after attribute", fixIts: ["insert declaration"]) + ], + fixedSource: """ + @frozen <#declaration#> + + #if true + func foo() {} + #endif + """ + ) + } } diff --git a/Tests/SwiftParserTest/StatementTests.swift b/Tests/SwiftParserTest/StatementTests.swift index 39872342bbe..d56274bb449 100644 --- a/Tests/SwiftParserTest/StatementTests.swift +++ b/Tests/SwiftParserTest/StatementTests.swift @@ -703,7 +703,7 @@ final class StatementTests: ParserTestCase { func testRecoveryInFrontOfAccessorIntroducer() { assertParse( """ - subscript1️⃣(2️⃣{3️⃣@self _modify + subscript1️⃣(2️⃣{@attr _modify3️⃣ """, diagnostics: [ DiagnosticSpec( @@ -723,14 +723,10 @@ final class StatementTests: ParserTestCase { notes: [NoteSpec(locationMarker: "2️⃣", message: "to match this opening '{'")], fixIts: ["insert '}'"] ), - DiagnosticSpec( - locationMarker: "3️⃣", - message: "extraneous code '@self _modify' at top level" - ), ], fixedSource: """ - subscript() -> <#type#> { - }@self _modify + subscript() -> <#type#> {@attr _modify + } """ ) }