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.

Author: beccadax

CodeGeneration/Sources/SyntaxSupport/DeclNodes.swift

@@ -157,7 +157,7 @@ public let DECL_NODES: [Node] = [
   Node(
     kind: .accessorParameters,
     base: .syntax,
-    nameForDiagnostics: nil,
+    nameForDiagnostics: "accessor parameter",
     traits: [
       "Parenthesized"
     ],
@@ -209,6 +209,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation: "The name of the actor. If the name matches a reserved keyword use backticks to escape it."
       ),
       Child(
@@ -301,6 +302,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation: "The name of this associated type."
       ),
       Child(
@@ -389,6 +391,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation: "The name of the class."
       ),
       Child(
@@ -770,6 +773,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation: "The name of this case."
       ),
       Child(
@@ -833,6 +837,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation:
           "Declares the name of this enum. If the name matches a reserved keyword use backticks to escape it."
       ),
@@ -1009,6 +1014,7 @@ public let DECL_NODES: [Node] = [
           .token(.prefixOperator),
           .token(.postfixOperator),
         ]),
+        nameForDiagnostics: "name",
         documentation: "The name of the function. If the name matches a reserved keyword use backticks to escape it."
       ),
       Child(
@@ -1492,7 +1498,8 @@ public let DECL_NODES: [Node] = [
       ),
       Child(
         name: "name",
-        kind: .token(choices: [.token(.identifier)])
+        kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name"
       ),
       Child(
         name: "genericParameterClause",
@@ -1678,7 +1685,8 @@ public let DECL_NODES: [Node] = [
       ),
       Child(
         name: "name",
-        kind: .token(choices: [.token(.binaryOperator), .token(.prefixOperator), .token(.postfixOperator)])
+        kind: .token(choices: [.token(.binaryOperator), .token(.prefixOperator), .token(.postfixOperator)]),
+        nameForDiagnostics: "custom operator"
       ),
       Child(
         name: "operatorPrecedenceAndTypes",
@@ -1971,7 +1979,7 @@ public let DECL_NODES: [Node] = [
   Node(
     kind: .precedenceGroupDecl,
     base: .decl,
-    nameForDiagnostics: "precedencegroup",
+    nameForDiagnostics: "precedence group",
     documentation: "A Swift `precedencegroup` declaration.",
     traits: [
       "Braced",
@@ -1999,6 +2007,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation: "The name of this precedence group."
       ),
       Child(
@@ -2119,6 +2128,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation: "The name of the protocol."
       ),
       Child(
@@ -2299,6 +2309,7 @@ public let DECL_NODES: [Node] = [
       Child(
         name: "name",
         kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name",
         documentation:
           "Declares the name of this struct. If the name matches a reserved keyword use backticks to escape it."
       ),
@@ -2465,7 +2476,8 @@ public let DECL_NODES: [Node] = [
       ),
       Child(
         name: "name",
-        kind: .token(choices: [.token(.identifier)])
+        kind: .token(choices: [.token(.identifier)]),
+        nameForDiagnostics: "name"
       ),
       Child(
         name: "genericParameterClause",

CodeGeneration/Sources/SyntaxSupport/ExprNodes.swift

@@ -773,7 +773,7 @@ public let EXPR_NODES: [Node] = [
   Node(
     kind: .discardAssignmentExpr,
     base: .expr,
-    nameForDiagnostics: nil,
+    nameForDiagnostics: "wildcard expression",
     documentation: """
       A `_` that discards a value inside an assignment.
 
@@ -1516,7 +1516,7 @@ public let EXPR_NODES: [Node] = [
   Node(
     kind: .nilLiteralExpr,
     base: .expr,
-    nameForDiagnostics: nil,
+    nameForDiagnostics: "'nil' keyword",
     children: [
       Child(
         name: "nilKeyword",
@@ -1886,7 +1886,7 @@ public let EXPR_NODES: [Node] = [
   Node(
     kind: .superExpr,
     base: .expr,
-    nameForDiagnostics: nil,
+    nameForDiagnostics: "'super' keyword",
     children: [
       Child(
         name: "superKeyword",

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -497,6 +497,71 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         ],
         handledNodes: [node.id, otherNode.id]
       )
+    } else if let (moduleName, colonColon) = node.twoPresentTokens(
+      firstSatisfying: { $0.tokenKind.isIdentifier },
+      secondSatisfying: { $0.tokenKind == .colonColon }
+    ) {
+      // Looks like a module selector.
+      var fixIts: [FixIt] = []
+
+      func canExplicitlyInitializeVariable() -> TokenSyntax? {
+        if let capture = node.parent?.as(ClosureCaptureSyntax.self),
+           capture.unexpectedBeforeSpecifier == node || capture.unexpectedBetweenSpecifierAndName == node,
+           capture.initializer == nil {
+          return capture.name
+        }
+
+        if let pattern = node.parent?.as(IdentifierPatternSyntax.self),
+           pattern.unexpectedBeforeIdentifier == node,
+           pattern.parent?.is(OptionalBindingConditionSyntax.self) ?? false {
+          return pattern.identifier
+        }
+
+        return nil
+      }
+
+      if let explicitVarName = canExplicitlyInitializeVariable() {
+        // Suggest turning `Foo::bar` into `bar = Foo::bar`
+        fixIts.append(
+          FixIt(
+            message: ExplicitlyInitializeFixIt(newVariableName: explicitVarName),
+            changes: [
+              FixIt.MultiNodeChange(
+                .replace(
+                  oldNode: Syntax(node),
+                  newNode: Syntax(
+                    UnexpectedNodesSyntax(
+                      [
+                        Syntax(
+                          explicitVarName
+                            .with(\.leadingTrivia, moduleName.leadingTrivia)
+                            .with(\.trailingTrivia, [.spaces(1)])
+                        ),
+                        Syntax(TokenSyntax.equalToken(trailingTrivia: [.spaces(1)])),
+                        Syntax(moduleName.with(\.leadingTrivia, [])),
+                        Syntax(colonColon)
+                      ]
+                    )
+                  )
+                )
+              )
+            ]
+          )
+        )
+      }
+      // Suggest removing the module selector.
+      fixIts.append(
+        FixIt(
+          message: RemoveNodesFixIt([Syntax(node)]),
+          changes: [.makeMissing(node, transferTrivia: true)]
+        )
+      )
+      addDiagnostic(
+        node,
+        CannotUseModuleSelectorError(unexpectedNodes: node),
+        highlights: [Syntax(node)],
+        fixIts: fixIts
+      )
     } else {
       addDiagnostic(node, UnexpectedNodesError(unexpectedNodes: node), highlights: [Syntax(node)])
     }

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -13,11 +13,11 @@
 #if compiler(>=6)
 public import SwiftDiagnostics
 @_spi(Diagnostics) internal import SwiftParser
-@_spi(RawSyntax) public import SwiftSyntax
+@_spi(ExperimentalLanguageFeatures) @_spi(RawSyntax) public import SwiftSyntax
 #else
 import SwiftDiagnostics
 @_spi(Diagnostics) import SwiftParser
-@_spi(RawSyntax) import SwiftSyntax
+@_spi(ExperimentalLanguageFeatures) @_spi(RawSyntax) import SwiftSyntax
 #endif
 
 fileprivate let diagnosticDomain: String = "SwiftParser"
@@ -309,6 +309,60 @@ public struct CannotParseVersionTuple: ParserError {
   }
 }
 
+public struct CannotUseModuleSelectorError: ParserError {
+  public let unexpectedNodes: UnexpectedNodesSyntax
+
+  public var message: String {
+    var qualifiedSyntaxDescription = "code"
+    if let parent = unexpectedNodes.parent {
+      if let parentDeclRef = parent.as(DeclReferenceExprSyntax.self),
+         parentDeclRef.moduleSelector == nil,
+         parentDeclRef.unexpectedBeforeModuleSelector == unexpectedNodes {
+        // We only insert a module selector here if the `baseName` is some sort of exotic token that can't take a
+        // module selector. Blame that token.
+        qualifiedSyntaxDescription = self.exoticNameDescription(for: parentDeclRef.baseName)
+      } else if let parentTypeName = parent.ancestorOrSelf(mapping: self.parentDescription(for:)) {
+        qualifiedSyntaxDescription = parentTypeName
+        // If not first child, and subsequent child has name, add it.
+        let siblings = parent.children(viewMode: .sourceAccurate)
+        if siblings.first(where: { $0.totalLength.utf8Length > 0 })?.id != unexpectedNodes.id,
+           let nextSibling = siblings.suffix(from: siblings.index(of: self.unexpectedNodes)!).dropFirst().first,
+           let nextSiblingChildName = nextSibling.childNameInParent {
+          qualifiedSyntaxDescription += " \(nextSiblingChildName)"
+        }
+      } else if let parentTypeName = parent.ancestorOrSelf(mapping: self.parentDescription(for:)) {
+        qualifiedSyntaxDescription = "code in \(parentTypeName)"
+      }
+    }
+
+    return "\(qualifiedSyntaxDescription) cannot be qualified with a module selector"
+  }
+
+  private func parentDescription(for node: Syntax) -> String? {
+    if let name = node.nodeTypeNameForDiagnostics(allowBlockNames: false) {
+      return name
+    }
+
+    // Special case for otherwise undistinguishable syntax inside an attribute's argument list.
+    if node.keyPathInParent == \AttributeSyntax.arguments {
+      return "attribute argument"
+    }
+
+    return nil
+  }
+
+  private func exoticNameDescription(for nameToken: TokenSyntax) -> String {
+    switch nameToken.tokenKind {
+    case .keyword(let keyword):
+      return "'\(keyword.defaultText)' keyword"
+    case .binaryOperator(let text), .dollarIdentifier(let text), .identifier(let text), .postfixOperator(let text), .prefixOperator(let text):
+      return "\(nameToken.tokenKind.nameForDiagnostics) '\(text)'"
+    default:
+      return nameToken.tokenKind.nameForDiagnostics
+    }
+  }
+}
+
 public struct DeclarationNotPermittedInContext: ParserError {
   public var missingDecl: MissingDeclSyntax
   public var invalidDecl: DeclSyntax
@@ -741,6 +795,14 @@ extension FixItMessage where Self == StaticParserFixIt {
   }
 }
 
+public struct ExplicitlyInitializeFixIt: ParserFixIt {
+  public let newVariableName: TokenSyntax
+
+  public var message: String {
+    "explicitly initialize variable '\(newVariableName.text)'"
+  }
+}
+
 public struct InsertFixIt: ParserFixIt {
   public let tokenToBeInserted: TokenSyntax
 

Sources/SwiftParserDiagnostics/SyntaxExtensions.swift

@@ -90,6 +90,9 @@ extension SyntaxProtocol {
     if !allowBlockNames && (syntax.is(CodeBlockSyntax.self) || syntax.is(MemberBlockSyntax.self)) {
       return nil
     }
+    if let relation = syntax.as(PrecedenceGroupRelationSyntax.self) {
+      return "'\(relation.higherThanOrLowerThanLabel.text)' property of precedencegroup"
+    }
     return syntax.kind.nameForDiagnostics
   }