Skip to content

Commit a07dda6

Browse files
author
Nathan Hawes
authored
Merge pull request #34448 from nathawes/completion-dedupe-ambiguous
[CodeCompletion] Dedupe completion results if multiple lookups were made
2 parents 848d823 + 62a5fb1 commit a07dda6

File tree

2 files changed

+90
-4
lines changed

2 files changed

+90
-4
lines changed

‎lib/IDE/CodeCompletion.cpp

Lines changed: 54 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1813,6 +1813,9 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
18131813
bool PreferFunctionReferencesToCalls = false;
18141814
bool HaveLeadingSpace = false;
18151815

1816+
bool CheckForDuplicates = false;
1817+
llvm::DenseSet<std::pair<const Decl *, Type>> PreviouslySeen;
1818+
18161819
bool IncludeInstanceMembers = false;
18171820

18181821
/// True if we are code completing inside a static method.
@@ -1998,6 +2001,10 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
19982001
IsKeyPathExpr = true;
19992002
}
20002003

2004+
void shouldCheckForDuplicates(bool value = true) {
2005+
CheckForDuplicates = value;
2006+
}
2007+
20012008
void setIsSwiftKeyPathExpr(bool onRoot) {
20022009
IsSwiftKeyPathExpr = true;
20032010
IsAfterSwiftKeyPathRoot = onRoot;
@@ -2895,8 +2902,13 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
28952902
IsImplicitlyCurriedInstanceMethod = isImplicitlyCurriedInstanceMethod(FD);
28962903

28972904
// Strip off '(_ self: Self)' if needed.
2898-
if (AFT && !IsImplicitlyCurriedInstanceMethod)
2905+
if (AFT && !IsImplicitlyCurriedInstanceMethod) {
28992906
AFT = AFT->getResult()->getAs<AnyFunctionType>();
2907+
2908+
// Check for duplicates with the adjusted type too.
2909+
if (isDuplicate(FD, AFT))
2910+
return;
2911+
}
29002912
}
29012913

