Merge pull request #1620 from artemcm/60DepScanSourceLocs

DougGregor · web-flow · commit f9b6dca13786 · 2024-05-25T08:06:52.000-07:00
[6.0 🍒][Explicit Module Builds] Get source locations from the dependency scanner and emit diagnostics with them
diff --git a/Sources/CSwiftScan/include/swiftscan_header.h b/Sources/CSwiftScan/include/swiftscan_header.h
@@ -44,6 +44,7 @@ typedef struct swiftscan_dependency_info_s *swiftscan_dependency_info_t;
 typedef struct swiftscan_dependency_graph_s *swiftscan_dependency_graph_t;
 typedef struct swiftscan_import_set_s *swiftscan_import_set_t;
 typedef struct swiftscan_diagnostic_info_s *swiftscan_diagnostic_info_t;
+typedef struct swiftscan_source_location_s *swiftscan_source_location_t;
 
 typedef enum {
   SWIFTSCAN_DIAGNOSTIC_SEVERITY_ERROR = 0,
@@ -270,9 +271,19 @@ typedef struct {
   (*swiftscan_diagnostic_get_message)(swiftscan_diagnostic_info_t);
   swiftscan_diagnostic_severity_t
   (*swiftscan_diagnostic_get_severity)(swiftscan_diagnostic_info_t);
+  swiftscan_source_location_t
+  (*swiftscan_diagnostic_get_source_location)(swiftscan_diagnostic_info_t);
   void
   (*swiftscan_diagnostics_set_dispose)(swiftscan_diagnostic_set_t*);
 
+  //=== Source Location -----------------------------------------------------===//
+  swiftscan_string_ref_t
+  (*swiftscan_source_location_get_buffer_identifier)(swiftscan_source_location_t);
+  int64_t
+  (*swiftscan_source_location_get_line_number)(swiftscan_source_location_t);
+  int64_t
+  (*swiftscan_source_location_get_column_number)(swiftscan_source_location_t);
+
   //=== Scanner Cache Functions ---------------------------------------------===//
   void (*swiftscan_scanner_cache_serialize)(swiftscan_scanner_t scanner, const char * path);
   bool (*swiftscan_scanner_cache_load)(swiftscan_scanner_t scanner, const char * path);
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/InterModuleDependencyOracle.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/InterModuleDependencies/InterModuleDependencyOracle.swift
@@ -167,6 +167,13 @@ public class InterModuleDependencyOracle {
     return swiftScan.canQueryPerScanDiagnostics
   }
 
+  @_spi(Testing) public func supportsDiagnosticSourceLocations() throws -> Bool {
+    guard let swiftScan = swiftScanLibInstance else {
+      fatalError("Attempting to query supported scanner API with no scanner instance.")
+    }
+    return swiftScan.supportsDiagnosticSourceLocations
+  }
+
   @_spi(Testing) public func getScannerDiagnostics() throws -> [ScannerDiagnosticPayload]? {
     guard let swiftScan = swiftScanLibInstance else {
       fatalError("Attempting to reset scanner cache with no scanner instance.")
diff --git a/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift b/Sources/SwiftDriver/ExplicitModuleBuilds/ModuleDependencyScanning.swift
@@ -33,7 +33,7 @@ extension Diagnostic.Message {
     .error("Swift Caching enabled - libSwiftScan load failed (\(libPath)).")
   }
   static func scanner_diagnostic_error(_ message: String) -> Diagnostic.Message {
-    .error(message)
+    return .error(message)
   }
   static func scanner_diagnostic_warn(_ message: String) -> Diagnostic.Message {
     .warning(message)
@@ -247,15 +247,20 @@ public extension Driver {
       for diagnostic in diagnostics {
         switch diagnostic.severity {
         case .error:
-          diagnosticEngine.emit(.scanner_diagnostic_error(diagnostic.message))
+          diagnosticEngine.emit(.scanner_diagnostic_error(diagnostic.message),
+                                location: diagnostic.sourceLocation)
         case .warning:
-          diagnosticEngine.emit(.scanner_diagnostic_warn(diagnostic.message))
+          diagnosticEngine.emit(.scanner_diagnostic_warn(diagnostic.message),
+                                location: diagnostic.sourceLocation)
         case .note:
-          diagnosticEngine.emit(.scanner_diagnostic_note(diagnostic.message))
+          diagnosticEngine.emit(.scanner_diagnostic_note(diagnostic.message),
+                                location: diagnostic.sourceLocation)
         case .remark:
-          diagnosticEngine.emit(.scanner_diagnostic_remark(diagnostic.message))
+          diagnosticEngine.emit(.scanner_diagnostic_remark(diagnostic.message),
+                                location: diagnostic.sourceLocation)
         case .ignored:
-          diagnosticEngine.emit(.scanner_diagnostic_error(diagnostic.message))
+          diagnosticEngine.emit(.scanner_diagnostic_error(diagnostic.message),
+                                location: diagnostic.sourceLocation)
         }
       }
   }
diff --git a/Sources/SwiftDriver/SwiftScan/SwiftScan.swift b/Sources/SwiftDriver/SwiftScan/SwiftScan.swift
@@ -21,6 +21,7 @@ import struct Foundation.Data
 import protocol TSCBasic.DiagnosticData
 import struct TSCBasic.AbsolutePath
 import struct TSCBasic.Diagnostic
+import protocol TSCBasic.DiagnosticLocation
 
 public enum DependencyScanningError: LocalizedError, DiagnosticData, Equatable {
   case missingRequiredSymbol(String)
@@ -70,9 +71,19 @@ public enum DependencyScanningError: LocalizedError, DiagnosticData, Equatable {
   }
 }
 
-@_spi(Testing) public struct ScannerDiagnosticPayload {
-  @_spi(Testing) public let severity: Diagnostic.Behavior
-  @_spi(Testing) public let message: String
+public struct ScannerDiagnosticSourceLocation : DiagnosticLocation {
+  public var description: String {
+    return "\(bufferIdentifier):\(lineNumber):\(columnNumber)"
+  }
+  public let bufferIdentifier: String
+  public let lineNumber: Int
+  public let columnNumber: Int
+}
+
+public struct ScannerDiagnosticPayload {
+  public let severity: Diagnostic.Behavior
+  public let message: String
+  public let sourceLocation: ScannerDiagnosticSourceLocation?
 }
 
 internal extension swiftscan_diagnostic_severity_t {
@@ -92,6 +103,12 @@ internal extension swiftscan_diagnostic_severity_t {
   }
 }
 
+private extension String {
+  func stripNewline() -> String {
+    return self.hasSuffix("\n") ? String(self.dropLast()) : self
+  }
+}
+
 /// Wrapper for libSwiftScan, taking care of initialization, shutdown, and dispatching dependency scanning queries.
 @_spi(Testing) public final class SwiftScan {
   /// The path to the libSwiftScan dylib.
@@ -148,6 +165,8 @@ internal extension swiftscan_diagnostic_severity_t {
     guard let importSetRef = importSetRefOrNull else {
       throw DependencyScanningError.dependencyScanFailed("Unable to produce import set")
     }
+    defer { api.swiftscan_import_set_dispose(importSetRef) }
+
     if canQueryPerScanDiagnostics {
       let diagnosticsSetRefOrNull = api.swiftscan_import_set_get_diagnostics(importSetRef)
       guard let diagnosticsSetRef = diagnosticsSetRefOrNull else {
@@ -156,11 +175,7 @@ internal extension swiftscan_diagnostic_severity_t {
       diagnostics = try mapToDriverDiagnosticPayload(diagnosticsSetRef)
     }
 
-    let importSet = try constructImportSet(from: importSetRef, with: moduleAliases)
-    // Free the memory allocated for the in-memory representation of the import set
-    // returned by the scanner, now that we have translated it.
-    api.swiftscan_import_set_dispose(importSetRef)
-    return importSet
+    return try constructImportSet(from: importSetRef, with: moduleAliases)
   }
 
   func scanDependencies(workingDirectory: AbsolutePath,
@@ -184,6 +199,8 @@ internal extension swiftscan_diagnostic_severity_t {
     guard let graphRef = graphRefOrNull else {
       throw DependencyScanningError.dependencyScanFailed("Unable to produce dependency graph")
     }
+    defer { api.swiftscan_dependency_graph_dispose(graphRef) }
+
     if canQueryPerScanDiagnostics {
       let diagnosticsSetRefOrNull = api.swiftscan_dependency_graph_get_diagnostics(graphRef)
       guard let diagnosticsSetRef = diagnosticsSetRefOrNull else {
@@ -192,12 +209,7 @@ internal extension swiftscan_diagnostic_severity_t {
       diagnostics = try mapToDriverDiagnosticPayload(diagnosticsSetRef)
     }
 
-    let dependencyGraph = try constructGraph(from: graphRef, moduleAliases: moduleAliases)
-    // Free the memory allocated for the in-memory representation of the dependency
-    // graph returned by the scanner, now that we have translated it into an
-    // `InterModuleDependencyGraph`.
-    api.swiftscan_dependency_graph_dispose(graphRef)
-    return dependencyGraph
+    return try constructGraph(from: graphRef, moduleAliases: moduleAliases)
   }
 
   func batchScanDependencies(workingDirectory: AbsolutePath,
@@ -345,6 +357,13 @@ internal extension swiftscan_diagnostic_severity_t {
            api.swiftscan_import_set_get_diagnostics != nil
   }
 
+  @_spi(Testing) public var supportsDiagnosticSourceLocations : Bool {
+    return api.swiftscan_diagnostic_get_source_location != nil &&
+           api.swiftscan_source_location_get_buffer_identifier != nil &&
+           api.swiftscan_source_location_get_line_number != nil &&
+           api.swiftscan_source_location_get_column_number != nil
+  }
+
   func serializeScannerCache(to path: AbsolutePath) {
     api.swiftscan_scanner_cache_serialize(scanner,
                                           path.description.cString(using: String.Encoding.utf8))
@@ -367,9 +386,24 @@ internal extension swiftscan_diagnostic_severity_t {
       guard let diagnosticRef = diagnosticRefOrNull else {
         throw DependencyScanningError.dependencyScanFailed("Unable to produce scanner diagnostics")
       }
-      let message = try toSwiftString(api.swiftscan_diagnostic_get_message(diagnosticRef))
+      let message = try toSwiftString(api.swiftscan_diagnostic_get_message(diagnosticRef)).stripNewline()
       let severity = api.swiftscan_diagnostic_get_severity(diagnosticRef)
-      result.append(ScannerDiagnosticPayload(severity: severity.toDiagnosticBehavior(), message: message))
+
+      var sourceLoc: ScannerDiagnosticSourceLocation? = nil
+      if supportsDiagnosticSourceLocations {
+        let sourceLocRefOrNull = api.swiftscan_diagnostic_get_source_location(diagnosticRef)
+        if let sourceLocRef = sourceLocRefOrNull {
+          let bufferName = try toSwiftString(api.swiftscan_source_location_get_buffer_identifier(sourceLocRef))
+          let lineNumber = api.swiftscan_source_location_get_line_number(sourceLocRef)
+          let columnNumber = api.swiftscan_source_location_get_column_number(sourceLocRef)
+          sourceLoc = ScannerDiagnosticSourceLocation(bufferIdentifier: bufferName,
+                                                      lineNumber: Int(lineNumber),
+                                                      columnNumber: Int(columnNumber))
+        }
+      }
+      result.append(ScannerDiagnosticPayload(severity: severity.toDiagnosticBehavior(),
+                                             message: message,
+                                             sourceLocation: sourceLoc))
     }
     return result
   }
@@ -597,6 +631,11 @@ private extension swiftscan_functions_t {
     self.swiftscan_cache_replay_result_get_stderr = try loadOptional("swiftscan_cache_replay_result_get_stderr")
     self.swiftscan_cache_replay_result_dispose = try loadOptional("swiftscan_cache_replay_result_dispose")
 
+    self.swiftscan_diagnostic_get_source_location = try loadOptional("swiftscan_diagnostic_get_source_location")
+    self.swiftscan_source_location_get_buffer_identifier = try loadOptional("swiftscan_source_location_get_buffer_identifier")
+    self.swiftscan_source_location_get_line_number = try loadOptional("swiftscan_source_location_get_line_number")
+    self.swiftscan_source_location_get_column_number = try loadOptional("swiftscan_source_location_get_column_number")
+
     // Swift Overlay Dependencies
     self.swiftscan_swift_textual_detail_get_swift_overlay_dependencies =
       try loadOptional("swiftscan_swift_textual_detail_get_swift_overlay_dependencies")
diff --git a/Sources/swift-driver/main.swift b/Sources/swift-driver/main.swift
@@ -120,6 +120,13 @@ do {
   }
 
   let jobs = try driver.planBuild()
+
+  // Planning may result in further errors emitted
+  // due to dependency scanning failures.
+  guard !driver.diagnosticEngine.hasErrors else {
+    throw Driver.ErrorDiagnostics.emitted
+  }
+
   try driver.run(jobs: jobs)
 
   if driver.diagnosticEngine.hasErrors {
diff --git a/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift b/Tests/SwiftDriverTests/ExplicitModuleBuildTests.swift
@@ -1408,6 +1408,7 @@ final class ExplicitModuleBuildTests: XCTestCase {
       throw XCTSkip("libSwiftScan does not support diagnostics query.")
     }
 
+    // Missing Swift Interface dependency
     try withTemporaryDirectory { path in
       let main = path.appending(component: "testDependencyScanning.swift")
       try localFileSystem.writeFileContents(main, bytes: "import S;")
@@ -1450,8 +1451,21 @@ final class ExplicitModuleBuildTests: XCTestCase {
       XCTAssertEqual(potentialDiags?.count, 5)
       let diags = try XCTUnwrap(potentialDiags)
       let error = diags[0]
-      XCTAssertEqual(error.message, "Unable to find module dependency: 'unknown_module'")
       XCTAssertEqual(error.severity, .error)
+      if try dependencyOracle.supportsDiagnosticSourceLocations() {
+        XCTAssertEqual(error.message,
+        """
+        Unable to find module dependency: 'unknown_module'
+        import unknown_module
+               ^
+        """)
+        let sourceLoc = try XCTUnwrap(error.sourceLocation)
+        XCTAssertTrue(sourceLoc.bufferIdentifier.hasSuffix("I.swiftinterface"))
+        XCTAssertEqual(sourceLoc.lineNumber, 3)
+        XCTAssertEqual(sourceLoc.columnNumber, 8)
+      } else {
+        XCTAssertEqual(error.message, "Unable to find module dependency: 'unknown_module'")
+      }
       let noteI = diags[1]
       XCTAssertTrue(noteI.message.starts(with: "a dependency of Swift module 'I':"))
       XCTAssertEqual(noteI.severity, .note)
@@ -1462,7 +1476,82 @@ final class ExplicitModuleBuildTests: XCTestCase {
       XCTAssertTrue(noteS.message.starts(with: "a dependency of Swift module 'S':"))
       XCTAssertEqual(noteS.severity, .note)
       let noteTest = diags[4]
-      XCTAssertEqual(noteTest.message, "a dependency of main module 'testDependencyScanning'")
+      if try dependencyOracle.supportsDiagnosticSourceLocations() {
+        XCTAssertEqual(noteTest.message,
+        """
+        a dependency of main module 'testDependencyScanning'
+        import unknown_module
+               ^
+        """
+        )
+      } else {
+        XCTAssertEqual(noteTest.message, "a dependency of main module 'testDependencyScanning'")
+      }
+      XCTAssertEqual(noteTest.severity, .note)
+    }
+
+    // Missing main module dependency
+    try withTemporaryDirectory { path in
+      let main = path.appending(component: "testDependencyScanning.swift")
+      try localFileSystem.writeFileContents(main, bytes: "import FooBar")
+      let sdkArgumentsForTesting = (try? Driver.sdkArgumentsForTesting()) ?? []
+      var driver = try Driver(args: ["swiftc",
+                                     "-I", stdlibPath.nativePathString(escaped: true),
+                                     "-I", shimsPath.nativePathString(escaped: true),
+                                     "-explicit-module-build",
+                                     "-working-directory", path.nativePathString(escaped: true),
+                                     "-disable-clang-target",
+                                     main.nativePathString(escaped: true)] + sdkArgumentsForTesting,
+                              env: ProcessEnv.vars)
+      let resolver = try ArgsResolver(fileSystem: localFileSystem)
+      var scannerCommand = try driver.dependencyScannerInvocationCommand().1.map { try resolver.resolve($0) }
+      if scannerCommand.first == "-frontend" {
+        scannerCommand.removeFirst()
+      }
+      var scanDiagnostics: [ScannerDiagnosticPayload] = []
+      let _ =
+          try dependencyOracle.getDependencies(workingDirectory: path,
+                                               commandLine: scannerCommand,
+                                               diagnostics: &scanDiagnostics)
+      let potentialDiags: [ScannerDiagnosticPayload]?
+      if try dependencyOracle.supportsPerScanDiagnostics() {
+        potentialDiags = scanDiagnostics
+        print("Using Per-Scan diagnostics")
+      } else {
+        potentialDiags = try dependencyOracle.getScannerDiagnostics()
+        print("Using Scanner-Global diagnostics")
+      }
+      XCTAssertEqual(potentialDiags?.count, 2)
+      let diags = try XCTUnwrap(potentialDiags)
+      let error = diags[0]
+      XCTAssertEqual(error.severity, .error)
+      if try dependencyOracle.supportsDiagnosticSourceLocations() {
+        XCTAssertEqual(error.message,
+        """
+        Unable to find module dependency: 'FooBar'
+        import FooBar
+               ^
+        """)
+
+        let sourceLoc = try XCTUnwrap(error.sourceLocation)
+        XCTAssertTrue(sourceLoc.bufferIdentifier.hasSuffix("testDependencyScanning.swift"))
+        XCTAssertEqual(sourceLoc.lineNumber, 1)
+        XCTAssertEqual(sourceLoc.columnNumber, 8)
+      } else {
+        XCTAssertEqual(error.message, "Unable to find module dependency: 'FooBar'")
+      }
+      let noteTest = diags[1]
+      if try dependencyOracle.supportsDiagnosticSourceLocations() {
+        XCTAssertEqual(noteTest.message,
+        """
+        a dependency of main module 'testDependencyScanning'
+        import FooBar
+               ^
+        """
+        )
+      } else {
+        XCTAssertEqual(noteTest.message, "a dependency of main module 'testDependencyScanning'")
+      }
       XCTAssertEqual(noteTest.severity, .note)
     }
   }
@@ -1712,8 +1801,31 @@ final class ExplicitModuleBuildTests: XCTestCase {
         for scanIndex in 0..<numFiles {
           let diagnostics = scanDiagnostics[scanIndex]
           XCTAssertEqual(diagnostics.count, 2)
-          XCTAssertEqual(diagnostics[0].message, "Unable to find module dependency: 'UnknownModule\(scanIndex)'")
-          XCTAssertEqual(diagnostics[1].message, "a dependency of main module 'testParallelDependencyScanningDiagnostics\(scanIndex)'")
+          // Diagnostic source locations came after per-scan diagnostics, only meaningful
+          // on this test code-path
+          if try dependencyOracle.supportsDiagnosticSourceLocations() {
+              let sourceLoc = try XCTUnwrap(diagnostics[0].sourceLocation)
+              XCTAssertEqual(sourceLoc.lineNumber, 1)
+              XCTAssertEqual(sourceLoc.columnNumber, 8)
+              XCTAssertEqual(diagnostics[0].message,
+              """
+              Unable to find module dependency: 'UnknownModule\(scanIndex)'
+              import UnknownModule\(scanIndex);
+                     ^
+              """)
+              let noteSourceLoc = try XCTUnwrap(diagnostics[1].sourceLocation)
+              XCTAssertEqual(noteSourceLoc.lineNumber, 1)
+              XCTAssertEqual(noteSourceLoc.columnNumber, 8)
+              XCTAssertEqual(diagnostics[1].message,
+              """
+              a dependency of main module 'testParallelDependencyScanningDiagnostics\(scanIndex)'
+              import UnknownModule\(scanIndex);
+                     ^
+              """)
+          } else {
+              XCTAssertEqual(diagnostics[0].message, "Unable to find module dependency: 'UnknownModule\(scanIndex)'")
+              XCTAssertEqual(diagnostics[1].message, "a dependency of main module 'testParallelDependencyScanningDiagnostics\(scanIndex)'")
+          }
         }
       } else {
         let globalDiagnostics = try dependencyOracle.getScannerDiagnostics()
