Merge pull request #73600 from artemcm/DepScanSourceLocsV2

[Dependency Scanning] Specify Source Locations For Missing Module Dependencies

Author: artemcm

include/swift-c/DependencyScan/DependencyScan.h

@@ -25,7 +25,7 @@
 /// SWIFTSCAN_VERSION_MINOR should increase when there are API additions.
 /// SWIFTSCAN_VERSION_MAJOR is intended for "major" source/ABI breaking changes.
 #define SWIFTSCAN_VERSION_MAJOR 0
-#define SWIFTSCAN_VERSION_MINOR 8
+#define SWIFTSCAN_VERSION_MINOR 9
 
 SWIFTSCAN_BEGIN_DECLS
 
@@ -55,6 +55,9 @@ typedef struct swiftscan_import_set_s *swiftscan_import_set_t;
 /// Opaque container to contain the info of a diagnostics emitted by the scanner.
 typedef struct swiftscan_diagnostic_info_s *swiftscan_diagnostic_info_t;
 
+/// Opaque container to contain the info of a source location.
+typedef struct swiftscan_source_location_s *swiftscan_source_location_t;
+
 /// Full Dependency Graph (Result)
 typedef struct {
   swiftscan_dependency_info_t *modules;
@@ -419,9 +422,22 @@ swiftscan_diagnostic_get_message(swiftscan_diagnostic_info_t diagnostic);
 SWIFTSCAN_PUBLIC swiftscan_diagnostic_severity_t
 swiftscan_diagnostic_get_severity(swiftscan_diagnostic_info_t diagnostic);
 
+SWIFTSCAN_PUBLIC swiftscan_source_location_t
+swiftscan_diagnostic_get_source_location(swiftscan_diagnostic_info_t diagnostic);
+
 SWIFTSCAN_PUBLIC void
 swiftscan_diagnostics_set_dispose(swiftscan_diagnostic_set_t* diagnostics);
 
+//=== Source Location -----------------------------------------------------===//
+SWIFTSCAN_PUBLIC swiftscan_string_ref_t
+swiftscan_source_location_get_buffer_identifier(swiftscan_source_location_t source_location);
+
+SWIFTSCAN_PUBLIC int64_t
+swiftscan_source_location_get_line_number(swiftscan_source_location_t source_location);
+
+SWIFTSCAN_PUBLIC int64_t
+swiftscan_source_location_get_column_number(swiftscan_source_location_t source_location);
+
 //=== Scanner Cache Operations --------------------------------------------===//
 // The following operations expose an implementation detail of the dependency
 // scanner: its module dependencies cache. This is done in order

include/swift/AST/DiagnosticsSema.def

@@ -2224,6 +2224,8 @@ ERROR(alignment_not_power_of_two,none,
 
 // Dependency Scanning
 ERROR(dependency_scan_module_not_found, none, "Unable to find module dependency: '%0'", (StringRef))
+NOTE(unresolved_import_location,none,
+     "also imported here", ())
 NOTE(dependency_as_imported_by_main_module,none,
      "a dependency of main module '%0'", (StringRef))
 NOTE(dependency_as_imported_by, none,

include/swift/AST/ModuleDependencies.h

@@ -129,6 +129,39 @@ namespace dependencies {
                                   bool);
 }
 
+struct ScannerImportStatementInfo {
+  struct ImportDiagnosticLocationInfo {
+    ImportDiagnosticLocationInfo() = delete;
+    ImportDiagnosticLocationInfo(std::string bufferIdentifier,
+                                 uint32_t lineNumber,
+                                 uint32_t columnNumber)
+    : bufferIdentifier(bufferIdentifier),
+      lineNumber(lineNumber),
+      columnNumber(columnNumber) {}
+    std::string bufferIdentifier;
+    uint32_t lineNumber;
+    uint32_t columnNumber;
+  };
+
+  ScannerImportStatementInfo(std::string importIdentifier)
+  : importLocations(),
+    importIdentifier(importIdentifier) {}
+
+  ScannerImportStatementInfo(std::string importIdentifier,
+                             ImportDiagnosticLocationInfo location)
+  : importLocations({location}),
+    importIdentifier(importIdentifier) {}
+
+  void addImportLocation(ImportDiagnosticLocationInfo location) {
+    importLocations.push_back(location);
+  }
+
+  // Buffer, line & column number of the import statement
+  SmallVector<ImportDiagnosticLocationInfo, 4> importLocations;
+  // Imported module string. e.g. "Foo.Bar" in 'import Foo.Bar'
+  std::string importIdentifier;
+};
+
 /// Base class for the variant storage of ModuleDependencyInfo.
 ///
 /// This class is mostly an implementation detail for \c ModuleDependencyInfo.
@@ -141,25 +174,27 @@ class ModuleDependencyInfoStorageBase {
       : dependencyKind(dependencyKind), moduleCacheKey(moduleCacheKey.str()),
         resolved(false), finalized(false) {}
 
-  ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind,
-                                  const std::vector<std::string> &moduleImports,
-                                  const std::vector<std::string> &optionalModuleImports,
-                                  StringRef moduleCacheKey = "")
+  ModuleDependencyInfoStorageBase(
+      ModuleDependencyKind dependencyKind,
+      const std::vector<ScannerImportStatementInfo> &moduleImports,
+      const std::vector<ScannerImportStatementInfo> &optionalModuleImports,
+      StringRef moduleCacheKey = "")
       : dependencyKind(dependencyKind), moduleImports(moduleImports),
         optionalModuleImports(optionalModuleImports),
-        moduleCacheKey(moduleCacheKey.str()), resolved(false), finalized(false)  {}
+        moduleCacheKey(moduleCacheKey.str()), resolved(false),
+        finalized(false) {}
 
   virtual ModuleDependencyInfoStorageBase *clone() const = 0;
 
   virtual ~ModuleDependencyInfoStorageBase();
 
   /// The set of modules on which this module depends.
-  std::vector<std::string> moduleImports;
+  std::vector<ScannerImportStatementInfo> moduleImports;
 
   /// The set of modules which constitute optional module
   /// dependencies for this module, such as `@_implementationOnly`
   /// or `internal` imports.
-  std::vector<std::string> optionalModuleImports;
+  std::vector<ScannerImportStatementInfo> optionalModuleImports;
 
   /// The set of modules on which this module depends, resolved
   /// to Module IDs, qualified by module kind: Swift, Clang, etc.
@@ -320,21 +355,23 @@ class SwiftSourceModuleDependenciesStorage :
   }
 };
 
