Merge pull request #69552 from artemcm/DepScanTautologicalImports

[Dependency Scanning] Detect and ignore (and warn about) tautological imports

Author: artemcm

include/swift/AST/ModuleDependencies.h

@@ -123,6 +123,10 @@ using ModuleDependencyIDSetVector =
 
 namespace dependencies {
   std::string createEncodedModuleKindAndName(ModuleDependencyID id);
+  bool checkImportNotTautological(const ImportPath::Module, 
+                                  const SourceLoc,
+                                  const SourceFile&,
+                                  bool);
 }
 
 /// Base class for the variant storage of ModuleDependencyInfo.

lib/AST/ModuleDependencies.cpp

@@ -16,6 +16,7 @@
 #include "swift/AST/ModuleDependencies.h"
 #include "swift/AST/Decl.h"
 #include "swift/AST/DiagnosticsFrontend.h"
+#include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/SourceFile.h"
 #include "swift/Frontend/Frontend.h"
 #include "llvm/CAS/CASProvidingFileSystem.h"
@@ -145,6 +146,11 @@ void ModuleDependencyInfo::addModuleImport(
     if (importedModuleName == BUILTIN_NAME)
       continue;
 
+    // Ignore/diagnose tautological imports akin to import resolution
+    if (!swift::dependencies::checkImportNotTautological(
+            realPath, importDecl->getLoc(), sf, importDecl->isExported()))
+      continue;
+
     addModuleImport(realPath, &alreadyAddedModules);
 
     // Additionally, keep track of which dependencies of a Source
@@ -408,6 +414,35 @@ SwiftDependencyScanningService::SwiftDependencyScanningService() {
   SharedFilesystemCache.emplace();
 }
 
+bool
+swift::dependencies::checkImportNotTautological(const ImportPath::Module modulePath, 
+                                                const SourceLoc importLoc,
+                                                const SourceFile &SF,
+                                                bool isExported) {
+  if (modulePath.front().Item != SF.getParentModule()->getName() ||
+      // Overlays use an @_exported self-import to load their clang module.
+      isExported ||
+      // Imports of your own submodules are allowed in cross-language libraries.
+      modulePath.size() != 1 ||
+      // SIL files self-import to get decls from the rest of the module.
+      SF.Kind == SourceFileKind::SIL)
+    return true;
+
+  ASTContext &ctx = SF.getASTContext();
+
+  StringRef filename = llvm::sys::path::filename(SF.getFilename());
+  if (filename.empty())
+    ctx.Diags.diagnose(importLoc, diag::sema_import_current_module,
+                       modulePath.front().Item);
+  else
+    ctx.Diags.diagnose(importLoc, diag::sema_import_current_module_with_file,
+                       filename, modulePath.front().Item);
+
+  return false;
+
+  return false;
+}
+
 void SwiftDependencyTracker::addCommonSearchPathDeps(
     const SearchPathOptions &Opts) {
   // Add SDKSetting file.

lib/Sema/ImportResolution.cpp

@@ -17,6 +17,7 @@
 #define DEBUG_TYPE "swift-import-resolution"
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSema.h"
+#include "swift/AST/ModuleDependencies.h"
 #include "swift/AST/ModuleLoader.h"
 #include "swift/AST/ModuleNameLookup.h"
 #include "swift/AST/NameLookup.h"
@@ -602,28 +603,9 @@ UnboundImport::UnboundImport(ImportDecl *ID)
 }
 
 bool UnboundImport::checkNotTautological(const SourceFile &SF) {
-  // Exit early if this is not a self-import.
-  auto modulePath = import.module.getModulePath();
-  if (modulePath.front().Item != SF.getParentModule()->getName() ||
-      // Overlays use an @_exported self-import to load their clang module.
-      import.options.contains(ImportFlags::Exported) ||
-      // Imports of your own submodules are allowed in cross-language libraries.
-      modulePath.size() != 1 ||
-      // SIL files self-import to get decls from the rest of the module.
-      SF.Kind == SourceFileKind::SIL)
-    return true;
-
-  ASTContext &ctx = SF.getASTContext();
-
-  StringRef filename = llvm::sys::path::filename(SF.getFilename());
-  if (filename.empty())
-    ctx.Diags.diagnose(importLoc, diag::sema_import_current_module,
-                       modulePath.front().Item);
-  else
-    ctx.Diags.diagnose(importLoc, diag::sema_import_current_module_with_file,
-                       filename, modulePath.front().Item);
-
-  return false;
+  return swift::dependencies::checkImportNotTautological(
+      import.module.getModulePath(), importLoc, SF,
+      import.options.contains(ImportFlags::Exported));
 }
 
 bool UnboundImport::checkModuleLoaded(ModuleDecl *M, SourceFile &SF) {

test/ScanDependencies/module_deps_cross_import_overlay.swift

@@ -11,7 +11,7 @@
 // REQUIRES: executable_test
 // REQUIRES: objc_interop
 
-import CrossImportTestModule
+@_exported import CrossImportTestModule
 import EWrapper
 import SubEWrapper
 

test/ScanDependencies/no_main_module_cross_import.swift

@@ -9,7 +9,7 @@
 
 // Ordinarily, importing `E` and `SubE` triggers a cross-import of `_cross_import_E`, but not here, because we are building `SubE` Swift module itself.
 import EWrapper
-import SubE
+@_exported import SubE
 
 // CHECK:  "directDependencies": [
 // CHECK-DAG:   "swift": "EWrapper"

test/ScanDependencies/tautological_import.swift

@@ -0,0 +1,10 @@
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/clang-module-cache)
+
+// RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -module-name TautologicalModule -verify
+// Check the contents of the JSON output
+// RUN: %validate-json %t/deps.json | %FileCheck %s
+
+import TautologicalModule // expected-warning {{file 'tautological_import.swift' is part of module 'TautologicalModule'; ignoring import}}
+
+// CHECK: "mainModuleName": "TautologicalModule"