[Dependency Scanning] Construct a hollow output on query failure to carry diagnostic output

artemcm · artemcm · commit acd2da76a121 · 2024-07-11T11:49:55.000-07:00
diff --git a/lib/DependencyScan/DependencyScanningTool.cpp b/lib/DependencyScan/DependencyScanningTool.cpp
@@ -189,6 +189,71 @@ swiftscan_diagnostic_set_t *mapCollectedDiagnosticsForOutput(
   return diagnosticOutput;
 }
 
+// Generate an instance of the `swiftscan_dependency_graph_s` which contains no
+// module dependnecies but captures the diagnostics emitted during the attempted
+// scan query.
+static swiftscan_dependency_graph_t generateHollowDiagnosticOutput(
+    const DependencyScanDiagnosticCollector &ScanDiagnosticConsumer) {
+  // Create a dependency graph instance
+  swiftscan_dependency_graph_t hollowResult = new swiftscan_dependency_graph_s;
+
+  // Populate the `modules` with a single info for the main module
+  // containing no dependencies
+  swiftscan_dependency_set_t *dependencySet = new swiftscan_dependency_set_t;
+  dependencySet->count = 1;
+  dependencySet->modules = new swiftscan_dependency_info_t[1];
+  swiftscan_dependency_info_s *hollowMainModuleInfo =
+      new swiftscan_dependency_info_s;
+  dependencySet->modules[0] = hollowMainModuleInfo;
+  hollowResult->dependencies = dependencySet;
+
+  // Other main module details empty
+  hollowMainModuleInfo->direct_dependencies =
+      c_string_utils::create_empty_set();
+  hollowMainModuleInfo->source_files = c_string_utils::create_empty_set();
+  hollowMainModuleInfo->module_path = c_string_utils::create_null();
+  hollowResult->main_module_name = c_string_utils::create_clone("unknown");
+  hollowMainModuleInfo->module_name =
+      c_string_utils::create_clone("swiftTextual:unknown");
+
+  // Hollow info details
+  swiftscan_module_details_s *hollowDetails = new swiftscan_module_details_s;
+  hollowDetails->kind = SWIFTSCAN_DEPENDENCY_INFO_SWIFT_TEXTUAL;
+  hollowDetails->swift_textual_details = {c_string_utils::create_null(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_null(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_empty_set(),
+                                          c_string_utils::create_null(),
+                                          false,
+                                          c_string_utils::create_null(),
+                                          c_string_utils::create_null(),
+                                          c_string_utils::create_null()};
+  hollowMainModuleInfo->details = hollowDetails;
+
+  // Populate the diagnostic info
+  hollowResult->diagnostics =
+      mapCollectedDiagnosticsForOutput(&ScanDiagnosticConsumer);
+  return hollowResult;
+}
+
+// Generate an instance of the `swiftscan_import_set_t` which contains no
+// imports but captures the diagnostics emitted during the attempted
+// scan query.
+static swiftscan_import_set_t generateHollowDiagnosticOutputImportSet(
+    const DependencyScanDiagnosticCollector &ScanDiagnosticConsumer) {
+  // Create an dependency graph instance
+  swiftscan_import_set_t hollowResult = new swiftscan_import_set_s;
+  hollowResult->imports = c_string_utils::create_empty_set();
+  hollowResult->diagnostics =
+      mapCollectedDiagnosticsForOutput(&ScanDiagnosticConsumer);
+  return hollowResult;
+}
+
 DependencyScanningTool::DependencyScanningTool()
     : ScanningService(std::make_unique<SwiftDependencyScanningService>()),
       VersionedPCMInstanceCacheCache(
@@ -203,18 +268,13 @@ DependencyScanningTool::getDependencies(
   // There may be errors as early as in instance initialization, so we must ensure
   // we can catch those.
   auto ScanDiagnosticConsumer = std::make_shared<DependencyScanDiagnosticCollector>();
-  auto produceDiagnosticStateOnFailure = [&ScanDiagnosticConsumer]() {
-    swiftscan_dependency_graph_t result = new swiftscan_dependency_graph_s;
-    result->diagnostics = mapCollectedDiagnosticsForOutput(ScanDiagnosticConsumer.get());
-    return result;
-  };
 
   // The primary instance used to scan the query Swift source-code
   auto QueryContextOrErr = initCompilerInstanceForScan(Command,
                                                        WorkingDirectory,
                                                        ScanDiagnosticConsumer);
   if (QueryContextOrErr.getError())
-    return produceDiagnosticStateOnFailure();
+    return generateHollowDiagnosticOutput(*ScanDiagnosticConsumer);
 
   auto QueryContext = std::move(*QueryContextOrErr);
 
@@ -228,9 +288,9 @@ DependencyScanningTool::getDependencies(
                                              QueryContext.ScanDiagnostics.get(),
                                              cache);
   if (DependenciesOrErr.getError())
-    return produceDiagnosticStateOnFailure();
+    return generateHollowDiagnosticOutput(*ScanDiagnosticConsumer);
 
-  return std::move(*DependenciesOrErr);;
+  return std::move(*DependenciesOrErr);
 }
 
 llvm::ErrorOr<swiftscan_import_set_t>
@@ -243,11 +303,9 @@ DependencyScanningTool::getImports(ArrayRef<const char *> Command,
   auto QueryContextOrErr = initCompilerInstanceForScan(Command,
                                                        WorkingDirectory,
                                                        ScanDiagnosticConsumer);
-  if (QueryContextOrErr.getError()) {
-    swiftscan_import_set_t result = new swiftscan_import_set_s;
-    result->diagnostics = mapCollectedDiagnosticsForOutput(ScanDiagnosticConsumer.get());
-    return result;
-  }
+  if (QueryContextOrErr.getError())
+    return generateHollowDiagnosticOutputImportSet(*ScanDiagnosticConsumer);
+
   auto QueryContext = std::move(*QueryContextOrErr);
 
   // Local scan cache instance, wrapping the shared global cache.
@@ -259,10 +317,9 @@ DependencyScanningTool::getImports(ArrayRef<const char *> Command,
                                                 QueryContext.ScanDiagnostics.get(),
                                                 cache);
   if (DependenciesOrErr.getError())
-    return std::make_error_code(std::errc::not_supported);
-  auto Dependencies = std::move(*DependenciesOrErr);
+    return generateHollowDiagnosticOutputImportSet(*ScanDiagnosticConsumer);
 
-  return Dependencies;
+  return std::move(*DependenciesOrErr);
 }
 
 std::vector<llvm::ErrorOr<swiftscan_dependency_graph_t>>
diff --git a/lib/DependencyScan/ScanDependencies.cpp b/lib/DependencyScan/ScanDependencies.cpp
@@ -579,28 +579,6 @@ static void bridgeDependencyIDs(const ArrayRef<ModuleDependencyID> dependencies,
   }
 }
 
-static swiftscan_macro_dependency_set_t *createMacroDependencySet(
-    const std::map<std::string, MacroPluginDependency> &macroDeps) {
-  swiftscan_macro_dependency_set_t *set = new swiftscan_macro_dependency_set_t;
-  if (macroDeps.empty()) {
-    set->count = 0;
-    set->macro_dependencies = nullptr;
-    return set;
-  }
-  set->count = macroDeps.size();
-  set->macro_dependencies = new swiftscan_macro_dependency_t[set->count];
-  unsigned SI = 0;
-  for (auto &entry : macroDeps) {
-    set->macro_dependencies[SI] = new swiftscan_macro_dependency_s;
-    set->macro_dependencies[SI]->moduleName = create_clone(entry.first.c_str());
-    set->macro_dependencies[SI]->libraryPath =
-        create_clone(entry.second.LibraryPath.c_str());
-    set->macro_dependencies[SI]->executablePath =
-        create_clone(entry.second.ExecutablePath.c_str());
-  }
-  return set;
-}
-
 static swiftscan_dependency_graph_t
 generateFullDependencyGraph(const CompilerInstance &instance,
                             const DependencyScanDiagnosticCollector *diagnosticCollector,
diff --git a/unittests/DependencyScan/ModuleDeps.cpp b/unittests/DependencyScan/ModuleDeps.cpp
@@ -13,6 +13,7 @@
 #include "ScanFixture.h"
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/Platform.h"
+#include "swift/DependencyScan/DependencyScanImpl.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Host.h"
@@ -243,9 +244,87 @@ public func overlayFuncA() { }\n"));
     CommandB.push_back(command.c_str());
   }
 
-  auto instanceA = ScannerTool.initCompilerInstanceForScan(CommandA, {});
-  auto instanceB = ScannerTool.initCompilerInstanceForScan(CommandB, {});
+  auto ScanDiagnosticConsumer = std::make_shared<DependencyScanDiagnosticCollector>();
+  auto instanceA = ScannerTool.initCompilerInstanceForScan(CommandA, {}, ScanDiagnosticConsumer);
+  auto instanceB = ScannerTool.initCompilerInstanceForScan(CommandB, {}, ScanDiagnosticConsumer);
   // Ensure that scans that only differ in module name have distinct scanning context hashes
   ASSERT_NE(instanceA->ScanInstance.get()->getInvocation().getModuleScanningHash(),
             instanceB->ScanInstance.get()->getInvocation().getModuleScanningHash());
 }
+
+TEST_F(ScanTest, TestModuleCycle) {
+  SmallString<256> tempDir;
+  ASSERT_FALSE(llvm::sys::fs::createUniqueDirectory("ScanTest.TestModuleCycle", tempDir));
+  SWIFT_DEFER { llvm::sys::fs::remove_directories(tempDir); };
+
+  // Create test input file
+  std::string TestPathStr = createFilename(tempDir, "foo.swift");
+  ASSERT_FALSE(emitFileWithContents(tempDir, "foo.swift", "import A\n"));
+
+  // Create includes
+  std::string IncludeDirPath = createFilename(tempDir, "include");
+  ASSERT_FALSE(llvm::sys::fs::create_directory(IncludeDirPath));
+  std::string SwiftDirPath = createFilename(IncludeDirPath, "Swift");
+  ASSERT_FALSE(llvm::sys::fs::create_directory(SwiftDirPath));
+
+  // Create imported module Swift interface files
+  ASSERT_FALSE(emitFileWithContents(SwiftDirPath, "A.swiftinterface",
+                                    "// swift-interface-format-version: 1.0\n\
+// swift-module-flags: -module-name A\n\
+import Swift\n\
+import B\n\
+public func funcA() { }\n"));
+  ASSERT_FALSE(emitFileWithContents(SwiftDirPath, "B.swiftinterface",
+                                    "// swift-interface-format-version: 1.0\n\
+// swift-module-flags: -module-name B\n\
+import Swift\n\
+import A\n\
+public func funcB() { }\n"));
+
+  // Paths to shims and stdlib
+  llvm::SmallString<128> ShimsLibDir = StdLibDir;
+  llvm::sys::path::append(ShimsLibDir, "shims");
+  auto Target = llvm::Triple(llvm::sys::getDefaultTargetTriple());
+  llvm::sys::path::append(StdLibDir, getPlatformNameForTriple(Target));
+
+  std::vector<std::string> BaseCommandStrArr = {
+    TestPathStr,
+    std::string("-I ") + SwiftDirPath,
+    std::string("-I ") + StdLibDir.str().str(),
+    std::string("-I ") + ShimsLibDir.str().str(),
+  };
+
+  std::vector<std::string> CommandStr = BaseCommandStrArr;
+  CommandStr.push_back("-module-name");
+  CommandStr.push_back("test");
+
+  // On Windows we need to add an extra escape for path separator characters because otherwise
+  // the command line tokenizer will treat them as escape characters.
+  for (size_t i = 0; i < CommandStr.size(); ++i) {
+    std::replace(CommandStr[i].begin(), CommandStr[i].end(), '\\', '/');
+  }
+  std::vector<const char*> Command;
+  for (auto &command : CommandStr)
+    Command.push_back(command.c_str());
+
+  auto ScanDiagnosticConsumer = std::make_shared<DependencyScanDiagnosticCollector>();
+
+  auto DependenciesOrErr = ScannerTool.getDependencies(Command, {}, {});
+
+  // Ensure a hollow output with diagnostic info is produced
+  ASSERT_FALSE(DependenciesOrErr.getError());
+  auto Dependencies = DependenciesOrErr.get();
+  auto Diagnostics = Dependencies->diagnostics;
+  ASSERT_TRUE(Diagnostics->count == 1);
+  auto Diagnostic = Diagnostics->diagnostics[0];
+  ASSERT_TRUE(Diagnostic->severity == SWIFTSCAN_DIAGNOSTIC_SEVERITY_ERROR);
+  auto Message = std::string((const char*)Diagnostic->message.data,
+                             Diagnostic->message.length);
+  ASSERT_TRUE(Message == "module dependency cycle: 'A.swiftinterface -> B.swiftinterface -> A.swiftinterface'\n");
+
+  // Ensure hollow output is hollow
+  ASSERT_TRUE(Dependencies->dependencies->count == 1);
+  ASSERT_TRUE(Dependencies->dependencies->modules[0]->source_files->count == 0);
+  ASSERT_TRUE(Dependencies->dependencies->modules[0]->direct_dependencies->count == 0);
+  swiftscan_dependency_graph_dispose(Dependencies);
+}
