Skip to content

Commit

Permalink
Merge pull request swiftlang#16789 from rintaro/refactoring-nested-if
Browse files Browse the repository at this point in the history 
[Refactoring] Re-implement "collapse nested if" action
  • Loading branch information
@rintaro
rintaro authored May 23, 2018
2 parents 2403fb8 + 639fb85 commit 025f4dd
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 112 deletions.
2 changes: 1 addition & 1 deletion include/swift/IDE/RefactoringKinds.def
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ CURSOR_REFACTORING(LocalizeString, "Localize String", localize.string)

CURSOR_REFACTORING(SimplifyNumberLiteral, "Simplify Long Number Literal", simplify.long.number.literal)

CURSOR_REFACTORING(CollapseNestedIfExpr, "Collapse Nested If Expression", collapse.nested.if.expr)
CURSOR_REFACTORING(CollapseNestedIfStmt, "Collapse Nested If Statements", collapse.nested.ifstmt)

CURSOR_REFACTORING(ConvertToDoCatch, "Convert To Do/Catch", convert.do.catch)

Expand Down
145 changes: 53 additions & 92 deletions lib/IDE/Refactoring.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1668,109 +1668,70 @@ bool RefactoringActionReplaceBodiesWithFatalError::performChange() {
return false;
}

struct CollapsibleNestedIfInfo {
IfStmt *OuterIf;
IfStmt *InnerIf;
bool FinishedOuterIf;
bool FoundNonCollapsibleItem;
CollapsibleNestedIfInfo():
OuterIf(nullptr), InnerIf(nullptr),
FinishedOuterIf(false), FoundNonCollapsibleItem(false) {}
bool isValid() {
return OuterIf && InnerIf && FinishedOuterIf && !FoundNonCollapsibleItem;
}
};