-/// Describes the dependencies of a pre-built Swift module (with no .swiftinterface).
+/// Describes the dependencies of a pre-built Swift module (with no
+/// .swiftinterface).
 ///
 /// This class is mostly an implementation detail for \c ModuleDependencyInfo.
-class SwiftBinaryModuleDependencyStorage : public ModuleDependencyInfoStorageBase {
+class SwiftBinaryModuleDependencyStorage
+    : public ModuleDependencyInfoStorageBase {
 public:
-  SwiftBinaryModuleDependencyStorage(const std::string &compiledModulePath,
-                                     const std::string &moduleDocPath,
-                                     const std::string &sourceInfoPath,
-                                     const std::vector<std::string> &moduleImports,
-                                     const std::vector<std::string> &optionalModuleImports,
-                                     const std::string &headerImport,
-                                     const bool isFramework,
-                                     const std::string &moduleCacheKey)
+  SwiftBinaryModuleDependencyStorage(
+      const std::string &compiledModulePath, const std::string &moduleDocPath,
+      const std::string &sourceInfoPath,
+      const std::vector<ScannerImportStatementInfo> &moduleImports,
+      const std::vector<ScannerImportStatementInfo> &optionalModuleImports,
+      const std::string &headerImport, const bool isFramework,
+      const std::string &moduleCacheKey)
       : ModuleDependencyInfoStorageBase(ModuleDependencyKind::SwiftBinary,
-                                        moduleImports, optionalModuleImports, moduleCacheKey),
+                                        moduleImports, optionalModuleImports,
+                                        moduleCacheKey),
         compiledModulePath(compiledModulePath), moduleDocPath(moduleDocPath),
         sourceInfoPath(sourceInfoPath), headerImport(headerImport),
         isFramework(isFramework) {}
@@ -520,8 +557,8 @@ class ModuleDependencyInfo {
       const std::string &compiledModulePath,
       const std::string &moduleDocPath,
       const std::string &sourceInfoPath,
-      const std::vector<std::string> &moduleImports,
-      const std::vector<std::string> &optionalModuleImports,
+      const std::vector<ScannerImportStatementInfo> &moduleImports,
+      const std::vector<ScannerImportStatementInfo> &optionalModuleImports,
       const std::string &headerImport,
       bool isFramework, const std::string &moduleCacheKey) {
     return ModuleDependencyInfo(
@@ -572,12 +609,12 @@ class ModuleDependencyInfo {
   }
 
   /// Retrieve the module-level imports.
-  ArrayRef<std::string> getModuleImports() const {
+  ArrayRef<ScannerImportStatementInfo> getModuleImports() const {
     return storage->moduleImports;
   }
 
   /// Retrieve the module-level optional imports.
-  ArrayRef<std::string> getOptionalModuleImports() const {
+  ArrayRef<ScannerImportStatementInfo> getOptionalModuleImports() const {
     return storage->optionalModuleImports;
   }
 
@@ -754,31 +791,24 @@ class ModuleDependencyInfo {
   void addOptionalModuleImport(StringRef module,
                                llvm::StringSet<> *alreadyAddedModules = nullptr);
 
+  /// Add all of the module imports in the given source
+  /// file to the set of module imports.
+  void addModuleImports(const SourceFile &sourceFile,
+                        llvm::StringSet<> &alreadyAddedModules,
+                        const SourceManager *sourceManager);
 
   /// Add a dependency on the given module, if it was not already in the set.
-  void addModuleImport(StringRef module,
-                       llvm::StringSet<> *alreadyAddedModules = nullptr);
+  void addModuleImport(ImportPath::Module module,
+                       llvm::StringSet<> *alreadyAddedModules = nullptr,
+                       const SourceManager *sourceManager = nullptr,
+                       SourceLoc sourceLocation = SourceLoc());
 
   /// Add a dependency on the given module, if it was not already in the set.
-  void addModuleImport(ImportPath::Module module,
-                       llvm::StringSet<> *alreadyAddedModules = nullptr) {
-    std::string ImportedModuleName = module.front().Item.str().str();
-    auto submodulePath = module.getSubmodulePath();
-    if (submodulePath.size() > 0 && !submodulePath[0].Item.empty()) {
-      auto submoduleComponent = submodulePath[0];
-      // Special case: a submodule named "Foo.Private" can be moved to a top-level
-      // module named "Foo_Private". ClangImporter has special support for this.
-      if (submoduleComponent.Item.str() == "Private")
-        addOptionalModuleImport(ImportedModuleName + "_Private", alreadyAddedModules);
-    }
-
-    addModuleImport(ImportedModuleName, alreadyAddedModules);
-  }
+  void addModuleImport(StringRef module,
+                       llvm::StringSet<> *alreadyAddedModules = nullptr,
+                       const SourceManager *sourceManager = nullptr,
+                       SourceLoc sourceLocation = SourceLoc());
 
-  /// Add all of the module imports in the given source
-  /// file to the set of module imports.
-  void addModuleImport(const SourceFile &sf,
-                       llvm::StringSet<> &alreadyAddedModules);
   /// Add a kind-qualified module dependency ID to the set of
   /// module dependencies.
   void addModuleDependency(ModuleDependencyID dependencyID);

include/swift/DependencyScan/DependencyScanImpl.h

@@ -21,7 +21,7 @@
 namespace swift {
 namespace dependencies {
 class DependencyScanningTool;
-}
+} // namespace dependencies
 } // namespace swift
 
 struct swiftscan_dependency_graph_s {
@@ -212,7 +212,13 @@ struct swiftscan_scan_invocation_s {
 struct swiftscan_diagnostic_info_s {
   swiftscan_string_ref_t message;
   swiftscan_diagnostic_severity_t severity;
-  // TODO: SourceLoc
+  swiftscan_source_location_t source_location;
+};
+
+struct swiftscan_source_location_s {
+  swiftscan_string_ref_t buffer_identifier;
+  uint32_t line_number;
+  uint32_t column_number;
 };
 
 #endif // SWIFT_C_DEPENDENCY_SCAN_IMPL_H

include/swift/DependencyScan/DependencyScanningTool.h

@@ -37,10 +37,11 @@ class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
   struct ScannerDiagnosticInfo {
     std::string Message;
     llvm::SourceMgr::DiagKind Severity;
+    std::optional<ScannerImportStatementInfo::ImportDiagnosticLocationInfo> ImportLocation;
   };
+  std::vector<ScannerDiagnosticInfo> Diagnostics;
 
   void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;
-  std::vector<ScannerDiagnosticInfo> Diagnostics;
 
 protected:
   virtual void addDiagnostic(SourceManager &SM, const DiagnosticInfo &Info);

lib/AST/ModuleDependencies.cpp

@@ -121,16 +121,67 @@ void ModuleDependencyInfo::addOptionalModuleImport(
 }
 
 void ModuleDependencyInfo::addModuleImport(
-    StringRef module, llvm::StringSet<> *alreadyAddedModules) {
-  if (!alreadyAddedModules || alreadyAddedModules->insert(module).second)
-    storage->moduleImports.push_back(module.str());
+    StringRef module, llvm::StringSet<> *alreadyAddedModules,
+    const SourceManager *sourceManager, SourceLoc sourceLocation) {
+  auto scannerImportLocToDiagnosticLocInfo =
+      [&sourceManager](SourceLoc sourceLocation) {
+        auto lineAndColumnNumbers =
+            sourceManager->getLineAndColumnInBuffer(sourceLocation);
+        return ScannerImportStatementInfo::ImportDiagnosticLocationInfo(
+            sourceManager->getDisplayNameForLoc(sourceLocation).str(),
+            lineAndColumnNumbers.first, lineAndColumnNumbers.second);
+      };
+  bool validSourceLocation = sourceManager && sourceLocation.isValid() &&
+                             sourceManager->isOwning(sourceLocation);
+
+  if (alreadyAddedModules && alreadyAddedModules->contains(module)) {
+    if (validSourceLocation) {
+      // Find a prior import of this module and add import location
+      for (auto &existingImport : storage->moduleImports) {
+        if (existingImport.importIdentifier == module) {
+          existingImport.addImportLocation(
+              scannerImportLocToDiagnosticLocInfo(sourceLocation));
+          break;
+        }
+      }
+    }
+  } else {
+    if (alreadyAddedModules)
+      alreadyAddedModules->insert(module);
+
+    if (validSourceLocation)
+      storage->moduleImports.push_back(ScannerImportStatementInfo(
+          module.str(), scannerImportLocToDiagnosticLocInfo(sourceLocation)));
+    else
+      storage->moduleImports.push_back(
+          ScannerImportStatementInfo(module.str()));
+  }
 }
 
 void ModuleDependencyInfo::addModuleImport(
-    const SourceFile &sf, llvm::StringSet<> &alreadyAddedModules) {
+    ImportPath::Module module, llvm::StringSet<> *alreadyAddedModules,
+    const SourceManager *sourceManager, SourceLoc sourceLocation) {
+  std::string ImportedModuleName = module.front().Item.str().str();
+  auto submodulePath = module.getSubmodulePath();
+  if (submodulePath.size() > 0 && !submodulePath[0].Item.empty()) {
+    auto submoduleComponent = submodulePath[0];
+    // Special case: a submodule named "Foo.Private" can be moved to a top-level
+    // module named "Foo_Private". ClangImporter has special support for this.
+    if (submoduleComponent.Item.str() == "Private")
+      addOptionalModuleImport(ImportedModuleName + "_Private",
+                              alreadyAddedModules);
+  }
+
+  addModuleImport(ImportedModuleName, alreadyAddedModules,
+                  sourceManager, sourceLocation);
+}
+
+void ModuleDependencyInfo::addModuleImports(
+    const SourceFile &sourceFile, llvm::StringSet<> &alreadyAddedModules,
+    const SourceManager *sourceManager) {
   // Add all of the module dependencies.
   SmallVector<Decl *, 32> decls;
-  sf.getTopLevelDecls(decls);
+  sourceFile.getTopLevelDecls(decls);
   for (auto decl : decls) {
     auto importDecl = dyn_cast<ImportDecl>(decl);
     if (!importDecl)
@@ -149,10 +200,12 @@ void ModuleDependencyInfo::addModuleImport(
 
     // Ignore/diagnose tautological imports akin to import resolution
     if (!swift::dependencies::checkImportNotTautological(
-            realPath, importDecl->getLoc(), sf, importDecl->isExported()))
+            realPath, importDecl->getLoc(), sourceFile,
+            importDecl->isExported()))
       continue;
 
-    addModuleImport(realPath, &alreadyAddedModules);
+    addModuleImport(realPath, &alreadyAddedModules,
+                    sourceManager, importDecl->getLoc());
 
     // Additionally, keep track of which dependencies of a Source
     // module are `@Testable`.
@@ -161,7 +214,7 @@ void ModuleDependencyInfo::addModuleImport(
       addTestableImport(realPath);
   }
 
-  auto fileName = sf.getFilename();
+  auto fileName = sourceFile.getFilename();
   if (fileName.empty())
     return;