SR-5739 Add refactoring action to collapse nested if (#11851)

willtellis · nkcsgexi · commit 57454eb8c9ad · 2017-09-18T16:16:01.000-07:00
diff --git a/include/swift/IDE/RefactoringKinds.def b/include/swift/IDE/RefactoringKinds.def
@@ -38,6 +38,8 @@ 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)
+
 RANGE_REFACTORING(ExtractExpr, "Extract Expression", extract.expr)
 
 RANGE_REFACTORING(ExtractFunction, "Extract Method", extract.function)
diff --git a/lib/IDE/Refactoring.cpp b/lib/IDE/Refactoring.cpp
@@ -1448,6 +1448,113 @@ bool RefactoringActionExtractRepeatedExpr::performChange() {
                                           EditConsumer).performChange();
 }
 
+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) {
+  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;
+      }
+
+      bool StmtIsOuterIfBrace =
+        IfInfo.OuterIf && !IfInfo.InnerIf && S->getKind() == StmtKind::Brace;
+      if (StmtIsOuterIfBrace) {
+        return true;
+      }
+
+      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;
+    }
+
+  } Walker(CursorInfo.TrailingStmt->getStartLoc());
+  Walker.walk(CursorInfo.TrailingStmt);
+  return Walker.IfInfo;
+}
+
+bool RefactoringActionCollapseNestedIfExpr::isApplicable(ResolvedCursorInfo Tok) {
+  return findCollapseNestedIfTarget(Tok).isValid();
+}
+
+bool RefactoringActionCollapseNestedIfExpr::performChange() {
+  auto Target = findCollapseNestedIfTarget(CursorInfo);
+  if (!Target.isValid())
+    return true;
+  auto OuterIfConds = Target.OuterIf->getCond().vec();
+  auto InnerIfConds = Target.InnerIf->getCond().vec();
+
+  llvm::SmallString<64> DeclBuffer;
+  llvm::raw_svector_ostream OS(DeclBuffer);
+  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;
+
+  auto SourceRange = Lexer::getCharSourceRangeFromSourceRange(
+    SM, Target.OuterIf->getSourceRange());
+  EditConsumer.accept(SM, SourceRange, DeclBuffer.str());
+  return false;
+}
+
 /// The helper class analyzes a given nominal decl or an extension decl to
 /// decide whether stubs are required to filled in and the context in which
 /// these stubs should be filled.
diff --git a/test/refactoring/CollapseNestedIf/Outputs/basic/L15-3.swift.expected b/test/refactoring/CollapseNestedIf/Outputs/basic/L15-3.swift.expected
@@ -0,0 +1,22 @@
+func testCollapseNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+  }
+}
+func testMultiConditionalNestedIf() {
+  let a = 3
+  if a > 2, a != 4 {
+    if a < 10, a != 5 {}
+  }
+}
+func testLetNestedIf() {
+  let a: Int? = 3
+  if let b = a, b != 5 {}
+}
+func testCaseLetNestedIf() {
+  let a: Int? = 3
+  if case .some(let b) = a {
+    if b != 5 {}
+  }
+}
diff --git a/test/refactoring/CollapseNestedIf/Outputs/basic/L21-3.swift.expected b/test/refactoring/CollapseNestedIf/Outputs/basic/L21-3.swift.expected
@@ -0,0 +1,22 @@
+func testCollapseNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+  }
+}
+func testMultiConditionalNestedIf() {
+  let a = 3
+  if a > 2, a != 4 {
+    if a < 10, a != 5 {}
+  }
+}
+func testLetNestedIf() {
+  let a: Int? = 3
+  if let b = a {
+    if b != 5 {}
+  }
+}
+func testCaseLetNestedIf() {
+  let a: Int? = 3
+  if case .some(let b) = a, b != 5 {}
+}
diff --git a/test/refactoring/CollapseNestedIf/Outputs/basic/L3-3.swift.expected b/test/refactoring/CollapseNestedIf/Outputs/basic/L3-3.swift.expected
@@ -0,0 +1,22 @@
+func testCollapseNestedIf() {
+  let a = 3
+  if a > 2, a < 10 {}
+}
+func testMultiConditionalNestedIf() {
+  let a = 3
+  if a > 2, a != 4 {
+    if a < 10, a != 5 {}
+  }
+}
+func testLetNestedIf() {
+  let a: Int? = 3
+  if let b = a {
+    if b != 5 {}
+  }
+}
+func testCaseLetNestedIf() {
+  let a: Int? = 3
+  if case .some(let b) = a {
+    if b != 5 {}
+  }
+}
diff --git a/test/refactoring/CollapseNestedIf/Outputs/basic/L9-3.swift.expected b/test/refactoring/CollapseNestedIf/Outputs/basic/L9-3.swift.expected
@@ -0,0 +1,22 @@
+func testCollapseNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+  }
+}
+func testMultiConditionalNestedIf() {
+  let a = 3
+  if a > 2, a != 4, a < 10, a != 5 {}
+}
+func testLetNestedIf() {
+  let a: Int? = 3
+  if let b = a {
+    if b != 5 {}
+  }
+}
+func testCaseLetNestedIf() {
+  let a: Int? = 3
+  if case .some(let b) = a {
+    if b != 5 {}
+  }
+}
diff --git a/test/refactoring/CollapseNestedIf/basic.swift b/test/refactoring/CollapseNestedIf/basic.swift
@@ -0,0 +1,33 @@
+func testCollapseNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+  }
+}
+func testMultiConditionalNestedIf() {
+  let a = 3
+  if a > 2, a != 4 {
+    if a < 10, a != 5 {}
+  }
+}
+func testLetNestedIf() {
+  let a: Int? = 3
+  if let b = a {
+    if b != 5 {}
+  }
+}
+func testCaseLetNestedIf() {
+  let a: Int? = 3
+  if case .some(let b) = a {
+    if b != 5 {}
+  }
+}
+// RUN: rm -rf %t.result && mkdir -p %t.result
+// RUN: %refactor -collapse-nested-if -source-filename %s -pos=3:3 > %t.result/L3-3.swift
+// RUN: diff -u %S/Outputs/basic/L3-3.swift.expected %t.result/L3-3.swift
+// RUN: %refactor -collapse-nested-if -source-filename %s -pos=9:3 > %t.result/L9-3.swift
+// RUN: diff -u %S/Outputs/basic/L9-3.swift.expected %t.result/L9-3.swift
+// RUN: %refactor -collapse-nested-if -source-filename %s -pos=15:3 > %t.result/L15-3.swift
+// RUN: diff -u %S/Outputs/basic/L15-3.swift.expected %t.result/L15-3.swift
+// RUN: %refactor -collapse-nested-if -source-filename %s -pos=21:3 > %t.result/L21-3.swift
+// RUN: diff -u %S/Outputs/basic/L21-3.swift.expected %t.result/L21-3.swift
diff --git a/test/refactoring/RefactoringKind/basic.swift b/test/refactoring/RefactoringKind/basic.swift
@@ -174,6 +174,35 @@ func testStringLiteral() -> String {
   return "abc"
 }
 
