Improve module selector expr lookup diagnostics

Author: beccadax

include/swift/AST/DiagnosticsSema.def

@@ -1161,8 +1161,14 @@ 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(type_not_in_module,none,
       "type %0 is not imported through module %1", (DeclName, Identifier))
+ERROR(decl_not_in_module,none,
+      "declaration %0 is not imported through module %1", (DeclName, Identifier))
 NOTE(note_change_module_selector,none,
      "did you mean module %0?", (Identifier))
+NOTE(note_remove_module_selector,none,
+     "did you mean the local declaration?", ())
+NOTE(note_add_explicit_self_with_module_selector,none,
+     "did you mean the member of 'self'?", ())
 NOTE(note_typo_candidate_implicit_member,none,
      "did you mean the implicitly-synthesized %kindbase0?", (const ValueDecl *))
 NOTE(note_remapped_type,none,

lib/Sema/PreCheckTarget.cpp

@@ -635,6 +635,52 @@ Expr *TypeChecker::resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE,
       return errorResult();
     }
 
+    // Is there an incorrect module selector?
+    if (Name.hasModuleSelector()) {
+      auto anyModuleName = DeclNameRef(LookupName.getFullName());
+      auto anyModuleResults = TypeChecker::lookupUnqualified(DC, anyModuleName,
+                                                             Loc,
+                                                             lookupOptions);
+      if (!anyModuleResults.empty()) {
+        Context.Diags.diagnose(UDRE->getNameLoc(), diag::decl_not_in_module,
+                               LookupName.getFullName(),
+                               LookupName.getModuleSelector());
+
+        SourceLoc moduleSelectorLoc = UDRE->getNameLoc().getModuleSelectorLoc();
+
+        for (auto result : anyModuleResults) {
+          ValueDecl * decl = result.getValueDecl();
+          Identifier moduleName = decl->getModuleContext()->getName();
+
+          if (moduleName != Name.getModuleSelector()) {
+            SmallString<64> replacement;
+            if (decl->isInstanceMember())
+              replacement += "self.";
+            replacement += moduleName.str();
+
+            Context.Diags.diagnose(moduleSelectorLoc,
+                                   diag::note_change_module_selector,
+                                   moduleName)
+                .fixItReplace(moduleSelectorLoc, replacement);
+          } else {
+            // It's just something we need to pick up contextually.
+            if (decl->getDeclContext()->getLocalContext())
+              decl->diagnose(diag::note_remove_module_selector)
+                  .fixItRemoveChars(moduleSelectorLoc,
+                                    UDRE->getNameLoc().getBaseNameLoc());
+
+            if (decl->isInstanceMember())
+              Context.Diags.diagnose(moduleSelectorLoc,
+                  diag::note_add_explicit_self_with_module_selector)
+                  .fixItInsert(moduleSelectorLoc, "self.");
+          }
+        }
+
+        // FIXME: Can we recover by assuming the first/best result is correct?
+        return new (Context) ErrorExpr(UDRE->getSourceRange());
+      }
+    }
+
     // Try ignoring access control.
     NameLookupOptions relookupOptions = lookupOptions;
     relookupOptions |= NameLookupFlags::IgnoreAccessControl;

test/NameLookup/module_selector.swift

