diff --git a/include/swift/IDE/RefactoringKinds.def b/include/swift/IDE/RefactoringKinds.def index cbb0e1eb9c4d4..3764fdf3fc92d 100644 --- a/include/swift/IDE/RefactoringKinds.def +++ b/include/swift/IDE/RefactoringKinds.def @@ -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) diff --git a/lib/IDE/Refactoring.cpp b/lib/IDE/Refactoring.cpp index 3fdb42ee10451..88a382e37d237 100644 --- a/lib/IDE/Refactoring.cpp +++ b/lib/IDE/Refactoring.cpp @@ -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 +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(CursorInfo.TrailingStmt); + if (!OuterIf) + return {}; + if (OuterIf->getElseStmt()) + return {}; - auto *IFS = dyn_cast(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(OuterIf->getThenStmt()); + if (!Body || Body->getNumElements() != 1) + return {}; + + IfStmt *InnerIf = + dyn_cast_or_null(Body->getElement(0).dyn_cast()); + 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(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; } diff --git a/test/refactoring/RefactoringKind/basic.swift b/test/refactoring/RefactoringKind/basic.swift index 436c4f93df64a..e434eeb8b3acf 100644 --- a/test/refactoring/RefactoringKind/basic.swift +++ b/test/refactoring/RefactoringKind/basic.swift @@ -174,21 +174,21 @@ 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 {} @@ -196,11 +196,31 @@ func testExtraDeclNestedIf() { } } -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") + } } } @@ -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 @@ -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 diff --git a/tools/swift-refactor/swift-refactor.cpp b/tools/swift-refactor/swift-refactor.cpp index eeceb52304bfe..6c9f6b69f5c42 100644 --- a/tools/swift-refactor/swift-refactor.cpp +++ b/tools/swift-refactor/swift-refactor.cpp @@ -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"),