Sema: Prefer submodule imports for access-level-on-import diagnostics

lib/AST/Module.cpp

@@ -2971,10 +2971,10 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
   assert(targetModule != getParentModule() &&
          "getImportAccessLevel doesn't support checking for a self-import");
 
-  /// Order of relevancy of `import` to reach `targetModule`.
-  /// Lower is better/more authoritative.
+  /// Order of relevancy of `import` to reach `targetModule` assuming the same
+  /// visibility level. Lower is better/more authoritative.
   auto rateImport = [&](const ImportAccessLevel import) -> int {
-    auto importedModule = import->module.importedModule;
+    auto importedModule = import->module.importedModule->getTopLevelModule();
 
     // Prioritize public names:
     if (targetModule->getExportAsName() == importedModule->getBaseIdentifier())
@@ -2989,9 +2989,14 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
     if (targetModule == importedModule->getUnderlyingModuleIfOverlay())
       return 3;
 
-    // Any import in the sources.
-    if (import->importLoc.isValid())
-      return 4;
+    // Any import in the sources:
+    if (import->importLoc.isValid()) {
+      // Prefer clang submodules to their top level modules:
+      if (import->module.importedModule != importedModule)
+        return 4;
+
+      return 5;
+    }
 
     return 10;
   };
@@ -3001,11 +3006,12 @@ SourceFile::getImportAccessLevel(const ModuleDecl *targetModule) const {
   auto &imports = getASTContext().getImportCache();
   ImportAccessLevel restrictiveImport = std::nullopt;
   for (auto &import : *Imports) {
+    auto importedModule = import.module.importedModule->getTopLevelModule();
     if ((!restrictiveImport.has_value() ||
          import.accessLevel > restrictiveImport->accessLevel ||
          (import.accessLevel == restrictiveImport->accessLevel &&
           rateImport(import) < rateImport(restrictiveImport))) &&
-        imports.isImportedBy(targetModule, import.module.importedModule)) {
+        imports.isImportedBy(targetModule, importedModule)) {
       restrictiveImport = import;
     }
   }

lib/Sema/ResilienceDiagnostics.cpp

@@ -162,7 +162,8 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
         ModuleDecl *importedVia = attributedImport.module.importedModule,
                    *sourceModule = D->getModuleContext();
         ctx.Diags.diagnose(loc, diag::module_api_import_aliases, D, importedVia,
-                           sourceModule, importedVia == sourceModule);
+                           sourceModule,
+                           importedVia->getTopLevelModule() == sourceModule);
       });
 
   auto ignoredDowngradeToWarning = DowngradeToWarning::No;
@@ -283,7 +284,8 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
           ModuleDecl *importedVia = attributedImport.module.importedModule,
                      *sourceModule = D->getModuleContext();
           ctx.Diags.diagnose(loc, diag::module_api_import, D, importedVia,
-                             sourceModule, importedVia == sourceModule,
+                             sourceModule,
+                             importedVia->getTopLevelModule() == sourceModule,
                              /*isImplicit*/ false);
         }
       });
@@ -426,7 +428,7 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
         ctx.Diags.diagnose(loc, diag::module_api_import_conformance,
                            rootConf->getType(), rootConf->getProtocol(),
                            importedVia, sourceModule,
-                           importedVia == sourceModule);
+                           importedVia->getTopLevelModule() == sourceModule);
       });
 
   auto originKind = getDisallowedOriginKind(ext, where);

lib/Sema/TypeCheckAccess.cpp

@@ -2768,7 +2768,7 @@ void swift::recordRequiredImportAccessLevelForDecl(const ValueDecl *decl,
                    *sourceModule = decl->getModuleContext();
         dc->getASTContext().Diags.diagnose(
             diagLoc, diag::module_api_import, decl, importedVia, sourceModule,
-            importedVia == sourceModule, loc.isInvalid());
+            importedVia->getTopLevelModule() == sourceModule, loc.isInvalid());
       });
 }
 

test/Sema/access-level-import-inconsistent-ordering.swift

