Skip to content

Commit a927dc8

Browse files
committed
[CodeCompletion] Hide literals that don't match the type context in SourceKit
If there is a type context, hide literal suggesetions that don't match it, unless they are keywords and we have filtered to their names. Incidentally fix an output buffering issue when combining filtering with the -raw flag in complete-test. Part of rdar://problem/23865118
1 parent 0327fc3 commit a927dc8

File tree

9 files changed

+133
-32
lines changed

9 files changed

+133
-32
lines changed

include/swift/IDE/CodeCompletion.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -693,6 +693,7 @@ class CodeCompletionContext {
693693
public:
694694
CodeCompletionCache &Cache;
695695
CompletionKind CodeCompletionKind = CompletionKind::None;
696+
bool HasExpectedTypeRelation = false;
696697

697698
CodeCompletionContext(CodeCompletionCache &Cache)
698699
: Cache(Cache) {}

lib/IDE/CodeCompletion.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1546,9 +1546,14 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
15461546
}
15471547

15481548
void setExpectedTypes(ArrayRef<Type> Types) {
1549-
ExpectedTypes = Types;
1549+
ExpectedTypes.reserve(Types.size());
1550+
for (auto T : Types)
1551+
if (T)
1552+
ExpectedTypes.push_back(T);
15501553
}
15511554

1555+
bool hasExpectedTypes() const { return !ExpectedTypes.empty(); }
1556+
15521557
bool needDot() const {
15531558
return NeedLeadingDot;
15541559
}
@@ -4777,6 +4782,8 @@ void CodeCompletionCallbacksImpl::doneParsing() {
47774782
Lookup.RequestedCachedResults.reset();
47784783
}
47794784

4785+
CompletionContext.HasExpectedTypeRelation = Lookup.hasExpectedTypes();
4786+
47804787
deliverCompletionResults();
47814788
}
47824789

test/IDE/complete_value_literals.swift

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// RUN: %target-swift-ide-test -code-completion -source-filename=%s -code-completion-token=NO_CONTEXT_0 | FileCheck %s -check-prefix=NO_CONTEXT_0
12
// RUN: %target-swift-ide-test -code-completion -source-filename=%s -code-completion-token=NIL_0 | FileCheck %s -check-prefix=NIL_0
23
// RUN: %target-swift-ide-test -code-completion -source-filename=%s -code-completion-token=NIL_1 | FileCheck %s -check-prefix=NIL_1
34
// RUN: %target-swift-ide-test -code-completion -source-filename=%s -code-completion-token=NIL_2 | FileCheck %s -check-prefix=NIL_2
@@ -25,6 +26,22 @@
2526
// RUN: %target-swift-ide-test -code-completion -source-filename=%s -code-completion-token=IMAGE_0 | FileCheck %s -check-prefix=IMAGE_0
2627
// RUN: %target-swift-ide-test -code-completion -source-filename=%s -code-completion-token=IMAGE_1 | FileCheck %s -check-prefix=IMAGE_1
2728

29+
func testAll0() {
30+
// Not type context.
31+
let x = #^NO_CONTEXT_0^#
32+
// NO_CONTEXT_0-DAG: Begin completions
33+
// NO_CONTEXT_0-DAG: Literal[Integer]/None: 0[#Int#];
34+
// NO_CONTEXT_0-DAG: Literal[Boolean]/None: true[#Bool#];
35+
// NO_CONTEXT_0-DAG: Literal[Boolean]/None: false[#Bool#];
36+
// NO_CONTEXT_0-DAG: Literal[Nil]/None: nil;
37+
// NO_CONTEXT_0-DAG: Literal[String]/None: "{#(abc)#}"[#String#];
38+
// NO_CONTEXT_0-DAG: Literal[Array]/None: [{#(values)#}][#Array#];
39+
// NO_CONTEXT_0-DAG: Literal[Dictionary]/None: [{#(key)#}: {#(value)#}][#Dictionary#];
40+
// NO_CONTEXT_0-DAG: Literal[_Color]/None: [#Color({#colorLiteralRed: Float#}, {#green: Float#}, {#blue: Float#}, {#alpha: Float#})#];
41+
// NO_CONTEXT_0-DAG: Literal[_Image]/None: [#Image({#imageLiteral: String#})#];
42+
// NO_CONTEXT_0: End completions
43+
}
44+
2845
struct MyNil1: NilLiteralConvertible {
2946
init(nilLiteral: ()) {}
3047
}

