Merge pull request #84160 from tshortli/member-import-visibility-key-paths-downgrade-to-warning

Sema: Downgrade missing import diagnostics for key paths

Author: tshortli

include/swift/AST/SourceFile.h

@@ -137,8 +137,8 @@ class SourceFile final : public FileUnit {
 
   /// 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>>;
+  using DelayedMissingImportForMemberDiags = llvm::SmallDenseMap<
+      const ValueDecl *, std::vector<std::pair<SourceLoc, DiagnosticBehavior>>>;
   DelayedMissingImportForMemberDiags MissingImportForMemberDiagnostics;
 
   /// A unique identifier representing this file; used to mark private decls
@@ -518,8 +518,9 @@ class SourceFile final : public FileUnit {
   /// 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);
+                                                  SourceLoc loc,
+                                                  DiagnosticBehavior limit) {
+    MissingImportForMemberDiagnostics[decl].push_back({loc, limit});
   }
 
   DelayedMissingImportForMemberDiags

lib/Sema/MiscDiagnostics.cpp

@@ -6382,20 +6382,25 @@ static void diagnoseMissingMemberImports(const Expr *E, const DeclContext *DC) {
       if (auto *KPE = dyn_cast<KeyPathExpr>(E)) {
         for (const auto &component : KPE->getComponents()) {
           if (component.hasDeclRef())
-            checkDecl(component.getDeclRef().getDecl(), component.getLoc());
+            checkDecl(component.getDeclRef().getDecl(), component.getLoc(),
+                      /*downgradeToWarning=*/true);
         }
       }
 
       return Action::Continue(E);
     }
 
-    void checkDecl(const ValueDecl *decl, SourceLoc loc) {
+    void checkDecl(const ValueDecl *decl, SourceLoc loc,
+                   bool downgradeToWarning = false) {
       // Only diagnose uses of members.
       if (!decl->getDeclContext()->isTypeContext())
         return;
 
       if (!dc->isDeclImported(decl))
-        maybeDiagnoseMissingImportForMember(decl, dc, loc);
+        maybeDiagnoseMissingImportForMember(
+            decl, dc, loc,
+            downgradeToWarning ? DiagnosticBehavior::Warning
+                               : DiagnosticBehavior::Unspecified);
     }
   };
 

lib/Sema/TypeCheckNameLookup.cpp

@@ -924,20 +924,23 @@ class MissingImportFixItCache {
   }
 };
 
-static void
-diagnoseMissingImportsForMember(const ValueDecl *decl,
-                                SmallVectorImpl<ModuleDecl *> &modulesToImport,
-                                SourceFile *sf, SourceLoc loc) {
+static void diagnoseMissingImportsForMember(
+    const ValueDecl *decl, SmallVectorImpl<ModuleDecl *> &modulesToImport,
+    SourceFile *sf, SourceLoc loc, DiagnosticBehavior limit) {
   auto &ctx = sf->getASTContext();
   auto count = modulesToImport.size();
   ASSERT(count > 0);
 
   if (count > 1) {
-    ctx.Diags.diagnose(loc, diag::member_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])
+        .limitBehavior(limit);
   } else {
-    ctx.Diags.diagnose(loc, diag::member_from_missing_import, decl,
-                       modulesToImport.front());
+    ctx.Diags
+        .diagnose(loc, diag::member_from_missing_import, decl,
+                  modulesToImport.front())
+        .limitBehavior(limit);
   }
 }
 
