Sema: Add an access level to the fix-its for missing imports when appropriate.

When emitting fix-its for missing imports, include an access level when the
module has been imported with an access level in other source files. For now,
the suggested access level for will always be `internal`, even when uses of
members in the file would actually require `public` or `package` visibility. In
order to suggest the correct access level, name lookup will need to be
refactored to repair references to inaccessible declarations, instead of
leaving error nodes in the AST. In anticipation of that refactoring of name
lookup, missing import diagnostics are now delayed until type checking a source
file is finished so that a consistent access level can be suggested for each
import fix-it for a given module.

Partially resolves rdar://126637855.

Author: tshortli

include/swift/AST/SourceFile.h

@@ -163,6 +163,12 @@ class SourceFile final : public FileUnit {
   /// were not written in the source file.
   llvm::SmallDenseSet<ImportedModule> ImplicitImportsForModuleInterface;
 
+  /// Associates a list of source locations to the member declarations that must
+  /// be diagnosed as being out of scope due to a missing import.
+  using DelayedMissingImportForMemberDiags =
+      llvm::SmallDenseMap<const ValueDecl *, std::vector<SourceLoc>>;
+  DelayedMissingImportForMemberDiags MissingImportForMemberDiagnostics;
+
   /// A unique identifier representing this file; used to mark private decls
   /// within the file to keep them from conflicting with other files in the
   /// same module.
@@ -467,8 +473,11 @@ 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;
+  /// Enumerates each of the direct imports of \p module in the file.
+  using AttributedImportCallback =
+      llvm::function_ref<void(AttributedImport<ImportedModule> &)>;
+  void forEachImportOfModule(const ModuleDecl *module,
+                             AttributedImportCallback callback);
 
   /// Get the most permissive restriction applied to the imports of \p module.
   RestrictedImportKind getRestrictedImportKind(const ModuleDecl *module) const;
@@ -528,6 +537,20 @@ class SourceFile final : public FileUnit {
   void getImplicitImportsForModuleInterface(
       SmallVectorImpl<ImportedModule> &imports) const override;
 
+  /// Add a source location for which a delayed missing import for member
+  /// diagnostic should be emited.
+  void addDelayedMissingImportForMemberDiagnostic(const ValueDecl *decl,
+                                                  SourceLoc loc) {
+    MissingImportForMemberDiagnostics[decl].push_back(loc);
+  }
+
+  DelayedMissingImportForMemberDiags
+  takeDelayedMissingImportForMemberDiagnostics() {
+    DelayedMissingImportForMemberDiags diags;
+    std::swap(diags, MissingImportForMemberDiagnostics);
+    return diags;
+  }
+
   /// Record the source range info for a parsed \c #if clause.
   void recordIfConfigClauseRangeInfo(const IfConfigClauseRangeInfo &range);
 

lib/AST/Module.cpp

@@ -2817,13 +2817,13 @@ bool SourceFile::hasImportsWithFlag(ImportFlags flag) const {
       ctx.evaluator, HasImportsMatchingFlagRequest{mutableThis, flag}, false);
 }
 
-ImportFlags SourceFile::getImportFlags(const ModuleDecl *module) const {
-  unsigned flags = 0x0;
+void SourceFile::forEachImportOfModule(
+    const ModuleDecl *module,
+    llvm::function_ref<void(AttributedImport<ImportedModule> &)> callback) {
   for (auto import : *Imports) {
     if (import.module.importedModule == module)
-      flags |= import.options.toRaw();
+      callback(import);
   }
-  return ImportFlags(flags);
 }
 
 bool SourceFile::hasTestableOrPrivateImport(

lib/Sema/CSDiagnostics.cpp

@@ -6188,7 +6188,7 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
 
   auto loc = nameLoc.isValid() ? nameLoc.getStartLoc() : ::getLoc(anchor);
   if (IsMissingImport) {
-    diagnoseMissingImportForMember(Member, getDC(), loc);
+    maybeDiagnoseMissingImportForMember(Member, getDC(), loc);
     return true;
   }
 

lib/Sema/PreCheckExpr.cpp

@@ -629,7 +629,7 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
         TypeChecker::lookupUnqualified(DC, LookupName, Loc, relookupOptions);
     if (nonImportedResults) {
       const ValueDecl *first = nonImportedResults.front().getValueDecl();
-      diagnoseMissingImportForMember(first, DC, Loc);
+      maybeDiagnoseMissingImportForMember(first, DC, Loc);
 
       // Don't try to recover here; we'll get more access-related diagnostics
       // downstream if the type of the inaccessible decl is also inaccessible.

lib/Sema/TypeCheckNameLookup.cpp

@@ -15,6 +15,8 @@
 // declaration of members (such as constructors).
 //
 //===----------------------------------------------------------------------===//
+
+#include "TypeCheckAvailability.h"
 #include "TypeChecker.h"
 #include "TypoCorrection.h"
 #include "swift/AST/ExistentialLayout.h"
@@ -755,43 +757,87 @@ TypoCorrectionResults::claimUniqueCorrection() {
   return SyntacticTypoCorrection(WrittenName, Loc, uniqueCorrectedName);
 }
 
-bool swift::diagnoseMissingImportForMember(const ValueDecl *decl,
-                                           const DeclContext *dc,
-                                           SourceLoc loc) {
-  if (decl->findImport(dc))
-    return false;
+struct MissingImportFixItInfo {
+  OptionSet<ImportFlags> flags;
+  std::optional<AccessLevel> accessLevel;
+};
+
+class MissingImportFixItCache {
+  SourceFile &sf;
+  llvm::DenseMap<const ModuleDecl *, MissingImportFixItInfo> infos;
+
+public:
+  MissingImportFixItCache(SourceFile &sf) : sf(sf){};
+
+  MissingImportFixItInfo getInfo(const ModuleDecl *mod) {
+    auto existing = infos.find(mod);
+    if (existing != infos.end())
+      return existing->getSecond();
+
+    MissingImportFixItInfo info;
+
+    // Find imports of the defining module in other source files and aggregate
+    // the attributes and access level usage on those imports collectively. This
+    // information can be used to emit a fix-it that is consistent with
+    // how the module is imported in the rest of the module.
+    auto parentModule = sf.getParentModule();
+    bool foundImport = false;
+    bool anyImportHasAccessLevel = false;
+    for (auto file : parentModule->getFiles()) {
+      if (auto otherSF = dyn_cast<SourceFile>(file)) {
+        unsigned flagsInSourceFile = 0x0;
+        otherSF->forEachImportOfModule(
+            mod, [&](AttributedImport<ImportedModule> &import) {
+              foundImport = true;
+              anyImportHasAccessLevel |= import.accessLevelRange.isValid();
+              flagsInSourceFile |= import.options.toRaw();
+            });
+        info.flags |= ImportFlags(flagsInSourceFile);
+      }
+    }
 
-  auto &ctx = dc->getASTContext();
+    // Add an appropriate access level as long as it would not conflict with
+    // existing imports that lack access levels.
+    if (!foundImport || anyImportHasAccessLevel)
+      info.accessLevel = sf.getMaxAccessLevelUsingImport(mod);
+
+    infos[mod] = info;
+    return info;
+  }
+};
+
+static void diagnoseMissingImportForMember(const ValueDecl *decl,
+                                           SourceFile *sf, SourceLoc loc) {
+  auto &ctx = sf->getASTContext();
   auto definingModule = decl->getModuleContext();
   ctx.Diags.diagnose(loc, diag::candidate_from_missing_import,
                      decl->getDescriptiveKind(), decl->getName(),
                      definingModule);
+}
+
+static void
+diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
+                                     SourceLoc loc,
+                                     MissingImportFixItCache &fixItCache) {
+
+  diagnoseMissingImportForMember(decl, sf, loc);
 
-  SourceLoc bestLoc =
-      ctx.Diags.getBestAddImportFixItLoc(decl, dc->getParentSourceFile());
+  auto &ctx = sf->getASTContext();
+  auto definingModule = decl->getModuleContext();
+  SourceLoc bestLoc = ctx.Diags.getBestAddImportFixItLoc(decl, sf);
   if (!bestLoc.isValid())
-    return false;
+    return;
 
   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::ImplementationOnly))
+  auto fixItInfo = fixItCache.getInfo(definingModule);
+  if (fixItInfo.flags.contains(ImportFlags::ImplementationOnly))
     importText += "@_implementationOnly ";
-  if (flags.contains(ImportFlags::WeakLinked))
+  if (fixItInfo.flags.contains(ImportFlags::WeakLinked))
     importText += "@_weakLinked ";
-  if (flags.contains(ImportFlags::SPIOnly))
+  if (fixItInfo.flags.contains(ImportFlags::SPIOnly))
     importText += "@_spiOnly ";
 
-  // FIXME: Access level should be considered, too.
-
   // @_spi imports.
   if (decl->isSPI()) {
     auto spiGroups = decl->getSPIGroups();
@@ -802,11 +848,49 @@ bool swift::diagnoseMissingImportForMember(const ValueDecl *decl,
     }
   }
 
+  if (auto accessLevel = fixItInfo.accessLevel) {
+    importText += getAccessLevelSpelling(*accessLevel);
+    importText += " ";
+  }
+
   importText += "import ";
   importText += definingModule->getName().str();
   importText += "\n";
   ctx.Diags.diagnose(bestLoc, diag::candidate_add_import, definingModule)
       .fixItInsert(bestLoc, importText);
+}
 
-  return true;
+bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
+                                                const DeclContext *dc,
+                                                SourceLoc loc) {
+  if (decl->findImport(dc))
+    return false;
+
+  auto sf = dc->getParentSourceFile();
+  if (!sf)
+    return false;
+
+  auto &ctx = dc->getASTContext();
+
+  // 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) {
+    diagnoseMissingImportForMember(decl, sf, loc);
+    return true;
+  }
+
+  sf->addDelayedMissingImportForMemberDiagnostic(decl, loc);
+  return false;
+}
+
+void swift::diagnoseMissingImports(SourceFile &sf) {
+  auto delayedDiags = sf.takeDelayedMissingImportForMemberDiagnostics();
+  auto fixItCache = MissingImportFixItCache(sf);
+
+  for (auto declAndLocs : delayedDiags) {
+    for (auto loc : declAndLocs.second) {
+      diagnoseAndFixMissingImportForMember(declAndLocs.first, &sf, loc,
+                                           fixItCache);
+    }
+  }
 }

