Merge pull request #75899 from tshortli/member-import-visibility-spi

Sema: Only add `@_spiOnly` to import fix-its with a public access level

Author: tshortli

lib/Sema/TypeCheckNameLookup.cpp

@@ -830,15 +830,26 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
 
   llvm::SmallString<64> importText;
 
+  // Add flags that must be used consistently on every import in every file.
   auto fixItInfo = fixItCache.getInfo(definingModule);
   if (fixItInfo.flags.contains(ImportFlags::ImplementationOnly))
     importText += "@_implementationOnly ";
   if (fixItInfo.flags.contains(ImportFlags::WeakLinked))
     importText += "@_weakLinked ";
-  if (fixItInfo.flags.contains(ImportFlags::SPIOnly))
-    importText += "@_spiOnly ";
 
-  // @_spi imports.
+  auto explicitAccessLevel = fixItInfo.accessLevel;
+  bool isPublicImport =
+      explicitAccessLevel
+          ? *explicitAccessLevel >= AccessLevel::Public
+          : !ctx.LangOpts.hasFeature(Feature::InternalImportsByDefault);
+
+  // Add flags that are only appropriate on public imports.
+  if (isPublicImport) {
+    if (fixItInfo.flags.contains(ImportFlags::SPIOnly))
+      importText += "@_spiOnly ";
+  }
+
+  // Add @_spi groups if needed for the declaration.
   if (decl->isSPI()) {
     auto spiGroups = decl->getSPIGroups();
     if (!spiGroups.empty()) {
@@ -848,8 +859,8 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
     }
   }
 
