Improve expression diagnostics for incorrect module selectors

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

@@ -637,6 +637,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

@@ -93,15 +93,18 @@ extension B: main::Equatable {
     // FIXME incorrect: expected-error@-3 {{variable used within its own initial value}}
 
     if main::Bool.main::random() {
-    // FIXME improve: expected-error@-1 {{use of unresolved identifier 'main::Bool'}}
+    // 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, suggest adding 'self.': expected-error@-1 {{use of unresolved identifier 'main::negate'}}
+      // 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 {{use of unresolved identifier 'main::B'}}
-      // 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.main::myNegate()
@@ -152,10 +155,12 @@ extension ModuleSelectorTestingKit::C: ModuleSelectorTestingKit::Equatable {
     // FIXME incorrect: expected-error@-3 {{variable used within its own initial value}}
 
     if ModuleSelectorTestingKit::Bool.ModuleSelectorTestingKit::random() {
-    // FIXME improve: expected-error@-1 {{use of unresolved identifier 'ModuleSelectorTestingKit::Bool'}}
+    // expected-error@-1 {{declaration 'Bool' is not imported through module 'ModuleSelectorTestingKit'}}
+    // expected-note@-2 {{did you mean module 'Swift'?}} {{8-31=Swift}}
 
       ModuleSelectorTestingKit::negate()
-      // FIXME improve, suggest adding 'self.': expected-error@-1 {{use of unresolved identifier 'ModuleSelectorTestingKit::negate'}}
+      // 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)
@@ -205,12 +210,14 @@ extension D: @retroactive Swift::Equatable {
     // FIXME: expected-error@-2 {{variable used within its own initial value}}
     if Swift::Bool.Swift::random() {
       Swift::negate()
-      // FIXME improve: expected-error@-1 {{use of unresolved identifier 'Swift::negate'}}
+      // 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 {{use of unresolved identifier 'Swift::D'}}
-      // 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.Swift::myNegate()
@@ -315,10 +322,10 @@ func main::decl1(
   // expected-error@-2 {{name of constant declaration cannot be qualified with module selector}}
 
   // From uses in the switch statements below:
-  // expected-note@-5 3{{did you mean 'decl1g'?}}
+  // expected-note@-5 3{{did you mean the local declaration?}}
 
   switch Optional(main::decl1g) {
-  // expected-error@-1 {{use of unresolved identifier 'main::decl1g'}}
+  // expected-error@-1 {{declaration 'decl1g' is not imported through module 'main'}}
   case Optional.some(let main::decl1i):
     // expected-error@-1 {{name of constant declaration cannot be qualified with module selector}}
     break
@@ -327,7 +334,7 @@ func main::decl1(
   }
 
   switch Optional(main::decl1g) {
-  // expected-error@-1 {{use of unresolved identifier 'main::decl1g'}}
+  // expected-error@-1 {{declaration 'decl1g' is not imported through module 'main'}}
   case let Optional.some(main::decl1j):
     // expected-error@-1 {{name of constant declaration cannot be qualified with module selector}}
     break
@@ -336,7 +343,7 @@ func main::decl1(
   }
 
   switch Optional(main::decl1g) {
- // expected-error@-1 {{use of unresolved identifier 'main::decl1g'}}
+ // expected-error@-1 {{declaration 'decl1g' is not imported through module 'main'}}
  case let main::decl1k?:
     // expected-error@-1 {{name of constant declaration cannot be qualified with module selector}}
     break