@@ -0,0 +1,109 @@
+/// The order of imports in sources shouldn't matter for access-levels.
+
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t --leading-lines
+
+/// Build the libraries.
+// RUN: %target-swift-frontend -emit-module %t/Lib1.swift -module-name Lib1 -o %t
+// RUN: %target-swift-frontend -emit-module %t/Lib2.swift -module-name Lib2 -o %t
+
+/// Test main cases.
+// RUN: %target-swift-frontend -typecheck %t/Client.swift -I %t -package-name pkg \
+// RUN:   -verify -verify-ignore-unrelated
+// RUN: %target-swift-frontend -typecheck %t/Client_Clang.swift -I %t \
+// RUN:   -verify -verify-ignore-unrelated
+// RUN: %target-swift-frontend -typecheck %t/Client_Clang.swift -I %t -DINVERT \
+// RUN:   -verify -verify-ignore-unrelated
+// RUN: %target-swift-frontend -typecheck %t/Client_Clang_Submodules.swift -I %t \
+// RUN:   -verify -verify-ignore-unrelated
+// RUN: %target-swift-frontend -typecheck %t/Client_Clang_Submodules.swift -I %t -DINVERT \
+// RUN:   -verify -verify-ignore-unrelated
+
+// REQUIRES: VENDOR=apple
+
+//--- Lib1.swift
+public struct Type1 {}
+
+//--- Lib2.swift
+public struct Type2 {}
+
+//--- Client.swift
+
+/// Simple public vs internal.
+package import Lib1 // expected-note {{imported 'package' here}}
+// expected-note @-1 {{struct 'Type1' imported as 'package' from 'Lib1' here}}
+internal import Lib1 // expected-warning {{module 'Lib1' is imported as 'package' from the same file; this 'internal' access level will be ignored}}
+
+/// Simple package vs internal, inverted.
+internal import Lib2 // expected-warning {{module 'Lib2' is imported as 'package' from the same file; this 'internal' access level will be ignored}}
+package import Lib2 // expected-note {{imported 'package' here}}
+// expected-note @-1 {{struct 'Type2' imported as 'package' from 'Lib2' here}}
+
+public func dummyAPI(t1: Type1) {} // expected-error {{function cannot be declared public because its parameter uses a package type}}
+// expected-note @-1 {{struct 'Type1' is imported by this file as 'package' from 'Lib1'}}
+
+public func dummyAPI(t2: Type2) {} // expected-error {{function cannot be declared public because its parameter uses a package type}}
+// expected-note @-1 {{struct 'Type2' is imported by this file as 'package' from 'Lib2'}}
+
+//--- Client_Clang.swift
+
+#if INVERT
+private import ClangLib
+#endif
+
+internal import ClangLib2 // expected-note {{struct 'ClangType' imported as 'internal' from 'ClangLib2' here}}
+
+#if !INVERT
+private import ClangLib
+#endif
+
+public func dummyAPI(t2: ClangType) {}
+// expected-error @-1 {{function cannot be declared public because its parameter uses an internal type}}
+// expected-note @-2 {{struct 'ClangType' is imported by this file as 'internal' from 'ClangLib2'}}
+
+//--- Client_Clang_Submodules.swift
+
+#if INVERT
+private import ClangLib.Sub1
+#endif
+
+internal import ClangLib.Sub2 // expected-note {{struct 'SubType' imported as 'internal' from 'Sub2' here}}
+
+#if !INVERT
+private import ClangLib.Sub1
+#endif
+
+public func dummyAPI(t2: SubType) {}
+// expected-error @-1 {{function cannot be declared public because its parameter uses an internal type}}
+// expected-note @-2 {{struct 'SubType' is imported by this file as 'internal' from 'Sub2'}}
+
+//--- module.modulemap
+
+module ClangLib {
+  header "ClangLib1.h"
+
+  explicit module Sub1 {
+    header "Sub1.h"
+  }
+
+  explicit module Sub2 {
+    header "Sub2.h"
+  }
+}
+
+module ClangLib2 {
+  header "ClangLib2.h"
+  export *
+}
+
+//--- ClangLib1.h
+struct ClangType {};
+
+//--- ClangLib2.h
+#include <ClangLib1.h>
+
+//--- Sub1.h
+struct SubType {};
+
+//--- Sub2.h
+#include "Sub1.h"

test/Sema/superfluously-public-imports.swift

@@ -337,8 +337,8 @@ public import ClangSubmoduleUnused.ClangSubmoduleUnsuedSubmodule // expected-war
 public import ClangTopModule.ClangTopModuleSubmodule
 
 public func clangUser(a: ClangSimpleType) {} // expected-remark {{struct 'ClangSimpleType' is imported via 'ClangSimple'}}
-public func clangUser(a: ClangSubmoduleSubmoduleType) {} // expected-remark {{struct 'ClangSubmoduleSubmoduleType' is imported via 'ClangSubmodule'}}
-public func clangUser(a: ClangTopModuleType) {} // expected-remark {{struct 'ClangTopModuleType' is imported via 'ClangTopModule'}}
+public func clangUser(a: ClangSubmoduleSubmoduleType) {} // expected-remark {{struct 'ClangSubmoduleSubmoduleType' is imported via 'ClangSubmoduleSubmodule'}}
+public func clangUser(a: ClangTopModuleType) {} // expected-remark {{struct 'ClangTopModuleType' is imported via 'ClangTopModuleSubmodule'}}
 
 //--- ClientOfClangReexportedSubmodules.swift
 public import ClangReexportedSubmodulePublic.ClangReexportedSubmodulePublicSub