Skip to content

Commit b7a6b4e

Browse files
beccadaxbeccadax
committed
Improve module selector (and other) diagnostics
Add tailored diagnostics for unexpected module selectors which offer either one or two fix-its: • Remove the module selector • Convert `Foo::bar` to `bar = Foo::bar` (in certain declaration syntaxes) Making these messages clear required adding `nameForDiagnostics` properties to a bunch of children, which also impacted other existing diagnostics (for the better, IMHO). If we don’t like those changes or think they need more work, this commit can be severed from the rest of the PR.
1 parent 7a707ad commit b7a6b4e

16 files changed

+761
-301
lines changed

‎CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ public let DECL_NODES: [Node] = [
157157
Node(
158158
kind: .accessorParameters,
159159
base: .syntax,
160-
nameForDiagnostics: nil,
160+
nameForDiagnostics: "accessor parameter",
161161
traits: [
162162
"Parenthesized"
163163
],
@@ -209,6 +209,7 @@ public let DECL_NODES: [Node] = [
209209
Child(
210210
name: "name",
211211
kind: .token(choices: [.token(.identifier)]),
212+
nameForDiagnostics: "name",
212213
documentation: "The name of the actor. If the name matches a reserved keyword use backticks to escape it."
213214
),
214215
Child(
@@ -301,6 +302,7 @@ public let DECL_NODES: [Node] = [
301302
Child(
302303
name: "name",
303304
kind: .token(choices: [.token(.identifier)]),
305+
nameForDiagnostics: "name",
304306
documentation: "The name of this associated type."
305307
),
306308
Child(
@@ -389,6 +391,7 @@ public let DECL_NODES: [Node] = [
389391
Child(
390392
name: "name",
391393
kind: .token(choices: [.token(.identifier)]),
394+
nameForDiagnostics: "name",
392395
documentation: "The name of the class."
393396
),
394397
Child(
@@ -770,6 +773,7 @@ public let DECL_NODES: [Node] = [
770773
Child(
771774
name: "name",
772775
kind: .token(choices: [.token(.identifier)]),
776+
nameForDiagnostics: "name",
773777
documentation: "The name of this case."
774778
),
775779
Child(
@@ -833,6 +837,7 @@ public let DECL_NODES: [Node] = [
833837
Child(
834838
name: "name",
835839
kind: .token(choices: [.token(.identifier)]),
840+
nameForDiagnostics: "name",
836841
documentation:
837842
"Declares the name of this enum. If the name matches a reserved keyword use backticks to escape it."
838843
),
@@ -1009,6 +1014,7 @@ public let DECL_NODES: [Node] = [
10091014
.token(.prefixOperator),
10101015
.token(.postfixOperator),
10111016
]),
1017+
nameForDiagnostics: "name",
10121018
documentation: "The name of the function. If the name matches a reserved keyword use backticks to escape it."
10131019
),
10141020
Child(
@@ -1492,7 +1498,8 @@ public let DECL_NODES: [Node] = [
14921498
),
14931499
Child(
14941500
name: "name",
1495-
kind: .token(choices: [.token(.identifier)])
1501+
kind: .token(choices: [.token(.identifier)]),
1502+
nameForDiagnostics: "name"
14961503
),
14971504
Child(
14981505
name: "genericParameterClause",
@@ -1678,7 +1685,8 @@ public let DECL_NODES: [Node] = [
16781685
),
16791686
Child(
16801687
name: "name",
1681-
kind: .token(choices: [.token(.binaryOperator), .token(.prefixOperator), .token(.postfixOperator)])
1688+
kind: .token(choices: [.token(.binaryOperator), .token(.prefixOperator), .token(.postfixOperator)]),
1689+
nameForDiagnostics: "custom operator"
16821690
),
16831691
Child(
16841692
name: "operatorPrecedenceAndTypes",
@@ -1971,7 +1979,7 @@ public let DECL_NODES: [Node] = [
19711979
Node(
19721980
kind: .precedenceGroupDecl,
19731981
base: .decl,
1974-
nameForDiagnostics: "precedencegroup",
1982+
nameForDiagnostics: "precedence group",
19751983
documentation: "A Swift `precedencegroup` declaration.",
19761984
traits: [
19771985
"Braced",
@@ -1999,6 +2007,7 @@ public let DECL_NODES: [Node] = [
19992007
Child(
20002008
name: "name",
20012009
kind: .token(choices: [.token(.identifier)]),
2010+
nameForDiagnostics: "name",
20022011
documentation: "The name of this precedence group."
20032012
),
20042013
Child(
@@ -2119,6 +2128,7 @@ public let DECL_NODES: [Node] = [
21192128
Child(
21202129
name: "name",
21212130
kind: .token(choices: [.token(.identifier)]),
2131+
nameForDiagnostics: "name",
21222132
documentation: "The name of the protocol."
21232133
),
21242134
Child(
@@ -2299,6 +2309,7 @@ public let DECL_NODES: [Node] = [
22992309
Child(
23002310
name: "name",
23012311
kind: .token(choices: [.token(.identifier)]),
2312+
nameForDiagnostics: "name",
23022313
documentation:
23032314
"Declares the name of this struct. If the name matches a reserved keyword use backticks to escape it."
23042315
),
@@ -2465,7 +2476,8 @@ public let DECL_NODES: [Node] = [
24652476
),
24662477
Child(
24672478
name: "name",
2468-
kind: .token(choices: [.token(.identifier)])
2479+
kind: .token(choices: [.token(.identifier)]),
2480+
nameForDiagnostics: "name"
24692481
),
24702482
Child(
24712483
name: "genericParameterClause",

‎CodeGeneration/Sources/SyntaxSupport/ExprNodes.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ public let EXPR_NODES: [Node] = [
773773
Node(
774774
kind: .discardAssignmentExpr,
775775
base: .expr,
776-
nameForDiagnostics: nil,
776+
nameForDiagnostics: "wildcard expression",
777777
documentation: """
778778
A `_` that discards a value inside an assignment.
779779
@@ -1516,7 +1516,7 @@ public let EXPR_NODES: [Node] = [
15161516
Node(
15171517
kind: .nilLiteralExpr,
15181518
base: .expr,
1519-
nameForDiagnostics: nil,
1519+
nameForDiagnostics: "'nil' keyword",
15201520
children: [
15211521
Child(
15221522
name: "nilKeyword",
@@ -1886,7 +1886,7 @@ public let EXPR_NODES: [Node] = [
18861886
Node(
18871887
kind: .superExpr,
18881888
base: .expr,
1889-
nameForDiagnostics: nil,
1889+
nameForDiagnostics: "'super' keyword",
18901890
children: [
18911891
Child(
18921892
name: "superKeyword",

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,71 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
497497
],
498498
handledNodes: [node.id, otherNode.id]
499499
)
500+
} else if let (moduleName, colonColon) = node.twoPresentTokens(
501+
firstSatisfying: { $0.tokenKind.isIdentifier },
502+
secondSatisfying: { $0.tokenKind == .colonColon }
503+
) {
504+
// Looks like a module selector.
505+
var fixIts: [FixIt] = []
506+
507+
func canExplicitlyInitializeVariable() -> TokenSyntax? {
508+
if let capture = node.parent?.as(ClosureCaptureSyntax.self),
509+
capture.unexpectedBeforeSpecifier == node || capture.unexpectedBetweenSpecifierAndName == node,
510+
capture.initializer == nil {
511+
return capture.name
512+
}
513+
514+
if let pattern = node.parent?.as(IdentifierPatternSyntax.self),
515+
pattern.unexpectedBeforeIdentifier == node,
516+
pattern.parent?.is(OptionalBindingConditionSyntax.self) ?? false {
517+
return pattern.identifier
518+
}
519+
520+
return nil
521+
}
522+
523+
if let explicitVarName = canExplicitlyInitializeVariable() {
524+
// Suggest turning `Foo::bar` into `bar = Foo::bar`
525+
fixIts.append(
526+
FixIt(
527+
message: ExplicitlyInitializeFixIt(newVariableName: explicitVarName),
528+
changes: [
529+
FixIt.MultiNodeChange(
530+
.replace(
531+
oldNode: Syntax(node),
532+
newNode: Syntax(
533+
UnexpectedNodesSyntax(
534+
[
535+
Syntax(
536+
explicitVarName
537+
.with(\.leadingTrivia, moduleName.leadingTrivia)
538+
.with(\.trailingTrivia, [.spaces(1)])
539+
),
540+
Syntax(TokenSyntax.equalToken(trailingTrivia: [.spaces(1)])),
541+
Syntax(moduleName.with(\.leadingTrivia, [])),
542+
Syntax(colonColon)
543+
]
544+
)
545+
)
546+
)
547+
)
548+
]
549+
)
550+
)
551+
}
552+
// Suggest removing the module selector.
553+
fixIts.append(
554+
FixIt(
555+
message: RemoveNodesFixIt([Syntax(node)]),
556+
changes: [.makeMissing(node, transferTrivia: true)]
557+
)
558+
)
559+
addDiagnostic(
560+
node,
561+
CannotUseModuleSelectorError(unexpectedNodes: node),
562+
highlights: [Syntax(node)],
563+
fixIts: fixIts
564+
)
500565
} else {
501566
addDiagnostic(node, UnexpectedNodesError(unexpectedNodes: node), highlights: [Syntax(node)])
502567
}

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift‎

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@
1313
#if compiler(>=6)
1414
public import SwiftDiagnostics
1515
@_spi(Diagnostics) internal import SwiftParser
16-
@_spi(RawSyntax) public import SwiftSyntax
16+
@_spi(ExperimentalLanguageFeatures) @_spi(RawSyntax) public import SwiftSyntax
1717
#else
1818
import SwiftDiagnostics
1919
@_spi(Diagnostics) import SwiftParser
20-
@_spi(RawSyntax) import SwiftSyntax
20+
@_spi(ExperimentalLanguageFeatures) @_spi(RawSyntax) import SwiftSyntax
2121
#endif
2222

2323
fileprivate let diagnosticDomain: String = "SwiftParser"
@@ -309,6 +309,60 @@ public struct CannotParseVersionTuple: ParserError {
309309
}
310310
}
311311

312+
public struct CannotUseModuleSelectorError: ParserError {
313+
public let unexpectedNodes: UnexpectedNodesSyntax
314+
315+
public var message: String {
316+
var qualifiedSyntaxDescription = "code"
317+
if let parent = unexpectedNodes.parent {
318+
if let parentDeclRef = parent.as(DeclReferenceExprSyntax.self),
319+
parentDeclRef.moduleSelector == nil,
320+
parentDeclRef.unexpectedBeforeModuleSelector == unexpectedNodes {
321+
// We only insert a module selector here if the `baseName` is some sort of exotic token that can't take a
322+
// module selector. Blame that token.
323+
qualifiedSyntaxDescription = self.exoticNameDescription(for: parentDeclRef.baseName)
324+
} else if let parentTypeName = parent.ancestorOrSelf(mapping: self.parentDescription(for:)) {
325+
qualifiedSyntaxDescription = parentTypeName
326+
// If not first child, and subsequent child has name, add it.
327+
let siblings = parent.children(viewMode: .sourceAccurate)
328+
if siblings.first(where: { $0.totalLength.utf8Length > 0 })?.id != unexpectedNodes.id,
329+
let nextSibling = siblings.suffix(from: siblings.index(of: self.unexpectedNodes)!).dropFirst().first,
330+
let nextSiblingChildName = nextSibling.childNameInParent {
331+
qualifiedSyntaxDescription += " \(nextSiblingChildName)"
332+
}
333+
} else if let parentTypeName = parent.ancestorOrSelf(mapping: self.parentDescription(for:)) {
334+
qualifiedSyntaxDescription = "code in \(parentTypeName)"
335+
}
336+
}
337+
338+
return "\(qualifiedSyntaxDescription) cannot be qualified with a module selector"
339+
}
340+
341+
private func parentDescription(for node: Syntax) -> String? {
342+
if let name = node.nodeTypeNameForDiagnostics(allowBlockNames: false) {
343+
return name
344+
}
345+
346+
// Special case for otherwise undistinguishable syntax inside an attribute's argument list.
347+
if node.keyPathInParent == \AttributeSyntax.arguments {
348+
return "attribute argument"
349+
}
350+
351+
return nil
352+
}
353+
354+
private func exoticNameDescription(for nameToken: TokenSyntax) -> String {
355+
switch nameToken.tokenKind {
356+
case .keyword(let keyword):
357+
return "'\(keyword.defaultText)' keyword"
358+
case .binaryOperator(let text), .dollarIdentifier(let text), .identifier(let text), .postfixOperator(let text), .prefixOperator(let text):
359+
return "\(nameToken.tokenKind.nameForDiagnostics) '\(text)'"
360+
default:
361+
return nameToken.tokenKind.nameForDiagnostics
362+
}
363+
}
364+
}
365+
312366
public struct DeclarationNotPermittedInContext: ParserError {
313367
public var missingDecl: MissingDeclSyntax
314368
public var invalidDecl: DeclSyntax
@@ -741,6 +795,14 @@ extension FixItMessage where Self == StaticParserFixIt {
741795
}
742796
}
743797

798+
public struct ExplicitlyInitializeFixIt: ParserFixIt {
799+
public let newVariableName: TokenSyntax
800+
801+
public var message: String {
802+
"explicitly initialize variable '\(newVariableName.text)'"
803+
}
804+
}
805+
744806
public struct InsertFixIt: ParserFixIt {
745807
public let tokenToBeInserted: TokenSyntax
746808

‎Sources/SwiftParserDiagnostics/SyntaxExtensions.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ extension SyntaxProtocol {
9090
if !allowBlockNames && (syntax.is(CodeBlockSyntax.self) || syntax.is(MemberBlockSyntax.self)) {
9191
return nil
9292
}
93+
if let relation = syntax.as(PrecedenceGroupRelationSyntax.self) {
94+
return "'\(relation.higherThanOrLowerThanLabel.text)' property of precedencegroup"
95+
}
9396
return syntax.kind.nameForDiagnostics
9497
}
9598

0 commit comments

Comments
 (0)