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

The note will point the user to where the "other" module with the same name is located and mention whether it is an SDK module. This is nice to have in various circumstances where developers attempt to define a module with the same name as a Swift module that already exists on their search paths, for example in the SDK.

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 *
+}