Merge pull request #75694 from tshortli/member-import-visibility-import-access-level

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

Author: tshortli

include/swift/AST/FileUnit.h

@@ -273,8 +273,8 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
                      ModuleDecl::ImportFilter filter) const {}
 
   /// Lists modules that are not imported from this file and used in API.
-  virtual void
-  getMissingImportedModules(SmallVectorImpl<ImportedModule> &imports) const {}
+  virtual void getImplicitImportsForModuleInterface(
+      SmallVectorImpl<ImportedModule> &imports) const {}
 
   /// \see ModuleDecl::getImportedModulesForLookup
   virtual void getImportedModulesForLookup(

include/swift/AST/Module.h

@@ -1034,8 +1034,8 @@ class ModuleDecl
                           ImportFilter filter = ImportFilterKind::Exported) const;
 
   /// Lists modules that are not imported from a file and used in API.
-  void
-  getMissingImportedModules(SmallVectorImpl<ImportedModule> &imports) const;
+  void getImplicitImportsForModuleInterface(
+      SmallVectorImpl<ImportedModule> &imports) const;
 
   /// Looks up which modules are imported by this module, ignoring any that
   /// won't contain top-level decls.
@@ -1319,11 +1319,6 @@ inline SourceLoc extractNearestSourceLoc(const ModuleDecl *mod) {
   return extractNearestSourceLoc(static_cast<const Decl *>(mod));
 }
 
-/// 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

@@ -159,6 +159,16 @@ class SourceFile final : public FileUnit {
   /// The highest access level of declarations referencing each import.
   llvm::DenseMap<const ModuleDecl *, AccessLevel> ImportsUseAccessLevel;
 
+  /// Imports that should be printed in the module interface even though they
+  /// 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.
@@ -463,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;
@@ -510,10 +523,32 @@ class SourceFile final : public FileUnit {
 
   SWIFT_DEBUG_DUMPER(dumpSeparatelyImportedOverlays());
 
-  llvm::SmallDenseSet<ImportedModule> MissingImportedModules;
+  /// Record an import that should be printed in the module interface even
+  /// though it was not written in the source file. These imports are needed in
+  /// Swift 5 mode to preserve the integrity of swiftinterface files when code
+  /// publicly use declarations from modules that were \c `@_implementationOnly`
+  /// imported in other source files.
+  void addImplicitImportForModuleInterface(ImportedModule module) {
+    ImplicitImportsForModuleInterface.insert(module);
+  }
+
+  /// Gather implicit imports that should printed in swiftinterfaces for
+  /// compatibility with code in some Swift 5 modules.
+  void getImplicitImportsForModuleInterface(
+      SmallVectorImpl<ImportedModule> &imports) const override;
 
-  void addMissingImportedModule(ImportedModule module) const {
-     const_cast<SourceFile *>(this)->MissingImportedModules.insert(module);
+  /// 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.
@@ -527,10 +562,10 @@ class SourceFile final : public FileUnit {
   ArrayRef<IfConfigClauseRangeInfo>
   getIfConfigClausesWithin(SourceRange outer) const;
 
-  void getMissingImportedModules(
-         SmallVectorImpl<ImportedModule> &imports) const override;
-
+  /// Record visible declarations for use in code completion and refactoring.
   void cacheVisibleDecls(SmallVectorImpl<ValueDecl *> &&globals) const;
+
+  /// Retrieve visible declarations for use in code completion and refactoring.
   const SmallVectorImpl<ValueDecl *> &getCachedVisibleDecls() const;
 
   virtual void lookupValue(DeclName name, NLKind lookupKind,

lib/AST/Module.cpp

@@ -1720,9 +1720,9 @@ void ModuleDecl::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
   FORWARD(getImportedModules, (modules, filter));
 }
 
-void ModuleDecl::getMissingImportedModules(
+void ModuleDecl::getImplicitImportsForModuleInterface(
     SmallVectorImpl<ImportedModule> &imports) const {
-  FORWARD(getMissingImportedModules, (imports));
+  FORWARD(getImplicitImportsForModuleInterface, (imports));
 }
 
 const llvm::DenseMap<const clang::Module *, ModuleDecl *> &
@@ -1811,9 +1811,9 @@ SourceFile::getImportedModules(SmallVectorImpl<ImportedModule> &modules,
   }
 }
 
-void SourceFile::getMissingImportedModules(
+void SourceFile::getImplicitImportsForModuleInterface(
     SmallVectorImpl<ImportedModule> &modules) const {
-  for (auto module : MissingImportedModules)
+  for (auto module : ImplicitImportsForModuleInterface)
     modules.push_back(module);
 }
 
@@ -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(
@@ -4093,62 +4093,3 @@ 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/Frontend/ModuleInterfaceSupport.cpp

@@ -87,7 +87,7 @@ static void printToolVersionAndFlagsComment(raw_ostream &out,
 
     SmallVector<ImportedModule> imports;
     M->getImportedModules(imports, filter);
-    M->getMissingImportedModules(imports);
+    M->getImplicitImportsForModuleInterface(imports);
 
     for (ImportedModule import: imports) {
       StringRef importedName = import.importedModule->getNameStr();
@@ -338,7 +338,7 @@ static void printImports(raw_ostream &out,
   M->getImportedModules(allImports, allImportFilter);
 
   if (Opts.PrintMissingImports)
-    M->getMissingImportedModules(allImports);
+    M->getImplicitImportsForModuleInterface(allImports);
 
   ImportedModule::removeDuplicates(allImports);
   diagnoseScopedImports(ctx.Diags, allImports);

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/ResilienceDiagnostics.cpp

@@ -45,7 +45,7 @@ static bool addMissingImport(SourceLoc loc, const Decl *D,
   // API.
   auto missingImport = ImportedModule(ImportPath::Access(),
                                       const_cast<ModuleDecl *>(M));
-  SF->addMissingImportedModule(missingImport);
+  SF->addImplicitImportForModuleInterface(missingImport);
   ctx.Diags.diagnose(loc, diag::missing_import_inserted, M->getName());
   return true;
 }