Improve module selector constraint solver diagnostics

beccadax · beccadax · commit 4df114ebc864 · 2025-05-15T16:29:19.000-07:00
diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h
@@ -186,6 +186,10 @@ enum class FixKind : uint8_t {
   /// no access control.
   AllowInaccessibleMember,
 
+  /// If a module selector prevented us from selecting a member, pretend that it
+  /// was not specified.
+  AllowMemberFromWrongModule,
+
   /// Allow KeyPaths to use AnyObject as root type
   AllowAnyObjectKeyPathRoot,
 
@@ -1930,6 +1934,25 @@ class AllowInaccessibleMember final : public AllowInvalidMemberRef {
   }
 };
 
+class AllowMemberFromWrongModule final : public AllowInvalidMemberRef {
+  AllowMemberFromWrongModule(ConstraintSystem &cs, Type baseType,
+                             ValueDecl *member, DeclNameRef name,
+                             ConstraintLocator *locator)
+      : AllowInvalidMemberRef(cs, FixKind::AllowMemberFromWrongModule, baseType,
+                              member, name, locator) {}
+
+public:
+  std::string getName() const override {
+    return "allow reference to member from wrong module";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  static AllowMemberFromWrongModule *create(ConstraintSystem &cs, Type baseType,
+                                            ValueDecl *member, DeclNameRef name,
+                                            ConstraintLocator *locator);
+};
+
 class AllowAnyObjectKeyPathRoot final : public ConstraintFix {
 
   AllowAnyObjectKeyPathRoot(ConstraintSystem &cs, ConstraintLocator *locator)
diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h
@@ -2033,6 +2033,9 @@ struct MemberLookupResult {
     /// 'accesses' attributes of the init accessor and therefore canno
     /// t be referenced in its body.
     UR_UnavailableWithinInitAccessor,
+
+    /// The module selector in the `DeclNameRef` does not match this candidate.
+    UR_WrongModule
   };
 
   /// This is a list of considered (but rejected) candidates, along with a
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -6322,10 +6322,7 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
 
   if (baseExpr) {
     auto *locator = getConstraintLocator(baseExpr, ConstraintLocator::Member);
-    const auto &solution = getSolution();
-    if (llvm::any_of(solution.Fixes, [&locator](const ConstraintFix *fix) {
-          return fix->getLocator() == locator;
-        }))
+    if (hasFixFor(getSolution(), locator))
       return false;
   }
 
@@ -6345,6 +6342,54 @@ bool InaccessibleMemberFailure::diagnoseAsError() {
   return true;
 }
 
+bool MemberFromWrongModuleFailure::diagnoseAsError() {
+  auto anchor = getRawAnchor();
+  // Let's try to avoid over-diagnosing chains of wrong-module
+  // members e.g.:
+  //
+  // struct A {
+  //   struct B {
+  //     struct C {}
+  //   }
+  // }
+  //
+  // _ = A.Other::B.Other::C()
+  Expr *baseExpr = nullptr;
+  DeclNameLoc nameLoc;
+  if (auto *UDE = getAsExpr<UnresolvedDotExpr>(anchor)) {
+    baseExpr = UDE->getBase();
+    nameLoc = UDE->getNameLoc();
+  } else if (auto *UME = getAsExpr<UnresolvedMemberExpr>(anchor)) {
+    nameLoc = UME->getNameLoc();
+  } else if (auto *SE = getAsExpr<SubscriptExpr>(anchor)) {
+    baseExpr = SE->getBase();
+  } else if (auto *call = getAsExpr<CallExpr>(anchor)) {
+    baseExpr = call->getFn();
+  }
+
+  if (baseExpr) {
+    auto *locator = getConstraintLocator(baseExpr, ConstraintLocator::Member);
+    if (hasFixFor(getSolution(), locator))
+      return false;
+  }
+
+  auto modSelLoc = nameLoc.getModuleSelectorLoc();
+  auto loc = nameLoc.isValid() ? nameLoc.getStartLoc() : ::getLoc(anchor);
+
+  emitDiagnosticAt(loc, diag::decl_not_in_module, Member->getName(),
+                   Name.getModuleSelector());
+
+  Identifier actualModuleName = Member->getModuleContext()->getName();
+  assert(actualModuleName != Name.getModuleSelector() &&
+         "Module selector failure on member in same module?");
+  emitDiagnosticAt(loc, diag::note_change_module_selector, actualModuleName)
+      .fixItReplace(modSelLoc, actualModuleName.str());
+
+  emitDiagnosticAt(Member, diag::decl_declared_here, Member);
+
+  return true;
+}
+
 SourceLoc AnyObjectKeyPathRootFailure::getLoc() const {
   auto anchor = getAnchor();
 
diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h
@@ -1644,6 +1644,27 @@ class InaccessibleMemberFailure final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+/// Diagnose an attempt to reference member from the wrong module with a module
+/// selector, e.g.
+///
+/// ```swift
+/// import Foo
+/// import Bar
+///
+/// SomeType.Bar::methodDefinedInFoo()
+/// ```
+class MemberFromWrongModuleFailure final : public FailureDiagnostic {
+  ValueDecl *Member;
+  DeclNameRef Name;
+
+public:
+  MemberFromWrongModuleFailure(const Solution &solution, DeclNameRef name,
+                               ValueDecl *member, ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), Member(member), Name(name) {}
+
+  bool diagnoseAsError() override;
+};
+
 /// Diagnose an attempt to reference member marked as `mutating`
 /// on immutable base e.g. `let` variable:
 ///
diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp
@@ -1207,6 +1207,21 @@ AllowInaccessibleMember::create(ConstraintSystem &cs, Type baseType,
       AllowInaccessibleMember(cs, baseType, member, name, locator);
 }
 
+bool AllowMemberFromWrongModule::diagnose(const Solution &solution,
+                                          bool asNote) const {
+  MemberFromWrongModuleFailure failure(solution, getMemberName(), getMember(),
+                                       getLocator());
+  return failure.diagnose(asNote);
+}
+
+AllowMemberFromWrongModule *
+AllowMemberFromWrongModule::create(ConstraintSystem &cs, Type baseType,
+                                   ValueDecl *member, DeclNameRef name,
+                                   ConstraintLocator *locator) {
+  return new (cs.getAllocator())
+      AllowMemberFromWrongModule(cs, baseType, member, name, locator);
+}
+
 bool AllowAnyObjectKeyPathRoot::diagnose(const Solution &solution,
                                          bool asNote) const {
   AnyObjectKeyPathRootFailure failure(solution, getLocator());
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -10760,8 +10760,8 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
   }
 
   // If we have no viable or unviable candidates, and we're generating,
-  // diagnostics, rerun the query with inaccessible members included, so we can
-  // include them in the unviable candidates list.
+  // diagnostics, rerun the query with various excluded members included, so we
+  // can include them in the unviable candidates list.
   if (result.ViableCandidates.empty() && result.UnviableCandidates.empty() &&
       includeInaccessibleMembers) {
     NameLookupOptions lookupOptions =
@@ -10770,7 +10770,8 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
     // Local function that looks up additional candidates using the given lookup
     // options, recording the results as unviable candidates.
     auto lookupUnviable =
-        [&](NameLookupOptions lookupOptions,
+        [&](DeclNameRef memberName,
+            NameLookupOptions lookupOptions,
             MemberLookupResult::UnviableReason reason) -> bool {
       auto lookup = TypeChecker::lookupMember(DC, instanceTy, memberName,
                                               memberLoc, lookupOptions);
@@ -10794,9 +10795,20 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
       return !lookup.empty();
     };
 
+    // Look for members that were excluded because of a module selector.
+    if (memberName.hasModuleSelector()) {
+      DeclNameRef unqualifiedMemberName{memberName.getFullName()};
+
+      if (lookupUnviable(unqualifiedMemberName,
+                         lookupOptions,
+                         MemberLookupResult::UR_WrongModule))
+        return result;
+    }
+
     // Ignore access control so we get candidates that might have been missed
     // before.
-    if (lookupUnviable(lookupOptions | NameLookupFlags::IgnoreAccessControl,
+    if (lookupUnviable(memberName,
+                       lookupOptions | NameLookupFlags::IgnoreAccessControl,
                        MemberLookupResult::UR_Inaccessible))
       return result;
   }
@@ -10997,6 +11009,11 @@ static ConstraintFix *fixMemberRef(
                  : nullptr;
     }
 
+    case MemberLookupResult::UR_WrongModule:
+      assert(choice.isDecl());
+      return AllowMemberFromWrongModule::create(cs, baseTy, choice.getDecl(),
+                                                memberName, locator);
+
     case MemberLookupResult::UR_Inaccessible:
       assert(choice.isDecl());
       return AllowInaccessibleMember::create(cs, baseTy, choice.getDecl(),
@@ -15880,6 +15897,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
   case FixKind::AllowInvalidInitRef:
   case FixKind::AllowClosureParameterDestructuring:
   case FixKind::AllowInaccessibleMember:
+  case FixKind::AllowMemberFromWrongModule:
   case FixKind::AllowAnyObjectKeyPathRoot:
   case FixKind::AllowMultiArgFuncKeyPathMismatch:
   case FixKind::TreatKeyPathSubscriptIndexAsHashable:
diff --git a/test/NameLookup/module_selector.swift b/test/NameLookup/module_selector.swift
@@ -123,8 +123,8 @@ extension B: @retroactive main::Equatable {
       // expected-error@-3 {{cannot infer contextual base in reference to member 'main::min'}}
 
       self = B.main::init(value: .min)
-      // FIXME improve: expected-error@-1 {{'B' cannot be constructed because it has no accessible initializers}}
-      // expected-error@-2 {{cannot infer contextual base in reference to member 'min'}}
+      // expected-error@-1 {{declaration 'init(value:)' is not imported through module 'main'}}
+      // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{16-20=ModuleSelectorTestingKit}}
     }
 
     self.main::myNegate()
@@ -163,7 +163,7 @@ extension C: @retroactive ModuleSelectorTestingKit::Equatable {
   @_dynamicReplacement(for: ModuleSelectorTestingKit::negate())
 
   mutating func myNegate() {
-  // FIXME improve: expected-note@-1 {{did you mean 'myNegate'?}}
+  // expected-note@-1 {{'myNegate()' declared here}}
 
     let fn: (ModuleSelectorTestingKit::Int, ModuleSelectorTestingKit::Int) -> ModuleSelectorTestingKit::Int =
     // expected-error@-1 3{{type 'Int' is not imported through module 'ModuleSelectorTestingKit'}}
@@ -189,13 +189,15 @@ extension C: @retroactive ModuleSelectorTestingKit::Equatable {
     }
     else {
       self = ModuleSelectorTestingKit::C(value: .ModuleSelectorTestingKit::min)
-      // FIXME improve: expected-error@-1 {{type 'Int' has no member 'ModuleSelectorTestingKit::min'}}
+      // expected-error@-1 {{declaration 'min' is not imported through module 'ModuleSelectorTestingKit'}}
+      // expected-note@-2 {{did you mean module 'Swift'?}} {{50-74=Swift}}
 
       self = C.ModuleSelectorTestingKit::init(value: .min)
     }
 
     self.ModuleSelectorTestingKit::myNegate()
-    // FIXME improve: expected-error@-1 {{value of type 'C' has no member 'ModuleSelectorTestingKit::myNegate'}}
+    // expected-error@-1 {{declaration 'myNegate()' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'main'?}} {{10-34=main}}
 
     ModuleSelectorTestingKit::fatalError()
     // expected-error@-1 {{declaration 'fatalError' is not imported through module 'ModuleSelectorTestingKit'}}
@@ -231,7 +233,7 @@ extension D: @retroactive Swift::Equatable {
   // FIXME improve: expected-error@-1 {{replaced function 'Swift::negate()' could not be found}}
 
   mutating func myNegate() {
-    // expected-note@-1 {{did you mean 'myNegate'?}}
+  // expected-note@-1 {{'myNegate()' declared here}}
 
     let fn: (Swift::Int, Swift::Int) -> Swift::Int =
       (Swift::+)
@@ -255,12 +257,13 @@ extension D: @retroactive Swift::Equatable {
       // expected-error@-3 {{cannot infer contextual base in reference to member 'Swift::min'}}
 
       self = D.Swift::init(value: .min)
-      // FIXME improve: expected-error@-1 {{'D' cannot be constructed because it has no accessible initializers}}
-      // expected-error@-2 {{cannot infer contextual base in reference to member 'min'}}
+      // expected-error@-1 {{declaration 'init(value:)' is not imported through module 'Swift'}}
+      // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{16-21=ModuleSelectorTestingKit}}
     }
 
     self.Swift::myNegate()
-    // FIXME improve: expected-error@-1 {{value of type 'D' has no member 'Swift::myNegate'}}
+    // expected-error@-1 {{declaration 'myNegate()' is not imported through module 'Swift'}}
+    // expected-note@-2 {{did you mean module 'main'?}} {{10-15=main}}
 
     Swift::fatalError()
   }
@@ -490,8 +493,8 @@ func badModuleNames() {
   // FIXME redundant: expected-note@-3  {{did you mean module 'Swift'?}}
 
   _ = "foo".NonexistentModule::count
-  // FIXME improve: expected-error@-1 {{value of type 'String' has no member 'NonexistentModule::count'}}
-  // FIXME: expected-EVENTUALLY-note@-2 {{did you mean module 'Swift'?}} {{13-30=Swift}}
+  // expected-error@-1 {{declaration 'count' is not imported through module 'NonexistentModule'}}
+  // expected-note@-2 {{did you mean module 'Swift'?}} {{13-30=Swift}}
 
   let x: NonexistentModule::MyType = NonexistentModule::MyType()
   // expected-error@-1 {{cannot find type 'NonexistentModule::MyType' in scope}}
