IDEA-129766 (Structural replace of try/finally block deletes catch block)

java/structuralsearch-java/src/com/intellij/structuralsearch/JavaReplaceHandler.java

@@ -1,7 +1,6 @@
 package com.intellij.structuralsearch;
 
 import com.intellij.openapi.fileTypes.StdFileTypes;
-import com.intellij.openapi.project.Project;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.psi.*;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
@@ -257,7 +256,6 @@ public class JavaReplaceHandler extends StructuralReplaceHandler {
     PsiElement elementToReplace = info.getMatch(0);
     PsiElement elementParent = elementToReplace.getParent();
     String replacementToMake = info.getReplacement();
-    Project project = myContext.getProject();
     PsiElement el = findRealSubstitutionElement(elementToReplace);
     boolean listContext = isListContext(el);
 
@@ -298,21 +296,13 @@ public class JavaReplaceHandler extends StructuralReplaceHandler {
 
         if (replacement instanceof PsiTryStatement) {
           final List<PsiCatchSection> unmatchedCatchSections = el.getUserData(JavaMatchingVisitor.UNMATCHED_CATCH_SECTION_CONTENT_VAR_KEY);
-          final PsiCatchSection[] catches = ((PsiTryStatement)replacement).getCatchSections();
-
           if (unmatchedCatchSections != null) {
+            final PsiTryStatement tryStatement = (PsiTryStatement)replacement;
+            final PsiCatchSection[] catches = tryStatement.getCatchSections();
+            final PsiElement anchor = catches.length == 0 ? tryStatement.getTryBlock() : catches[catches.length - 1];
             for (int i = unmatchedCatchSections.size() - 1; i >= 0; --i) {
-              final PsiParameter parameter = unmatchedCatchSections.get(i).getParameter();
-              final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory();
-              final PsiCatchSection catchSection = elementFactory.createCatchSection(parameter.getType(), parameter.getName(), null);
-
-              catchSection.getCatchBlock().replace(
-                unmatchedCatchSections.get(i).getCatchBlock()
-              );
-              replacement.addAfter(
-                catchSection, catches[catches.length - 1]
-              );
-              replacement.addBefore(createWhiteSpace(replacement), replacement.getLastChild());
+              replacement.addAfter(unmatchedCatchSections.get(i), anchor);
+              replacement.addAfter(createWhiteSpace(replacement), anchor);
             }
           }
         }

java/structuralsearch-java/src/com/intellij/structuralsearch/impl/matcher/JavaMatchingVisitor.java

@@ -1437,7 +1437,7 @@ public class JavaMatchingVisitor extends JavaElementVisitor {
         myMatchingVisitor.setResult(myMatchingVisitor.matchSons(finally1, finally2));
       }
 
-      if (myMatchingVisitor.getResult() && unmatchedCatchSections.size() > 0 && !looseMatching) {
+      if (myMatchingVisitor.getResult() && unmatchedCatchSections.size() > 0) {
         try2.putUserData(UNMATCHED_CATCH_SECTION_CONTENT_VAR_KEY, unmatchedCatchSections);
       }
     }

platform/structuralsearch/testSource/com/intellij/structuralsearch/StructuralReplaceTest.java

@@ -17,7 +17,9 @@ public class StructuralReplaceTest extends StructuralReplaceTestCase {
   @Override
   protected void setUp() throws Exception {
     super.setUp();
-    options.getMatchOptions().setFileType(StdFileTypes.JAVA);
+    final MatchOptions matchOptions = this.options.getMatchOptions();
+    matchOptions.setFileType(StdFileTypes.JAVA);
+    matchOptions.setLooseMatching(true);
   }
 
   public void testReplaceInLiterals() {
@@ -449,13 +451,24 @@ public class StructuralReplaceTest extends StructuralReplaceTestCase {
                               "  aaa5();\n" +
                               "}\n";
 
-    actualResult = replacer.testReplace(s52,s53,s54,options);
+    options.getMatchOptions().setLooseMatching(false);
+    try {
+      actualResult = replacer.testReplace(s52, s53, s54, options);
+      assertEquals("Try/finally unwrapped with strict matching", expectedResult19, actualResult);
+    } finally {
+      options.getMatchOptions().setLooseMatching(true);
+    }
+
+    String expectedResult19Loose = "aaa();\n" +
+                                   "aaa2();\n" +
+                                   "try {\n" +
+                                   "  aaa4();\n" +
+                                   "} catch(Exception ex) {\n" +
+                                   "  aaa5();\n" +
+                                   "}\n";
+    actualResult = replacer.testReplace(s52, s53, s54, options);
+    assertEquals("Try/finally unwrapped with loose matching", expectedResult19Loose, actualResult);
 
-    assertEquals(
-      "Try/ catch/ finally is replace with try/finally",
-      expectedResult19,
-      actualResult
-    );
 
     String s55 = "for(Iterator<String> iterator = stringlist.iterator(); iterator.hasNext();) {\n" +
                  "      String str = iterator.next();\n" +
@@ -523,7 +536,7 @@ public class StructuralReplaceTest extends StructuralReplaceTestCase {
     String s61 = "try { 1=1; } catch(Exception e) { 1=1; } catch(Throwable t) { 2=2; }";
     String s62 = "try { '_a; } catch(Exception e) { '_b; }";
     String s63 = "try { $a$; } catch(Exception1 e) { $b$; } catch(Exception2 e) { $b$; }";
-    String expectedResult22 = "try { 1=1; } catch(Exception1 e) { 1=1; } catch(Exception2 e) { 1=1; } catch (Throwable t) { 2=2; }";
+    String expectedResult22 = "try { 1=1; } catch(Exception1 e) { 1=1; } catch(Exception2 e) { 1=1; } catch(Throwable t) { 2=2; }";
 
     actualResult = replacer.testReplace(s61,s62,s63,options);
 
@@ -1971,6 +1984,37 @@ public class StructuralReplaceTest extends StructuralReplaceTestCase {
       expected,
       actualResult
     );
+
+    final String in1 = "try {\n" +
+                       "  System.out.println(1);\n" +
+                       "} catch (RuntimeException e) {\n" +
+                       "  System.out.println(2);\n" +
+                       "} finally {\n" +
+                       "  System.out.println(3);\n" +
+                       "}\n";
+    final String what1 = "try {\n" +
+                         "  '_Statement1;\n" +
+                         "} finally {\n" +
+                         "  '_Statement2;\n" +
+                         "}";
+    final String by1 = "try {\n" +
+                       "  // comment1\n" +
+                       "  $Statement1$;\n" +
+                       "} finally {\n" +
+                       "  // comment2\n" +
+                       "  $Statement2$;\n" +
+                       "}";
+    final String expected1 = "try {\n" +
+                             "  // comment1\n" +
+                             "  System.out.println(1);\n" +
+                             "} catch (RuntimeException e) {\n" +
+                             "  System.out.println(2);\n" +
+                             "} finally {\n" +
+                             "  // comment2\n" +
+                             "  System.out.println(3);\n" +
+                             "}\n";
+    final String actualResult1 = replacer.testReplace(in1, what1, by1, options);
+    assertEquals("Replacing try/finally should leave unmatched catch sections alone", expected1, actualResult1);
   }
 
   public void testReplaceExtraSemicolon() {