Merge pull request #20904 from rintaro/ast-lookupvisibledecls-forward

[AST] Don't suggest unusable values for typo correction and code completion

Author: rintaro

lib/AST/LookupVisibleDecls.cpp

@@ -942,11 +942,10 @@ static void lookupVisibleMemberDecls(
     Consumer.foundDecl(DeclAndReason.D, DeclAndReason.Reason);
 }
 
-void swift::lookupVisibleDecls(VisibleDeclConsumer &Consumer,
-                               const DeclContext *DC,
-                               LazyResolver *TypeResolver,
-                               bool IncludeTopLevel,
-                               SourceLoc Loc) {
+static void lookupVisibleDeclsImpl(VisibleDeclConsumer &Consumer,
+                                   const DeclContext *DC,
+                                   LazyResolver *TypeResolver,
+                                   bool IncludeTopLevel, SourceLoc Loc) {
   const ModuleDecl &M = *DC->getParentModule();
   const SourceManager &SM = DC->getASTContext().SourceMgr;
   auto Reason = DeclVisibilityKind::MemberOfCurrentNominal;
@@ -1067,6 +1066,65 @@ void swift::lookupVisibleDecls(VisibleDeclConsumer &Consumer,
   }
 }
 
+void swift::lookupVisibleDecls(VisibleDeclConsumer &Consumer,
+                               const DeclContext *DC,
+                               LazyResolver *TypeResolver,
+                               bool IncludeTopLevel,
+                               SourceLoc Loc) {
+  if (Loc.isInvalid()) {
+    lookupVisibleDeclsImpl(Consumer, DC, TypeResolver, IncludeTopLevel, Loc);
+    return;
+  }
+
+  // Filtering out unusable values.
+  class LocalConsumer : public VisibleDeclConsumer {
+    const SourceManager &SM;
+    SourceLoc Loc;
+    VisibleDeclConsumer &Consumer;
+
+    bool isUsableValue(ValueDecl *VD, DeclVisibilityKind Reason) {
+
+      // Check "use within its own initial value" case.
+      if (auto *varD = dyn_cast<VarDecl>(VD))
+        if (auto *PBD = varD->getParentPatternBinding())
+          if (!PBD->isImplicit() &&
+              SM.rangeContainsTokenLoc(PBD->getSourceRange(), Loc))
+            return false;
+
+      switch (Reason) {
+      case DeclVisibilityKind::LocalVariable:
+        // Use of 'TypeDecl's before declaration is allowed.
+        if (isa<TypeDecl>(VD))
+          return true;
+
+        return SM.isBeforeInBuffer(VD->getLoc(), Loc);
+
+      case DeclVisibilityKind::VisibleAtTopLevel:
+        // TODO: Implement forward reference rule for script mode? Currently,
+        // it's not needed because the rest of the file hasn't been parsed.
+        // See: https://bugs.swift.org/browse/SR-284 for the rule.
+        return true;
+
+      default:
+        // Other visibility kind are always usable.
+        return true;
+      }
+    }
+
+  public:
+    LocalConsumer(const SourceManager &SM, SourceLoc Loc,
+                  VisibleDeclConsumer &Consumer)
+        : SM(SM), Loc(Loc), Consumer(Consumer) {}
+
+    void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason) {
+      if (isUsableValue(VD, Reason))
+        Consumer.foundDecl(VD, Reason);
+    }
+  } LocalConsumer(DC->getASTContext().SourceMgr, Loc, Consumer);
+
+  lookupVisibleDeclsImpl(LocalConsumer, DC, TypeResolver, IncludeTopLevel, Loc);
+}
+
 void swift::lookupVisibleMemberDecls(VisibleDeclConsumer &Consumer, Type BaseTy,
                                      const DeclContext *CurrDC,
                                      LazyResolver *TypeResolver,

lib/Sema/TypeCheckNameLookup.cpp

@@ -637,15 +637,6 @@ static bool isPlausibleTypo(DeclRefKind refKind, DeclName typedName,
   return true;
 }
 
-static bool isLocInVarInit(TypeChecker &TC, VarDecl *var, SourceLoc loc) {
-  auto binding = var->getParentPatternBinding();
-  if (!binding || binding->isImplicit())
-    return false;
-
-  auto initRange = binding->getSourceRange();
-  return TC.Context.SourceMgr.rangeContainsTokenLoc(initRange, loc);
-}
-
 void TypeChecker::performTypoCorrection(DeclContext *DC, DeclRefKind refKind,
                                         Type baseTypeOrNull,
                                         NameLookupOptions lookupOptions,
@@ -670,12 +661,6 @@ void TypeChecker::performTypoCorrection(DeclContext *DC, DeclRefKind refKind,
     if (!isPlausibleTypo(refKind, corrections.WrittenName, decl))
       return;
 
-    // Don't suggest a variable within its own initializer.
-    if (auto var = dyn_cast<VarDecl>(decl)) {
-      if (isLocInVarInit(*this, var, corrections.Loc.getBaseNameLoc()))
-        return;
-    }
-
     auto candidateName = decl->getFullName();
 
     // Don't waste time computing edit distances that are more than

test/IDE/complete_expr_postfix_begin.swift

@@ -75,6 +75,14 @@
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_TUPLE_1 | %FileCheck %s -check-prefix=IN_TUPLE_1
 // RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_TUPLE_2 | %FileCheck %s -check-prefix=IN_TUPLE_2
 
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_1 | %FileCheck %s -check-prefix=OWN_INIT_1
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_2 | %FileCheck %s -check-prefix=OWN_INIT_2
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_3 | %FileCheck %s -check-prefix=OWN_INIT_3
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_4 | %FileCheck %s -check-prefix=OWN_INIT_4
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_5 | %FileCheck %s -check-prefix=OWN_INIT_5
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_6 | %FileCheck %s -check-prefix=OWN_INIT_6
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_7 | %FileCheck %s -check-prefix=OWN_INIT_7
+
 //
 // Test code completion at the beginning of expr-postfix.
 //
@@ -435,8 +443,7 @@ func testInForEach1(arg: Int) {
   let after = 4
 // IN_FOR_EACH_1-NOT: Decl[LocalVar]
 // IN_FOR_EACH_1: Decl[LocalVar]/Local:               local[#Int#];
-// FIXME: shouldn't show 'after' here.
-// IN_FOR_EACH_1: Decl[LocalVar]/Local:               after[#Int#];
+// IN_FOR_EACH_1-NOT: after
 // IN_FOR_EACH_1: Decl[LocalVar]/Local:               arg[#Int#];
 // IN_FOR_EACH_1-NOT: Decl[LocalVar]
 }
@@ -448,8 +455,7 @@ func testInForEach2(arg: Int) {
   let after = 4
 // IN_FOR_EACH_2-NOT: Decl[LocalVar]
 // IN_FOR_EACH_2: Decl[LocalVar]/Local/TypeRelation[Identical]: local[#Int#];
-// FIXME: shouldn't show 'after' here.
-// IN_FOR_EACH_2: Decl[LocalVar]/Local/TypeRelation[Identical]: after[#Int#];
+// IN_FOR_EACH_2-NOT: after
 // IN_FOR_EACH_2: Decl[LocalVar]/Local/TypeRelation[Identical]: arg[#Int#];
 // IN_FOR_EACH_2-NOT: Decl[LocalVar]
 }
@@ -463,8 +469,7 @@ func testInForEach3(arg: Int) {
 // IN_FOR_EACH_3: Decl[LocalVar]/Local:               index[#Int#];
 // IN_FOR_EACH_3-NOT: Decl[LocalVar]
 // IN_FOR_EACH_3: Decl[LocalVar]/Local:               local[#Int#];
-// FIXME: shouldn't show 'after' here.
-// IN_FOR_EACH_3: Decl[LocalVar]/Local:               after[#Int#];
+// IN_FOR_EACH_3-NOT: after
 // IN_FOR_EACH_3: Decl[LocalVar]/Local:               arg[#Int#];
 // IN_FOR_EACH_3-NOT: Decl[LocalVar]
 }
@@ -503,8 +508,7 @@ func testInForEach9(arg: Int) {
 // NOTE: [Convertible] to AnyHashable.
 // IN_FOR_EACH_4-NOT: Decl[LocalVar]
 // IN_FOR_EACH_4: Decl[LocalVar]/Local/TypeRelation[Convertible]: local[#Int#];
-// FIXME: shouldn't show 'after' here.
-// IN_FOR_EACH_4: Decl[LocalVar]/Local/TypeRelation[Convertible]: after[#Int#];
+// IN_FOR_EACH_4-NOT: after
 // IN_FOR_EACH_4: Decl[LocalVar]/Local/TypeRelation[Convertible]: arg[#Int#];
 // IN_FOR_EACH_4-NOT: Decl[LocalVar]
 }
@@ -555,3 +559,31 @@ func testTuple(localInt: Int) {
 // IN_TUPLE_2: Decl[LocalVar]/Local:               localStr[#String#]; name=localStr
 // IN_TUPLE_2: Decl[LocalVar]/Local/TypeRelation[Identical]: localInt[#Int#]; name=localInt
 // IN_TUPLE_2: End completions
+
+var ownInit1: Int = #^OWN_INIT_1^#
+// OWN_INIT_1: Begin completions
+// OWN_INIT_1-NOT: ownInit1
+var ownInit2: () -> Void = { #^OWN_INIT_2^# }
+// OWN_INIT_2: Begin completions
+// OWN_INIT_2-NOT: ownInit2
+struct OwnInitTester {
+  var ownInit3: Int = #^OWN_INIT_3^#
+  // OWN_INIT_3: Begin completions
+  // OWN_INIT_3-NOT: ownInit3
+  var ownInit4: () -> Void = { #^OWN_INIT_4^# }
+  // OWN_INIT_4: Begin completions
+  // OWN_INIT_4-NOT: ownInit4
+}
+func ownInitTesting() {
+  var ownInit5: Int = #^OWN_INIT_5^#
+  // OWN_INIT_5: Begin completions
+  // OWN_INIT_5-NOT: ownInit5
+  var ownInit6: () -> Void = { #^OWN_INIT_6^# }
+  // OWN_INIT_6: Begin completions
+  // OWN_INIT_6-NOT: ownInit6
+}
+func ownInitTestingShadow(ownInit7: Int) {
+  var ownInit7: Int = #^OWN_INIT_7^#
+  // OWN_INIT_7: Begin completions
+  // OWN_INIT_7: Decl[LocalVar]/Local/TypeRelation[Identical]: ownInit7[#Int#];
+}

test/Sema/typo_correction.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift -typo-correction-limit 22
+// RUN: %target-typecheck-verify-swift -typo-correction-limit 23
 // RUN: not %target-swift-frontend -typecheck -disable-typo-correction %s 2>&1 | %FileCheck %s -check-prefix=DISABLED
 // RUN: not %target-swift-frontend -typecheck -typo-correction-limit 0 %s 2>&1 | %FileCheck %s -check-prefix=DISABLED
 // RUN: not %target-swift-frontend -typecheck -DIMPORT_FAIL %s 2>&1 | %FileCheck %s -check-prefix=DISABLED
@@ -191,3 +191,9 @@ func test_underscored_match() {
   _ = _fggs + 1
   // expected-error@-1 {{use of unresolved identifier '_fggs'; did you mean '_eggs'?}}
 }
+
+// Don't show values before declaration.
+func testFwdRef() {
+  let _ = forward_refX + 1 // expected-error {{use of unresolved identifier 'forward_refX'}}
+  let forward_ref1 = 4
+}