Skip to content

[5.5][Refactorings] Add cursor refactorings for the start of the range #38983

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
Aug 25, 2021
Merged

[5.5][Refactorings] Add cursor refactorings for the start of the range #38983

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
51 changes: 13 additions & 38 deletions lib/IDE/Refactoring.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3788,43 +3788,18 @@ bool RefactoringActionTrailingClosure::performChange() {
return false;
}

static bool rangeStartMayNeedRename(const ResolvedRangeInfo &Info) {
switch(Info.Kind) {
case RangeKind::SingleExpression: {
Expr *E = Info.ContainedNodes[0].get<Expr*>();
// We should show rename for the selection of "foo()"
if (auto *CE = dyn_cast<CallExpr>(E)) {
if (CE->getFn()->getKind() == ExprKind::DeclRef)
return true;

// When callling an instance method inside another instance method,
// we have a dot syntax call whose dot and base are both implicit. We
// need to explicitly allow the specific case here.
if (auto *DSC = dyn_cast<DotSyntaxCallExpr>(CE->getFn())) {
if (DSC->getBase()->isImplicit() &&
DSC->getFn()->getStartLoc() == Info.TokensInRange.front().getLoc())
return true;
}
}
return false;
}
case RangeKind::PartOfExpression: {
if (auto *CE = dyn_cast<CallExpr>(Info.CommonExprParent)) {
if (auto *DSC = dyn_cast<DotSyntaxCallExpr>(CE->getFn())) {
if (DSC->getFn()->getStartLoc() == Info.TokensInRange.front().getLoc())
return true;
}
}
return false;
}
case RangeKind::SingleDecl:
case RangeKind::MultiTypeMemberDecl:
case RangeKind::SingleStatement:
case RangeKind::MultiStatement:
case RangeKind::Invalid:
return false;
static bool collectRangeStartRefactorings(const ResolvedRangeInfo &Info) {
switch (Info.Kind) {
case RangeKind::SingleExpression:
case RangeKind::SingleStatement:
case RangeKind::SingleDecl:
case RangeKind::PartOfExpression:
return true;
case RangeKind::MultiStatement:
case RangeKind::MultiTypeMemberDecl:
case RangeKind::Invalid:
return false;
}
llvm_unreachable("unhandled kind");
}

bool RefactoringActionConvertToComputedProperty::
Expand Down Expand Up @@ -8308,7 +8283,7 @@ void swift::ide::collectAvailableRefactorings(
}

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

RangeStartMayNeedRename = rangeStartMayNeedRename(Result);
CollectRangeStartRefactorings = collectRangeStartRefactorings(Result);
}

bool swift::ide::
Expand Down
25 changes: 18 additions & 7 deletions test/SourceKit/Refactoring/basic.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,12 @@ HasInitWithDefaultArgs(y: 45, z: 89)
func `hasBackticks`(`x`: Int) {}
`hasBackticks`(`x`:2)

func hasAsyncAlternative(completion: @escaping (String?, Error?) -> Void) { }
func hasCallToAsyncAlternative() {
hasAsyncAlternative { str, err in print(str!) }
struct ConvertAsync {
func hasAsyncAlternative(completion: @escaping (String?, Error?) -> Void) { }
}
func hasCallToAsyncAlternative(c: ConvertAsync) {
((((c)).hasAsyncAlternative)) { str, err in print(str!) }
c.hasAsyncAlternative() { str, err in print(str!) }
}

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

// RUN: %sourcekitd-test -req=cursor -pos=119:6 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-ASYNC
// RUN: %sourcekitd-test -req=cursor -pos=121:3 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC

// RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
// RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT

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

// RUN: %sourcekitd-test -req=cursor -pos=120:8 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-ASYNC
// RUN: %sourcekitd-test -req=cursor -pos=123:11 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=123:11 -end-pos=123:30 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:30 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:60 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:46 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=123:3 -end-pos=123:58 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=124:3 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=124:3 -end-pos=124:26 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC
// RUN: %sourcekitd-test -req=cursor -pos=124:3 -end-pos=124:54 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-CALLASYNC

// CHECK-NORENAME-NOT: Global Rename
// CHECK-NORENAME-NOT: Local Rename

Expand Down
5 changes: 1 addition & 4 deletions test/SourceKit/Refactoring/rename-objc.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,11 @@ func foo1() {
// 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
// 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
// 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
// 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
// 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

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

// CHECK2: ACTIONS BEGIN
// CHECK2-NEXT: ACTIONS END

// REQUIRES: OS=macosx || OS=linux-gnu
32 changes: 16 additions & 16 deletions tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,24 +1448,24 @@ static void resolveCursor(
Range.Line = Pair.first;
Range.Column = Pair.second;
Range.Length = Length;
bool RangeStartMayNeedRename = false;
bool CollectRangeStartRefactorings = false;
collectAvailableRefactorings(&AstUnit->getPrimarySourceFile(), Range,
RangeStartMayNeedRename, Kinds, {});
CollectRangeStartRefactorings, Kinds, {});
for (RefactoringKind Kind : Kinds) {
Actions.emplace_back(SwiftLangSupport::getUIDForRefactoringKind(Kind),
getDescriptiveRefactoringKindName(Kind),
/*UnavailableReason*/ StringRef());
}
if (!RangeStartMayNeedRename) {
// If Length is given, then the cursor-info request should only about
// collecting available refactorings for the range.
if (!CollectRangeStartRefactorings) {
// If Length is given then this request is only for refactorings,
// return straight away unless we need cursor based refactorings as
// well.
CursorInfoData Data;
Data.AvailableActions = llvm::makeArrayRef(Actions);
Receiver(RequestResult<CursorInfoData>::fromResult(Data));
return;
}
// If the range start may need rename, we fall back to a regular cursor
// info request to get the available rename kinds.
// Fall through to collect cursor based refactorings
}

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

if (CursorInfo.isInvalid()) {
CursorInfoData Info;
Info.InternalDiagnostic = "Unable to resolve cursor info.";
Receiver(RequestResult<CursorInfoData>::fromResult(Info));
return;
}
CompilerInvocation CompInvok;
ASTInvok->applyTo(CompInvok);

Expand Down Expand Up @@ -1526,9 +1520,15 @@ static void resolveCursor(
Receiver(RequestResult<CursorInfoData>::fromResult(Info));
return;
}
case CursorInfoKind::Invalid: {
llvm_unreachable("bad sema token kind");
}
case CursorInfoKind::Invalid:
CursorInfoData Data;
if (Actionables) {
Data.AvailableActions = llvm::makeArrayRef(Actions);
} else {
Data.InternalDiagnostic = "Unable to resolve cursor info.";
}
Receiver(RequestResult<CursorInfoData>::fromResult(Data));
return;
}
}

Expand Down
6 changes: 3 additions & 3 deletions tools/swift-refactor/swift-refactor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,9 @@ int main(int argc, char *argv[]) {

if (options::Action == RefactoringKind::None) {
llvm::SmallVector<RefactoringKind, 32> Kinds;
bool RangeStartMayNeedRename = false;
collectAvailableRefactorings(SF, Range, RangeStartMayNeedRename, Kinds,
{&PrintDiags});
bool CollectRangeStartRefactorings = false;
collectAvailableRefactorings(SF, Range, CollectRangeStartRefactorings,
Kinds, {&PrintDiags});
llvm::outs() << "Action begins\n";
for (auto Kind : Kinds) {
llvm::outs() << getDescriptiveRefactoringKindName(Kind) << "\n";
Expand Down