Merge pull request swiftlang#73094 from tshortli/rename-extension-import-visibility-feature-6.0

[6.0] Rename ExtensionImportVisibility to MemberImportVisibility and fix bugs

Author: tshortli

include/swift/AST/Decl.h

@@ -2788,7 +2788,7 @@ class ValueDecl : public Decl {
 public:
   /// Find the import that makes the given declaration available.
   std::optional<AttributedImport<ImportedModule>>
-  findImport(const DeclContext *fromDC);
+  findImport(const DeclContext *fromDC) const;
 
   /// Return true if this protocol member is a protocol requirement.
   ///

include/swift/AST/DiagnosticsSema.def

@@ -167,9 +167,9 @@ ERROR(init_candidate_inaccessible,none,
 
 ERROR(candidate_from_missing_import,none,
       "%0 %1 is not available due to missing import of defining module %2",
-      (DescriptiveDeclKind, DeclName, DeclName))
+      (DescriptiveDeclKind, DeclName, ModuleDecl *))
 NOTE(candidate_add_import,none,
-     "add import of module %0", (DeclName))
+     "add import of module %0", (ModuleDecl *))
 
 ERROR(cannot_pass_rvalue_mutating_subelement,none,
       "cannot use mutating member on immutable value: %0",

include/swift/AST/Module.h

@@ -1269,6 +1269,11 @@ void collectParsedExportedImports(const ModuleDecl *M,
                                   llvm::SmallDenseMap<ModuleDecl *, SmallPtrSet<Decl *, 4>, 4> &QualifiedImports,
                                   llvm::function_ref<bool(AttributedImport<ImportedModule>)> includeImport = nullptr);
 
+/// If the import that would make the given declaration visibile is absent,
+/// emit a diagnostic and a fix-it suggesting adding the missing import.
+bool diagnoseMissingImportForMember(const ValueDecl *decl,
+                                    const DeclContext *dc, SourceLoc loc);
+
 } // end namespace swift
 
 #endif

include/swift/AST/SourceFile.h

@@ -457,6 +457,9 @@ class SourceFile final : public FileUnit {
   /// If not, we can fast-path module checks.
   bool hasImportsWithFlag(ImportFlags flag) const;
 
+  /// Returns the import flags that are active on imports of \p module.
+  ImportFlags getImportFlags(const ModuleDecl *module) const;
+
   /// Get the most permissive restriction applied to the imports of \p module.
   RestrictedImportKind getRestrictedImportKind(const ModuleDecl *module) const;
 

include/swift/Basic/Features.def

@@ -364,8 +364,8 @@ EXPERIMENTAL_FEATURE(ClosureIsolation, true)
 // Enable isolated(any) attribute on function types.
 CONDITIONALLY_SUPPRESSIBLE_EXPERIMENTAL_FEATURE(IsolatedAny, true)
 
-// Whether members of extensions respect the enclosing file's imports.
-EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE(ExtensionImportVisibility, true)
+// Whether lookup of members respects the enclosing file's imports.
+EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE(MemberImportVisibility, true)
 
 // Alias for IsolatedAny
 EXPERIMENTAL_FEATURE(IsolatedAny2, true)

lib/AST/Decl.cpp

@@ -3856,7 +3856,7 @@ ValueDecl::getSatisfiedProtocolRequirements(bool Sorted) const {
 }
 
 std::optional<AttributedImport<ImportedModule>>
-ValueDecl::findImport(const DeclContext *fromDC) {
+ValueDecl::findImport(const DeclContext *fromDC) const {
   // If the type is from the current module, there's no import.
   auto module = getModuleContext();
   if (module == fromDC->getParentModule())

lib/AST/FeatureSet.cpp

@@ -680,7 +680,7 @@ static bool usesFeatureIsolatedAny(Decl *decl) {
   });
 }
 
-UNINTERESTING_FEATURE(ExtensionImportVisibility)
+UNINTERESTING_FEATURE(MemberImportVisibility)
 UNINTERESTING_FEATURE(IsolatedAny2)
 
 UNINTERESTING_FEATURE(ObjCImplementation)

lib/AST/Module.cpp

@@ -2702,6 +2702,15 @@ bool SourceFile::hasImportsWithFlag(ImportFlags flag) const {
       ctx.evaluator, HasImportsMatchingFlagRequest{mutableThis, flag}, false);
 }
 