+func testCollapseNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+  }
+}
+
+func testMultiConditionalNestedIf() {
+  let a = 3
+  if a > 2, a != 4 {
+    if a < 10 {}
+  }
+}
+
+func testExtraDeclNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+    let b = 0
+  }
+}
+
+func testExtraIfNestedIf() {
+  let a = 3
+  if a > 2 {
+    if a < 10 {}
+    let b = 0
+  }
+}
 // RUN: %refactor -source-filename %s -pos=2:1 -end-pos=5:13 | %FileCheck %s -check-prefix=CHECK1
 // RUN: %refactor -source-filename %s -pos=3:1 -end-pos=5:13 | %FileCheck %s -check-prefix=CHECK1
 // RUN: %refactor -source-filename %s -pos=4:1 -end-pos=5:13 | %FileCheck %s -check-prefix=CHECK1
@@ -245,6 +274,11 @@ func testStringLiteral() -> String {
 
 // 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=193:3 | %FileCheck %s -check-prefix=CHECK-NONE
+// RUN: %refactor -source-filename %s -pos=201:3 | %FileCheck %s -check-prefix=CHECK-NONE
+
 // CHECK1: Action begins
 // CHECK1-NEXT: Extract Method
 // CHECK1-NEXT: Action ends
@@ -279,3 +313,5 @@ func testStringLiteral() -> String {
 // CHECK-EXTRCT-METHOD-NEXT: Action ends
 
 // CHECK-LOCALIZE-STRING: Localize String
+
+// CHECK-COLLAPSE-NESTED-IF-EXPRESSION: Collapse Nested If Expression
diff --git a/tools/swift-refactor/swift-refactor.cpp b/tools/swift-refactor/swift-refactor.cpp
@@ -38,6 +38,8 @@ Action(llvm::cl::desc("kind:"), llvm::cl::init(RefactoringKind::None),
                       "expand-default", "Perform expand default statement refactoring"),
            clEnumValN(RefactoringKind::LocalizeString,
                       "localize-string", "Perform string localization refactoring"),
+           clEnumValN(RefactoringKind::CollapseNestedIfExpr,
+                       "collapse-nested-if", "Perform collapse nested if statements"),
            clEnumValN(RefactoringKind::SimplifyNumberLiteral,
                       "simplify-long-number", "Perform simplify long number literal refactoring"),
            clEnumValN(RefactoringKind::ExtractFunction,
