Skip to content

Commit 07bf225

Browse files
rintarorintaro
authored
Merge pull request #3136 from rintaro/parser-attr-recover
[SwiftParser] Improve attribute parsing and recovering
2 parents c4002f2 + 556a2f3 commit 07bf225

File tree

11 files changed

+275
-114
lines changed

11 files changed

+275
-114
lines changed

‎Sources/SwiftParser/Attributes.swift

Lines changed: 81 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,56 @@
1717
#endif
1818

1919
extension Parser {
20-
mutating func parseAttributeList() -> RawAttributeListSyntax {
21-
guard self.at(.atSign, .poundIf) else {
22-
return self.emptyCollection(RawAttributeListSyntax.self)
20+
private mutating func parseAttributeListElement() -> RawAttributeListSyntax.Element {
21+
if self.at(.poundIf) {
22+
// 'consumeIfConfigOfAttributes()' check in 'parseAttributeList()' already guarantees
23+
// that this '#if' only contains attribute list elements. We don't check
24+
// `consumeIfConfigOfAttributes()` here again because it's not only redundant, but it
25+
// does *not* work for cases like:
26+
//
27+
// #if COND1
28+
// @attr
29+
// #if COND2
30+
// #endif
31+
// #endif
32+
// func fn() {}
33+
//
34+
// In such cases, the second `#if` is not `consumeIfConfigOfAttributes()`.
35+
return .ifConfigDecl(
36+
self.parsePoundIfDirective { (parser, _) -> RawAttributeListSyntax.Element in
37+
return parser.parseAttributeListElement()
38+
} syntax: { parser, attributes in
39+
return .attributes(RawAttributeListSyntax(elements: attributes, arena: parser.arena))
40+
}
41+
)
42+
} else {
43+
return .attribute(self.parseAttribute())
2344
}
45+
}
2446

47+
mutating func parseAttributeList() -> RawAttributeListSyntax {
2548
var elements = [RawAttributeListSyntax.Element]()
49+
50+
func shouldContinue() -> Bool {
51+
if self.at(.atSign) {
52+
return true
53+
}
54+
if self.at(.poundIf) && self.withLookahead({ $0.consumeIfConfigOfAttributes() }) {
55+
return true
56+
}
57+
return false
58+
}
59+
2660
var loopProgress = LoopProgressCondition()
27-
repeat {
28-
let attribute = self.parseAttribute()
61+
while self.hasProgressed(&loopProgress) && shouldContinue() {
62+
let attribute = self.parseAttributeListElement()
2963
elements.append(attribute)
30-
} while self.at(.atSign, .poundIf) && self.hasProgressed(&loopProgress)
31-
return RawAttributeListSyntax(elements: elements, arena: self.arena)
64+
}
65+
if elements.isEmpty {
66+
return self.emptyCollection(RawAttributeListSyntax.self)
67+
} else {
68+
return RawAttributeListSyntax(elements: elements, arena: self.arena)
69+
}
3270
}
3371
}
3472

@@ -148,7 +186,7 @@ extension Parser {
148186
parseMissingArguments: (
149187
(inout Parser) -> (unexpectedBefore: RawUnexpectedNodesSyntax?, arguments: RawAttributeSyntax.Arguments)
150188
)? = nil
151-
) -> RawAttributeListSyntax.Element {
189+
) -> RawAttributeSyntax {
152190
var (unexpectedBeforeAtSign, atSign) = self.expect(.atSign)
153191
if atSign.trailingTriviaByteLength > 0 || self.currentToken.leadingTriviaByteLength > 0 {
154192
let diagnostic = TokenDiagnostic(
@@ -163,9 +201,7 @@ extension Parser {
163201
case .required:
164202
shouldParseArgument = true
165203
case .customAttribute:
166-
shouldParseArgument =
167-
self.withLookahead { $0.atAttributeOrSpecifierArgument() }
168-
&& self.at(TokenSpec(.leftParen, allowAtStartOfLine: false))
204+
shouldParseArgument = self.withLookahead { $0.atAttributeOrSpecifierArgument() }
169205
case .optional:
170206
shouldParseArgument = self.at(.leftParen)
171207
case .noArgument:
@@ -190,46 +226,32 @@ extension Parser {
190226
(unexpectedBeforeArguments, argument) = parseArguments(&self)
191227
}
192228
let (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
193-
return .attribute(
194-
RawAttributeSyntax(
195-
unexpectedBeforeAtSign,
196-
atSign: atSign,
197-
attributeName: attributeName,
198-
unexpectedBeforeLeftParen,
199-
leftParen: leftParen,
200-
unexpectedBeforeArguments,
201-
arguments: argument,
202-
unexpectedBeforeRightParen,
203-
rightParen: rightParen,
204-
arena: self.arena
205-
)
229+
return RawAttributeSyntax(
230+
unexpectedBeforeAtSign,
231+
atSign: atSign,
232+
attributeName: attributeName,
233+
unexpectedBeforeLeftParen,
234+
leftParen: leftParen,
235+
unexpectedBeforeArguments,
236+
arguments: argument,
237+
unexpectedBeforeRightParen,
238+
rightParen: rightParen,
239+
arena: self.arena
206240
)
207241
} else {
208-
return .attribute(
209-
RawAttributeSyntax(
210-
unexpectedBeforeAtSign,
211-
atSign: atSign,
212-
attributeName: attributeName,
213-
leftParen: nil,
214-
arguments: nil,
215-
rightParen: nil,
216-
arena: self.arena
217-
)
242+
return RawAttributeSyntax(
243+
unexpectedBeforeAtSign,
244+
atSign: atSign,
245+
attributeName: attributeName,
246+
leftParen: nil,
247+
arguments: nil,
248+
rightParen: nil,
249+
arena: self.arena
218250
)
219251
}
220252
}
221253

222-
mutating func parseAttribute() -> RawAttributeListSyntax.Element {
223-
if self.at(.poundIf) {
224-
return .ifConfigDecl(
225-
self.parsePoundIfDirective { (parser, _) -> RawAttributeListSyntax.Element in
226-
return parser.parseAttribute()
227-
} syntax: { parser, attributes in
228-
return .attributes(RawAttributeListSyntax(elements: attributes, arena: parser.arena))
229-
}
230-
)
231-
}
232-
254+
mutating func parseAttribute() -> RawAttributeSyntax {
233255
switch peek(isAtAnyIn: DeclarationAttributeWithSpecialSyntax.self) {
234256
case .abi:
235257
return parseAttribute(argumentMode: .required) { parser in
@@ -299,17 +321,15 @@ extension Parser {
299321
case .rethrows:
300322
let (unexpectedBeforeAtSign, atSign) = self.expect(.atSign)
301323
let (unexpectedBeforeAttributeName, attributeName) = self.expect(TokenSpec(.rethrows, remapping: .identifier))
302-
return .attribute(
303-
RawAttributeSyntax(
304-
unexpectedBeforeAtSign,
305-
atSign: atSign,
306-
unexpectedBeforeAttributeName,
307-
attributeName: RawIdentifierTypeSyntax(name: attributeName, genericArgumentClause: nil, arena: self.arena),
308-
leftParen: nil,
309-
arguments: nil,
310-
rightParen: nil,
311-
arena: self.arena
312-
)
324+
return RawAttributeSyntax(
325+
unexpectedBeforeAtSign,
326+
atSign: atSign,
327+
unexpectedBeforeAttributeName,
328+
attributeName: RawIdentifierTypeSyntax(name: attributeName, genericArgumentClause: nil, arena: self.arena),
329+
leftParen: nil,
330+
arguments: nil,
331+
rightParen: nil,
332+
arena: self.arena
313333
)
314334
case .Sendable:
315335
return parseAttribute(argumentMode: .noArgument) { parser in
@@ -1023,6 +1043,10 @@ extension Parser {
10231043

10241044
extension Parser.Lookahead {
10251045
mutating func atAttributeOrSpecifierArgument() -> Bool {
1046+
if !self.at(TokenSpec(.leftParen, allowAtStartOfLine: false)) {
1047+
return false
1048+
}
1049+
10261050
var lookahead = self.lookahead()
10271051
lookahead.skipSingle()
10281052

@@ -1055,9 +1079,7 @@ extension Parser.Lookahead {
10551079
return false
10561080
}
10571081

1058-
if self.at(TokenSpec(.leftParen, allowAtStartOfLine: false))
1059-
&& self.withLookahead({ $0.atAttributeOrSpecifierArgument() })
1060-
{
1082+
if self.withLookahead({ $0.atAttributeOrSpecifierArgument() }) {
10611083
self.skipSingle()
10621084
}
10631085

‎Sources/SwiftParser/Declarations.swift

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,16 @@ extension TokenConsumer {
6565

6666
var hasAttribute = false
6767
var attributeProgress = LoopProgressCondition()
68-
while subparser.hasProgressed(&attributeProgress) && subparser.at(.atSign) {
69-
hasAttribute = true
70-
_ = subparser.consumeAttributeList()
68+
while subparser.hasProgressed(&attributeProgress) {
69+
if subparser.at(.atSign) {
70+
_ = subparser.consumeAttributeList()
71+
hasAttribute = true
72+
} else if subparser.at(.poundIf) && subparser.consumeIfConfigOfAttributes() {
73+
subparser.skipSingle()
74+
hasAttribute = true
75+
} else {
76+
break
77+
}
7178
}
7279

7380
var hasModifier = false
@@ -94,11 +101,6 @@ extension TokenConsumer {
94101
}
95102
}
96103

97-
if subparser.at(.poundIf) {
98-
var attrLookahead = subparser.lookahead()
99-
return attrLookahead.consumeIfConfigOfAttributes()
100-
}
101-
102104
let declStartKeyword: DeclarationKeyword?
103105
if allowRecovery {
104106
declStartKeyword =
@@ -175,6 +177,10 @@ extension TokenConsumer {
175177
allowRecovery: allowRecovery
176178
)
177179
}
180+
if allowRecovery && (hasAttribute || hasModifier) {
181+
// If we found any attributes or modifiers, consider it's a missing decl.
182+
return true
183+
}
178184
return false
179185
}
180186
}
@@ -492,11 +498,11 @@ extension Parser {
492498
arena: self.arena
493499
)
494500

495-
if self.at(.atSign), case .attribute(let attribute) = self.parseAttribute() {
501+
if self.at(.atSign) {
496502
return RawUsingDeclSyntax(
497503
unexpectedBeforeUsingKeyword,
498504
usingKeyword: usingKeyword,
499-
specifier: .attribute(attribute),
505+
specifier: .attribute(self.parseAttribute()),
500506
arena: self.arena
501507
)
502508
}

‎Sources/SwiftParser/Lookahead.swift

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,8 @@ extension Parser.Lookahead {
221221
_ = self.consumeGenericArguments()
222222
} while self.consume(if: .period) != nil
223223

224-
if self.consume(if: .leftParen) != nil {
225-
while !self.at(.endOfFile, .rightParen, .poundEndif) {
226-
self.skipSingle()
227-
}
228-
self.consume(if: .rightParen)
224+
if self.atAttributeOrSpecifierArgument() {
225+
self.skipSingle()
229226
}
230227
}
231228
return true

‎Sources/SwiftParser/TokenSpecSet.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,9 @@ enum ContextualDeclKeyword: TokenSpecSet {
267267
}
268268
}
269269

270-
/// A `DeclarationKeyword` that is not a `ValueBindingPatternSyntax.BindingSpecifierOptions`.
270+
/// A `DeclarationKeyword` that is not a `VariableDeclSyntax.BindingSpecifierOptions`.
271271
///
272-
/// `ValueBindingPatternSyntax.BindingSpecifierOptions` are injected into
272+
/// `VariableDeclSyntax.BindingSpecifierOptions` are injected into
273273
/// `DeclarationKeyword` via an `EitherTokenSpecSet`.
274274
enum PureDeclarationKeyword: TokenSpecSet {
275275
case actor
@@ -344,7 +344,7 @@ enum PureDeclarationKeyword: TokenSpecSet {
344344

345345
typealias DeclarationKeyword = EitherTokenSpecSet<
346346
PureDeclarationKeyword,
347-
ValueBindingPatternSyntax.BindingSpecifierOptions
347+
VariableDeclSyntax.BindingSpecifierOptions
348348
>
349349

350350
enum DeclarationModifier: TokenSpecSet {

‎Sources/SwiftParser/TopLevel.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,10 @@ extension Parser {
251251
return self.parseStatementItem()
252252
} else if self.atStartOfExpression() {
253253
return .expr(self.parseExpression(flavor: .basic, pattern: .none))
254-
} else if self.atStartOfDeclaration(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl, allowRecovery: true) {
255-
return .decl(self.parseDeclaration())
256254
} else if self.atStartOfStatement(allowRecovery: true, preferExpr: false) {
257255
return self.parseStatementItem()
256+
} else if self.atStartOfDeclaration(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl, allowRecovery: true) {
257+
return .decl(self.parseDeclaration())
258258
} else {
259259
return .init(expr: RawMissingExprSyntax(arena: self.arena))
260260
}

‎Sources/SwiftParser/Types.swift

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,17 +1308,21 @@ extension Parser {
13081308
return .attribute(self.parseDifferentiableAttribute())
13091309

13101310
case .isolated:
1311-
return parseAttribute(argumentMode: .required) { parser in
1312-
return (nil, .argumentList(parser.parseIsolatedAttributeArguments()))
1313-
}
1311+
return .attribute(
1312+
parseAttribute(argumentMode: .required) { parser in
1313+
return (nil, .argumentList(parser.parseIsolatedAttributeArguments()))
1314+
}
1315+
)
13141316
case .convention, ._opaqueReturnTypeOf, nil: // Custom attribute
1315-
return parseAttribute(argumentMode: .customAttribute) { parser in
1316-
let arguments = parser.parseArgumentListElements(
1317-
pattern: .none,
1318-
allowTrailingComma: true
1319-
)
1320-
return (nil, .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena)))
1321-
}
1317+
return .attribute(
1318+
parseAttribute(argumentMode: .customAttribute) { parser in
1319+
let arguments = parser.parseArgumentListElements(
1320+
pattern: .none,
1321+
allowTrailingComma: true
1322+
)
1323+
return (nil, .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena)))
1324+
}
1325+
)
13221326

13231327
}
13241328
}

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -482,21 +482,36 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
482482
otherNode.lastToken(viewMode: .sourceAccurate)?.tokenKind == .poundEndif
483483
{
484484
let diagnoseOn = parent.parent ?? parent
485-
addDiagnostic(
486-
diagnoseOn,
487-
IfConfigDeclNotAllowedInContext(context: diagnoseOn),
488-
highlights: [Syntax(node), Syntax(otherNode)],
489-
fixIts: [
490-
FixIt(
491-
message: RemoveNodesFixIt([Syntax(node), Syntax(otherNode)]),
492-
changes: [
493-
.makeMissing([Syntax(node)], transferTrivia: false),
494-
.makeMissing([Syntax(otherNode)], transferTrivia: false),
495-
]
496-
)
497-
],
498-
handledNodes: [node.id, otherNode.id]
499-
)
485+
if node == otherNode {
486+
addDiagnostic(
487+
diagnoseOn,
488+
IfConfigDeclNotAllowedInContext(context: diagnoseOn),
489+
highlights: [Syntax(node)],
490+
fixIts: [
491+
FixIt(
492+
message: RemoveNodesFixIt([Syntax(node)]),
493+
changes: .makeMissing([Syntax(node)], transferTrivia: false)
494+
)
495+
],
496+
handledNodes: [node.id]
497+
)
498+
} else {
499+
addDiagnostic(
500+
diagnoseOn,
501+
IfConfigDeclNotAllowedInContext(context: diagnoseOn),
502+
highlights: [Syntax(node), Syntax(otherNode)],
503+
fixIts: [
504+
FixIt(
505+
message: RemoveNodesFixIt([Syntax(node), Syntax(otherNode)]),
506+
changes: [
507+
.makeMissing([Syntax(node)], transferTrivia: false),
508+
.makeMissing([Syntax(otherNode)], transferTrivia: false),
509+
]
510+
)
511+
],
512+
handledNodes: [node.id, otherNode.id]
513+
)
514+
}
500515
} else {
501516
addDiagnostic(node, UnexpectedNodesError(unexpectedNodes: node), highlights: [Syntax(node)])
502517
}

0 commit comments

Comments
 (0)