+ImportFlags SourceFile::getImportFlags(const ModuleDecl *module) const {
+  unsigned flags = 0x0;
+  for (auto import : *Imports) {
+    if (import.module.importedModule == module)
+      flags |= import.options.toRaw();
+  }
+  return ImportFlags(flags);
+}
+
 bool SourceFile::hasTestableOrPrivateImport(
     AccessLevel accessLevel, const swift::ValueDecl *ofDecl,
     SourceFile::ImportQueryKind queryKind) const {
@@ -4001,3 +4010,62 @@ version::Version ModuleDecl::getLanguageVersionBuiltWith() const {
 
   return version::Version();
 }
+
+bool swift::diagnoseMissingImportForMember(const ValueDecl *decl,
+                                           const DeclContext *dc,
+                                           SourceLoc loc) {
+  if (decl->findImport(dc))
+    return false;
+
+  auto &ctx = dc->getASTContext();
+  auto definingModule = decl->getModuleContext();
+  ctx.Diags.diagnose(loc, diag::candidate_from_missing_import,
+                     decl->getDescriptiveKind(), decl->getName(),
+                     definingModule);
+
+  SourceLoc bestLoc =
+      ctx.Diags.getBestAddImportFixItLoc(decl, dc->getParentSourceFile());
+  if (!bestLoc.isValid())
+    return false;
+
+  llvm::SmallString<64> importText;
+
+  // Check other source files for import flags that should be applied to the
+  // fix-it for consistency with the rest of the imports in the module.
+  auto parentModule = dc->getParentModule();
+  OptionSet<ImportFlags> flags;
+  for (auto file : parentModule->getFiles()) {
+    if (auto sf = dyn_cast<SourceFile>(file))
+      flags |= sf->getImportFlags(definingModule);
+  }
+
+  if (flags.contains(ImportFlags::Exported) ||
+      parentModule->isClangOverlayOf(definingModule))
+    importText += "@_exported ";
+  if (flags.contains(ImportFlags::ImplementationOnly))
+    importText += "@_implementationOnly ";
+  if (flags.contains(ImportFlags::WeakLinked))
+    importText += "@_weakLinked ";
+  if (flags.contains(ImportFlags::SPIOnly))
+    importText += "@_spiOnly ";
+
+  // FIXME: Access level should be considered, too.
+
+  // @_spi imports.
+  if (decl->isSPI()) {
+    auto spiGroups = decl->getSPIGroups();
+    if (!spiGroups.empty()) {
+      importText += "@_spi(";
+      importText += spiGroups[0].str();
+      importText += ") ";
+    }
+  }
+
+  importText += "import ";
+  importText += definingModule->getName().str();
+  importText += "\n";
+  ctx.Diags.diagnose(bestLoc, diag::candidate_add_import, definingModule)
+      .fixItInsert(bestLoc, importText);
+
+  return true;
+}

lib/AST/NameLookup.cpp

@@ -2293,7 +2293,7 @@ static bool isAcceptableLookupResult(const DeclContext *dc,
     // makes this decl visible.
     if (decl->getDeclContext()->getParentModule() != dc->getParentModule() &&
         dc->getASTContext().LangOpts.hasFeature(
-            Feature::ExtensionImportVisibility) &&
+            Feature::MemberImportVisibility) &&
         !decl->findImport(dc))
       return false;
   }

lib/Sema/CSDiagnostics.cpp

@@ -6187,40 +6187,11 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
 
   auto loc = nameLoc.isValid() ? nameLoc.getStartLoc() : ::getLoc(anchor);
   auto accessLevel = Member->getFormalAccessScope().accessLevelForDiagnostics();
-  bool suppressDeclHereNote = false;
   if (accessLevel == AccessLevel::Public &&
-      !Member->findImport(getDC())) {
-    auto definingModule = Member->getDeclContext()->getParentModule();
-    emitDiagnosticAt(loc, diag::candidate_from_missing_import,
-                     Member->getDescriptiveKind(), Member->getName(),
-                     definingModule->getName());
-
-    auto enclosingSF = getDC()->getParentSourceFile();
-    SourceLoc bestLoc =
-        getBestAddImportFixItLocation(Member, getDC()->getParentSourceFile());
-    if (bestLoc.isValid()) {
-      llvm::SmallString<64> importText;
-
-      // @_spi imports.
-      if (Member->isSPI()) {
-        auto spiGroups = Member->getSPIGroups();
-        if (!spiGroups.empty()) {
-          importText += "@_spi(";
-          importText += spiGroups[0].str();
-          importText += ") ";
-        }
-      }
-
-      importText += "import ";
-      importText += definingModule->getName().str();
-      importText += "\n";
-      emitDiagnosticAt(bestLoc, diag::candidate_add_import,
-                       definingModule->getName())
-        .fixItInsert(bestLoc, importText);
-    }
+      diagnoseMissingImportForMember(Member, getDC(), loc))
+    return true;
 
-    suppressDeclHereNote = true;
-  } else if (auto *CD = dyn_cast<ConstructorDecl>(Member)) {
+  if (auto *CD = dyn_cast<ConstructorDecl>(Member)) {
     emitDiagnosticAt(loc, diag::init_candidate_inaccessible,
                      CD->getResultInterfaceType(), accessLevel)
         .highlight(nameLoc.getSourceRange());
@@ -6230,8 +6201,7 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
         .highlight(nameLoc.getSourceRange());
   }
 
-  if (!suppressDeclHereNote)
-    emitDiagnosticAt(Member, diag::decl_declared_here, Member);
+  emitDiagnosticAt(Member, diag::decl_declared_here, Member);
   return true;
 }