Merge pull request #82695 from artemcm/DepScanCycleShadow

[Dependency Scanning] Emit a note if a dependency cycle is between the source target and another Swift module with the same name

Author: artemcm

include/swift/AST/DiagnosticsCommon.def

@@ -214,6 +214,9 @@ ERROR(scanner_find_cycle, none,
 NOTE(scanner_find_cycle_swift_overlay_path, none,
      "Swift Overlay dependency of '%0' on '%1' via Clang module dependency: '%2'", (StringRef, StringRef, StringRef))
 
+NOTE(scanner_cycle_source_target_shadow_module, none,
+     "source target '%0' shadowing a%select{ |n SDK }2Swift module with the same name at: '%1'", (StringRef, StringRef, bool))
+
 ERROR(scanner_arguments_invalid, none,
       "dependencies scanner cannot be configured with arguments: '%0'", (StringRef))
 

include/swift/AST/ModuleDependencies.h

@@ -862,6 +862,12 @@ class ModuleDependencyInfo {
   /// Retrieve the dependencies for a Clang module.
   const ClangModuleDependencyStorage *getAsClangModule() const;
 
+  /// Get the path to the module-defining file:
+  /// `SwiftInterface`: Textual Interface path
+  /// `SwiftBinary`: Binary module path
+  /// `Clang`: Module map path
+  std::string getModuleDefiningPath() const;
+
   /// Add a dependency on the given module, if it was not already in the set.
   void
   addOptionalModuleImport(StringRef module, bool isExported,

lib/AST/ModuleDependencies.cpp

@@ -88,6 +88,30 @@ ModuleDependencyInfo::getAsClangModule() const {
   return dyn_cast<ClangModuleDependencyStorage>(storage.get());
 }
 
+std::string ModuleDependencyInfo::getModuleDefiningPath() const {
+  std::string path = "";
+  switch (getKind()) {
+  case swift::ModuleDependencyKind::SwiftInterface:
+    path = getAsSwiftInterfaceModule()->swiftInterfaceFile;
+    break;
+  case swift::ModuleDependencyKind::SwiftBinary:
+    path = getAsSwiftBinaryModule()->compiledModulePath;
+    break;
+  case swift::ModuleDependencyKind::Clang:
+    path = getAsClangModule()->moduleMapFile;
+    break;
+  case swift::ModuleDependencyKind::SwiftSource:
+    path = getAsSwiftSourceModule()->sourceFiles.front();
+    break;
+  default:
+    llvm_unreachable("Unexpected dependency kind");
+  }
+
+  // Relative to the `module.modulemap` or `.swiftinterface` or `.swiftmodule`,
+  // the defininig path is the parent directory of the file.
+  return llvm::sys::path::parent_path(path).str();
+}
+
 void ModuleDependencyInfo::addTestableImport(ImportPath::Module module) {
   assert(getAsSwiftSourceModule() && "Expected source module for addTestableImport.");
   dyn_cast<SwiftSourceModuleDependenciesStorage>(storage.get())->addTestableImport(module);

lib/DependencyScan/ModuleDependencyScanner.cpp

@@ -1868,28 +1868,6 @@ void ModuleDependencyScanner::diagnoseScannerFailure(
   }
 }
 
-static std::string getModuleDefiningPath(const ModuleDependencyInfo &info) {
-  std::string path = "";
-  switch (info.getKind()) {
-  case swift::ModuleDependencyKind::SwiftInterface:
-    path = info.getAsSwiftInterfaceModule()->swiftInterfaceFile;
-    break;
-  case swift::ModuleDependencyKind::SwiftBinary:
-    path = info.getAsSwiftBinaryModule()->compiledModulePath;
-    break;
-  case swift::ModuleDependencyKind::Clang:
-    path = info.getAsClangModule()->moduleMapFile;
-    break;
-  case swift::ModuleDependencyKind::SwiftSource:
-  default:
-    llvm_unreachable("Unexpected dependency kind");
-  }
-
-  // Relative to the `module.modulemap` or `.swiftinterface` or `.swiftmodule`,
-  // the defininig path is the parent directory of the file.
-  return llvm::sys::path::parent_path(path).str();
-}
-
 std::optional<std::pair<ModuleDependencyID, std::string>>
 ModuleDependencyScanner::attemptToFindResolvingSerializedSearchPath(
     const ScannerImportStatementInfo &moduleImport,
@@ -1922,13 +1900,13 @@ ModuleDependencyScanner::attemptToFindResolvingSerializedSearchPath(
               /* isTestableImport */ false);
           if (!result.empty())
             return std::make_pair(binaryDepID,
-                                  getModuleDefiningPath(result[0].second));
+                                  result[0].second.getModuleDefiningPath());
 
           result = ScanningWorker->scanFilesystemForClangModuleDependency(
               getModuleImportIdentifier(moduleImport.importIdentifier), {});
           if (!result.empty())
-            return std::make_pair(binaryDepID,
-                                  getModuleDefiningPath(result[0].second));
+            return std::make_pair(
+                binaryDepID, result[0].second.getModuleDefiningPath());
           return std::nullopt;
         });
     if (result)

lib/DependencyScan/ScanDependencies.cpp

@@ -1201,6 +1201,24 @@ static bool diagnoseCycle(const CompilerInstance &instance,
             thisID.ModuleName, nextID.ModuleName, noteBuffer.str());
       }
     }