test/SourceKit/CodeComplete/complete_literals.swift

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,46 @@ func foo(x: Int) {
3333
// RUN: %complete-test -tok=EXPR1 %s -raw | FileCheck %s -check-prefix=LITERALS
3434
// RUN: %complete-test -tok=EXPR2 %s -raw | FileCheck %s -check-prefix=LITERALS
3535
// RUN: %complete-test -tok=EXPR3 %s -raw | FileCheck %s -check-prefix=LITERALS
36-
let x = #^EXPR1^#
37-
x + #^EXPR2^#
36+
let x1 = #^EXPR1^#
37+
x1 + #^EXPR2^#
3838
if #^EXPR3^# { }
39-
foo(#^EXPR3^#)
39+
40+
// RUN: %complete-test -tok=EXPR4 %s -raw | FileCheck %s -check-prefix=LITERAL_INT
41+
foo(#^EXPR4^#)
42+
// LITERAL_INT-NOT: source.lang.swift.literal
43+
// LITERAL_INT: key.kind: source.lang.swift.literal.integer
44+
// LITERAL_INT-NOT: source.lang.swift.literal
45+
46+
// RUN: %complete-test -tok=EXPR5 %s -raw | FileCheck %s -check-prefix=LITERAL_TUPLE
47+
let x2: (String, Int) = #^EXPR5^#
48+
// LITERAL_TUPLE-NOT: source.lang.swift.literal
49+
// LITERAL_TUPLE: key.kind: source.lang.swift.literal.tuple
50+
// LITERAL_TUPLE-NOT: source.lang.swift.literal
51+
52+
// RUN: %complete-test -tok=EXPR6 %s -raw | FileCheck %s -check-prefix=LITERAL_NO_TYPE
53+
// When there is a type context that doesn't match, we should see no literals
54+
// except the keywords and they should be prioritized like keywords not
55+
// literals.
56+
struct Opaque {}
57+
let x3: Opaque = #^EXPR6,,colo,ni,tru,fals^#
58+
// LITERAL_NO_TYPE-LABEL: Results for filterText: [
59+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
60+
// LITERAL_NO_TYPE: ]
61+
// LITERAL_NO_TYPE-LABEL: Results for filterText: colo [
62+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
63+
// LITERAL_NO_TYPE: ]
64+
// LITERAL_NO_TYPE-LABEL: Results for filterText: ni [
65+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
66+
// LITERAL_NO_TYPE: key.kind: source.lang.swift.literal.nil
67+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
68+
// LITERAL_NO_TYPE: ]
69+
// LITERAL_NO_TYPE-LABEL: Results for filterText: tru [
70+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
71+
// LITERAL_NO_TYPE: key.kind: source.lang.swift.literal.boolean
72+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
73+
// LITERAL_NO_TYPE: ]
74+
// LITERAL_NO_TYPE-LABEL: Results for filterText: fals [
75+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
76+
// LITERAL_NO_TYPE: key.kind: source.lang.swift.literal.boolean
77+
// LITERAL_NO_TYPE-NOT: source.lang.swift.literal
78+
// LITERAL_NO_TYPE: ]

tools/SourceKit/lib/SwiftLang/CodeCompletionOrganizer.cpp

Lines changed: 45 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,13 @@ bool SourceKit::CodeCompletion::addCustomCompletions(
188188
class CodeCompletionOrganizer::Impl {
189189
std::unique_ptr<Group> rootGroup;
190190
CompletionKind completionKind;
191+
bool completionHasExpectedTypes;
191192

192193
void groupStemsRecursive(Group *group, bool recurseIntoNewGroups,
193194
StringRef(getStem)(StringRef));
194195

195196
public:
196-
Impl(CompletionKind kind);
197+
Impl(CompletionKind kind, bool hasExpectedTypes);
197198

198199
void addCompletionsWithFilter(ArrayRef<Completion *> completions,
199200
StringRef filterText, Options options,
@@ -249,8 +250,9 @@ class CodeCompletionOrganizer::Impl {
249250
//===----------------------------------------------------------------------===//
250251

251252
CodeCompletionOrganizer::CodeCompletionOrganizer(const Options &options,
252-
CompletionKind kind)
253-
: impl(*new Impl(kind)), options(options) {}
253+
CompletionKind kind,
254+
bool hasExpectedTypes)
255+
: impl(*new Impl(kind, hasExpectedTypes)), options(options) {}
254256
CodeCompletionOrganizer::~CodeCompletionOrganizer() { delete &impl; }
255257

256258
void CodeCompletionOrganizer::preSortCompletions(
@@ -383,8 +385,8 @@ static std::unique_ptr<Result> make_result(Completion *result) {
383385
// CodeCompletionOrganizer::Impl implementation
384386
//===----------------------------------------------------------------------===//
385387

386-
CodeCompletionOrganizer::Impl::Impl(CompletionKind kind)
387-
: completionKind(kind) {
388+
CodeCompletionOrganizer::Impl::Impl(CompletionKind kind, bool hasExpectedTypes)
389+
: completionKind(kind), completionHasExpectedTypes(hasExpectedTypes) {
388390
assert(!rootGroup && "initialized twice");
389391
rootGroup = make_group("");
390392
}
@@ -516,7 +518,8 @@ void CodeCompletionOrganizer::Impl::addCompletionsWithFilter(
516518
}
517519
if (completion->getExpectedTypeRelation() >= Completion::Convertible ||
518520
(completion->getKind() == Completion::Literal &&
519-
completionKind != CompletionKind::StmtOrExpr))
521+
completionKind != CompletionKind::StmtOrExpr &&
522+
!completionHasExpectedTypes))
520523
break;
521524

522525
if (completion->getKind() == Completion::Keyword &&
@@ -543,6 +546,16 @@ void CodeCompletionOrganizer::Impl::addCompletionsWithFilter(
543546
if (rules.hideCompletion(completion))
544547
continue;
545548

549+
// Hide literals other than the ones that are also keywords if they don't
550+
// match the expected types.
551+
if (completion->getKind() == Completion::Literal &&
552+
completionHasExpectedTypes &&
553+
completion->getExpectedTypeRelation() < Completion::Convertible &&
554+
completion->getLiteralKind() !=
555+
CodeCompletionLiteralKind::BooleanLiteral &&
556+
completion->getLiteralKind() != CodeCompletionLiteralKind::NilLiteral)
557+
continue;
558+
546559
bool match = false;
547560
if (options.fuzzyMatching && filterText.size() >= options.minFuzzyLength) {
548561
match = pattern.matchesCandidate(completion->getName());
@@ -641,7 +654,7 @@ enum class ResultBucket {
641654
};
642655
} // end anonymous namespace
643656

644-
static ResultBucket getResultBucket(Item &item) {
657+
static ResultBucket getResultBucket(Item &item, bool hasExpectedTypes) {
645658
if (isa<Group>(item))
646659
return ResultBucket::Normal; // FIXME: take best contained result.
647660
auto *completion = cast<Result>(item).value;
@@ -655,7 +668,15 @@ static ResultBucket getResultBucket(Item &item) {
655668

656669
switch (completion->getKind()) {
657670
case Completion::Literal:
658-
return matchesType ? ResultBucket::LiteralTypeMatch : ResultBucket::Literal;
671+
if (matchesType) {
672+
return ResultBucket::LiteralTypeMatch;
673+
} else if (!hasExpectedTypes) {
674+
return ResultBucket::Literal;
675+
} else {
676+
// When we have type context, we still show literals that are keywords,
677+
// but we treat them as keywords instead of literals for prioritization.
678+
return ResultBucket::Normal;
679+
}
659680
case Completion::Keyword:
660681
return isHighPriorityKeyword(completion->getKeywordKind())
661682
? ResultBucket::HighPriorityKeyword
@@ -721,13 +742,14 @@ static int compareLiterals(Item &a_, Item &b_) {
721742
return 0;
722743
}
723744

724-
static void sortRecursive(const Options &options, Group *group) {
745+
static void sortRecursive(const Options &options, Group *group,
746+
bool hasExpectedTypes) {
725747
// Sort all of the subgroups first, and fill in the bucket for each result.
726748
auto &contents = group->contents;
727749
double best = -1.0;
728750
for (auto &item : contents) {
729751
if (Group *g = dyn_cast<Group>(item.get())) {
730-
sortRecursive(options, g);
752+
sortRecursive(options, g, hasExpectedTypes);
731753
} else {
732754
Result *r = cast<Result>(item.get());
733755
item->finalScore = combinedScore(options, item->matchScore, r->value);
@@ -749,40 +771,40 @@ static void sortRecursive(const Options &options, Group *group) {
749771
return;
750772
}
751773

752-
llvm::array_pod_sort(contents.begin(), contents.end(), [](const std::unique_ptr<Item> *a_, const std::unique_ptr<Item> *b_) {
753-
Item &a = **a_;
754-
Item &b = **b_;
774+
std::sort(contents.begin(), contents.end(), [=](const std::unique_ptr<Item> &a_, const std::unique_ptr<Item> &b_) {
775+
Item &a = *a_;
776+
Item &b = *b_;
755777

756-
auto bucketA = getResultBucket(a);
757-
auto bucketB = getResultBucket(b);
778+
auto bucketA = getResultBucket(a, hasExpectedTypes);
779+
auto bucketB = getResultBucket(b, hasExpectedTypes);
758780
if (bucketA < bucketB)
759-
return 1;
781+
return false;
760782
else if (bucketB < bucketA)
761-
return -1;
783+
return true;
762784

763785
// Special internal orderings.
764786
switch (bucketA) {
765787
case ResultBucket::HighPriorityKeyword:
766-
return compareHighPriorityKeywords(a, b);
788+
return compareHighPriorityKeywords(a, b) < 0;
767789
case ResultBucket::Literal:
768790
case ResultBucket::LiteralTypeMatch:
769-
return compareLiterals(a, b);
791+
return compareLiterals(a, b) < 0;
770792
default:
771793
break;
772794
}
773795

774796
// "Normal" order.
775797
if (a.finalScore < b.finalScore)
776-
return 1;
798+
return false;
777799
else if (b.finalScore < a.finalScore)
778-
return -1;
800+
return true;
779801

780-
return compareResultName(a, b);
802+
return compareResultName(a, b) < 0;
781803
});
782804
}
783805

784806
void CodeCompletionOrganizer::Impl::sort(Options options) {
785-
sortRecursive(options, rootGroup.get());
807+
sortRecursive(options, rootGroup.get(), completionHasExpectedTypes);
786808
}
787809

788810
void CodeCompletionOrganizer::Impl::groupStemsRecursive(

tools/SourceKit/lib/SwiftLang/CodeCompletionOrganizer.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ class CodeCompletionOrganizer {
7575
Impl &impl;
7676
const Options &options;
7777
public:
78-
CodeCompletionOrganizer(const Options &options, CompletionKind kind);
78+
CodeCompletionOrganizer(const Options &options, CompletionKind kind,
79+
bool hasExpectedTypes);
7980
~CodeCompletionOrganizer();
8081

8182
static void

tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,10 @@ CompletionKind CodeCompletion::SessionCache::getCompletionKind() {
697697
llvm::sys::ScopedLock L(mtx);
698698
return completionKind;
699699
}
700+
bool CodeCompletion::SessionCache::getCompletionHasExpectedTypes() {
701+
llvm::sys::ScopedLock L(mtx);
702+
return completionHasExpectedTypes;
703+
}
700704
const CodeCompletion::FilterRules &
701705
CodeCompletion::SessionCache::getFilterRules() {
702706
llvm::sys::ScopedLock L(mtx);
@@ -928,7 +932,8 @@ static void transformAndForwardResults(
928932
};
929933

930934
CodeCompletion::CodeCompletionOrganizer organizer(
931-
options, session->getCompletionKind());
935+
options, session->getCompletionKind(),
936+
session->getCompletionHasExpectedTypes());
932937

933938
bool hasEarlyInnerResults =
934939
session->getCompletionKind() == CompletionKind::PostfixExpr;
@@ -1060,10 +1065,12 @@ void SwiftLangSupport::codeCompleteOpen(
10601065
}
10611066

10621067
CompletionKind completionKind = CompletionKind::None;
1068+
bool hasExpectedTypes = false;
10631069

10641070
SwiftCodeCompletionConsumer swiftConsumer(
10651071
[&](ArrayRef<CodeCompletionResult *> results, SwiftCompletionInfo &info) {
10661072
completionKind = info.completionContext->CodeCompletionKind;
1073+
hasExpectedTypes = info.completionContext->HasExpectedTypeRelation;
10671074
completions =
10681075
extendCompletions(results, sink, info, nameToPopularity, CCOpts);
10691076
});
@@ -1097,7 +1104,7 @@ void SwiftLangSupport::codeCompleteOpen(
10971104
std::vector<std::string> argsCopy(extendedArgs.begin(), extendedArgs.end());
10981105
SessionCacheRef session{new SessionCache(
10991106
std::move(sink), std::move(bufferCopy), std::move(argsCopy),
1100-
completionKind, std::move(filterRules))};
1107+
completionKind, hasExpectedTypes, std::move(filterRules))};
11011108
session->setSortedCompletions(std::move(completions));
11021109

11031110
if (!CCSessions.set(name, offset, session)) {

tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,22 +137,26 @@ class SessionCache : public ThreadSafeRefCountedBase<SessionCache> {
137137
CompletionSink sink;
138138
std::vector<Completion *> sortedCompletions;
139139
CompletionKind completionKind;
140+
bool completionHasExpectedTypes;
140141
FilterRules filterRules;
141142
llvm::sys::Mutex mtx;
142143

143144
public:
144145
SessionCache(CompletionSink &&sink,
145146
std::unique_ptr<llvm::MemoryBuffer> &&buffer,
146147
std::vector<std::string> &&args, CompletionKind completionKind,
147-
FilterRules filterRules)
148+
bool hasExpectedTypes, FilterRules filterRules)
148149
: buffer(std::move(buffer)), args(std::move(args)), sink(std::move(sink)),
149-
completionKind(completionKind), filterRules(std::move(filterRules)) {}
150+
completionKind(completionKind),
151+
completionHasExpectedTypes(hasExpectedTypes),
152+
filterRules(std::move(filterRules)) {}
150153
void setSortedCompletions(std::vector<Completion *> &&completions);
151154
ArrayRef<Completion *> getSortedCompletions();
152155
llvm::MemoryBuffer *getBuffer();
153156
ArrayRef<std::string> getCompilerArgs();
154157
const FilterRules &getFilterRules();
155158
CompletionKind getCompletionKind();
159+
bool getCompletionHasExpectedTypes();
156160
};
157161
typedef RefPtr<SessionCache> SessionCacheRef;
158162

tools/SourceKit/tools/complete-test/complete-test.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -537,6 +537,7 @@ static void printResponse(sourcekitd_response_t resp, bool raw, bool structure,
537537
}
538538
ResponsePrinter p(llvm::outs(), 4, indentation, structure);
539539
p.printResponse(resp);
540+
llvm::outs().flush();
540541
}
541542

542543
static std::unique_ptr<llvm::MemoryBuffer>
@@ -770,9 +771,11 @@ static int handleTestInvocation(TestOptions &options) {
770771
return true;
771772
}
772773
llvm::outs() << "Results for filterText: " << prefix << " [\n";
774+
llvm::outs().flush();
773775
printResponse(response, options.rawOutput, options.structureOutput,
774776
/*indentation*/ 4);
775777
llvm::outs() << "]\n";
778+
llvm::outs().flush();
776779
return false;
777780
});
778781
if (isError)

0 commit comments

Comments
 (0)