@@ -983,7 +986,7 @@ static void emitMissingImportNoteAndFixIt(
 
 static void
 diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
-                                     SourceLoc loc,
+                                     SourceLoc loc, DiagnosticBehavior limit,
                                      MissingImportFixItCache &fixItCache) {
 
   auto modulesAndFixits =
@@ -994,7 +997,7 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
   if (modulesToImport.empty())
     return;
 
-  diagnoseMissingImportsForMember(decl, modulesToImport, sf, loc);
+  diagnoseMissingImportsForMember(decl, modulesToImport, sf, loc, limit);
 
   auto &ctx = sf->getASTContext();
   SourceLoc bestLoc = ctx.Diags.getBestAddImportFixItLoc(sf);
@@ -1008,7 +1011,8 @@ diagnoseAndFixMissingImportForMember(const ValueDecl *decl, SourceFile *sf,
 
 bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
                                                 const DeclContext *dc,
-                                                SourceLoc loc) {
+                                                SourceLoc loc,
+                                                DiagnosticBehavior limit) {
   if (dc->isDeclImported(decl))
     return false;
 
@@ -1040,11 +1044,11 @@ bool swift::maybeDiagnoseMissingImportForMember(const ValueDecl *decl,
     if (modulesToImport.empty())
       return false;
 
-    diagnoseMissingImportsForMember(decl, modulesToImport, sf, loc);
+    diagnoseMissingImportsForMember(decl, modulesToImport, sf, loc, limit);
     return true;
   }
 
-  sf->addDelayedMissingImportForMemberDiagnostic(decl, loc);
+  sf->addDelayedMissingImportForMemberDiagnostic(decl, loc, limit);
   return false;
 }
 
@@ -1103,7 +1107,8 @@ void migrateToMemberImportVisibility(SourceFile &sf) {
       if (locs == delayedDiags.end())
         continue;
 
-      for (auto loc : locs->second) {
+      for (auto locAndLimit : locs->second) {
+        auto loc = locAndLimit.first;
         ctx.Diags.diagnose(loc, diag::decl_from_module_used_here, decl, mod);
       }
     }
@@ -1122,8 +1127,9 @@ void swift::diagnoseMissingImports(SourceFile &sf) {
   auto fixItCache = MissingImportFixItCache(sf);
 
   for (auto declAndLocs : delayedDiags) {
-    for (auto loc : declAndLocs.second) {
-      diagnoseAndFixMissingImportForMember(declAndLocs.first, &sf, loc,
+    for (auto locAndLimit : declAndLocs.second) {
+      auto [loc, limit] = locAndLimit;
+      diagnoseAndFixMissingImportForMember(declAndLocs.first, &sf, loc, limit,
                                            fixItCache);
     }
   }

lib/Sema/TypeChecker.h

@@ -1531,8 +1531,9 @@ void diagnoseUnnecessaryPublicImports(SourceFile &SF);
 /// 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);
+bool maybeDiagnoseMissingImportForMember(
+    const ValueDecl *decl, const DeclContext *dc, SourceLoc loc,
+    DiagnosticBehavior limit = DiagnosticBehavior::Unspecified);
 
 /// Emit delayed diagnostics regarding imports that should be added to the
 /// source file.

test/NameLookup/member_import_visibility.swift

@@ -47,11 +47,11 @@ func testExtensionMembers(x: X, y: Y<Z>) {
   func takesKeyPath<T, U>(_ t: T, _ keyPath: KeyPath<T, U>) -> () { }
 
   takesKeyPath(x, \.propXinA)
-  takesKeyPath(x, \.propXinB) // expected-member-visibility-error{{property 'propXinB' is not available due to missing import of defining module 'members_B'}}
+  takesKeyPath(x, \.propXinB) // expected-member-visibility-warning{{property 'propXinB' is not available due to missing import of defining module 'members_B'}}
   takesKeyPath(x, \.propXinC)
 
   takesKeyPath(x, \.propXinA.description)
-  takesKeyPath(x, \.propXinB.description) // expected-member-visibility-error{{property 'propXinB' is not available due to missing import of defining module 'members_B'}}
+  takesKeyPath(x, \.propXinB.description) // expected-member-visibility-warning{{property 'propXinB' is not available due to missing import of defining module 'members_B'}}
   takesKeyPath(x, \.propXinC.description)
 }
 

test/NameLookup/member_import_visibility_migrate.swift

@@ -48,6 +48,18 @@ func internalFunc(_ x: Int) {
   _ = x.memberInInternalUsesOnlyTransitivelyImported // expected-note {{property 'memberInInternalUsesOnlyTransitivelyImported' from 'InternalUsesOnlyTransitivelyImported' used here}}
 }
 
+func keyPaths(_ x: Int) {
+  func takesKeyPath<T, U>(_ t: T, _ keyPath: KeyPath<T, U>) -> () { }
+
+  takesKeyPath(x, \.memberInInternalUsesOnly) // expected-note {{property 'memberInInternalUsesOnly' from 'InternalUsesOnly' used here}}
+  takesKeyPath(x, \.memberInInternalUsesOnlyDefaultedImport) // expected-note {{property 'memberInInternalUsesOnlyDefaultedImport' from 'InternalUsesOnlyDefaultedImport' used here}}
+  takesKeyPath(x, \.memberInMixedUses) // expected-note {{property 'memberInMixedUses' from 'MixedUses' used here}}
+  takesKeyPath(x, \.memberInInternalUsesOnlyReexported) // expected-note {{property 'memberInInternalUsesOnlyReexported' from 'InternalUsesOnlyReexported' used here}}
+  takesKeyPath(x, \.memberInInternalUsesOnlySPIOnly) // expected-note {{property 'memberInInternalUsesOnlySPIOnly' from 'InternalUsesOnlySPIOnly' used here}}
+  takesKeyPath(x, \.memberInInternalUsesOnlyDefaultedImportSPIOnly) // expected-note {{property 'memberInInternalUsesOnlyDefaultedImportSPIOnly' from 'InternalUsesOnlyDefaultedImportSPIOnly' used here}}
+  takesKeyPath(x, \.memberInInternalUsesOnlyTransitivelyImported) // expected-note {{property 'memberInInternalUsesOnlyTransitivelyImported' from 'InternalUsesOnlyTransitivelyImported' used here}}
+}
+
 @inlinable package func packageInlinableFunc(_ x: Int) {
   _ = x.memberInPackageUsesOnly // expected-note {{property 'memberInPackageUsesOnly' from 'PackageUsesOnly' used here}}
   _ = x.memberInMixedUses // expected-note {{property 'memberInMixedUses' from 'MixedUses' used here}}