Improve module selector type lookup diagnostics

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -1191,6 +1191,10 @@ ERROR(cannot_find_type_in_cast_expression,none,
       "type-casting operator expects a type on its right-hand side (got: %kind0)", (const ValueDecl *))
 ERROR(cannot_find_type_in_scope_did_you_mean,none,
       "cannot find type %0 in scope; did you mean to use '%1'?", (DeclNameRef, StringRef))
+ERROR(wrong_module_selector,none,
+      "%0 is not imported through module %1", (DeclName, Identifier))
+NOTE(note_change_module_selector,none,
+     "did you mean module %0?", (Identifier))
 NOTE(note_typo_candidate_implicit_member,none,
      "did you mean the implicitly-synthesized %kindbase0?", (const ValueDecl *))
 NOTE(note_remapped_type,none,

include/swift/AST/NameLookup.h

@@ -155,11 +155,18 @@ class LookupResult {
       : Results(Results.begin(), Results.end()),
         IndexOfFirstOuterResult(indexOfFirstOuterResult) {}
 
+  using const_iterator = SmallVectorImpl<LookupResultEntry>::const_iterator;
+  const_iterator begin() const { return Results.begin(); }
+  const_iterator end() const {
+    return Results.begin() + IndexOfFirstOuterResult;
+  }
+
   using iterator = SmallVectorImpl<LookupResultEntry>::iterator;
   iterator begin() { return Results.begin(); }
   iterator end() {
     return Results.begin() + IndexOfFirstOuterResult;
   }
+
   unsigned size() const { return innerResults().size(); }
   bool empty() const { return innerResults().empty(); }
 

lib/Sema/TypeCheckNameLookup.cpp

@@ -1153,3 +1153,45 @@ void swift::diagnoseMissingImports(SourceFile &sf) {
     }
   }
 }
+
+ModuleSelectorCorrection::
+ModuleSelectorCorrection(const LookupResult &candidates) {
+  // Produce a list of *unique* module selector diagnostics so we don't
+  // emit a bunch of duplicates.
+  for (auto result : candidates) {
+    ValueDecl * decl = result.getValueDecl();
+    auto owningModule = decl->getModuleContext();
+    candidateModules.insert(owningModule->getName());
+  }
+}
+
+ModuleSelectorCorrection::
+ModuleSelectorCorrection(const LookupTypeResult &candidates) {
+  // Produce a list of *unique* module selector diagnostics so we don't
+  // emit a bunch of duplicates.
+  for (auto result : candidates) {
+    auto owningModule = result.Member->getModuleContext();
+    candidateModules.insert(owningModule->getName());
+  }
+}
+
+bool ModuleSelectorCorrection::diagnose(ASTContext &ctx,
+                                        DeclNameLoc nameLoc,
+                                        DeclNameRef originalName) const {
+  if (candidateModules.empty())
+    return false;
+
+  ctx.Diags.diagnose(nameLoc, diag::wrong_module_selector,
+                     originalName.getFullName(),
+                     originalName.getModuleSelector());
+
+  SourceLoc moduleSelectorLoc = nameLoc.getModuleSelectorLoc();
+
+  for (auto moduleName : candidateModules) {
+    ctx.Diags.diagnose(moduleSelectorLoc, diag::note_change_module_selector,
+                       moduleName)
+        .fixItReplace(moduleSelectorLoc, moduleName.str());
+  }
+
+  return true;
+}

lib/Sema/TypeCheckType.cpp

@@ -1534,6 +1534,18 @@ static Type diagnoseUnknownType(const TypeResolution &resolution,
       return ErrorType::get(ctx);
     }
 
+    // Is there an incorrect module selector?
+    if (repr->getNameRef().hasModuleSelector()) {
+      auto anyModuleName = DeclNameRef(repr->getNameRef().getFullName());
+      ModuleSelectorCorrection correction(
+        TypeChecker::lookupUnqualifiedType(dc, anyModuleName, repr->getLoc(),
+                                           lookupOptions));
+      if (correction.diagnose(ctx, repr->getNameLoc(), repr->getNameRef())) {
+        // FIXME: Can we recover by assuming the first/best result is correct?
+        return ErrorType::get(ctx);
+      }
+    }
+
     // Try ignoring access control.
     NameLookupOptions relookupOptions = lookupOptions;
     relookupOptions |= NameLookupFlags::IgnoreAccessControl;
