[Sema] Report inconsistent access level on import

When targeting Swift 5 and earlier, the default import is public. To
notify of a possible unintentional public import, raise errors on
default imports when other imports of the same target have an explicit
type.

This check isn't necessary in the tentative Swift 6 mode.

Author: xymus

include/swift/AST/DiagnosticsSema.def

@@ -3160,6 +3160,18 @@ ERROR(error_public_import_of_private_module,none,
       "private module %0 is imported publicly from the public module %1",
       (Identifier, Identifier))
 
+ERROR(inconsistent_implicit_access_level_on_import,none,
+      "ambiguous implicit access level for import of %0; it is imported as "
+      "'%select{private|fileprivate|internal|package|public|%error}1' elsewhere",
+      (Identifier, AccessLevel))
+NOTE(inconsistent_implicit_access_level_on_import_here,none,
+     "imported "
+     "'%select{private|fileprivate|internal|package|public|%error}0' here",
+     (AccessLevel))
+FIXIT(inconsistent_implicit_access_level_on_import_fixit,
+     "%select{private|fileprivate|internal|package|public|%error}0 ",
+     (AccessLevel))
+
 ERROR(implementation_only_decl_non_override,none,
       "'@_implementationOnly' can only be used on overrides", ())
 ERROR(implementation_only_override_changed_type,none,

include/swift/AST/TypeCheckRequests.h

@@ -3367,6 +3367,27 @@ class CheckInconsistentSPIOnlyImportsRequest
   bool isCached() const { return true; }
 };
 
+/// Report default imports if other imports of the same target from this
+/// module have an explicitly defined access level. In such a case, all imports
+/// of the target module need an explicit access level or it may be made public
+/// by error. This applies only to pre-Swift 6 mode.
+class CheckInconsistentAccessLevelOnImport
+    : public SimpleRequest<CheckInconsistentAccessLevelOnImport,
+                           evaluator::SideEffect(SourceFile *),
+                           RequestFlags::Cached> {
+public:
+  using SimpleRequest::SimpleRequest;
+
+private:
+  friend SimpleRequest;
+
+  evaluator::SideEffect evaluate(Evaluator &evaluator, SourceFile *mod) const;
+
+public:
+  // Cached.
+  bool isCached() const { return true; }
+};
+
 /// Checks to see if any of the imports in a module use \c @_weakLinked
 /// in one file and not in another.
 ///

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -35,6 +35,8 @@ SWIFT_REQUEST(TypeChecker, CheckInconsistentImplementationOnlyImportsRequest,
               evaluator::SideEffect(ModuleDecl *), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, CheckInconsistentSPIOnlyImportsRequest,
               evaluator::SideEffect(SourceFile *), Cached, NoLocationInfo)