29022914
bool trivialTrailingClosure = false;
@@ -3369,10 +3381,10 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
33693381
DynamicLookupInfo dynamicLookupInfo) {
33703382
auto funcTy =
33713383
getTypeOfMember(AFD, dynamicLookupInfo)->getAs<AnyFunctionType>();
3372-
if (funcTy && AFD->getDeclContext()->isTypeContext() &&
3373-
!isImplicitlyCurriedInstanceMethod(AFD)) {
3384+
bool dropCurryLevel = funcTy && AFD->getDeclContext()->isTypeContext() &&
3385+
!isImplicitlyCurriedInstanceMethod(AFD);
3386+
if (dropCurryLevel)
33743387
funcTy = funcTy->getResult()->getAs<AnyFunctionType>();
3375-
}
33763388

33773389
bool useFunctionReference = PreferFunctionReferencesToCalls;
33783390
if (!useFunctionReference && funcTy) {
@@ -3384,6 +3396,10 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
33843396
if (!useFunctionReference)
33853397
return false;
33863398

3399+
// Check for duplicates with the adjusted type too.
3400+
if (dropCurryLevel && isDuplicate(AFD, funcTy))
3401+
return true;
3402+
33873403
CommandWordsPairs Pairs;
33883404
CodeCompletionResultBuilder Builder(
33893405
Sink, CodeCompletionResult::ResultKind::Declaration,
@@ -3421,6 +3437,29 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
34213437
return true;
34223438
}
34233439

3440+
private:
3441+
3442+
/// Returns true if duplicate checking is enabled (via
3443+
/// \c shouldCheckForDuplicates) and this decl + type combination has been
3444+
/// checked previously. Returns false otherwise.
3445+
bool isDuplicate(const ValueDecl *D, Type Ty) {
3446+
if (!CheckForDuplicates)
3447+
return false;
3448+
return !PreviouslySeen.insert({D, Ty}).second;
3449+
}
3450+
3451+
/// Returns true if duplicate checking is enabled (via
3452+
/// \c shouldCheckForDuplicates) and this decl has been checked previously
3453+
/// with the type according to \c getTypeOfMember. Returns false otherwise.
3454+
bool isDuplicate(const ValueDecl *D, DynamicLookupInfo dynamicLookupInfo) {
3455+
if (!CheckForDuplicates)
3456+
return false;
3457+
Type Ty = getTypeOfMember(D, dynamicLookupInfo);
3458+
return !PreviouslySeen.insert({D, Ty}).second;
3459+
}
3460+
3461+
public:
3462+
34243463
// Implement swift::VisibleDeclConsumer.
34253464
void foundDecl(ValueDecl *D, DeclVisibilityKind Reason,
34263465
DynamicLookupInfo dynamicLookupInfo) override {
@@ -3436,6 +3475,11 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
34363475

34373476
if (IsSwiftKeyPathExpr && !SwiftKeyPathFilter(D, Reason))
34383477
return;
3478+
3479+
// If we've seen this decl+type before (possible when multiple lookups are
3480+
// performed e.g. because of ambiguous base types), bail.
3481+
if (isDuplicate(D, dynamicLookupInfo))
3482+
return;
34393483

34403484
// FIXME(InterfaceTypeRequest): Remove this.
34413485
(void)D->getInterfaceType();
@@ -3526,6 +3570,11 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
35263570
Type funcType = getTypeOfMember(FD, dynamicLookupInfo)
35273571
->castTo<AnyFunctionType>()
35283572
->getResult();
3573+
3574+
// Check for duplicates with the adjusted type too.
3575+
if (isDuplicate(FD, funcType))
3576+
return;
3577+
35293578
addFunctionCallPattern(
35303579
funcType->castTo<AnyFunctionType>(), FD,
35313580
getSemanticContext(FD, Reason, dynamicLookupInfo));
@@ -6039,6 +6088,7 @@ void deliverDotExprResults(
60396088
Lookup.setPreferFunctionReferencesToCalls();
60406089
}
60416090

6091+
Lookup.shouldCheckForDuplicates(Results.size() > 1);
60426092
for (auto &Result: Results) {
60436093
Lookup.setIsStaticMetatype(Result.BaseIsStaticMetaType);
60446094
Lookup.getPostfixKeywordCompletions(Result.BaseTy, BaseExpr);

‎test/IDE/complete_ambiguous.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=SIMPLE | %FileCheck %s --check-prefix=SIMPLE
22
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=SIMPLE_EXTRAARG | %FileCheck %s --check-prefix=SIMPLE
33
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=SIMPLE_MEMBERS | %FileCheck %s --check-prefix=SIMPLE
4+
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=SKIP_DUPLICATES > %t
5+
// RUN: %FileCheck %s --input-file %t --check-prefixes=SKIP_DUPLICATES,SKIP_DUPLICATES_PROPERTY
6+
// RUN: %FileCheck %s --input-file %t --check-prefixes=SKIP_DUPLICATES,SKIP_DUPLICATES_METHOD
7+
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=SKIP_COMPOUND_DUPLICATES | %FileCheck %s --check-prefix=SKIP_COMPOUND_DUPLICATES
8+
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=SKIP_CALLASFUNCTION_DUPLICATES | %FileCheck %s --check-prefix=SKIP_CALLASFUNCTION_DUPLICATES
49
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=RELATED | %FileCheck %s --check-prefix=RELATED
510
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=RELATED_EXTRAARG | %FileCheck %s --check-prefix=RELATED
611
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=RELATED_INERROREXPR | %FileCheck %s --check-prefix=RELATED
@@ -50,6 +55,37 @@ HasMembers().overloadedReturn().#^SIMPLE_MEMBERS^#
5055
func givenErrorExpr(_ a: String) -> A {}
5156
func givenErrorExpr(_ b: Int) -> B {}
5257

58+
func arrayWrapper<T>(a: T) -> [T]
59+
arrayWrapper(overloadedReturn()).#^SKIP_DUPLICATES^#
60+
61+
// SKIP_DUPLICATES: Begin completions
62+
// SKIP_DUPLICATES_PROPERTY: Decl[InstanceVar]/CurrNominal/IsSystem: count[#Int#]{{; name=.+$}}
63+
// SKIP_DUPLICATES_PROPERTY-NOT: count[#Int#]
64+
// SKIP_DUPLICATES_METHOD: Decl[InstanceMethod]/Super/IsSystem: formIndex({#(i): &Int#}, {#offsetBy: Int#})[#Void#]{{; name=.+$}}
65+
// SKIP_DUPLICATES_METHOD-NOT: formIndex({#(i): &Int#}, {#offsetBy: Int#})[#Void#]
66+
// SKIP_DUPLICATES: End completions
67+
68+
let x: (inout Int, Int) -> () = arrayWrapper(overloadedReturn()).#^SKIP_COMPOUND_DUPLICATES^#
69+
70+
// SKIP_COMPOUND_DUPLICATES: Begin completions
71+
// SKIP_COMPOUND_DUPLICATES: Decl[InstanceMethod]/Super/IsSystem/TypeRelation[Identical]: formIndex(_:offsetBy:)[#(inout Int, Int) -> ()#]{{; name=.+$}}
72+
// SKIP_COMPOUND_DUPLICATES-NOT: formIndex(_:offsetBy:)[#(inout Int, Int) -> ()#]
73+
// SKIP_COMPOUND_DUPLICATES: End completions
74+
75+
func testCallAsFunctionDeduplication() {
76+
struct Test<T> {
77+
func callAsFunction(x: Int) {}
78+
}
79+
80+
func overloaded() -> Test<A> { fatalError() }
81+
func overloaded() -> Test<B> { fatalError() }
82+
83+
overloaded()#^SKIP_CALLASFUNCTION_DUPLICATES^#
84+
}
85+
86+
// FIXME: Update this to check the callAsFunction pattern only appears once when PostfixExpr completion is migrated to the solver-based implementation (which handles ambiguity).
87+
// SKIP_CALLASFUNCTION_DUPLICATES-NOT: Begin completions
88+
5389
givenErrorExpr(undefined).#^ERROR_IN_BASE^#
5490

5591
// SIMPLE: Begin completions, 4 items

0 commit comments

Comments
 (0)