-  if (auto accessLevel = fixItInfo.accessLevel) {
-    importText += getAccessLevelSpelling(*accessLevel);
+  if (explicitAccessLevel) {
+    importText += getAccessLevelSpelling(*explicitAccessLevel);
     importText += " ";
   }
 

test/NameLookup/members_transitive_multifile_access_level.swift

@@ -8,18 +8,34 @@
 // RUN: %target-swift-frontend -emit-module -o %t %t/MixedUses.swift
 // RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyTransitivelyImported.swift
 // RUN: %target-swift-frontend -emit-module -o %t %t/Exports.swift -I %t
+// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlySPIOnly.swift -I %t
+// RUN: %target-swift-frontend -emit-module -o %t %t/InternalUsesOnlyDefaultedImportSPIOnly.swift -I %t
+// RUN: %target-swift-frontend -emit-module -o %t %t/PublicUsesOnlySPIOnly.swift -I %t
+
 // RUN: %target-swift-frontend -typecheck -verify -swift-version 5 \
 // RUN:   -primary-file %t/function_bodies.swift \
 // RUN:   -primary-file %t/function_signatures_unqualified.swift \
 // RUN:   -primary-file %t/function_signatures_qualified.swift \
 // RUN:   -primary-file %t/extensions.swift \
 // RUN:   %t/imports.swift \
 // RUN:   -I %t -package-name Package \
-// RUN:   -enable-experimental-feature MemberImportVisibility
+// RUN:   -enable-experimental-feature MemberImportVisibility \
+// RUN:   -verify-additional-prefix public-by-default-
+
+// RUN: %target-swift-frontend -typecheck -verify -swift-version 5 \
+// RUN:   -primary-file %t/function_bodies.swift \
+// RUN:   -primary-file %t/function_signatures_unqualified.swift \
+// RUN:   -primary-file %t/function_signatures_qualified.swift \
+// RUN:   -primary-file %t/extensions.swift \
+// RUN:   %t/imports.swift \
+// RUN:   -I %t -package-name Package \
+// RUN:   -enable-experimental-feature MemberImportVisibility \
+// RUN:   -enable-upcoming-feature InternalImportsByDefault \
+// RUN:   -verify-additional-prefix internal-by-default-
 
 //--- function_bodies.swift
 
-// FIXME: The access level is wrong on many of these fix-its.
+// FIXME: The access level of some of these fix-its should be public/package instead of internal.
 import Swift // Just here to anchor the fix-its
 // expected-note      {{add import of module 'InternalUsesOnly'}}{{1-1=internal import InternalUsesOnly\n}}
 // expected-note@-1   {{add import of module 'InternalUsesOnlyDefaultedImport'}}{{1-1=import InternalUsesOnlyDefaultedImport\n}}
@@ -28,12 +44,19 @@ import Swift // Just here to anchor the fix-its
 // expected-note@-4   {{add import of module 'PublicUsesOnlyDefaultedImport'}}{{1-1=import PublicUsesOnlyDefaultedImport\n}}
 // expected-note@-5 3 {{add import of module 'MixedUses'}}{{1-1=internal import MixedUses\n}}
 // expected-note@-6   {{add import of module 'InternalUsesOnlyTransitivelyImported'}}{{1-1=internal import InternalUsesOnlyTransitivelyImported\n}}
+// expected-note@-7   {{add import of module 'InternalUsesOnlySPIOnly'}}{{1-1=internal import InternalUsesOnlySPIOnly\n}}
+// expected-public-by-default-note@-8     {{add import of module 'InternalUsesOnlyDefaultedImportSPIOnly'}}{{1-1=@_spiOnly import InternalUsesOnlyDefaultedImportSPIOnly\n}}
+// expected-internal-by-default-note@-9   {{add import of module 'InternalUsesOnlyDefaultedImportSPIOnly'}}{{1-1=import InternalUsesOnlyDefaultedImportSPIOnly\n}}
+// expected-note@-10  {{add import of module 'PublicUsesOnlySPIOnly'}}{{1-1=internal import PublicUsesOnlySPIOnly\n}}
+
 
 func internalFunc(_ x: Int) {
   _ = x.memberInInternalUsesOnly // expected-error {{property 'memberInInternalUsesOnly' is not available due to missing import of defining module 'InternalUsesOnly'}}
   _ = x.memberInInternalUsesOnlyDefaultedImport // expected-error {{property 'memberInInternalUsesOnlyDefaultedImport' is not available due to missing import of defining module 'InternalUsesOnlyDefaultedImport'}}
   _ = x.memberInMixedUses // expected-error {{property 'memberInMixedUses' is not available due to missing import of defining module 'MixedUses'}}
   _ = x.memberInInternalUsesOnlyTransitivelyImported // expected-error {{property 'memberInInternalUsesOnlyTransitivelyImported' is not available due to missing import of defining module 'InternalUsesOnlyTransitivelyImported'}}
+  _ = x.memberInInternalUsesOnlySPIOnly // expected-error {{property 'memberInInternalUsesOnlySPIOnly' is not available due to missing import of defining module 'InternalUsesOnlySPIOnly'}}
+  _ = x.memberInInternalUsesOnlyDefaultedImportSPIOnly // expected-error {{property 'memberInInternalUsesOnlyDefaultedImportSPIOnly' is not available due to missing import of defining module 'InternalUsesOnlyDefaultedImportSPIOnly'}}
 }
 
 @inlinable package func packageInlinableFunc(_ x: Int) {
@@ -45,6 +68,7 @@ func internalFunc(_ x: Int) {
   _ = x.memberInPublicUsesOnly // expected-error {{property 'memberInPublicUsesOnly' is not available due to missing import of defining module 'PublicUsesOnly'}}
   _ = x.memberInPublicUsesOnlyDefaultedImport // expected-error {{property 'memberInPublicUsesOnlyDefaultedImport' is not available due to missing import of defining module 'PublicUsesOnlyDefaultedImport'}}
   _ = x.memberInMixedUses // expected-error {{property 'memberInMixedUses' is not available due to missing import of defining module 'MixedUses'}}
+  _ = x.memberInPublicUsesOnlySPIOnly // expected-error {{property 'memberInPublicUsesOnlySPIOnly' is not available due to missing import of defining module 'PublicUsesOnlySPIOnly'}}
 }
 
 //--- function_signatures_unqualified.swift
@@ -120,6 +144,9 @@ internal import PublicUsesOnly
 import PublicUsesOnlyDefaultedImport
 internal import MixedUses
 internal import Exports
+@_spiOnly public import InternalUsesOnlySPIOnly
+@_spiOnly import InternalUsesOnlyDefaultedImportSPIOnly
+@_spiOnly public import PublicUsesOnlySPIOnly
 
 //--- InternalUsesOnly.swift
 
@@ -180,3 +207,21 @@ extension Int {
 //--- Exports.swift
 
 @_exported import InternalUsesOnlyTransitivelyImported
+
+//--- InternalUsesOnlySPIOnly.swift
+
+extension Int {
+  public var memberInInternalUsesOnlySPIOnly: Int { return self }
+}
+
+//--- InternalUsesOnlyDefaultedImportSPIOnly.swift
+
+extension Int {
+  public var memberInInternalUsesOnlyDefaultedImportSPIOnly: Int { return self }
+}
+
+//--- PublicUsesOnlySPIOnly.swift
+
+extension Int {
+  public var memberInPublicUsesOnlySPIOnly: Int { return self }
+}