+
+    // Check if this is a case of a source target shadowing
+    // a module with the same name
+    if (sourceId.Kind == swift::ModuleDependencyKind::SwiftSource) {
+      auto sinkModuleDefiningPath =
+          cache.findKnownDependency(sinkId).getModuleDefiningPath();
+      auto SDKPath =
+          instance.getInvocation().getSearchPathOptions().getSDKPath();
+      auto sinkIsInSDK =
+          !SDKPath.empty() &&
+          hasPrefix(llvm::sys::path::begin(sinkModuleDefiningPath),
+                    llvm::sys::path::end(sinkModuleDefiningPath),
+                    llvm::sys::path::begin(SDKPath),
+                    llvm::sys::path::end(SDKPath));
+      instance.getASTContext().Diags.diagnose(
+          SourceLoc(), diag::scanner_cycle_source_target_shadow_module,
+          sourceId.ModuleName, sinkModuleDefiningPath, sinkIsInSDK);
+    }
   };
 
   // Start from the main module and check direct and overlay dependencies

test/ScanDependencies/diagnose_dependency_cycle_shadow.swift

@@ -0,0 +1,80 @@
+// REQUIRES: objc_interop
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/module-cache)
+// RUN: %empty-directory(%t/inputs)
+
+// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/A.framework/Modules/A.swiftmodule)
+// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/A.framework/Headers)
+// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Modules/CycleKit.swiftmodule)
+// RUN: %empty-directory(%t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Headers)
+
+// RUN: split-file %s %t
+
+// RUN: cp %t/inputs/A.swiftinterface %t/mock.sdk/System/Library/Frameworks/A.framework/Modules/A.swiftmodule/%target-swiftinterface-name
+// RUN: cp %t/inputs/framework_A.modulemap %t/mock.sdk/System/Library/Frameworks/A.framework/Modules/module.modulemap
+// RUN: cp %t/inputs/A-framework.h %t/mock.sdk/System/Library/Frameworks/A.framework/Headers/A.h
+
+// RUN: cp %t/inputs/CycleKit.swiftinterface %t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Modules/CycleKit.swiftmodule/%target-swiftinterface-name
+// RUN: cp %t/inputs/framework_CycleKit.modulemap %t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Modules/module.modulemap
+// RUN: cp %t/inputs/CycleKit.h %t/mock.sdk/System/Library/Frameworks/CycleKit.framework/Headers/CycleKit.h
+
+// Non-SDK dependency shadowing
+// RUN: not %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -I %t/inputs -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -module-name CycleKit  &> %t/out.txt
+// RUN: %FileCheck --check-prefix=CHECK-NONSDK %s < %t/out.txt
+
+// CHECK-NONSDK: note: source target 'CycleKit' shadowing a Swift module with the same name at: '{{.*}}{{/|\\}}diagnose_dependency_cycle_shadow.swift.tmp{{/|\\}}inputs'
+
+// SDK dependency shadowing
+// RUN: not %target-swift-frontend -scan-dependencies -module-cache-path %t/module-cache %t/test.swift -o %t/deps.json -disable-implicit-concurrency-module-import -disable-implicit-string-processing-module-import -module-name CycleKit -sdk %t/mock.sdk &> %t/out-sdk.txt
+// RUN: %FileCheck --check-prefix=CHECK-SDK %s < %t/out-sdk.txt
+
+// CHECK-SDK: note: source target 'CycleKit' shadowing an SDK Swift module with the same name at: '{{.*}}{{/|\\}}mock.sdk{{/|\\}}System{{/|\\}}Library{{/|\\}}Frameworks{{/|\\}}CycleKit.framework{{/|\\}}Modules{{/|\\}}CycleKit.swiftmodule'
+
+//--- test.swift
+import A
+
+//--- inputs/CycleKit.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name CycleKit -enable-library-evolution
+
+public func CycleKitFunc() {}
+
+//--- inputs/A.swiftinterface
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name A -enable-library-evolution
+@_exported import A
+public func AFunc() {}
+
+//--- inputs/A.h
+#import <CycleKit.h>
+void funcA(void);
+
+//--- inputs/A-framework.h
+#import <CycleKit/CycleKit.h>
+void funcA(void);
+
+//--- inputs/CycleKit.h
+void funcCycleKit(void);
+
+//--- inputs/module.modulemap
+module A {
+    header "A.h"
+    export *
+}
+
+module CycleKit {
+    header "CycleKit.h"
+    export *
+}
+
+//--- inputs/framework_A.modulemap
+framework module A {
+    header "A.h"
+    export *
+}
+
+//--- inputs/framework_CycleKit.modulemap
+framework module CycleKit {
+    header "CycleKit.h"
+    export *
+}