Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 4, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 63 additions & 5 deletions lib/AST/LookupVisibleDecls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 0 additions & 15 deletions lib/Sema/TypeCheckNameLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
48 changes: 40 additions & 8 deletions test/IDE/complete_expr_postfix_begin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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]
}
Expand All @@ -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]
}
Expand All @@ -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]
}
Expand Down Expand Up @@ -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]
}
Expand Down Expand Up @@ -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#];
}
8 changes: 7 additions & 1 deletion test/Sema/typo_correction.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
}