Skip to content

Commit 72fc098

Browse files
committed
Run ParseDiagnosticGenerator on tree with flipped token presence
This uncovered a couple of implicit assumptions, mostly around the fact that tokens insided `UnexpectedNodesSyntax` are present, which isn’t true in general for manually generated trees. It also uncovered an issue where we weren’t able to retrieve the trivia pieces of a token after it had been modified using `with` because after the modification the token was a `parsedToken` that no longer resided in a `ParsingSyntaxArena`.
1 parent ccadf41 commit 72fc098

File tree

8 files changed

+75
-34
lines changed

8 files changed

+75
-34
lines changed

CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ public let DECL_NODES: [Node] = [
464464
An editor placeholder, e.g. `<#declaration#>` that is used in a position that expects a declaration.
465465
""",
466466
kind: "Decl",
467-
traits:[
467+
traits: [
468468
"WithAttributes",
469469
"WithModifiers",
470470
],

Sources/SwiftParserDiagnostics/MissingTokenError.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import SwiftDiagnostics
1717
extension ParseDiagnosticsGenerator {
1818
func handleMissingToken(_ missingToken: TokenSyntax) {
1919
guard let invalidToken = missingToken.previousToken(viewMode: .all),
20+
invalidToken.presence == .present,
2021
let invalidTokenContainer = invalidToken.parent?.as(UnexpectedNodesSyntax.self),
2122
invalidTokenContainer.count == 1
2223
else {

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
133133
removeRedundantFixIt: (_ misplacedTokens: [TokenSyntax]) -> FixItMessage? = { _ in nil }
134134
) {
135135
guard let incorrectContainer = unexpected,
136-
let misplacedTokens = incorrectContainer.onlyTokens(satisfying: unexpectedTokenCondition)
136+
let misplacedTokens = incorrectContainer.onlyPresentTokens(satisfying: unexpectedTokenCondition)
137137
else {
138138
// If there are no unexpected nodes or the unexpected contain multiple tokens, don't emit a diagnostic.
139139
return
@@ -180,7 +180,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
180180
message: (TokenSyntax) -> Message
181181
) {
182182
guard let unexpected = unexpected,
183-
let misplacedToken = unexpected.onlyToken(where: predicate)
183+
let misplacedToken = unexpected.onlyPresentToken(where: predicate)
184184
else {
185185
// If there is no unexpected node or the unexpected doesn't have the
186186
// expected token, don't emit a diagnostic.
@@ -262,7 +262,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
262262
}
263263
if specifier.presence == .present {
264264
for case .some(let unexpected) in unexpectedNodes {
265-
for duplicateSpecifier in unexpected.tokens(satisfying: isOfSameKind) {
265+
for duplicateSpecifier in unexpected.presentTokens(satisfying: isOfSameKind) {
266266
addDiagnostic(
267267
duplicateSpecifier,
268268
DuplicateEffectSpecifiers(correctSpecifier: specifier, unexpectedSpecifier: duplicateSpecifier),
@@ -298,29 +298,36 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
298298
suppressRemainingDiagnostics = true
299299
return .skipChildren
300300
}
301-
if let tryKeyword = node.onlyToken(where: { $0.tokenKind == .keyword(.try) }),
301+
if let tryKeyword = node.onlyPresentToken(where: { $0.tokenKind == .keyword(.try) }),
302302
let nextToken = tryKeyword.nextToken(viewMode: .sourceAccurate),
303303
nextToken.tokenKind.isLexerClassifiedKeyword,
304304
!(node.parent?.is(TypeEffectSpecifiersSyntax.self) ?? false)
305305
{
306306
addDiagnostic(node, TryCannotBeUsed(nextToken: nextToken))
307-
} else if let semicolons = node.onlyTokens(satisfying: { $0.tokenKind == .semicolon }) {
307+
} else if let semicolons = node.onlyPresentTokens(satisfying: { $0.tokenKind == .semicolon }) {
308308
addDiagnostic(
309309
node,
310310
.unexpectedSemicolon,
311311
fixIts: [
312312
FixIt(message: RemoveNodesFixIt(semicolons), changes: semicolons.map { FixIt.MultiNodeChange.makeMissing($0) })
313313
]
314314
)
315-
} else if node.first?.as(TokenSyntax.self)?.tokenKind.isIdentifier == true,
315+
} else if let firstToken = node.first?.as(TokenSyntax.self),
316+
firstToken.tokenKind.isIdentifier == true,
317+
firstToken.presence == .present,
316318
let previousToken = node.previousToken(viewMode: .sourceAccurate),
317319
previousToken.tokenKind.isIdentifier,
318320
previousToken.parent?.is(DeclSyntax.self) == true || previousToken.parent?.is(IdentifierPatternSyntax.self) == true
319321
{
320322
// If multiple identifiers are used for a declaration name, offer to join them together.
321323
let tokens =
322324
node
323-
.prefix(while: { $0.as(TokenSyntax.self)?.tokenKind.isIdentifier == true })
325+
.prefix(while: {
326+
guard let token = $0.as(TokenSyntax.self) else {
327+
return false
328+
}
329+
return token.tokenKind.isIdentifier == true && token.presence == .present
330+
})
324331
.map({ $0.as(TokenSyntax.self)! })
325332
let joined = previousToken.text + tokens.map(\.text).joined()
326333
var fixIts: [FixIt] = [
@@ -449,7 +456,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
449456
return .skipChildren
450457
}
451458
if let unexpected = node.unexpectedBetweenPlatformAndVersion,
452-
unexpected.onlyToken(where: { $0.tokenKind == .binaryOperator(">=") }) != nil
459+
unexpected.onlyPresentToken(where: { $0.tokenKind == .binaryOperator(">=") }) != nil
453460
{
454461
addDiagnostic(
455462
unexpected,
@@ -552,7 +559,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
552559
}
553560

554561
if let unexpected = node.unexpectedBetweenBodyAndTrailingComma,
555-
let token = unexpected.tokens(satisfying: { $0.tokenKind == .binaryOperator("&&") }).first,
562+
let token = unexpected.presentTokens(satisfying: { $0.tokenKind == .binaryOperator("&&") }).first,
556563
let trailingComma = node.trailingComma,
557564
trailingComma.presence == .missing,
558565
let previous = node.unexpectedBetweenBodyAndTrailingComma?.previousToken(viewMode: .sourceAccurate)
@@ -583,7 +590,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
583590
return .skipChildren
584591
}
585592
if let unexpected = node.unexpectedBetweenDeinitKeywordAndBody,
586-
let name = unexpected.filter({ $0.as(TokenSyntax.self)?.tokenKind.isIdentifier == true }).only?.as(TokenSyntax.self)
593+
let name = unexpected.presentTokens(satisfying: { $0.tokenKind.isIdentifier == true }).only?.as(TokenSyntax.self)
587594
{
588595
addDiagnostic(
589596
name,
@@ -617,7 +624,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
617624
// Detect C-style for loops based on two semicolons which could not be parsed between the 'for' keyword and the '{'
618625
// This is mostly a proof-of-concept implementation to produce more complex diagnostics.
619626
if let unexpectedCondition = node.body.unexpectedBeforeLeftBrace,
620-
unexpectedCondition.tokens(withKind: .semicolon).count == 2
627+
unexpectedCondition.presentTokens(withKind: .semicolon).count == 2
621628
{
622629
// FIXME: This is aweful. We should have a way to either get all children between two cursors in a syntax node or highlight a range from one node to another.
623630
addDiagnostic(
@@ -690,7 +697,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
690697
message: { _ in .typeParameterPackEllipsis }
691698
)
692699
} else if let unexpected = node.unexpectedBetweenNameAndColon,
693-
let unexpectedEllipsis = unexpected.onlyToken(where: { $0.tokenKind == .ellipsis }),
700+
let unexpectedEllipsis = unexpected.onlyPresentToken(where: { $0.tokenKind == .ellipsis }),
694701
let each = node.each
695702
{
696703
addDiagnostic(
@@ -929,7 +936,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
929936
if shouldSkip(node) {
930937
return .skipChildren
931938
}
932-
if let token = node.unexpectedBetweenModuleLabelAndColon?.onlyToken(where: { $0.tokenKind.isIdentifier }),
939+
if let token = node.unexpectedBetweenModuleLabelAndColon?.onlyPresentToken(where: { $0.tokenKind.isIdentifier }),
933940
node.moduleLabel.presence == .missing
934941
{
935942
addDiagnostic(
@@ -976,9 +983,9 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
976983
return
977984
}
978985
let message: DiagnosticMessage?
979-
if let identifier = unexpected.onlyToken(where: { $0.tokenKind.isIdentifier }) {
986+
if let identifier = unexpected.onlyPresentToken(where: { $0.tokenKind.isIdentifier }) {
980987
message = IdentifierNotAllowedInOperatorName(identifier: identifier)
981-
} else if let tokens = unexpected.onlyTokens(satisfying: { _ in true }) {
988+
} else if let tokens = unexpected.onlyPresentTokens(satisfying: { _ in true }) {
982989
message = TokensNotAllowedInOperatorName(tokens: tokens)
983990
} else {
984991
message = nil
@@ -1068,7 +1075,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
10681075
if shouldSkip(node) {
10691076
return .skipChildren
10701077
}
1071-
if let singleQuote = node.unexpectedBetweenOpenDelimiterAndOpenQuote?.onlyToken(where: { $0.tokenKind == .singleQuote }) {
1078+
if let singleQuote = node.unexpectedBetweenOpenDelimiterAndOpenQuote?.onlyPresentToken(where: { $0.tokenKind == .singleQuote }) {
10721079
let fixIt = FixIt(
10731080
message: ReplaceTokensFixIt(replaceTokens: [singleQuote], replacement: node.openQuote),
10741081
changes: [
@@ -1094,7 +1101,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
10941101
addDiagnostic(diagnostic, handledNodes: handledNodes)
10951102
}
10961103
if case .stringSegment(let segment) = node.segments.last {
1097-
if let invalidContent = segment.unexpectedBeforeContent?.onlyToken(where: { $0.trailingTrivia.contains(where: { $0.isBackslash }) }) {
1104+
if let invalidContent = segment.unexpectedBeforeContent?.onlyPresentToken(where: { $0.trailingTrivia.contains(where: { $0.isBackslash }) }) {
10981105
let fixIt = FixIt(
10991106
message: .removeBackslash,
11001107
changes: [
@@ -1119,7 +1126,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
11191126
return .skipChildren
11201127
}
11211128
if let unexpected = node.unexpectedBetweenSubscriptKeywordAndGenericParameterClause,
1122-
let nameTokens = unexpected.onlyTokens(satisfying: { !$0.tokenKind.isLexerClassifiedKeyword })
1129+
let nameTokens = unexpected.onlyPresentTokens(satisfying: { !$0.tokenKind.isLexerClassifiedKeyword })
11231130
{
11241131
addDiagnostic(
11251132
unexpected,
@@ -1131,7 +1138,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
11311138
)
11321139
}
11331140
if let unexpected = node.indices.unexpectedBeforeLeftParen,
1134-
let nameTokens = unexpected.onlyTokens(satisfying: { !$0.tokenKind.isLexerClassifiedKeyword })
1141+
let nameTokens = unexpected.onlyPresentTokens(satisfying: { !$0.tokenKind.isLexerClassifiedKeyword })
11351142
{
11361143
addDiagnostic(
11371144
unexpected,
@@ -1255,7 +1262,8 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
12551262
return .skipChildren
12561263
}
12571264

1258-
if let token = node.unexpectedBetweenMessageLabelAndColon?.onlyToken(where: { $0.tokenKind.isIdentifier }),
1265+
if let token = node.unexpectedBetweenMessageLabelAndColon?.onlyPresentToken(where: { $0.tokenKind.isIdentifier }),
1266+
token.presence == .present,
12591267
node.messageLabel.presence == .missing
12601268
{
12611269
addDiagnostic(

Sources/SwiftParserDiagnostics/SyntaxExtensions.swift

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,30 @@
1414
@_spi(Diagnostics) import SwiftParser
1515

1616
extension UnexpectedNodesSyntax {
17-
func tokens(satisfying isIncluded: (TokenSyntax) -> Bool) -> [TokenSyntax] {
17+
func presentTokens(satisfying isIncluded: (TokenSyntax) -> Bool) -> [TokenSyntax] {
1818
return self.children(viewMode: .sourceAccurate).compactMap({ $0.as(TokenSyntax.self) }).filter(isIncluded)
1919
}
2020

21-
func tokens(withKind kind: TokenKind) -> [TokenSyntax] {
22-
return self.tokens(satisfying: { $0.tokenKind == kind })
21+
func presentTokens(withKind kind: TokenKind) -> [TokenSyntax] {
22+
return self.presentTokens(satisfying: { $0.tokenKind == kind })
2323
}
2424

25-
/// If this only contains a single item, which is a token satisfying `condition`, return that token, otherwise return `nil`.
26-
func onlyToken(where condition: (TokenSyntax) -> Bool) -> TokenSyntax? {
27-
if self.count == 1, let token = self.first?.as(TokenSyntax.self), condition(token) {
25+
/// If this only contains a single item, which is a present token satisfying `condition`, return that token, otherwise return `nil`.
26+
func onlyPresentToken(where condition: (TokenSyntax) -> Bool) -> TokenSyntax? {
27+
if self.count == 1,
28+
let token = self.first?.as(TokenSyntax.self),
29+
condition(token),
30+
token.presence == .present
31+
{
2832
return token
2933
} else {
3034
return nil
3135
}
3236
}
3337

34-
/// If this only contains tokens satisfying `condition`, return an array containing those tokens, otherwise return `nil`.
35-
func onlyTokens(satisfying condition: (TokenSyntax) -> Bool) -> [TokenSyntax]? {
36-
let tokens = tokens(satisfying: condition)
38+
/// If this only contains present tokens satisfying `condition`, return an array containing those tokens, otherwise return `nil`.
39+
func onlyPresentTokens(satisfying condition: (TokenSyntax) -> Bool) -> [TokenSyntax]? {
40+
let tokens = presentTokens(satisfying: condition)
3741
if tokens.count == self.count {
3842
return tokens
3943
} else {
@@ -79,7 +83,7 @@ extension SyntaxProtocol {
7983
} else {
8084
return "braces"
8185
}
82-
} else if let token = Syntax(self).as(UnexpectedNodesSyntax.self)?.onlyTokens(satisfying: { $0.tokenKind.isLexerClassifiedKeyword })?.only {
86+
} else if let token = Syntax(self).as(UnexpectedNodesSyntax.self)?.onlyPresentTokens(satisfying: { $0.tokenKind.isLexerClassifiedKeyword })?.only {
8387
return "'\(token.text)' keyword"
8488
} else if let token = Syntax(self).as(TokenSyntax.self) {
8589
return "'\(token.text)' keyword"

Sources/SwiftSyntax/Raw/RawSyntaxTokenView.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public struct RawSyntaxTokenView {
9595
public var leadingRawTriviaPieces: [RawTriviaPiece] {
9696
switch raw.rawData.payload {
9797
case .parsedToken(let dat):
98-
let arena = raw.arena as! ParsingSyntaxArena
98+
let arena = raw.arena.parsingArena!
9999
return arena.parseTrivia(source: dat.leadingTriviaText, position: .leading)
100100
case .materializedToken(let dat):
101101
return Array(dat.leadingTrivia)
@@ -108,7 +108,7 @@ public struct RawSyntaxTokenView {
108108
public var trailingRawTriviaPieces: [RawTriviaPiece] {
109109
switch raw.rawData.payload {
110110
case .parsedToken(let dat):
111-
let arena = raw.arena as! ParsingSyntaxArena
111+
let arena = raw.arena.parsingArena!
112112
return arena.parseTrivia(source: dat.trailingTriviaText, position: .trailing)
113113
case .materializedToken(let dat):
114114
return Array(dat.trailingTrivia)

Sources/SwiftSyntax/SyntaxArena.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,33 @@ public class SyntaxArena {
6969
}
7070
}
7171

72+
/// If this arena or any of its child arenas is a ``ParsingSyntaxArena``
73+
/// return one of these arenas, otherwise return `nil`.
74+
///
75+
/// If the arena has multiple child nodes that are ``ParsingSyntaxArena``s, it
76+
/// is undefined which one will be returned.
77+
///
78+
/// The use case for this is to get the trivia parsing function of the arena.
79+
/// Parsed tokens created by `SwiftParser` automatically reside in a
80+
/// ``ParsingSyntaxArena`` but if they are modified (e.g. using the `with`
81+
/// functions), they might reside in a new arena. But we still want to be able
82+
/// to retrieve trivia from those modified tokens, which requires calling into
83+
/// the `parseTrivia` function of the ``ParsingSyntaxArena`` that created the
84+
/// token. Since the modified syntax arena needs to keep the original
85+
/// ``ParsingSyntaxArena`` alive, we can search this arena’s `childRefs` for
86+
/// the ``ParsingSyntaxArena`` that created the token.
87+
var parsingArena: ParsingSyntaxArena? {
88+
if let parsingArena = self as? ParsingSyntaxArena {
89+
return parsingArena
90+
}
91+
for child in childRefs {
92+
if let parsingArena = child.value.parsingArena {
93+
return parsingArena
94+
}
95+
}
96+
return nil
97+
}
98+
7299
/// Allocates a buffer of `RawSyntax?` with the given count, then returns the
73100
/// uninitlialized memory range as a `UnsafeMutableBufferPointer<RawSyntax?>`.
74101
func allocateRawSyntaxBuffer(count: Int) -> UnsafeMutableBufferPointer<RawSyntax?> {

Sources/SwiftSyntax/generated/SyntaxRewriter.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6997,7 +6997,7 @@ open class SyntaxRewriter {
69976997
public func rewrite(_ node: Syntax) -> Syntax {
69986998
let rewritten = self.visit(node)
69996999
return withExtendedLifetime(rewritten) {
7000-
return Syntax(node.data.replacingSelf(rewritten.raw, arena: rewritten.raw.arena))
7000+
return Syntax(node.data.replacingSelf(rewritten.raw, arena: SyntaxArena()))
70017001
}
70027002
}
70037003
}

Tests/SwiftParserTest/Assertions.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,7 @@ func assertParse<S: SyntaxProtocol>(
681681
if enableLongTests {
682682
DispatchQueue.concurrentPerform(iterations: Array(tree.tokens(viewMode: .all)).count) { tokenIndex in
683683
let flippedTokenTree = TokenPresenceFlipper(flipTokenAtIndex: tokenIndex).rewrite(Syntax(tree))
684+
_ = ParseDiagnosticsGenerator.diagnostics(for: flippedTokenTree)
684685
assertRoundTrip(source: flippedTokenTree.syntaxTextBytes, parse, file: file, line: line)
685686
}
686687

0 commit comments

Comments
 (0)