@@ -109,15 +109,18 @@ extension B: @retroactive main::Equatable {
     _ = (fn, magnitude)
 
     if main::Bool.main::random() {
-      // FIXME improve: expected-error@-1 {{cannot find 'main::Bool' in scope}}
+      // expected-error@-1 {{declaration 'Bool' is not imported through module 'main'}}
+      // expected-note@-2 {{did you mean module 'Swift'?}} {{8-12=Swift}}
 
       main::negate()
-      // FIXME improve: expected-error@-1 {{cannot find 'main::negate' in scope}}
+      // expected-error@-1 {{declaration 'negate' is not imported through module 'main'}}
+      // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{7-11=self.ModuleSelectorTestingKit}}
     }
     else {
       self = main::B(value: .main::min)
-      // FIXME improve: expected-error@-1 {{cannot find 'main::B' in scope}}
-      // expected-error@-2 {{cannot infer contextual base in reference to member 'main::min'}}
+      // expected-error@-1 {{declaration 'B' is not imported through module 'main'}}
+      // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{14-18=ModuleSelectorTestingKit}}
+      // 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}}
@@ -127,7 +130,8 @@ extension B: @retroactive main::Equatable {
     self.main::myNegate()
 
     main::fatalError()
-    // FIXME improve: expected-error@-1 {{cannot find 'main::fatalError' in scope}}
+    // expected-error@-1 {{declaration 'fatalError' is not imported through module 'main'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{5-9=Swift}}
   }
 
   // FIXME: Can we test @convention(witness_method:)?
@@ -176,10 +180,12 @@ extension C: @retroactive ModuleSelectorTestingKit::Equatable {
     _ = (fn, magnitude)
 
     if ModuleSelectorTestingKit::Bool.ModuleSelectorTestingKit::random() {
-    // FIXME improve: expected-error@-1 {{cannot find 'ModuleSelectorTestingKit::Bool' in scope}}
+    // expected-error@-1 {{declaration 'Bool' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{8-32=Swift}}
 
       ModuleSelectorTestingKit::negate()
-      // expected-error@-1 {{cannot find 'ModuleSelectorTestingKit::negate' in scope}}
+      // expected-error@-1 {{declaration 'negate' is not imported through module 'ModuleSelectorTestingKit'}}
+      // expected-note@-2 {{did you mean the member of 'self'?}} {{7-7=self.}}
     }
     else {
       self = ModuleSelectorTestingKit::C(value: .ModuleSelectorTestingKit::min)
@@ -192,7 +198,8 @@ extension C: @retroactive ModuleSelectorTestingKit::Equatable {
     // FIXME improve: expected-error@-1 {{value of type 'C' has no member 'ModuleSelectorTestingKit::myNegate'}}
 
     ModuleSelectorTestingKit::fatalError()
-    // FIXME improve: expected-error@-1 {{cannot find 'ModuleSelectorTestingKit::fatalError' in scope}}
+    // expected-error@-1 {{declaration 'fatalError' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{5-29=Swift}}
   }
 
   // FIXME: Can we test @convention(witness_method:)?
@@ -224,24 +231,28 @@ 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'?}}
 
     let fn: (Swift::Int, Swift::Int) -> Swift::Int =
       (Swift::+)
 
     let magnitude: Int.Swift::Magnitude = Swift::magnitude
-    // expected-error@-1 {{cannot find 'Swift::magnitude' in scope}}
+    // expected-error@-1 {{declaration 'magnitude' is not imported through module 'Swift'}}
+    // expected-note@-2 {{did you mean module 'main'?}} {{43-48=main}}
 
     _ = (fn, magnitude)
 
     if Swift::Bool.Swift::random() {
 
       Swift::negate()
-      // FIXME improve: expected-error@-1 {{cannot find 'Swift::negate' in scope}}
+      // expected-error@-1 {{declaration 'negate' is not imported through module 'Swift'}}
+      // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{7-12=self.ModuleSelectorTestingKit}}
     }
     else {
       self = Swift::D(value: .Swift::min)
-      // FIXME improve: expected-error@-1 {{cannot find 'Swift::D' in scope}}
-      // expected-error@-2 {{cannot infer contextual base in reference to member 'Swift::min'}}
+      // expected-error@-1 {{declaration 'D' is not imported through module 'Swift'}}
+      // expected-note@-2 {{did you mean module 'ModuleSelectorTestingKit'?}} {{14-19=ModuleSelectorTestingKit}}
+      // 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}}
@@ -261,7 +272,8 @@ let mog: Never = fatalError()
 
 func localVarsCantBeAccessedByModuleSelector() {
   let mag: Int.Swift::Magnitude = main::mag
-  // expected-error@-1 {{cannot find 'main::mag' in scope}}
+  // expected-error@-1 {{declaration 'mag' is not imported through module 'main'}}
+  // expected-note@-2 {{did you mean the local declaration?}} {{35-41=}}
 
   let mog: Never = main::mog
 }
@@ -346,9 +358,10 @@ func main::decl1(
   guard let (main::decl1g, main::decl1h) = Optional(("g", "h")) else { return }
   // expected-error@-1 {{name in constant declaration cannot be qualified with a module selector}} expected-note@-1 {{remove module selector from this name}} {{14-20=}}
   // expected-error@-2 {{name in constant declaration cannot be qualified with a module selector}} expected-note@-2 {{remove module selector from this name}} {{28-34=}}
+  // From uses in the switch statements below: expected-note@-3 3 {{did you mean the local declaration?}}
 
   switch Optional(main::decl1g) {
-  // expected-error@-1 {{cannot find 'main::decl1g' in scope}}
+  // expected-error@-1 {{declaration 'decl1g' is not imported through module 'main'}}
   case Optional.some(let main::decl1i):
     // expected-error@-1 {{name in constant declaration cannot be qualified with a module selector}} expected-note@-1 {{remove module selector from this name}} {{26-32=}}
     break
@@ -357,7 +370,7 @@ func main::decl1(
   }
 
   switch Optional(main::decl1g) {
-  // expected-error@-1 {{cannot find 'main::decl1g' in scope}}
+  // expected-error@-1 {{declaration 'decl1g' is not imported through module 'main'}}
   case let Optional.some(main::decl1j):
     // expected-error@-1 {{name in constant declaration cannot be qualified with a module selector}} expected-note@-1 {{remove module selector from this name}} {{26-32=}}
     break
@@ -366,7 +379,7 @@ func main::decl1(
   }
 
   switch Optional(main::decl1g) {
-  // expected-error@-1 {{cannot find 'main::decl1g' in scope}}
+  // expected-error@-1 {{declaration 'decl1g' is not imported through module 'main'}}
  case let main::decl1k?:
     // expected-error@-1 {{name in constant declaration cannot be qualified with a module selector}} expected-note@-1 {{remove module selector from this name}} {{11-17=}}
     break
@@ -472,7 +485,9 @@ precedencegroup main::PG1 {
 
 func badModuleNames() {
   NonexistentModule::print()
-  // expected-error@-1 {{cannot find 'NonexistentModule::print' in scope}}
+  // expected-error@-1 {{declaration 'print' is not imported through module 'NonexistentModule'}}
+  // expected-note@-2 {{did you mean module 'Swift'?}} {{3-20=Swift}}
+  // 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'}}