Macro diagnostics fixes

lib/ASTGen/Sources/ASTGen/Diagnostics.swift

@@ -5,7 +5,6 @@ import SwiftSyntax
 fileprivate func emitDiagnosticParts(
   diagEnginePtr: UnsafeMutablePointer<UInt8>,
   sourceFileBuffer: UnsafeMutableBufferPointer<UInt8>,
-  nodeStartOffset: Int?,
   message: String,
   severity: DiagnosticSeverity,
   position: AbsolutePosition,
@@ -22,18 +21,8 @@ fileprivate func emitDiagnosticParts(
 
   // Form a source location for the given absolute position
   func sourceLoc(
-    at origPosition: AbsolutePosition
+    at position: AbsolutePosition
   ) -> UnsafeMutablePointer<UInt8>? {
-    // FIXME: Our tree is very confused about absolute offsets. Work around
-    // the issue in a very hacky way.
-    let position: AbsolutePosition
-    if let nodeStartOffset = nodeStartOffset,
-        origPosition.utf8Offset < nodeStartOffset {
-      position = origPosition + SourceLength(utf8Length: nodeStartOffset)
-    } else {
-      position = origPosition
-    }
-
     if let sourceFileBase = sourceFileBuffer.baseAddress,
       position.utf8Offset >= 0 &&
         position.utf8Offset < sourceFileBuffer.count {
@@ -99,8 +88,8 @@ fileprivate func emitDiagnosticParts(
 func emitDiagnostic(
   diagEnginePtr: UnsafeMutablePointer<UInt8>,
   sourceFileBuffer: UnsafeMutableBufferPointer<UInt8>,
-  nodeStartOffset: Int? = nil,
-  diagnostic: Diagnostic
+  diagnostic: Diagnostic,
+  messageSuffix: String? = nil
 ) {
   // Collect all of the Fix-It changes based on their Fix-It ID.
   var fixItChangesByID: [MessageID : [FixIt.Change]] = [:]
@@ -113,8 +102,7 @@ func emitDiagnostic(
   emitDiagnosticParts(
     diagEnginePtr: diagEnginePtr,
     sourceFileBuffer: sourceFileBuffer,
-    nodeStartOffset: nodeStartOffset,
-    message: diagnostic.diagMessage.message,
+    message: diagnostic.diagMessage.message + (messageSuffix ?? ""),
     severity: diagnostic.diagMessage.severity,
     position: diagnostic.position,
     highlights: diagnostic.highlights,
@@ -126,7 +114,6 @@ func emitDiagnostic(
     emitDiagnosticParts(
       diagEnginePtr: diagEnginePtr,
       sourceFileBuffer: sourceFileBuffer,
-      nodeStartOffset: nodeStartOffset,
       message: note.message,
       severity: .note, position: note.position,
       fixItChanges: fixItChangesByID[note.noteMessage.fixItID] ?? []

lib/ASTGen/Sources/ASTGen/Macros.swift

@@ -176,14 +176,13 @@ func evaluateMacro(
     }
 
     // Emit diagnostics accumulated in the context.
+    let macroName = parentExpansion.macro.withoutTrivia().description
     for diag in context.diagnostics {
-      // FIXME: Consider tacking on a note that says that this diagnostic
-      // came from a macro expansion.
       emitDiagnostic(
         diagEnginePtr: diagEnginePtr,
         sourceFileBuffer: .init(mutating: sourceFile.pointee.buffer),
-        nodeStartOffset: parentSyntax.position.utf8Offset,
-        diagnostic: diag
+        diagnostic: diag,
+        messageSuffix: " (from macro '\(macroName)')"
       )
     }
 

test/Index/index_macros.swift

@@ -3,9 +3,9 @@
 // REQUIRES: OS=macosx
 
 
-macro myLine: Int = _SwiftSyntaxMacros.LineMacro
-macro myFilename<T: ExpressibleByStringLiteral>: T = _SwiftSyntaxMacros.FilePathMacro
-macro myStringify<T>(_: T) -> (T, String) = _SwiftSyntaxMacros.StringifyMacro
+macro myLine: Int = MacroDefinition.LineMacro
+macro myFilename<T: ExpressibleByStringLiteral>: T = MacroDefinition.FileMacro
+macro myStringify<T>(_: T) -> (T, String) = MacroDefinition.StringifyMacro
 
 func test(x: Int) {
   _ = #myLine

test/Macros/Inputs/syntax_macro_definitions.swift

@@ -63,6 +63,10 @@ struct SimpleDiagnosticMessage: DiagnosticMessage {
   let severity: DiagnosticSeverity
 }
 
+extension SimpleDiagnosticMessage: FixItMessage {
+  var fixItID: MessageID { diagnosticID }
+}
+
 public struct AddBlocker: ExpressionMacro {
   public static func expansion(
     of node: MacroExpansionExprSyntax, in context: inout MacroExpansionContext
@@ -77,12 +81,13 @@ public struct AddBlocker: ExpressionMacro {
       context.diagnose(error.asDiagnostic)
     }
 
+    // Link the folded argument back into the tree.
+    var node = node.withArgumentList(node.argumentList.replacing(childAt: 0, with: node.argumentList.first!.withExpression(foldedArgument.as(ExprSyntax.self)!)))
+
    class AddVisitor: SyntaxVisitor {
       var diagnostics: [Diagnostic] = []
-      let startPosition: AbsolutePosition
 
-      init(startPosition: AbsolutePosition) {
-        self.startPosition = startPosition
+      init() {
         super.init(viewMode: .sourceAccurate)
       }
 
@@ -91,18 +96,38 @@ public struct AddBlocker: ExpressionMacro {
       ) -> SyntaxVisitorContinueKind {
         if let binOp = node.operatorOperand.as(BinaryOperatorExprSyntax.self) {
           if binOp.operatorToken.text == "+" {
+            let messageID = MessageID(domain: "silly", id: "addblock")
             diagnostics.append(
               Diagnostic(
                 node: Syntax(node.operatorOperand),
-                position: startPosition + SourceLength(utf8Length: binOp.operatorToken.position.utf8Offset),
                 message: SimpleDiagnosticMessage(
-                  message: "blocked an add",
-                  diagnosticID: MessageID(domain: "silly", id: "addblock"),
+                  message: "blocked an add; did you mean to subtract?",
+                  diagnosticID: messageID,
                   severity: .error
                 ),
                 highlights: [
                   Syntax(node.leftOperand.withoutTrivia()),
                   Syntax(node.rightOperand.withoutTrivia())
+                ],
+                fixIts: [
+                  FixIt(
+                    message: SimpleDiagnosticMessage(
+                      message: "use '-'",
+                      diagnosticID: messageID,
+                      severity: .error
+                    ),
+                    changes: [
+                      FixIt.Change.replace(
+                        oldNode: Syntax(binOp.operatorToken.withoutTrivia()),
+                        newNode: Syntax(
+                          TokenSyntax(
+                            .spacedBinaryOperator("-"),
+                            presence: .present
+                          )
+                        )
+                      )
+                    ]
+                  ),
                 ]
               )
             )
@@ -113,8 +138,8 @@ public struct AddBlocker: ExpressionMacro {
       }
     }
 
-    let visitor = AddVisitor(startPosition: argument.position)
-    visitor.walk(Syntax(foldedArgument))
+    let visitor = AddVisitor()
+    visitor.walk(Syntax(node))
 
     for diag in visitor.diagnostics {
       context.diagnose(diag)

test/Macros/macro_expand.swift

@@ -53,7 +53,6 @@ macro addBlocker<T>(_ value: T) -> T = MacroDefinition.AddBlocker
 func testAddBlocker(a: Int, b: Int, c: Int) {
   _ = #addBlocker(a * b * c)
 #if TEST_DIAGNOSTICS
-  // FIXME: Highlight locations are wrong.
-  _ = #addBlocker(a + b * c) // expected-error{{blocked an add}}
+  _ = #addBlocker(a + b * c) // expected-error{{blocked an add; did you mean to subtract? (from macro 'addBlocker')}}{{21-22=-}}
 #endif
 }

test/Macros/macros_diagnostics.swift

@@ -1,7 +1,9 @@
-// RUN: %target-typecheck-verify-swift -enable-experimental-feature Macros -module-name MacrosTest
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -I %swift-host-lib-dir -L %swift-host-lib-dir -emit-library -o %t/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath
+// RUN: %target-typecheck-verify-swift -enable-experimental-feature Macros -module-name MacrosTest -load-plugin-library %t/%target-library-name(MacroDefinition) -I %swift-host-lib-dir
 // REQUIRES: OS=macosx
 
-macro stringify<T>(_ value: T) -> (T, String) = _SwiftSyntaxMacros.StringifyMacro
+macro stringify<T>(_ value: T) -> (T, String) = MacroDefinition.StringifyMacro
 macro missingMacro1(_: Any) = MissingModule.MissingType // expected-note{{'missingMacro1' declared here}}
 macro missingMacro2(_: Any) = MissingModule.MissingType
 

test/ModuleInterface/macros.swift

@@ -7,14 +7,14 @@
 // RUN: %target-swift-frontend -compile-module-from-interface %t/Macros.swiftinterface -o %t/Macros.swiftmodule
 
 // CHECK: #if compiler(>=5.3) && $Macros
-// CHECK-NEXT: public macro publicStringify<T>(_ value: T) -> (T, Swift.String) = _SwiftSyntaxMacros.StringifyMacro
+// CHECK-NEXT: public macro publicStringify<T>(_ value: T) -> (T, Swift.String) = SomeModule.StringifyMacro
 // CHECK-NEXT: #endif
-public macro publicStringify<T>(_ value: T) -> (T, String) = _SwiftSyntaxMacros.StringifyMacro
+public macro publicStringify<T>(_ value: T) -> (T, String) = SomeModule.StringifyMacro
 
 // CHECK: #if compiler(>=5.3) && $Macros
-// CHECK: public macro publicLine<T>: T = _SwiftSyntaxMacros.Line where T : Swift.ExpressibleByIntegerLiteral
+// CHECK: public macro publicLine<T>: T = SomeModule.Line where T : Swift.ExpressibleByIntegerLiteral
 // CHECK-NEXT: #endif
-public macro publicLine<T: ExpressibleByIntegerLiteral>: T = _SwiftSyntaxMacros.Line
+public macro publicLine<T: ExpressibleByIntegerLiteral>: T = SomeModule.Line
 
 // CHECK-NOT: internalStringify
-macro internalStringify<T>(_ value: T) -> (T, String) = _SwiftSyntaxMacros.StringifyMacro
+macro internalStringify<T>(_ value: T) -> (T, String) = SomeModule.StringifyMacro

test/Serialization/Inputs/def_macros.swift

@@ -1,3 +1,3 @@
-public macro publicStringify<T>(_ value: T) -> (T, String) = _SwiftSyntaxMacros.StringifyMacro
+public macro publicStringify<T>(_ value: T) -> (T, String) = SomeModule.StringifyMacro
 
-macro internalStringify<T>(_ value: T) -> (T, String) = _SwiftSyntaxMacros.StringifyMacro
+macro internalStringify<T>(_ value: T) -> (T, String) = SomeModule.StringifyMacro

test/Serialization/macros.swift

@@ -10,6 +10,7 @@ import def_macros
 
 func test(a: Int, b: Int) {
   _ = #publicStringify(a + b)
+  // expected-error@-1{{external macro implementation type 'SomeModule.StringifyMacro' could not be found for macro 'publicStringify'; the type must be public and provided via '-load-plugin-library'}}
 
   _ = #internalStringify(a + b)
   // expected-error@-1{{macro 'internalStringify' is undefined}}