Skip to content

Commit 513fed7

Browse files
committed
[CodeCompletion] Enable ASTScope in code completion
* Re-create `ASTScope` for each completion * Add generic params and where clause scope even without missing body * Use `getOriginalBodySourceRange()` for `AbstractFunctionBodyScope` * Source range translations for replaced ranges when finding scopes * Bypass source range checks when the completion happens in the replaced range * Be lenient with ASTScope / DeclContext mismatch in code completion
1 parent 8a39c8d commit 513fed7

File tree

7 files changed

+87
-24
lines changed

7 files changed

+87
-24
lines changed

include/swift/AST/SourceFile.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ class SourceFile final : public FileUnit {
172172
bool IsPrimary;
173173

174174
/// The scope map that describes this source file.
175-
std::unique_ptr<ASTScope> Scope;
175+
NullablePtr<ASTScope> Scope = nullptr;
176176

177177
/// The set of validated opaque return type decls in the source file.
178178
llvm::SmallVector<OpaqueTypeDecl *, 4> OpaqueReturnTypes;
@@ -468,6 +468,10 @@ class SourceFile final : public FileUnit {
468468
/// Retrieve the scope that describes this source file.
469469
ASTScope &getScope();
470470

471+
void clearScope() {
472+
Scope = nullptr;
473+
}
474+
471475
/// Retrieves the previously set delayed parser state, asserting that it
472476
/// exists.
473477
PersistentParserState *getDelayedParserState() {

lib/AST/ASTScopeCreation.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,7 @@ void ASTSourceFileScope::
795795
void ASTSourceFileScope::expandFunctionBody(AbstractFunctionDecl *AFD) {
796796
if (!AFD)
797797
return;
798-
auto sr = AFD->getBodySourceRange();
798+
auto sr = AFD->getOriginalBodySourceRange();
799799
if (sr.isInvalid())
800800
return;
801801
ASTScopeImpl *bodyScope = findInnermostEnclosingScope(sr.Start, nullptr);
@@ -1596,11 +1596,6 @@ ASTScopeImpl *GenericTypeOrExtensionWholePortion::expandScope(
15961596
// Get now in case recursion emancipates scope
15971597
auto *const ip = scope->getParent().get();
15981598

1599-
// Prevent circular request bugs caused by illegal input and
1600-
// doing lookups that getExtendedNominal in the midst of getExtendedNominal.
1601-
if (scope->shouldHaveABody() && !scope->doesDeclHaveABody())
1602-
return ip;
1603-
16041599
auto *context = scope->getGenericContext();
16051600
auto *genericParams = (isa<TypeAliasDecl>(context)
16061601
? context->getParsedGenericParams()
@@ -1609,6 +1604,12 @@ ASTScopeImpl *GenericTypeOrExtensionWholePortion::expandScope(
16091604
scope->getDecl(), genericParams, scope);
16101605
if (context->getTrailingWhereClause())
16111606
scope->createTrailingWhereClauseScope(deepestScope, scopeCreator);
1607+
1608+
// Prevent circular request bugs caused by illegal input and
1609+
// doing lookups that getExtendedNominal in the midst of getExtendedNominal.
1610+
if (scope->shouldHaveABody() && !scope->doesDeclHaveABody())
1611+
return ip;
1612+
16121613
scope->createBodyScope(deepestScope, scopeCreator);
16131614
return ip;
16141615
}

lib/AST/ASTScopeLookup.cpp

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ const ASTScopeImpl *ASTScopeImpl::findStartingScopeForLookup(
8282
// Someday, just use the assertion below. For now, print out lots of info for
8383
// debugging.
8484
if (!startingScope) {
85+
86+
// Be lenient in code completion mode. There are cases where the decl
87+
// context doesn't match with the ASTScope. e.g. dangling attributes.
88+
// FIXME: Create ASTScope tree even for invalid code.
89+
if (innermost &&
90+
startingContext->getASTContext().SourceMgr.hasCodeCompletionBuffer()) {
91+
return innermost;
92+
}
93+
8594
llvm::errs() << "ASTScopeImpl: resorting to startingScope hack, file: "
8695
<< sourceFile->getFilename() << "\n";
8796
// The check is costly, and inactive lookups will end up here, so don't
@@ -143,6 +152,20 @@ bool ASTScopeImpl::checkSourceRangeOfThisASTNode() const {
143152
return true;
144153
}
145154

155+
/// If the \p loc is in a new buffer but \p range is not, consider the location
156+
/// is at the start of replaced range. Otherwise, returns \p loc as is.
157+
static SourceLoc translateLocForReplacedRange(SourceManager &sourceMgr,
158+
SourceRange range,
159+
SourceLoc loc) {
160+
if (const auto &replacedRange = sourceMgr.getReplacedRange()) {
161+
if (sourceMgr.rangeContainsTokenLoc(replacedRange.New, loc) &&
162+
!sourceMgr.rangeContains(replacedRange.New, range)) {
163+
return replacedRange.Original.Start;
164+
}
165+
}
166+
return loc;
167+
}
168+
146169
NullablePtr<ASTScopeImpl>
147170
ASTScopeImpl::findChildContaining(SourceLoc loc,
148171
SourceManager &sourceMgr) const {
@@ -152,24 +175,24 @@ ASTScopeImpl::findChildContaining(SourceLoc loc,
152175

153176
bool operator()(const ASTScopeImpl *scope, SourceLoc loc) {
154177
ASTScopeAssert(scope->checkSourceRangeOfThisASTNode(), "Bad range.");
155-
return -1 == ASTScopeImpl::compare(scope->getSourceRangeOfScope(), loc,
156-
sourceMgr,
178+
auto rangeOfScope = scope->getSourceRangeOfScope();
179+
loc = translateLocForReplacedRange(sourceMgr, rangeOfScope, loc);
180+
return -1 == ASTScopeImpl::compare(rangeOfScope, loc, sourceMgr,
157181
/*ensureDisjoint=*/false);
158182
}
159183
bool operator()(SourceLoc loc, const ASTScopeImpl *scope) {
160-
ASTScopeAssert(scope->checkSourceRangeOfThisASTNode(), "Bad range.");
161-
// Alternatively, we could check that loc < start-of-scope
162-
return 0 >= ASTScopeImpl::compare(loc, scope->getSourceRangeOfScope(),
163-
sourceMgr,
164-
/*ensureDisjoint=*/false);
184+
return !(*this)(scope, loc);
165185
}
166186
};
167187
auto *const *child = std::lower_bound(
168188
getChildren().begin(), getChildren().end(), loc, CompareLocs{sourceMgr});
169189

170-
if (child != getChildren().end() &&
171-
sourceMgr.rangeContainsTokenLoc((*child)->getSourceRangeOfScope(), loc))
172-
return *child;
190+
if (child != getChildren().end()) {
191+
auto rangeOfScope = (*child)->getSourceRangeOfScope();
192+
loc = translateLocForReplacedRange(sourceMgr, rangeOfScope, loc);
193+
if (sourceMgr.rangeContainsTokenLoc(rangeOfScope, loc))
194+
return *child;
195+
}
173196

174197
return nullptr;
175198
}

lib/AST/ASTScopeSourceRange.cpp

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,14 @@ ASTScopeImpl::widenSourceRangeForChildren(const SourceRange range,
6767
if (range.isInvalid())
6868
return childRange;
6969
auto r = range;
70+
71+
// HACK: For code completion. If the range of the child is from another
72+
// source buffer, don't widen using that range.
73+
if (const auto &replacedRange = getSourceManager().getReplacedRange()) {
74+
if (getSourceManager().rangeContains(replacedRange.Original, range) &&
75+
getSourceManager().rangeContains(replacedRange.New, childRange))
76+
return r;
77+
}
7078
r.widen(childRange);
7179
return r;
7280
}
@@ -114,6 +122,14 @@ bool ASTScopeImpl::verifyThatChildrenAreContainedWithin(
114122
getChildren().back()->getSourceRangeOfScope().End);
115123
if (getSourceManager().rangeContains(range, rangeOfChildren))
116124
return true;
125+
126+
// HACK: For code completion. Handle replaced range.
127+
if (const auto &replacedRange = getSourceManager().getReplacedRange()) {
128+
if (getSourceManager().rangeContains(replacedRange.Original, range) &&
129+
getSourceManager().rangeContains(replacedRange.New, rangeOfChildren))
130+
return true;
131+
}
132+
117133
auto &out = verificationError() << "children not contained in its parent\n";
118134
if (getChildren().size() == 1) {
119135
out << "\n***Only Child node***\n";
@@ -200,7 +216,7 @@ SourceRange DifferentiableAttributeScope::getSourceRangeOfThisASTNode(
200216

201217
SourceRange AbstractFunctionBodyScope::getSourceRangeOfThisASTNode(
202218
const bool omitAssertions) const {
203-
return decl->getBodySourceRange();
219+
return decl->getOriginalBodySourceRange();
204220
}
205221

206222
SourceRange TopLevelCodeScope::getSourceRangeOfThisASTNode(
@@ -357,7 +373,7 @@ SourceRange AbstractFunctionDeclScope::getSourceRangeOfThisASTNode(
357373
ASTScopeAssert(r.End.isValid(), "Start valid imples end valid.");
358374
return r;
359375
}
360-
return decl->getBodySourceRange();
376+
return decl->getOriginalBodySourceRange();
361377
}
362378

363379
SourceRange ParameterListScope::getSourceRangeOfThisASTNode(
@@ -609,7 +625,7 @@ SourceRange IterableTypeScope::sourceRangeForDeferredExpansion() const {
609625
return portion->sourceRangeForDeferredExpansion(this);
610626
}
611627
SourceRange AbstractFunctionBodyScope::sourceRangeForDeferredExpansion() const {
612-
const auto bsr = decl->getBodySourceRange();
628+
const auto bsr = decl->getOriginalBodySourceRange();
613629
const SourceLoc endEvenIfNoCloseBraceAndEndsWithInterpolatedStringLiteral =
614630
getLocEncompassingPotentialLookups(getSourceManager(), bsr.End);
615631
return SourceRange(bsr.Start,

lib/AST/Module.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2387,7 +2387,7 @@ StringRef SourceFile::getFilename() const {
23872387

23882388
ASTScope &SourceFile::getScope() {
23892389
if (!Scope)
2390-
Scope = std::unique_ptr<ASTScope>(new (getASTContext()) ASTScope(this));
2390+
Scope = new (getASTContext()) ASTScope(this);
23912391
return *Scope.get();
23922392
}
23932393

lib/IDE/CompletionInstance.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ bool CompletionInstance::performCachedOperationIfPossible(
418418
auto *AFD = cast<AbstractFunctionDecl>(DC);
419419
AFD->setBodyToBeReparsed(newBodyRange);
420420
SM.setReplacedRange({AFD->getOriginalBodySourceRange(), newBodyRange});
421+
oldSF->clearScope();
421422

422423
traceDC = AFD;
423424
break;
@@ -603,9 +604,6 @@ bool swift::ide::CompletionInstance::performOperation(
603604
// We don't need token list.
604605
Invocation.getLangOptions().CollectParsedToken = false;
605606

606-
// FIXME: ASTScopeLookup doesn't support code completion yet.
607-
Invocation.disableASTScopeLookup();
608-
609607
if (EnableASTCaching) {
610608
// Compute the signature of the invocation.
611609
llvm::hash_code ArgsHash(0);

test/IDE/complete_attributes.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TOP_LEVEL_ATTR_1 -code-completion-keywords=false | %FileCheck %s -check-prefix=ERROR_COMMON
22
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=MEMBER_DECL_ATTR_1 -code-completion-keywords=false | %FileCheck %s -check-prefix=ERROR_COMMON
3+
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ATTRARG_MEMBER | %FileCheck %s -check-prefix=MEMBER_MyValue
4+
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ATTRARG_MEMBER_IN_CLOSURE | %FileCheck %s -check-prefix=MEMBER_MyValue
35

46
// ERROR_COMMON: found code completion token
57
// ERROR_COMMON-NOT: Keyword/
@@ -9,3 +11,22 @@
911
class MemberDeclAttribute {
1012
@#^MEMBER_DECL_ATTR_1^# func memberDeclAttr1() {}
1113
}
14+
15+
struct MyValue {
16+
init() {}
17+
static var val: Int
18+
}
19+
20+
// MEMBER_MyValue: Begin completions, 4 items
21+
// MEMBER_MyValue-DAG: Keyword[self]/CurrNominal: self[#MyValue.Type#];
22+
// MEMBER_MyValue-DAG: Keyword/CurrNominal: Type[#MyValue.Type#];
23+
// MEMBER_MyValue-DAG: Decl[Constructor]/CurrNominal: init()[#MyValue#];
24+
// MEMBER_MyValue-DAG: Decl[StaticVar]/CurrNominal: val[#Int#];
25+
// MEMBER_MyValue: End completions
26+
27+
class TestUknownDanglingAttr1 {
28+
@UknownAttr(arg: MyValue.#^ATTRARG_MEMBER^#)
29+
}
30+
class TestUknownDanglingAttr2 {
31+
@UknownAttr(arg: { MyValue.#^ATTRARG_MEMBER_IN_CLOSURE^# })
32+
}

0 commit comments

Comments
 (0)