Skip to content

Commit 7bd8ee6

Browse files
bnbarhambnbarham
committed
[Refactorings] Add cursor refactorings for the start of the range
When a range is a single expression/statement/decl or part of expression, also return cursor based refactorings for the start of the range. This is a stop gap until the available refactorings are properly fixed to be more lenient in general - the current fix is a little odd as eg. if all of `foo.bar()` is selected, rename will be returned as an available refactoring for `foo`. Still an improvement over completely missing cursor based refactorings, however. Resolves rdar://82060063
1 parent d3cf72b commit 7bd8ee6

File tree

5 files changed

+51
-68
lines changed

5 files changed

+51
-68
lines changed

‎lib/IDE/Refactoring.cpp

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -3788,43 +3788,18 @@ bool RefactoringActionTrailingClosure::performChange() {
37883788
return false;
37893789
}
37903790

3791-
static bool rangeStartMayNeedRename(const ResolvedRangeInfo &Info) {
3792-
switch(Info.Kind) {
3793-
case RangeKind::SingleExpression: {
3794-
Expr *E = Info.ContainedNodes[0].get<Expr*>();
3795-
// We should show rename for the selection of "foo()"
3796-
if (auto *CE = dyn_cast<CallExpr>(E)) {
3797-
if (CE->getFn()->getKind() == ExprKind::DeclRef)
3798-
return true;
3799-
3800-
// When callling an instance method inside another instance method,
3801-
// we have a dot syntax call whose dot and base are both implicit. We
3802-
// need to explicitly allow the specific case here.
3803-
if (auto *DSC = dyn_cast<DotSyntaxCallExpr>(CE->getFn())) {
3804-
if (DSC->getBase()->isImplicit() &&
3805-
DSC->getFn()->getStartLoc() == Info.TokensInRange.front().getLoc())
3806-
return true;
3807-
}
3808-
}
3809-
return false;
3810-
}
3811-
case RangeKind::PartOfExpression: {
3812-
if (auto *CE = dyn_cast<CallExpr>(Info.CommonExprParent)) {
3813-
if (auto *DSC = dyn_cast<DotSyntaxCallExpr>(CE->getFn())) {
3814-
if (DSC->getFn()->getStartLoc() == Info.TokensInRange.front().getLoc())
3815-
return true;
3816-
}
3817-
}
3818-
return false;
3819-
}
3820-
case RangeKind::SingleDecl:
3821-
case RangeKind::MultiTypeMemberDecl:
3822-
case RangeKind::SingleStatement:
3823-
case RangeKind::MultiStatement:
3824-
case RangeKind::Invalid:
3825-
return false;
3791+
static bool collectRangeStartRefactorings(const ResolvedRangeInfo &Info) {
3792+
switch (Info.Kind) {
3793+
case RangeKind::SingleExpression:
3794+
case RangeKind::SingleStatement:
3795+
case RangeKind::SingleDecl:
3796+
case RangeKind::PartOfExpression:
3797+
return true;
3798+
case RangeKind::MultiStatement:
3799+
case RangeKind::MultiTypeMemberDecl:
3800+
case RangeKind::Invalid:
3801+
return false;
38263802
}
3827-
llvm_unreachable("unhandled kind");
38283803
}
38293804

38303805
bool RefactoringActionConvertToComputedProperty::
@@ -8308,7 +8283,7 @@ void swift::ide::collectAvailableRefactorings(
83088283
}
83098284

83108285
void swift::ide::collectAvailableRefactorings(
8311-
SourceFile *SF, RangeConfig Range, bool &RangeStartMayNeedRename,
8286+
SourceFile *SF, RangeConfig Range, bool &CollectRangeStartRefactorings,
83128287
SmallVectorImpl<RefactoringKind> &Kinds,
83138288
ArrayRef<DiagnosticConsumer *> DiagConsumers) {
83148289
if (Range.Length == 0) {
@@ -8337,7 +8312,7 @@ void swift::ide::collectAvailableRefactorings(
83378312
RANGE_REFACTORING(KIND, NAME, ID)
83388313
#include "swift/IDE/RefactoringKinds.def"
83398314

8340-
RangeStartMayNeedRename = rangeStartMayNeedRename(Result);
8315+
CollectRangeStartRefactorings = collectRangeStartRefactorings(Result);
83418316
}
83428317

83438318
bool swift::ide::

‎test/SourceKit/Refactoring/basic.swift

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,12 @@ HasInitWithDefaultArgs(y: 45, z: 89)
116116
func `hasBackticks`(`x`: Int) {}
117117
`hasBackticks`(`x`:2)
118118

119-
func hasAsyncAlternative(completion: @escaping (String?, Error?) -> Void) { }
120-
func hasCallToAsyncAlternative() {
121-
hasAsyncAlternative { str, err in print(str!) }
119+
struct ConvertAsync {
120+
func hasAsyncAlternative(completion: @escaping (String?, Error?) -> Void) { }
121+
}
122+
func hasCallToAsyncAlternative(c: ConvertAsync) {
123+
((((c)).hasAsyncAlternative)) { str, err in print(str!) }
124+
c.hasAsyncAlternative() { str, err in print(str!) }
122125
}
123126

124127
// RUN: %sourcekitd-test -req=cursor -pos=3:1 -end-pos=5:13 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK1
@@ -166,12 +169,9 @@ func hasCallToAsyncAlternative() {
166169
// RUN: %sourcekitd-test -req=cursor -pos=117:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
167170
// RUN: %sourcekitd-test -req=cursor -pos=117:17 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
168171

169-
// RUN: %sourcekitd-test -req=cursor -pos=119:6 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-ASYNC
170-
// RUN: %sourcekitd-test -req=cursor -pos=121:3 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
171-
172-
// RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
173172
// RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
174173

174+
// RUN: %sourcekitd-test -req=cursor -pos=54:10 -end-pos=54:22 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-LOCAL
175175
// RUN: %sourcekitd-test -req=cursor -pos=54:12 -end-pos=54:22 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-SELF-RENAME1
176176
// RUN: %sourcekitd-test -req=cursor -pos=54:23 -end-pos=54:33 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-SELF-RENAME2
177177
// RUN: %sourcekitd-test -req=cursor -pos=54:34 -end-pos=54:44 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-SELF-RENAME3
@@ -183,6 +183,17 @@ func hasCallToAsyncAlternative() {
183183
// RUN: %sourcekitd-test -req=cursor -pos=72:5 -end-pos=72:11 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
184184
// RUN: %sourcekitd-test -req=cursor -pos=78:3 -end-pos=78:9 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
185185

186+
// RUN: %sourcekitd-test -req=cursor -pos=120:8 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-ASYNC
187+
// RUN: %sourcekitd-test -req=cursor -pos=123:11 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
188+
// RUN: %sourcekitd-test -req=cursor -pos=123:11 -end-pos=123:30 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
189+
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:30 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
190+
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:60 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
191+
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:46 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
192+
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:58 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
193+
// RUN: %sourcekitd-test -req=cursor -pos=124:3 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
194+
// RUN: %sourcekitd-test -req=cursor -pos=124:3 -end-pos=124:26 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
195+
// RUN: %sourcekitd-test -req=cursor -pos=124:3 -end-pos=124:54 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
196+
186197
// CHECK-NORENAME-NOT: Global Rename
187198
// CHECK-NORENAME-NOT: Local Rename
188199

‎test/SourceKit/Refactoring/rename-objc.swift

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,11 @@ func foo1() {
88
// RUN: %sourcekitd-test -req=cursor -pos=4:30 -cursor-action %s -- -F %S/Inputs/mock-sdk -I %t.tmp %s | %FileCheck %s -check-prefix=CHECK1
99
// RUN: %sourcekitd-test -req=cursor -pos=4:30 -length=3 -cursor-action %s -- -F %S/Inputs/mock-sdk -I %t.tmp %s | %FileCheck %s -check-prefix=CHECK1
1010
// RUN: %sourcekitd-test -req=cursor -pos=4:20 -length=15 -cursor-action %s -- -F %S/Inputs/mock-sdk -I %t.tmp %s | %FileCheck %s -check-prefix=CHECK1
11-
// RUN: %sourcekitd-test -req=cursor -pos=4:20 -length=16 -cursor-action %s -- -F %S/Inputs/mock-sdk -I %t.tmp %s | %FileCheck %s -check-prefix=CHECK2
11+
// RUN: %sourcekitd-test -req=cursor -pos=4:20 -length=16 -cursor-action %s -- -F %S/Inputs/mock-sdk -I %t.tmp %s | %FileCheck %s -check-prefix=CHECK1
1212

1313
// CHECK1: ACTIONS BEGIN
1414
// CHECK1-NEXT: source.refactoring.kind.rename.global
1515
// CHECK1-NEXT: Global Rename
1616
// CHECK1-NEXT: cannot rename a Clang symbol from its Swift reference
1717

18-
// CHECK2: ACTIONS BEGIN
19-
// CHECK2-NEXT: ACTIONS END
20-
2118
// REQUIRES: OS=macosx || OS=linux-gnu

‎tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,24 +1448,24 @@ static void resolveCursor(
14481448
Range.Line = Pair.first;
14491449
Range.Column = Pair.second;
14501450
Range.Length = Length;
1451-
bool RangeStartMayNeedRename = false;
1451+
bool CollectRangeStartRefactorings = false;
14521452
collectAvailableRefactorings(&AstUnit->getPrimarySourceFile(), Range,
1453-
RangeStartMayNeedRename, Kinds, {});
1453+
CollectRangeStartRefactorings, Kinds, {});
14541454
for (RefactoringKind Kind : Kinds) {
14551455
Actions.emplace_back(SwiftLangSupport::getUIDForRefactoringKind(Kind),
14561456
getDescriptiveRefactoringKindName(Kind),
14571457
/*UnavailableReason*/ StringRef());
14581458
}
1459-
if (!RangeStartMayNeedRename) {
1460-
// If Length is given, then the cursor-info request should only about
1461-
// collecting available refactorings for the range.
1459+
if (!CollectRangeStartRefactorings) {
1460+
// If Length is given then this request is only for refactorings,
1461+
// return straight away unless we need cursor based refactorings as
1462+
// well.
14621463
CursorInfoData Data;
14631464
Data.AvailableActions = llvm::makeArrayRef(Actions);
14641465
Receiver(RequestResult<CursorInfoData>::fromResult(Data));
14651466
return;
14661467
}
1467-
// If the range start may need rename, we fall back to a regular cursor
1468-
// info request to get the available rename kinds.
1468+
// Fall through to collect cursor based refactorings
14691469
}
14701470

14711471
auto *File = &AstUnit->getPrimarySourceFile();
@@ -1474,12 +1474,6 @@ static void resolveCursor(
14741474
CursorInfoRequest{CursorInfoOwner(File, Loc)},
14751475
ResolvedCursorInfo());
14761476

1477-
if (CursorInfo.isInvalid()) {
1478-
CursorInfoData Info;
1479-
Info.InternalDiagnostic = "Unable to resolve cursor info.";
1480-
Receiver(RequestResult<CursorInfoData>::fromResult(Info));
1481-
return;
1482-
}
14831477
CompilerInvocation CompInvok;
14841478
ASTInvok->applyTo(CompInvok);
14851479

@@ -1526,9 +1520,15 @@ static void resolveCursor(
15261520
Receiver(RequestResult<CursorInfoData>::fromResult(Info));
15271521
return;
15281522
}
1529-
case CursorInfoKind::Invalid: {
1530-
llvm_unreachable("bad sema token kind");
1531-
}
1523+
case CursorInfoKind::Invalid:
1524+
CursorInfoData Data;
1525+
if (Actionables) {
1526+
Data.AvailableActions = llvm::makeArrayRef(Actions);
1527+
} else {
1528+
Data.InternalDiagnostic = "Unable to resolve cursor info.";
1529+
}
1530+
Receiver(RequestResult<CursorInfoData>::fromResult(Data));
1531+
return;
15321532
}
15331533
}
15341534

‎tools/swift-refactor/swift-refactor.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,9 +394,9 @@ int main(int argc, char *argv[]) {
394394

395395
if (options::Action == RefactoringKind::None) {
396396
llvm::SmallVector<RefactoringKind, 32> Kinds;
397-
bool RangeStartMayNeedRename = false;
398-
collectAvailableRefactorings(SF, Range, RangeStartMayNeedRename, Kinds,
399-
{&PrintDiags});
397+
bool CollectRangeStartRefactorings = false;
398+
collectAvailableRefactorings(SF, Range, CollectRangeStartRefactorings,
399+
Kinds, {&PrintDiags});
400400
llvm::outs() << "Action begins\n";
401401
for (auto Kind : Kinds) {
402402
llvm::outs() << getDescriptiveRefactoringKindName(Kind) << "\n";

0 commit comments

Comments
 (0)