AST/Sema: Make MemberImportVisibility a migratable feature.

tshortli · tshortli · commit aca604660fe6 · 2025-05-30T15:34:08.000-07:00
The migration to `MemberImportVisibility` can be performed mechanically by
adding missing import declarations, so offer automatic migration for the
feature.

Resolves rdar://151931597.
diff --git a/include/swift/AST/DiagnosticGroups.def b/include/swift/AST/DiagnosticGroups.def
@@ -49,6 +49,7 @@ GROUP(ErrorInFutureSwiftVersion, "error-in-future-swift-version")
 GROUP(ExistentialAny, "existential-any")
 GROUP(ExistentialMemberAccess, "existential-member-access-limitations")
 GROUP(IsolatedConformances, "isolated-conformances")
+GROUP(MemberImportVisibility, "member-import-visibility")
 GROUP(MultipleInheritance, "multiple-inheritance")
 GROUP(MutableGlobalVariable, "mutable-global-variable")
 GROUP(NominalTypes, "nominal-types")
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -167,16 +167,21 @@ ERROR(init_candidate_inaccessible,none,
       "'%select{private|fileprivate|internal|package|@_spi|@_spi}1' protection level",
       (Type, AccessLevel))
 
-ERROR(candidate_from_missing_import,none,
+GROUPED_ERROR(member_from_missing_import,MemberImportVisibility,none,
       "%kind0 is not available due to missing import of defining module %1",
       (const ValueDecl *, const ModuleDecl *))
-ERROR(candidate_from_missing_imports_2_or_more,none,
+GROUPED_ERROR(member_from_missing_imports_2_or_more,MemberImportVisibility,none,
       "%kind0 is not available due to missing imports of defining modules "
       "%2%select{ and|, }1 %3%select{|, and others}1",
       (const ValueDecl *, bool, const ModuleDecl *, const ModuleDecl *))
 NOTE(candidate_add_import,none,
      "add import of module %0", (const ModuleDecl *))
 
+GROUPED_WARNING(add_required_import_for_member,MemberImportVisibility,none,
+     "import of module %0 is required", (const ModuleDecl *))
+NOTE(decl_from_module_used_here,none,
+     "%kind0 from %1 used here", (const ValueDecl *, const ModuleDecl *))
+
 ERROR(cannot_pass_rvalue_mutating_subelement,none,
       "cannot use mutating member on immutable value: %0",
       (StringRef))
diff --git a/include/swift/Basic/Features.def b/include/swift/Basic/Features.def
@@ -287,7 +287,7 @@ UPCOMING_FEATURE(GlobalActorIsolatedTypesUsability, 0434, 6)
 // Swift 7
 MIGRATABLE_UPCOMING_FEATURE(ExistentialAny, 335, 7)
 UPCOMING_FEATURE(InternalImportsByDefault, 409, 7)
-UPCOMING_FEATURE(MemberImportVisibility, 444, 7)
+MIGRATABLE_UPCOMING_FEATURE(MemberImportVisibility, 444, 7)
 MIGRATABLE_UPCOMING_FEATURE(InferIsolatedConformances, 470, 7)
 MIGRATABLE_UPCOMING_FEATURE(NonisolatedNonsendingByDefault, 461, 7)
 
diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h
@@ -873,13 +873,17 @@ namespace swift {
 
     /// Returns whether the given feature is enabled.
     ///
-    /// If allowMigration is set, also returns true when the feature has been enabled for migration.
+    /// If allowMigration is set, also returns true when the feature has been
+    /// enabled for migration.
     bool hasFeature(Feature feature, bool allowMigration = false) const;
 
     /// Returns whether a feature with the given name is enabled. Returns
     /// `false` if a feature by this name is not known.
     bool hasFeature(llvm::StringRef featureName) const;
 
+    /// Returns whether the given feature is enabled for migration.
+    bool isMigratingToFeature(Feature feature) const;
+
     /// Enables the given feature (enables in migration mode if `forMigration`
     /// is `true`).
     void enableFeature(Feature feature, bool forMigration = false);
diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp
@@ -2371,7 +2371,9 @@ ObjCCategoryNameMap ClassDecl::getObjCCategoryNameMap() {
 /// the given context.
 static bool shouldRequireImportsInContext(const DeclContext *lookupContext) {
   auto &ctx = lookupContext->getASTContext();
-  if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility))
+
+  if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                               /*allowMigration=*/true))
     return false;
 
   // Code outside of the main module (which is often synthesized) isn't subject
@@ -3591,7 +3593,8 @@ ExtendedNominalRequest::evaluate(Evaluator &evaluator,
   // inaccessible due to missing imports. The missing imports will be diagnosed
   // elsewhere.
   if (referenced.first.empty() &&
-      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                              /*allowMigration=*/true)) {
     options |= DirectlyReferencedTypeLookupFlags::IgnoreMissingImports;
     referenced = directReferencesForTypeRepr(evaluator, ctx, typeRepr,
                                              ext->getParent(), options);
diff --git a/lib/Basic/LangOptions.cpp b/lib/Basic/LangOptions.cpp
@@ -363,6 +363,10 @@ bool LangOptions::hasFeature(llvm::StringRef featureName) const {
   return false;
 }
 
+bool LangOptions::isMigratingToFeature(Feature feature) const {
+  return featureStates.getState(feature).isEnabledForMigration();
+}
+
 void LangOptions::enableFeature(Feature feature, bool forMigration) {
   if (forMigration) {
     ASSERT(feature.isMigratable());
diff --git a/lib/Basic/SupportedFeatures.cpp b/lib/Basic/SupportedFeatures.cpp
@@ -35,6 +35,8 @@ static std::vector<DiagGroupID> migratableCategories(Feature feature) {
       return { DiagGroupID::ExistentialAny };
     case Feature::InnerKind::InferIsolatedConformances:
       return { DiagGroupID::IsolatedConformances };
+    case Feature::InnerKind::MemberImportVisibility:
+      return { DiagGroupID::MemberImportVisibility };
     case Feature::InnerKind::NonisolatedNonsendingByDefault:
       return { DiagGroupID::NonisolatedNonsendingByDefault };
     case Feature::InnerKind::StrictMemorySafety:
diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp
@@ -6322,7 +6322,8 @@ static void diagnoseMissingMemberImports(const Expr *E, const DeclContext *DC) {
   };
 
   auto &ctx = DC->getASTContext();
-  if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility))
+  if (!ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                               /*allowMigration=*/true))
     return;
 
   DiagnoseWalker walker(DC);
diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp
@@ -279,7 +279,9 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
   // Some diagnostics emitted with the `MemberImportVisibility` feature enabled
   // subsume these diagnostics.
   if (originKind == DisallowedOriginKind::MissingImport &&
-      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility) && SF)
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                              /*allowMigration=*/true) &&
+      SF)
     return false;
 
   if (auto accessor = dyn_cast<AccessorDecl>(D)) {
diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp
@@ -1532,7 +1532,8 @@ bool swift::checkOverrides(ValueDecl *decl) {
 
   auto &ctx = decl->getASTContext();
   if (overridden.empty() &&
-      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                              /*allowMigration=*/true)) {
     // If we didn't find anything, try broadening the search by ignoring missing
     // imports.
     if (!checkPotentialOverrides(decl, overridden,
@@ -2529,7 +2530,8 @@ OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
   // If we didn't find anything, try broadening the search by ignoring missing
   // imports.
   if (overridden.empty() &&
-      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+      ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                              /*allowMigration=*/true)) {
     overridden = computeOverriddenDecls(decl, true);
     if (!overridden.empty()) {
       auto first = overridden.front();
diff --git a/lib/Sema/TypeCheckNameLookup.cpp b/lib/Sema/TypeCheckNameLookup.cpp
@@ -852,6 +852,12 @@ class MissingImportFixItCache {
   llvm::DenseMap<const ModuleDecl *, MissingImportFixItInfo> infos;
   bool internalImportsByDefaultEnabled;
 
+public:
+  MissingImportFixItCache(SourceFile &sf)
+      : sf(sf),
+        internalImportsByDefaultEnabled(sf.getASTContext().LangOpts.hasFeature(
+            Feature::InternalImportsByDefault)) {};
+
   MissingImportFixItInfo getFixItInfo(ModuleDecl *mod) {
     auto existing = infos.find(mod);
     if (existing != infos.end())
@@ -900,12 +906,6 @@ class MissingImportFixItCache {
     return info;
   }
 
-public:
-  MissingImportFixItCache(SourceFile &sf) : sf(sf) {
-    internalImportsByDefaultEnabled = sf.getASTContext().LangOpts.hasFeature(
-        Feature::InternalImportsByDefault);
-  };
-
   std::pair<SmallVector<ModuleDecl *, 2>,
             SmallVector<MissingImportFixItInfo, 2>>
   getModulesAndFixIts(ModuleDecl *mod) {
@@ -929,20 +929,17 @@ diagnoseMissingImportsForMember(const ValueDecl *decl,
   ASSERT(count > 0);
 
   if (count > 1) {
-    ctx.Diags.diagnose(loc, diag::candidate_from_missing_imports_2_or_more,
-                       decl, bool(count > 2), modulesToImport[0],
-                       modulesToImport[1]);
+    ctx.Diags.diagnose(loc, diag::member_from_missing_imports_2_or_more, decl,
+                       bool(count > 2), modulesToImport[0], modulesToImport[1]);
   } else {
-    ctx.Diags.diagnose(loc, diag::candidate_from_missing_import, decl,
+    ctx.Diags.diagnose(loc, diag::member_from_missing_import, decl,
                        modulesToImport.front());
   }
 }
 
-static void emitMissingImportFixIt(SourceLoc loc,
-                                   const MissingImportFixItInfo &fixItInfo,
-                                   ASTContext &ctx) {
-  llvm::SmallString<64> importText;
-
+static void appendMissingImportFixIt(llvm::SmallString<64> &importText,
+                                     const MissingImportFixItInfo &fixItInfo,
+                                     ASTContext &ctx) {
   // Add flags that must be used consistently on every import in every file.
   if (fixItInfo.flags.contains(ImportFlags::ImplementationOnly))
     importText += "@_implementationOnly ";
@@ -969,6 +966,12 @@ static void emitMissingImportFixIt(SourceLoc loc,
   importText += "import ";
   importText += fixItInfo.moduleToImport->getName().str();
   importText += "\n";
+}
+
+static void emitMissingImportNoteAndFixIt(
+    SourceLoc loc, const MissingImportFixItInfo &fixItInfo, ASTContext &ctx) {
+  llvm::SmallString<64> importText;
+  appendMissingImportFixIt(importText, fixItInfo, ctx);
   ctx.Diags
       .diagnose(loc, diag::candidate_add_import, fixItInfo.moduleToImport)
       .fixItInsert(loc, importText);
@@ -995,7 +998,7 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
     return;
 
   for (auto &fixItInfo : fixItInfos) {
-    emitMissingImportFixIt(bestLoc, fixItInfo, ctx);
+    emitMissingImportNoteAndFixIt(bestLoc, fixItInfo, ctx);
   }
 }
 
@@ -1024,6 +1027,11 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
   // In lazy typechecking mode just emit the diagnostic immediately without a
   // fix-it since there won't be an opportunity to emit delayed diagnostics.
   if (ctx.TypeCheckerOpts.EnableLazyTypecheck) {
+    // Lazy type-checking and migration for MemberImportVisibility are
+    // completely incompatible, so just skip the diagnostic entirely.
+    if (ctx.LangOpts.isMigratingToFeature(Feature::MemberImportVisibility))
+      return false;
+
     auto modulesToImport = missingImportsForDefiningModule(definingModule, *sf);
     if (modulesToImport.empty())
       return false;
@@ -1036,7 +1044,76 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
   return false;
 }
 
+void migrateToMemberImportVisibility(SourceFile &sf) {
+  auto delayedDiags = sf.takeDelayedMissingImportForMemberDiagnostics();
+  if (delayedDiags.empty())
+    return;
+
+  auto &ctx = sf.getASTContext();
+  auto bestLoc = ctx.Diags.getBestAddImportFixItLoc(&sf);
+  if (bestLoc.isInvalid())
+    return;
+
+  // Collect the distinct modules that need to be imported and map them
+  // to the collection of declarations which are used in the file and belong
+  // to the module.
+  llvm::SmallVector<ModuleDecl *, 8> modulesToImport;
+  llvm::SmallDenseMap<ModuleDecl *, std::vector<const ValueDecl *>>
+      declsByModuleToImport;
+  for (auto declAndLocs : delayedDiags) {
+    auto decl = declAndLocs.first;
+    auto definingModules = missingImportsForDefiningModule(
+        decl->getModuleContextForNameLookup(), sf);
+
+    for (auto definingModule : definingModules) {
+      auto existing = declsByModuleToImport.find(definingModule);
+      if (existing != declsByModuleToImport.end()) {
+        existing->second.push_back(decl);
+      } else {
+        declsByModuleToImport[definingModule] = {decl};
+        modulesToImport.push_back(definingModule);
+      }
+    }
+  }
+
+  // Emit one warning for each module that needcs to be imported and emit notes
+  // for each reference to a declaration from that module in the file.
+  llvm::sort(modulesToImport, [](ModuleDecl *lhs, ModuleDecl *rhs) -> int {
+    return lhs->getName().compare(rhs->getName());
+  });
+
+  auto fixItCache = MissingImportFixItCache(sf);
+  for (auto mod : modulesToImport) {
+    auto fixItInfo = fixItCache.getFixItInfo(mod);
+    llvm::SmallString<64> importText;
+    appendMissingImportFixIt(importText, fixItInfo, ctx);
+    ctx.Diags.diagnose(bestLoc, diag::add_required_import_for_member, mod)
+        .fixItInsert(bestLoc, importText);
+
+    auto decls = declsByModuleToImport.find(mod);
+    if (decls == declsByModuleToImport.end())
+      continue;
+
+    for (auto decl : decls->second) {
+      auto locs = delayedDiags.find(decl);
+      if (locs == delayedDiags.end())
+        continue;
+
+      for (auto loc : locs->second) {
+        ctx.Diags.diagnose(loc, diag::decl_from_module_used_here, decl, mod);
+      }
+    }
+  }
+}
+
 void swift::diagnoseMissingImports(SourceFile &sf) {
+  // Missing import diagnostics should be emitted differently in "migrate" mode.
+  if (sf.getASTContext().LangOpts.isMigratingToFeature(
+          Feature::MemberImportVisibility)) {
+    migrateToMemberImportVisibility(sf);
+    return;
+  }
+
   auto delayedDiags = sf.takeDelayedMissingImportForMemberDiagnostics();
   auto fixItCache = MissingImportFixItCache(sf);
 
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -1772,7 +1772,8 @@ resolveUnqualifiedIdentTypeRepr(const TypeResolution &resolution,
   // If the look up did not yield any results, try again but allow members from
   // modules that are not directly imported to be accessible.
   bool didIgnoreMissingImports = false;
-  if (!globals && ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+  if (!globals && ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                                          /*allowMigration=*/true)) {
     lookupOptions |= NameLookupFlags::IgnoreMissingImports;
     globals = TypeChecker::lookupUnqualifiedType(DC, id, repr->getLoc(),
                                                  lookupOptions);
@@ -2035,8 +2036,8 @@ static Type resolveQualifiedIdentTypeRepr(const TypeResolution &resolution,
         DC, parentTy, repr->getNameRef(), repr->getLoc(), lookupOptions);
 
     // If no members were found, try ignoring missing imports.
-    if (!memberTypes &&
-        ctx.LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+    if (!memberTypes && ctx.LangOpts.hasFeature(Feature::MemberImportVisibility,
+                                                /*allowMigration=*/true)) {
       lookupOptions |= NameLookupFlags::IgnoreMissingImports;
       memberTypes = TypeChecker::lookupMemberType(
           DC, parentTy, repr->getNameRef(), repr->getLoc(), lookupOptions);
diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp
@@ -2698,7 +2698,8 @@ void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
       increaseScore(SK_Unavailable, locator);
 
     // If the declaration is from a module that hasn't been imported, note that.
-    if (getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility)) {
+    if (getASTContext().LangOpts.hasFeature(Feature::MemberImportVisibility,
+                                            /*allowMigration=*/true)) {
       if (!useDC->isDeclImported(decl))
         increaseScore(SK_MissingImport, locator);
     }
diff --git a/test/NameLookup/member_import_visibility_migrate.swift b/test/NameLookup/member_import_visibility_migrate.swift
diff --git a/userdocs/diagnostics/member-import-visibility.md b/userdocs/diagnostics/member-import-visibility.md
