swiftlang/swift-syntax

`SyntaxStringInterpolationError` should prepend `Internal macro error: ` if hit by a macro expansion

Closed this issue · 2 comments

Description

If a macro expansion hits a SyntaxStringInterpolationError, we should prepend some appropriate text to the description to mark it as an internal macro error, eg. Internal macro error: .

Tracked in Apple’s issue tracker as rdar://113608012

Looking! I've poked around SwiftSyntax, but Macros are a new area to me, so expect dumb questions 🙃

So far:

  • Looks like when macros are expanded, MacroExpansionContext.addDiagnostic will be invoked if any error / diagnostic comes up in the process, and it takes an error argument.
  • In SwiftSyntaxMacroExpansion/MacroSystem, there are visit, expandMacros, expandAccessors and expandFreeStandingMacro that all call addDiagnostic on errors. They explicitly throw MacroApplicationError.
  • My understanding is that when expanding a Macro, some Macros will make Syntax nodes with string interpolation, and when that errors out — it throws SyntaxStringInterpolationError. It will propagate to context.addDiagnostic.
  • There's also SwiftCompilerPluginMessageHandler that also does macro expansions and invokes context.addDiagnistics, and looks like it's reusing MacroExpansionContext, so we can fix this once there, and it'll work for the compiler plugin as well.
  • The easiest way to provide additional context to the user will be to hook into addDiagnistoc and do something like:
  public func addDiagnostics(from error: Error, node: some SyntaxProtocol) {
    // Inspect the error to form an appropriate set of diagnostics.
    var diagnostics: [Diagnostic]
    if let diagnosticsError = error as? DiagnosticsError {
      diagnostics = diagnosticsError.diagnostics
    } else if let message = error as? DiagnosticMessage {
      diagnostics = [Diagnostic(node: Syntax(node), message: message)]
    } else if let error = error as? SyntaxStringInterpolationError {
      diagnostics = [Diagnostic(node: Syntax(node), message: ThrownErrorDiagnostic(message: "Internal macro error: \( String(describing: error) )"))]
    } else {
      diagnostics = [Diagnostic(node: Syntax(node), message: ThrownErrorDiagnostic(message: String(describing: error)))]
    }

    ...
  }

To test that, we could make a macro for the test case that just goes ahead and throws a SyntaxStringInterpolationError in expansion(). And then in the test case, check that the resulting diagnostic as? ThrownErrorDiagnistic has a message which text starts with Internal macro error.

Internal macro does sound ironic :D

How does that look?