Skip to content

Commit ca63a16

Browse files
rintarorintaro
committed
[SwiftParser] Improve attribute parsing and recovering
Attributes followed by non-attribute `#if` were not `atStartOfDeclaration()` nor `parseStatementItem()`, so the rest of the code including the attribute were parsed as "unexpected code". * Improve attibute list checking in `atStartOfDeclaration()` and parse attributes without decl introducer as `MissingDeclSyntax` * Make sure `#if` contains only attributes before parsing them as an attribute list element * Remove `#if` parsing from `parseAttribute()`, and introduce `parseAttributeListElement()` for attribute list element parsing including `#if`. This prevents a crash when `#if` is passed to `AttributeSyntax.parse(from:)` #3134
1 parent 28dfc47 commit ca63a16

File tree

9 files changed

+217
-96
lines changed

9 files changed

+217
-96
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/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
}

‎Tests/SwiftParserTest/AttributeTests.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1400,4 +1400,17 @@ final class AttributeTests: ParserTestCase {
14001400
"""
14011401
)
14021402
}
1403+
1404+
func testAttributeParsable() {
1405+
let source = """
1406+
#if true
1407+
@discardableResult
1408+
#endif
1409+
"""
1410+
var parser = Parser(source)
1411+
let attr = AttributeSyntax.parse(from: &parser)
1412+
1413+
XCTAssertTrue(attr.description == source)
1414+
XCTAssertTrue(attr.tokens(viewMode: .fixedUp).allSatisfy({ $0.presence == .missing }))
1415+
}
14031416
}

‎Tests/SwiftParserTest/DeclarationTests.swift

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -567,11 +567,15 @@ final class DeclarationTests: ParserTestCase {
567567

568568
assertParse(
569569
"""
570-
1️⃣private(
570+
private(1️⃣
571571
""",
572572
diagnostics: [
573-
DiagnosticSpec(message: "extraneous code 'private(' at top level")
574-
]
573+
DiagnosticSpec(message: "expected 'set)' to end modifier", fixIts: ["insert 'set)'"]),
574+
DiagnosticSpec(message: "expected declaration after 'private' modifier", fixIts: ["insert declaration"]),
575+
],
576+
fixedSource: """
577+
private(set) <#declaration#>
578+
"""
575579
)
576580

577581
assertParse(

0 commit comments

Comments
 (0)