[Dependency Scanning] Specify Source Locations For Missing Module Dependencies

This change modifies the dependency scanner to keep track of source locations of each encountered 'import' statement, in order to be able to emit diagnostics with source locations if an import failed to resolve.

- Keep track of each 'import' statement's source buffer, line number, and column number when adding it. The dependency scanner utilizes separate compilation instances, and therefore separate Source Managers for scanning `import` statements of user sources and textual interfaces of Swift dependencies. Since import resolution may happen in the main scanner compilation instance while the `import` itself was found by an interface-scanning sub-instance, we cannot simply hold on to the import's `SourceLoc`.
- Add libSwiftScan API for diagnostics to carry above source locations to clients.

Author: artemcm

include/swift-c/DependencyScan/DependencyScan.h

@@ -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,25 @@ 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);
+
+SWIFTSCAN_PUBLIC void
+swiftscan_source_location_dispose(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

@@ -2217,6 +2217,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 ImportStatementInfo {
+  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;
+  };
+
+  ImportStatementInfo(std::string importIdentifier)
+  : importLocations(),
+    importIdentifier(importIdentifier) {}
+
+  ImportStatementInfo(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.
@@ -142,8 +175,8 @@ class ModuleDependencyInfoStorageBase {
         resolved(false), finalized(false) {}
 
   ModuleDependencyInfoStorageBase(ModuleDependencyKind dependencyKind,
-                                  const std::vector<std::string> &moduleImports,
-                                  const std::vector<std::string> &optionalModuleImports,
+                                  const std::vector<ImportStatementInfo> &moduleImports,
+                                  const std::vector<ImportStatementInfo> &optionalModuleImports,
                                   StringRef moduleCacheKey = "")
       : dependencyKind(dependencyKind), moduleImports(moduleImports),
         optionalModuleImports(optionalModuleImports),
@@ -154,12 +187,12 @@ class ModuleDependencyInfoStorageBase {
   virtual ~ModuleDependencyInfoStorageBase();
 
   /// The set of modules on which this module depends.
-  std::vector<std::string> moduleImports;
+  std::vector<ImportStatementInfo> 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<ImportStatementInfo> optionalModuleImports;
 
   /// The set of modules on which this module depends, resolved
   /// to Module IDs, qualified by module kind: Swift, Clang, etc.
@@ -328,8 +361,8 @@ class SwiftBinaryModuleDependencyStorage : public ModuleDependencyInfoStorageBas
   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::vector<ImportStatementInfo> &moduleImports,
+                                     const std::vector<ImportStatementInfo> &optionalModuleImports,
                                      const std::string &headerImport,
                                      const bool isFramework,
                                      const std::string &moduleCacheKey)
@@ -516,8 +549,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<ImportStatementInfo> &moduleImports,
+      const std::vector<ImportStatementInfo> &optionalModuleImports,
       const std::string &headerImport,
       bool isFramework, const std::string &moduleCacheKey) {
     return ModuleDependencyInfo(
@@ -567,12 +600,12 @@ class ModuleDependencyInfo {
   }
 
   /// Retrieve the module-level imports.
-  ArrayRef<std::string> getModuleImports() const {
+  ArrayRef<ImportStatementInfo> getModuleImports() const {
     return storage->moduleImports;
   }
 
   /// Retrieve the module-level optional imports.
-  ArrayRef<std::string> getOptionalModuleImports() const {
+  ArrayRef<ImportStatementInfo> getOptionalModuleImports() const {
     return storage->optionalModuleImports;
   }
 
@@ -749,31 +782,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,6 +37,7 @@ class DependencyScanDiagnosticCollector : public DiagnosticConsumer {
   struct ScannerDiagnosticInfo {
     std::string Message;
     llvm::SourceMgr::DiagKind Severity;
+    std::optional<ImportStatementInfo::ImportDiagnosticLocationInfo> ImportLocation;
   };
 
   void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override;

include/swift/DependencyScan/StringUtils.h

@@ -13,6 +13,7 @@
 #include "swift-c/CommonString/CommonString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/SmallVector.h"
 #include <string>
 #include <vector>
 

lib/AST/ModuleDependencies.cpp

@@ -121,16 +121,62 @@ 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 sourceLocToDiagnosticLocInfo =
+      [&sourceManager](SourceLoc sourceLocation) {
+        auto lineAndColumnNumbers =
+            sourceManager->getLineAndColumnInBuffer(sourceLocation);
+        return ImportStatementInfo::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(
+              sourceLocToDiagnosticLocInfo(sourceLocation));
+          break;
+        }
+      }
+    }
+  } else {
+    if (validSourceLocation)
+      storage->moduleImports.push_back(ImportStatementInfo(
+          module.str(), sourceLocToDiagnosticLocInfo(sourceLocation)));
+    else
+      storage->moduleImports.push_back(ImportStatementInfo(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 +195,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 +209,7 @@ void ModuleDependencyInfo::addModuleImport(
       addTestableImport(realPath);
   }
 
-  auto fileName = sf.getFilename();
+  auto fileName = sourceFile.getFilename();
   if (fileName.empty())
     return;
 

lib/DependencyScan/DependencyScanningTool.cpp

@@ -112,7 +112,14 @@ void DependencyScanDiagnosticCollector::addDiagnostic(
   DiagnosticEngine::formatDiagnosticText(Out, Info.FormatString,
                                          Info.FormatArgs);
   auto Msg = SM.GetMessage(Info.Loc, SMKind, Text, Ranges, FixIts);
-  Diagnostics.push_back(ScannerDiagnosticInfo{Msg.getMessage().str(), SMKind});
+
+  auto bufferIdentifier = SM.getDisplayNameForLoc(Info.Loc);
+  auto lineAndColumnNumbers = SM.getLineAndColumnInBuffer(Info.Loc);
+  auto importLocation = ImportStatementInfo::ImportDiagnosticLocationInfo(
+      bufferIdentifier.str(), lineAndColumnNumbers.first,
+      lineAndColumnNumbers.second);
+  Diagnostics.push_back(
+      ScannerDiagnosticInfo{Msg.getMessage().str(), SMKind, importLocation});
 }
 
 void LockingDependencyScanDiagnosticCollector::addDiagnostic(