@@ -1625,6 +1637,19 @@ static Type diagnoseUnknownType(const TypeResolution &resolution,
     return ErrorType::get(ctx);
   }
 
+  // Is there an incorrect module selector?
+  if (repr->getNameRef().hasModuleSelector()) {
+    auto anyModuleName = DeclNameRef(repr->getNameRef().getFullName());
+    ModuleSelectorCorrection correction(
+      TypeChecker::lookupMemberType(dc, parentType, anyModuleName,
+                                    repr->getLoc(), lookupOptions));
+    
+    if (correction.diagnose(ctx, repr->getNameLoc(), repr->getNameRef())) {
+      // FIXME: Can we recover by assuming the first/best result is correct?
+      return ErrorType::get(ctx);
+    }
+  }
+
   // Try ignoring access control.
   NameLookupOptions relookupOptions = lookupOptions;
   relookupOptions |= NameLookupFlags::IgnoreAccessControl;

lib/Sema/TypeChecker.h

@@ -95,9 +95,14 @@ class LookupTypeResult {
   SmallVector<LookupTypeResultEntry, 4> Results;
 
 public:
+  using const_iterator = SmallVectorImpl<LookupTypeResultEntry>::const_iterator;
+  const_iterator begin() const { return Results.begin(); }
+  const_iterator end() const { return Results.end(); }
+
   using iterator = SmallVectorImpl<LookupTypeResultEntry>::iterator;
   iterator begin() { return Results.begin(); }
   iterator end() { return Results.end(); }
+
   unsigned size() const { return Results.size(); }
 
   LookupTypeResultEntry operator[](unsigned index) const {
@@ -299,6 +304,29 @@ class CheckGenericArgumentsResult {
   CheckRequirementsResult getKind() const { return Kind; }
 };
 
+/// Helper type for diagnosing a lookup that failed merely because it was
+/// restricted by a module selector.
+///
+/// To use this, perform the lookup again without its module selector and
+/// construct a \c ModuleSelectorCorrection from the lookup result. If
+/// there are any results, the \c diagnose() method will emit an error and a
+/// set of fix-it notes.
+class ModuleSelectorCorrection {
+  using CandidateModule = Identifier;
+  SmallSetVector<CandidateModule, 4> candidateModules;
+
+public:
+  ModuleSelectorCorrection(const LookupResult &candidates);
+  ModuleSelectorCorrection(const LookupTypeResult &candidates);
+
+  /// Emit an error and warnings if there were any candidates.
+  ///
+  /// \returns \c true if an error was diagnosed.
+  bool diagnose(ASTContext &ctx,
+                DeclNameLoc nameLoc,
+                DeclNameRef originalName) const;
+};
+
 /// Describes the kind of checked cast operation being performed.
 enum class CheckedCastContextKind {
   /// None: we're just establishing how to perform the checked cast. This

test/NameLookup/module_selector.swift

@@ -69,18 +69,23 @@ extension A: @retroactive Swift::Equatable {
 // Test resolution of main:: using `B`
 
 extension main::B {}
-// FIXME improve: expected-error@-1 {{cannot find type 'main::B' in scope}}
+// expected-error@-1 {{'B' is not imported through module 'main'}}
+// expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{11-15=ModuleSelectorTestingKit}}
 
 extension B: @retroactive main::Equatable {
-  // FIXME improve: expected-error@-1 {{cannot find type 'main::Equatable' in scope}}
+  // expected-error@-1 {{'Equatable' is not imported through module 'main'}}
+  // expected-note@-2 {{did you mean module 'Swift'?}} {{27-31=Swift}}
 
   @_implements(main::Equatable, ==(_:_:))
-  // FIXME improve: expected-error@-1 {{cannot find type 'main::Equatable' in scope}}
+  // expected-error@-1 {{'Equatable' is not imported through module 'main'}}
+  // expected-note@-2 {{did you mean module 'Swift'?}} {{16-20=Swift}}
 
   public static func equals(_: main::B, _: main::B) -> main::Bool {
-  // FIXME improve: expected-error@-1 {{cannot find type 'main::B' in scope}}
-  // FIXME improve: expected-error@-2 {{cannot find type 'main::B' in scope}}
-  // FIXME improve: expected-error@-3 {{cannot find type 'main::Bool' in scope}}
+    // expected-error@-1 2{{'B' is not imported through module 'main'}}
+    // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{32-36=ModuleSelectorTestingKit}}
+    // expected-note@-3 {{did you mean module 'ModuleSelectorTestingKit'?}} {{44-48=ModuleSelectorTestingKit}}
+    // expected-error@-4 {{'Bool' is not imported through module 'main'}}
+    // expected-note@-5 {{did you mean module 'Swift'?}} {{56-60=Swift}}
     main::fatalError()
     // FIXME improve: expected-error@-1 {{cannot find 'main::fatalError' in scope}}
   }
@@ -93,12 +98,16 @@ extension B: @retroactive main::Equatable {
 
   mutating func myNegate() {
     let fn: (main::Int, main::Int) -> main::Int =
-    // FIXME improve: expected-error@-1 3{{cannot find type 'main::Int' in scope}}
+    // expected-error@-1 3{{'Int' is not imported through module 'main'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{14-18=Swift}}
+    // expected-note@-3 {{did you mean module 'Swift'?}} {{25-29=Swift}}
+    // expected-note@-4 {{did you mean module 'Swift'?}} {{39-43=Swift}}
       (main::+)
       // FIXME improve: expected-error@-1 {{cannot find operator 'main::+' in scope}}
 
     let magnitude: Int.main::Magnitude = main::magnitude
-    // FIXME improve: expected-error@-1 {{'main::Magnitude' is not a member type of struct 'Swift.Int'}}
+    // expected-error@-1 {{'Magnitude' is not imported through module 'main'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{24-28=Swift}}
 
     _ = (fn, magnitude)
 
@@ -124,7 +133,8 @@ extension B: @retroactive main::Equatable {
     // FIXME improve: expected-error@-1 {{cannot find 'main::fatalError' in scope}}
 
     _ = \main::A.magnitude
-    // FIXME improve: expected-error@-1 {{'main::A' in scope}} -- different diagnostic wording for legacy parser vs. ASTGen
+    // expected-error@-1 {{'A' is not imported through module 'main'}}
+    // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{10-14=ModuleSelectorTestingKit}}
     _ = \A.main::magnitude
     // FIXME improve: expected-error@-1 {{value of type 'A' has no member 'main::magnitude'}}
 
@@ -143,13 +153,16 @@ extension B: @retroactive main::Equatable {
 extension ModuleSelectorTestingKit::C {}
 
 extension C: @retroactive ModuleSelectorTestingKit::Equatable {
-// FIXME improve: expected-error@-1 {{cannot find type 'ModuleSelectorTestingKit::Equatable' in scope}}
+  // expected-error@-1 {{'Equatable' is not imported through module 'ModuleSelectorTestingKit'}}
+  // expected-note@-2 {{did you mean module 'Swift'?}} {{27-51=Swift}}
 
   @_implements(ModuleSelectorTestingKit::Equatable, ==(_:_:))
-  // FIXME improve: expected-error@-1 {{cannot find type 'ModuleSelectorTestingKit::Equatable' in scope}}
+  // expected-error@-1 {{'Equatable' is not imported through module 'ModuleSelectorTestingKit'}}
+  // expected-note@-2 {{did you mean module 'Swift'?}} {{16-40=Swift}}
 
   public static func equals(_: ModuleSelectorTestingKit::C, _: ModuleSelectorTestingKit::C) -> ModuleSelectorTestingKit::Bool {
-  // FIXME improve: expected-error@-1 {{cannot find type 'ModuleSelectorTestingKit::Bool' in scope}}
+    // expected-error@-1 {{'Bool' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{96-120=Swift}}
 
     ModuleSelectorTestingKit::fatalError()
     // FIXME improve: expected-error@-1 {{cannot find 'ModuleSelectorTestingKit::fatalError' in scope}}
@@ -162,13 +175,17 @@ extension C: @retroactive ModuleSelectorTestingKit::Equatable {
 
   mutating func myNegate() {
     let fn: (ModuleSelectorTestingKit::Int, ModuleSelectorTestingKit::Int) -> ModuleSelectorTestingKit::Int =
-    // FIXME improve: expected-error@-1 3{{cannot find type 'ModuleSelectorTestingKit::Int' in scope}}
+    // expected-error@-1 3{{'Int' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{14-38=Swift}}
+    // expected-note@-3 {{did you mean module 'Swift'?}} {{45-69=Swift}}
+    // expected-note@-4 {{did you mean module 'Swift'?}} {{79-103=Swift}}
       (ModuleSelectorTestingKit::+)
       // FIXME improve: expected-error@-1 {{cannot find operator 'ModuleSelectorTestingKit::+' in scope}}
 
     let magnitude: Int.ModuleSelectorTestingKit::Magnitude = ModuleSelectorTestingKit::magnitude
-    // FIXME improve: expected-error@-1 {{'ModuleSelectorTestingKit::Magnitude' is not a member type of struct 'Swift.Int'}}
-    // FIXME improve: expected-error@-2 {{cannot find 'ModuleSelectorTestingKit::magnitude' in scope}}
+    // expected-error@-1 {{'Magnitude' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{24-48=Swift}}
+    // FIXME improve: expected-error@-3 {{cannot find 'ModuleSelectorTestingKit::magnitude' in scope}}
 
     _ = (fn, magnitude)
 
@@ -206,15 +223,17 @@ extension C: @retroactive ModuleSelectorTestingKit::Equatable {
 // Test resolution of Swift:: using `D`
 
 extension Swift::D {}
-// FIXME improve: expected-error@-1 {{cannot find type 'Swift::D' in scope}}
+// expected-error@-1 {{'D' is not imported through module 'Swift'}}
+// expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{11-16=ModuleSelectorTestingKit}}
 
 extension D: @retroactive Swift::Equatable {
 // Caused by Swift::D failing to typecheck in `equals(_:_:)`: expected-error@-1 *{{extension outside of file declaring struct 'D' prevents automatic synthesis of '==' for protocol 'Equatable'}} expected-note@-1 *{{add stubs for conformance}}
 
   @_implements(Swift::Equatable, ==(_:_:))
   public static func equals(_: Swift::D, _: Swift::D) -> Swift::Bool {
-  // expected-error@-1 {{cannot find type 'Swift::D' in scope}}
-  // expected-error@-2 {{cannot find type 'Swift::D' in scope}}
+  // expected-error@-1 2{{'D' is not imported through module 'Swift'}}
+  // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{32-37=ModuleSelectorTestingKit}}
+  // expected-note@-3 {{did you mean module 'ModuleSelectorTestingKit'?}} {{45-50=ModuleSelectorTestingKit}}
     Swift::fatalError() // no-error -- not typechecking function bodies
   }
 
@@ -255,7 +274,8 @@ extension D: @retroactive Swift::Equatable {
     Swift::fatalError()
 
     _ = \Swift::A.magnitude
-    // FIXME improve: expected-error@-1 {{'Swift::A' in scope}} -- different diagnostic wording for legacy parser vs. ASTGen
+    // expected-error@-1 {{'A' is not imported through module 'Swift'}}
+    // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{10-15=ModuleSelectorTestingKit}}
     _ = \A.Swift::magnitude
     // FIXME improve: expected-error@-1 {{value of type 'A' has no member 'Swift::magnitude'}}
 
@@ -306,7 +326,8 @@ func builderUser4(@Swift::MyBuilder fn: () -> Void) {}
 
 func decl1(
   p1: main::A,
-  // FIXME: expected-error@-1 {{cannot find type 'main::A' in scope}}
+  // expected-error@-1 {{'A' is not imported through module 'main'}}
+  // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{7-11=ModuleSelectorTestingKit}}
   label p2: inout A,
   label p3: @escaping () -> A
 ) {
@@ -336,7 +357,8 @@ func decl1(
 }
 
 typealias decl5 = main::Bool
-// FIXME improve: expected-error@-1 {{cannot find type 'main::Bool' in scope}}
+// expected-error@-1 {{'Bool' is not imported through module 'main'}}
+// expected-note@-2 {{did you mean module 'Swift'?}} {{19-23=Swift}}
 
 func badModuleNames() {
   NonexistentModule::print()