Merge pull request #68776 from xymus/access-level-import-indirect-checks

Sema: report conformances and typealias underlying types behind non-public imports

Author: xymus

include/swift/AST/Decl.h

@@ -982,6 +982,16 @@ class alignas(1 << DeclAlignInBits) Decl : public ASTAllocated<Decl> {
   /// Returns the source range of the declaration including its attributes.
   SourceRange getSourceRangeIncludingAttrs() const;
 
+  using ImportAccessLevel = llvm::Optional<AttributedImport<ImportedModule>>;
+
+  /// Returns the import that may restrict the access to this decl
+  /// from \p useDC.
+  ///
+  /// If this decl and \p useDC are from the same module it returns
+  /// \c llvm::None. If there are many imports, it returns the most
+  /// permissive one.
+  ImportAccessLevel getImportAccessFrom(const DeclContext *useDC) const;
+
   SourceLoc TrailingSemiLoc;
 
   /// Whether this declaration is within a macro expansion relative to
@@ -2793,15 +2803,6 @@ class ValueDecl : public Decl {
                         bool forConformance = false,
                         bool allowUsableFromInline = false) const;
 
-  using ImportAccessLevel = llvm::Optional<AttributedImport<ImportedModule>>;
-
-  /// Returns the import that may restrict the access to this decl
-  /// from \p useDC.
-  ///
-  /// If this decl and \p useDC are from the same module it returns
-  /// \c llvm::None. If there are many imports, it returns the most
-  /// permissive one.
-  ImportAccessLevel getImportAccessFrom(const DeclContext *useDC) const;
 
   /// Returns whether this declaration should be treated as \c open from
   /// \p useDC. This is very similar to #getFormalAccess, but takes

include/swift/AST/DiagnosticsSema.def

@@ -2334,7 +2334,7 @@ NOTE(decl_import_via_here,none,
      "%kind0 imported as "
      "'%select{private|fileprivate|internal|package|%ERROR|%ERROR}1' "
      "from %2 here",
-     (const ValueDecl *, AccessLevel, const ModuleDecl*))
+     (const Decl *, AccessLevel, const ModuleDecl*))
 
 // Opaque return types
 ERROR(opaque_type_invalid_constraint,none,
@@ -3437,7 +3437,8 @@ ERROR(decl_from_hidden_module,none,
       "it is SPI|"
       "%2 was imported for SPI only|"
       "%2 was not imported by this file|"
-      "C++ types from imported module %2 do not support library evolution}3",
+      "C++ types from imported module %2 do not support library evolution|"
+      "<<ERROR>>}3",
       (const Decl *, unsigned, Identifier, unsigned))
 ERROR(typealias_desugars_to_type_from_hidden_module,none,
       "%0 aliases '%1.%2' and cannot be used %select{here|"
@@ -3450,7 +3451,8 @@ ERROR(typealias_desugars_to_type_from_hidden_module,none,
       "<<ERROR>>|"
       "%4 was imported for SPI only|"
       "%4 was not imported by this file|"
-      "C++ types from imported module %4 do not support library evolution}5",
+      "C++ types from imported module %4 do not support library evolution|"
+      "%4 was not imported publicly}5",
       (const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
 ERROR(conformance_from_implementation_only_module,none,
       "cannot use conformance of %0 to %1 %select{here|as property wrapper here|"
@@ -3462,7 +3464,8 @@ ERROR(conformance_from_implementation_only_module,none,
       "the conformance is declared as SPI|"
       "%3 was imported for SPI only|"
       "%3 was not imported by this file|"
-      "C++ types from imported module %3 do not support library evolution}4",
+      "C++ types from imported module %3 do not support library evolution|"
+      "%3 was not imported publicly}4",
       (Type, Identifier, unsigned, Identifier, unsigned))
 NOTE(assoc_conformance_from_implementation_only_module,none,
      "in associated type %0 (inferred as %1)", (Type, Type))
@@ -6568,7 +6571,8 @@ ERROR(inlinable_decl_ref_from_hidden_module,
       "it is SPI|"
       "%2 was imported for SPI only|"
       "%2 was not imported by this file|"
-      "C++ APIs from imported module %2 do not support library evolution}3",
+      "C++ APIs from imported module %2 do not support library evolution|"
+      "<<ERROR>>}3",
       (const ValueDecl *, unsigned, Identifier, unsigned))
 
 WARNING(inlinable_decl_ref_from_hidden_module_warn,
@@ -6585,7 +6589,8 @@ ERROR(inlinable_typealias_desugars_to_type_from_hidden_module,
       "<<ERROR>>|"
       "%4 was imported for SPI only|"
       "%4 was not imported by this file|"
-      "C++ types from imported module %4 do not support library evolution}5",
+      "C++ types from imported module %4 do not support library evolution|"
+      "%4 was not imported publicly}5",
       (const TypeAliasDecl *, StringRef, StringRef, unsigned, Identifier, unsigned))
 
 NOTE(missing_import_inserted,

lib/AST/Decl.cpp

@@ -4330,7 +4330,7 @@ bool ValueDecl::isAccessibleFrom(const DeclContext *useDC,
                      [&]() { return getFormalAccess(); });
 }
 
-ImportAccessLevel ValueDecl::getImportAccessFrom(const DeclContext *useDC) const {
+ImportAccessLevel Decl::getImportAccessFrom(const DeclContext *useDC) const {
   ModuleDecl *Mod = getModuleContext();
   if (useDC && useDC->getParentModule() != Mod) {
     if (auto useSF = useDC->getParentSourceFile()) {

lib/Sema/ResilienceDiagnostics.cpp

@@ -152,31 +152,44 @@ static bool diagnoseTypeAliasDeclRefExportability(SourceLoc loc,
 
   auto definingModule = D->getModuleContext();
   auto fragileKind = where.getFragileFunctionKind();
+  bool warnPreSwift6 = originKind != DisallowedOriginKind::SPIOnly &&
+                       originKind != DisallowedOriginKind::NonPublicImport;
   if (fragileKind.kind == FragileFunctionKind::None) {
     auto reason = where.getExportabilityReason();
     ctx.Diags
         .diagnose(loc, diag::typealias_desugars_to_type_from_hidden_module,
                   TAD, definingModule->getNameStr(), D->getNameStr(),
                   static_cast<unsigned>(*reason), definingModule->getName(),
                   static_cast<unsigned>(originKind))
-        .warnUntilSwiftVersionIf(originKind != DisallowedOriginKind::SPIOnly,
-                                 6);
+        .warnUntilSwiftVersionIf(warnPreSwift6, 6);
   } else {
     ctx.Diags
         .diagnose(loc,
                   diag::inlinable_typealias_desugars_to_type_from_hidden_module,
                   TAD, definingModule->getNameStr(), D->getNameStr(),
                   fragileKind.getSelector(), definingModule->getName(),
                   static_cast<unsigned>(originKind))
-        .warnUntilSwiftVersionIf(originKind != DisallowedOriginKind::SPIOnly,
-                                 6);
+        .warnUntilSwiftVersionIf(warnPreSwift6, 6);
   }
   D->diagnose(diag::kind_declared_here, DescriptiveDeclKind::Type);
 
   if (originKind == DisallowedOriginKind::MissingImport &&
       !ctx.LangOpts.isSwiftVersionAtLeast(6))
     addMissingImport(loc, D, where);
 
+  // If limited by an import, note which one.
+  if (originKind == DisallowedOriginKind::NonPublicImport) {
+    const DeclContext *DC = where.getDeclContext();
+    ImportAccessLevel limitImport = D->getImportAccessFrom(DC);
+    assert(limitImport.has_value() &&
+           limitImport->accessLevel < AccessLevel::Public &&
+           "The import should still be non-public");
+    ctx.Diags.diagnose(limitImport->accessLevelLoc,
+                       diag::decl_import_via_here, D,
+                       limitImport->accessLevel,
+                       limitImport->module.importedModule);
+  }
+
   return true;
 }
 
@@ -192,6 +205,10 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
   if (originKind == DisallowedOriginKind::None)
     return false;
 
+  // Access levels from imports are reported with the others access levels.
+  if (originKind == DisallowedOriginKind::NonPublicImport)
+    return false;
+
   auto diagName = D->getName();
   if (auto accessor = dyn_cast<AccessorDecl>(D)) {
     // Only diagnose accessors if their disallowed origin kind differs from
@@ -290,13 +307,27 @@ TypeChecker::diagnoseConformanceExportability(SourceLoc loc,
                      static_cast<unsigned>(originKind))
       .warnUntilSwiftVersionIf((useConformanceAvailabilityErrorsOption &&
                                 !ctx.LangOpts.EnableConformanceAvailabilityErrors &&
-                                originKind != DisallowedOriginKind::SPIOnly) ||
+                                originKind != DisallowedOriginKind::SPIOnly &&
+                                originKind != DisallowedOriginKind::NonPublicImport) ||
                                originKind == DisallowedOriginKind::MissingImport,
                                6);
 
   if (originKind == DisallowedOriginKind::MissingImport &&
       !ctx.LangOpts.isSwiftVersionAtLeast(6))
     addMissingImport(loc, ext, where);
 
+  // If limited by an import, note which one.
+  if (originKind == DisallowedOriginKind::NonPublicImport) {
+    const DeclContext *DC = where.getDeclContext();
+    ImportAccessLevel limitImport = ext->getImportAccessFrom(DC);
+    assert(limitImport.has_value() &&
+           limitImport->accessLevel < AccessLevel::Public &&
+           "The import should still be non-public");
+    ctx.Diags.diagnose(limitImport->accessLevelLoc,
+                       diag::decl_import_via_here, ext,
+                       limitImport->accessLevel,
+                       limitImport->module.importedModule);
+  }
+
   return true;
 }

lib/Sema/TypeCheckAccess.cpp

@@ -1915,6 +1915,13 @@ swift::getDisallowedOriginKind(const Decl *decl,
       isFragileClangNode(decl->getClangNode()))
     return DisallowedOriginKind::FragileCxxAPI;
 
+  // Report non-public import last as it can be ignored by the caller.
+  // See \c diagnoseValueDeclRefExportability.
+  auto importSource = decl->getImportAccessFrom(where.getDeclContext());
+  if (importSource.has_value() &&
+      importSource->accessLevel < AccessLevel::Public)
+    return DisallowedOriginKind::NonPublicImport;
+
   return DisallowedOriginKind::None;
 }
 

lib/Sema/TypeCheckAccess.h

@@ -47,6 +47,7 @@ enum class DisallowedOriginKind : uint8_t {
   SPIOnly,
   MissingImport,
   FragileCxxAPI,
+  NonPublicImport,
   None
 };
 

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

@@ -0,0 +1,31 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file --leading-lines %s %t
+
+/// Build the libraries.
+// RUN: %target-swift-frontend -emit-module %t/ConformanceBaseTypes.swift -o %t
+// RUN: %target-swift-frontend -emit-module %t/ConformanceDefinition.swift -o %t -I %t
+
+/// Check diagnostics.
+// RUN: %target-swift-frontend -typecheck -verify %t/Client.swift -I %t \
+// RUN:   -enable-experimental-feature AccessLevelOnImport
+
+//--- ConformanceBaseTypes.swift
+public protocol Proto {}
+public struct ConformingType {
+    public init () {}
+}
+
+//--- ConformanceDefinition.swift
+import ConformanceBaseTypes
+extension ConformingType : Proto  {}
+
+//--- Client.swift
+public import ConformanceBaseTypes
+internal import ConformanceDefinition // expected-note 2 {{extension of struct 'ConformingType' imported as 'internal' from 'ConformanceDefinition' here}}
+
+public func useInAPI(a: any Proto = ConformingType()) { // expected-error {{cannot use conformance of 'ConformingType' to 'Proto' here; 'ConformanceDefinition' was not imported publicly}}
+}
+
+@inlinable public func inlinableFunc() {
+  let _: any Proto = ConformingType() // expected-error {{cannot use conformance of 'ConformingType' to 'Proto' here; 'ConformanceDefinition' was not imported publicly}}
+}

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

@@ -0,0 +1,32 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-emit-module-interface(%t/Original.swiftinterface) %t/Original.swift
+// RUN: %target-swift-typecheck-module-from-interface(%t/Original.swiftinterface)
+
+// RUN: %target-swift-emit-module-interface(%t/Aliases.swiftinterface) %t/Aliases.swift -I %t
+// RUN: %target-swift-typecheck-module-from-interface(%t/Aliases.swiftinterface) -I %t
+
+// RUN: %target-swift-frontend -typecheck -verify %t/UsesAliasesNoImport.swift -I %t \
+// RUN:   -swift-version 5 -enable-library-evolution \
+// RUN:   -enable-experimental-feature AccessLevelOnImport
+
+//--- Original.swift
+open class Clazz {}
+
+//--- Aliases.swift
+import Original
+public typealias ClazzAlias = Clazz
+
+//--- UsesAliasesNoImport.swift
+public import Aliases
+internal import Original // expected-note 2 {{class 'Clazz' imported as 'internal' from 'Original' here}}
+
+// expected-error@+1 {{'ClazzAlias' aliases 'Original.Clazz' and cannot be used here because 'Original' was not imported publicly}}
+public class InheritsFromClazzAlias: ClazzAlias {}
+
+@inlinable public func inlinableFunc() {
+  // expected-error@+1 {{'ClazzAlias' aliases 'Original.Clazz' and cannot be used in an '@inlinable' function because 'Original' was not imported publicly}}
+  _ = ClazzAlias.self
+}
+