lib/Sema/TypeCheckType.cpp

@@ -1388,7 +1388,7 @@ static Type diagnoseUnknownType(const TypeResolution &resolution,
     if (!nonImportedResults.empty()) {
       auto first = cast<TypeDecl>(nonImportedResults.front().getValueDecl());
       auto nameLoc = repr->getNameLoc();
-      diagnoseMissingImportForMember(first, dc, nameLoc.getStartLoc());
+      maybeDiagnoseMissingImportForMember(first, dc, nameLoc.getStartLoc());
 
       // Don't try to recover here; we'll get more access-related diagnostics
       // downstream if we do.
@@ -1493,7 +1493,7 @@ static Type diagnoseUnknownType(const TypeResolution &resolution,
   if (nonImportedMembers) {
     const TypeDecl *first = nonImportedMembers.front().Member;
     auto nameLoc = repr->getNameLoc();
-    diagnoseMissingImportForMember(first, dc, nameLoc.getStartLoc());
+    maybeDiagnoseMissingImportForMember(first, dc, nameLoc.getStartLoc());
     // Don't try to recover here; we'll get more access-related diagnostics
     // downstream if we do.
     return ErrorType::get(ctx);

lib/Sema/TypeChecker.cpp

@@ -322,6 +322,7 @@ TypeCheckSourceFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
   if (!Ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation))
     diagnoseUnnecessaryPreconcurrencyImports(*SF);
   diagnoseUnnecessaryPublicImports(*SF);
+  diagnoseMissingImports(*SF);
 
   // Check to see if there are any inconsistent imports.
   // Whole-module @_implementationOnly imports.

lib/Sema/TypeChecker.h

@@ -1535,10 +1535,17 @@ void recordRequiredImportAccessLevelForDecl(
 /// Report imports that are marked public but are not used in API.
 void diagnoseUnnecessaryPublicImports(SourceFile &SF);
 
-/// 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);
+/// Emit or delay a diagnostic that suggests adding a missing import that is
+/// necessary to bring \p decl into scope in the containing source file. If
+/// delayed, the diagnostic will instead be emitted after type checking the
+/// entire file and will include an appropriate fix-it. Returns true if a
+/// diagnostic was emitted (and not delayed).
+bool maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
+                                         const DeclContext *dc, SourceLoc loc);
+
+/// Emit delayed diagnostics regarding imports that should be added to the
+/// source file.
+void diagnoseMissingImports(SourceFile &sf);
 } // end namespace swift
 
 #endif

test/NameLookup/members_transitive.swift

@@ -7,7 +7,7 @@
 // RUN: %target-swift-frontend -typecheck %s -I %t -verify -swift-version 5 -package-name TestPackage -enable-experimental-feature MemberImportVisibility -verify-additional-prefix member-visibility-
 
 import members_C
-// expected-member-visibility-note 11{{add import of module 'members_B'}}{{1-1=import members_B\n}}
+// expected-member-visibility-note 11{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}
 
 
 func testExtensionMembers(x: X, y: Y<Z>) {