+SWIFT_REQUEST(TypeChecker, CheckInconsistentAccessLevelOnImport,
+              evaluator::SideEffect(SourceFile *), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, CheckInconsistentWeakLinkedImportsRequest,
               evaluator::SideEffect(ModuleDecl *), Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, CheckRedeclarationRequest,

lib/Sema/ImportResolution.cpp

@@ -917,6 +917,41 @@ CheckInconsistentSPIOnlyImportsRequest::evaluate(
   return {};
 }
 
+evaluator::SideEffect
+CheckInconsistentAccessLevelOnImport::evaluate(
+    Evaluator &evaluator, SourceFile *SF) const {
+
+  auto mod = SF->getParentModule();
+  auto diagnose = [mod](const ImportDecl *implicitImport,
+                        const ImportDecl *otherImport) {
+    auto otherAccessLevel = otherImport->getAccessLevel();
+
+    auto &diags = mod->getDiags();
+    {
+      InFlightDiagnostic error =
+        diags.diagnose(implicitImport, diag::inconsistent_implicit_access_level_on_import,
+                       implicitImport->getModule()->getName(), otherAccessLevel);
+      error.fixItInsert(implicitImport->getStartLoc(),
+                        diag::inconsistent_implicit_access_level_on_import_fixit,
+                        otherAccessLevel);
+    }
+
+    SourceLoc accessLevelLoc = otherImport->getStartLoc();
+    if (auto attr = otherImport->getAttrs().getAttribute<AccessControlAttr>())
+      accessLevelLoc = attr->getLocation();
+    diags.diagnose(accessLevelLoc,
+                   diag::inconsistent_implicit_access_level_on_import_here,
+                   otherAccessLevel);
+  };
+
+  auto predicate = [](ImportDecl *decl) {
+    return !decl->isAccessLevelImplicit();
+  };
+
+  findInconsistentImportsAcrossModule(mod, predicate, diagnose);
+  return {};
+}
+
 evaluator::SideEffect
 CheckInconsistentWeakLinkedImportsRequest::evaluate(Evaluator &evaluator,
                                                     ModuleDecl *mod) const {

lib/Sema/TypeChecker.cpp

@@ -313,6 +313,13 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
       CheckInconsistentSPIOnlyImportsRequest{SF},
       {});
 
+  if (!Ctx.LangOpts.isSwiftVersionAtLeast(6)) {
+    evaluateOrDefault(
+      Ctx.evaluator,
+      CheckInconsistentAccessLevelOnImport{SF},
+      {});
+  }
+
   evaluateOrDefault(
       Ctx.evaluator,
       CheckInconsistentWeakLinkedImportsRequest{SF->getParentModule()}, {});

test/Sema/access-level-import-inconsistencies.swift

@@ -0,0 +1,64 @@
+/// Import inconsistencies.
+///
+/// Swift 5 case, with slow adoption of access level on imports.
+/// Report default imports if any other import of the same module
+/// has an access level below public (the default).
+///
+/// Swift 6 case, with default imports as internal.
+/// Don't report any mismatch.
+///
+/// RUN: %empty-directory(%t)
+/// RUN: split-file %s %t
+
+/// Build the library.
+// RUN: %target-swift-frontend -emit-module %t/Lib.swift -o %t
+
+/// Check diagnostics.
+
+//--- Lib.swift
+public struct LibType {}
+
+// RUN: %target-swift-frontend -typecheck %t/OneFile_AllExplicit.swift -I %t \
+// RUN:   -enable-experimental-feature AccessLevelOnImport -verify
+//--- OneFile_AllExplicit.swift
+public import Lib
+package import Lib
+internal import Lib
+fileprivate import Lib
+private import Lib
+
+// RUN: %target-swift-frontend -typecheck %t/ManyFiles_AllExplicit_File?.swift -I %t \
+// RUN:   -enable-experimental-feature AccessLevelOnImport -verify
+//--- ManyFiles_AllExplicit_FileA.swift
+public import Lib
+//--- ManyFiles_AllExplicit_FileB.swift
+package import Lib
+//--- ManyFiles_AllExplicit_FileC.swift
+internal import Lib
+//--- ManyFiles_AllExplicit_FileD.swift
+fileprivate import Lib
+//--- ManyFiles_AllExplicit_FileE.swift
+private import Lib
+
+// RUN: %target-swift-frontend -typecheck %t/ManyFiles_ImplicitVsInternal_FileB.swift -I %t \
+// RUN:   -enable-experimental-feature AccessLevelOnImport -verify \
+// RUN:   -primary-file %t/ManyFiles_ImplicitVsInternal_FileA.swift
+//--- ManyFiles_ImplicitVsInternal_FileA.swift
+import Lib // expected-error {{ambiguous implicit access level for import of 'Lib'; it is imported as 'internal' elsewhere}}
+//--- ManyFiles_ImplicitVsInternal_FileB.swift
+internal import Lib // expected-note {{imported 'internal' here}}
+
+// RUN: %target-swift-frontend -typecheck %t/ManyFiles_ImplicitVsPackage_FileB.swift -I %t \
+// RUN:   -enable-experimental-feature AccessLevelOnImport -verify \
+// RUN:   -primary-file %t/ManyFiles_ImplicitVsPackage_FileA.swift
+//--- ManyFiles_ImplicitVsPackage_FileA.swift
+import Lib // expected-error {{ambiguous implicit access level for import of 'Lib'; it is imported as 'package' elsewhere}}
+//--- ManyFiles_ImplicitVsPackage_FileB.swift
+package import Lib // expected-note {{imported 'package' here}} @:1
+
+// RUN: %target-swift-frontend -typecheck %t/ManyFiles_AmbiguitySwift6_File?.swift -I %t \
+// RUN:   -enable-experimental-feature AccessLevelOnImport -verify -swift-version 6
+//--- ManyFiles_AmbiguitySwift6_FileA.swift
+import Lib
+//--- ManyFiles_AmbiguitySwift6_FileB.swift
+internal import Lib