static CollapsibleNestedIfInfo findCollapseNestedIfTarget(ResolvedCursorInfo CursorInfo) {
static std::pair<IfStmt *, IfStmt *>
findCollapseNestedIfTarget(ResolvedCursorInfo CursorInfo) {
if (CursorInfo.Kind != CursorInfoKind::StmtStart)
return CollapsibleNestedIfInfo();
struct IfStmtFinder: public SourceEntityWalker {
SourceLoc StartLoc;
CollapsibleNestedIfInfo IfInfo;
IfStmtFinder(SourceLoc StartLoc): StartLoc(StartLoc), IfInfo() {}
bool finishedInnerIfButNotFinishedOuterIf() {
return IfInfo.InnerIf && !IfInfo.FinishedOuterIf;
}
bool walkToStmtPre(Stmt *S) {
if (finishedInnerIfButNotFinishedOuterIf()) {
IfInfo.FoundNonCollapsibleItem = true;
return false;
}
return {};

bool StmtIsOuterIfBrace =
IfInfo.OuterIf && !IfInfo.InnerIf && S->getKind() == StmtKind::Brace;
if (StmtIsOuterIfBrace) {
return true;
}
// Ensure the statement is 'if' statement. It must not have 'else' clause.
IfStmt *OuterIf = dyn_cast<IfStmt>(CursorInfo.TrailingStmt);
if (!OuterIf)
return {};
if (OuterIf->getElseStmt())
return {};

auto *IFS = dyn_cast<IfStmt>(S);
if (!IFS) {
return false;
}
if (!IfInfo.OuterIf) {
IfInfo.OuterIf = IFS;
return true;
} else {
IfInfo.InnerIf = IFS;
return false;
}
}
bool walkToStmtPost(Stmt *S) {
assert(S != IfInfo.InnerIf && "Should not traverse inner if statement");
if (S == IfInfo.OuterIf) {
IfInfo.FinishedOuterIf = true;
}
return true;
}
bool walkToDeclPre(Decl *D, CharSourceRange Range) {
if (finishedInnerIfButNotFinishedOuterIf()) {
IfInfo.FoundNonCollapsibleItem = true;
return false;
}
return true;
}
bool walkToExprPre(Expr *E) {
if (finishedInnerIfButNotFinishedOuterIf()) {
IfInfo.FoundNonCollapsibleItem = true;
return false;
}
return true;
}
// The body must contain a sole inner 'if' statement.
auto Body = dyn_cast_or_null<BraceStmt>(OuterIf->getThenStmt());
if (!Body || Body->getNumElements() != 1)
return {};

IfStmt *InnerIf =
dyn_cast_or_null<IfStmt>(Body->getElement(0).dyn_cast<Stmt *>());
if (!InnerIf)
return {};

} Walker(CursorInfo.TrailingStmt->getStartLoc());
Walker.walk(CursorInfo.TrailingStmt);
return Walker.IfInfo;
// Inner 'if' statement also cannot have 'else' clause.
if (InnerIf->getElseStmt())
return {};

return {OuterIf, InnerIf};
}

bool RefactoringActionCollapseNestedIfExpr::
isApplicable(ResolvedCursorInfo Tok, DiagnosticEngine &Diag) {
return findCollapseNestedIfTarget(Tok).isValid();
bool RefactoringActionCollapseNestedIfStmt::
isApplicable(ResolvedCursorInfo CursorInfo, DiagnosticEngine &Diag) {
return findCollapseNestedIfTarget(CursorInfo).first;
}

bool RefactoringActionCollapseNestedIfExpr::performChange() {
bool RefactoringActionCollapseNestedIfStmt::performChange() {
auto Target = findCollapseNestedIfTarget(CursorInfo);
if (!Target.isValid())
if (!Target.first)
return true;
auto OuterIfConds = Target.OuterIf->getCond().vec();
auto InnerIfConds = Target.InnerIf->getCond().vec();
auto OuterIf = Target.first;
auto InnerIf = Target.second;

EditorConsumerInsertStream OS(EditConsumer, SM,
Lexer::getCharSourceRangeFromSourceRange(
SM, Target.OuterIf->getSourceRange()));

OS << tok::kw_if << " ";
for (auto CI = OuterIfConds.begin(); CI != OuterIfConds.end(); ++CI) {
OS << (CI != OuterIfConds.begin() ? ", " : "");
OS << Lexer::getCharSourceRangeFromSourceRange(
SM, CI->getSourceRange()).str();
}
for (auto CI = InnerIfConds.begin(); CI != InnerIfConds.end(); ++CI) {
OS << ", " << Lexer::getCharSourceRangeFromSourceRange(
SM, CI->getSourceRange()).str();
}
auto ThenStatementText = Lexer::getCharSourceRangeFromSourceRange(
SM, Target.InnerIf->getThenStmt()->getSourceRange()).str();
OS << " " << ThenStatementText;
EditorConsumerInsertStream OS(
EditConsumer, SM,
Lexer::getCharSourceRangeFromSourceRange(SM, OuterIf->getSourceRange()));

OS << tok::kw_if << " ";

// Emit conditions.
bool first = true;
for (auto &C : llvm::concat<StmtConditionElement>(OuterIf->getCond(),
InnerIf->getCond())) {
if (first)
first = false;
else
OS << ", ";
OS << Lexer::getCharSourceRangeFromSourceRange(SM, C.getSourceRange())
.str();
}

// Emit body.
OS << " ";
OS << Lexer::getCharSourceRangeFromSourceRange(
SM, InnerIf->getThenStmt()->getSourceRange())
.str();
return false;
}

Expand Down
58 changes: 40 additions & 18 deletions test/refactoring/RefactoringKind/basic.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,33 +174,53 @@ func testStringLiteral() -> String {
return "abc"
}

func testCollapseNestedIf() {
func testCollapseNestedIf1() {
let a = 3
if a > 2 {
if a < 10 {}
}
}

func testMultiConditionalNestedIf() {
func testCollapseNestedIf2() {
let a = 3
if a > 2, a != 4 {
if a < 10 {}
}
}

func testExtraDeclNestedIf() {
func testCollapseNestedIf3() {
let a = 3
if a > 2 {
if a < 10 {}
let b = 0
}
}

func testExtraIfNestedIf() {
func testCollapseNestedIf4() {
let a = 3
if a > 2 {
if a < 10 {}
let b = 0
if a < 10 {}
}
}

func testCollapseNestedIf5() {
let a = 3
if a > 2 {
if a < 10 {}
} else {
print("else")
}
}

func testCollapseNestedIf6() {
let a = 3
if a > 2 {
if a < 10 {
print("if")
} else if a < 5 {
print("else")
}
}
}

Expand Down Expand Up @@ -313,25 +333,27 @@ func testConvertToTernaryExpr() {

// RUN: %refactor -source-filename %s -pos=173:3 -end-pos=173:27| %FileCheck %s -check-prefix=CHECK-EXTRCT-METHOD

// RUN: %refactor -source-filename %s -pos=179:3 | %FileCheck %s -check-prefix=CHECK-COLLAPSE-NESTED-IF-EXPRESSION
// RUN: %refactor -source-filename %s -pos=186:3 | %FileCheck %s -check-prefix=CHECK-COLLAPSE-NESTED-IF-EXPRESSION
// RUN: %refactor -source-filename %s -pos=179:3 | %FileCheck %s -check-prefix=CHECK-COLLAPSE-NESTED-IF
// RUN: %refactor -source-filename %s -pos=186:3 | %FileCheck %s -check-prefix=CHECK-COLLAPSE-NESTED-IF
// RUN: %refactor -source-filename %s -pos=193:3 | %FileCheck %s -check-prefix=CHECK-NONE
// RUN: %refactor -source-filename %s -pos=201:3 | %FileCheck %s -check-prefix=CHECK-NONE
// RUN: %refactor -source-filename %s -pos=209:3 | %FileCheck %s -check-prefix=CHECK-NONE
// RUN: %refactor -source-filename %s -pos=218:3 | %FileCheck %s -check-prefix=CHECK-NONE

// RUN: %refactor -source-filename %s -pos=210:11 -end-pos=210:24 | %FileCheck %s -check-prefix=CHECK-STRINGS-INTERPOLATION
// RUN: %refactor -source-filename %s -pos=211:11 -end-pos=211:26 | %FileCheck %s -check-prefix=CHECK-STRINGS-INTERPOLATION
// RUN: %refactor -source-filename %s -pos=212:11 -end-pos=212:21 | %FileCheck %s -check-prefix=CHECK-STRINGS-INTERPOLATION
// RUN: %refactor -source-filename %s -pos=230:11 -end-pos=230:24 | %FileCheck %s -check-prefix=CHECK-STRINGS-INTERPOLATION
// RUN: %refactor -source-filename %s -pos=231:11 -end-pos=231:26 | %FileCheck %s -check-prefix=CHECK-STRINGS-INTERPOLATION
// RUN: %refactor -source-filename %s -pos=232:11 -end-pos=232:21 | %FileCheck %s -check-prefix=CHECK-STRINGS-INTERPOLATION

// RUN: %refactor -source-filename %s -pos=217:11 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=217:12 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=217:13 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=217:14 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=237:11 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=237:12 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=237:13 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH
// RUN: %refactor -source-filename %s -pos=237:14 | %FileCheck %s -check-prefix=CHECK-TRY-CATCH

// RUN: %refactor -source-filename %s -pos=225:3 | %FileCheck %s -check-prefix=CHECK-EXPAND-SWITCH
// RUN: %refactor -source-filename %s -pos=245:3 | %FileCheck %s -check-prefix=CHECK-EXPAND-SWITCH

// RUN: %refactor -source-filename %s -pos=231:3 -end-pos=231:24 | %FileCheck %s -check-prefix=CHECK-EXPAND-TERNARY-EXPRESSEXPRESSION
// RUN: %refactor -source-filename %s -pos=251:3 -end-pos=251:24 | %FileCheck %s -check-prefix=CHECK-EXPAND-TERNARY-EXPRESSEXPRESSION

// RUN: %refactor -source-filename %s -pos=237:3 -end-pos=242:4 | %FileCheck %s -check-prefix=CHECK-CONVERT-TO-TERNARY-EXPRESSEXPRESSION
// RUN: %refactor -source-filename %s -pos=257:3 -end-pos=262:4 | %FileCheck %s -check-prefix=CHECK-CONVERT-TO-TERNARY-EXPRESSEXPRESSION

// CHECK1: Action begins
// CHECK1-NEXT: Extract Method
Expand Down Expand Up @@ -368,7 +390,7 @@ func testConvertToTernaryExpr() {

// CHECK-LOCALIZE-STRING: Localize String

// CHECK-COLLAPSE-NESTED-IF-EXPRESSION: Collapse Nested If Expression
// CHECK-COLLAPSE-NESTED-IF: Collapse Nested If Statements

// CHECK-STRINGS-INTERPOLATION: Convert to String Interpolation

Expand Down
2 changes: 1 addition & 1 deletion tools/swift-refactor/swift-refactor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Action(llvm::cl::desc("kind:"), llvm::cl::init(RefactoringKind::None),
"expand-switch-cases", "Perform switch cases expand refactoring"),
clEnumValN(RefactoringKind::LocalizeString,
"localize-string", "Perform string localization refactoring"),
clEnumValN(RefactoringKind::CollapseNestedIfExpr,
clEnumValN(RefactoringKind::CollapseNestedIfStmt,
"collapse-nested-if", "Perform collapse nested if statements"),
clEnumValN(RefactoringKind::ConvertToDoCatch,
"convert-to-do-catch", "Perform force try to do try catch refactoring"),
Expand Down

0 comments on commit 025f4dd

Please sign in to comment.