From c6213bcc0454e6a69524a6e023c21f1779d2eb03 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 29 May 2015 18:24:08 +0200 Subject: [PATCH] SSR: keep unmatched resource lists and finally blocks when replacing try/catch statements --- .../structuralsearch/JavaReplaceHandler.java | 31 ++++++++++++++----- .../impl/matcher/JavaMatchingVisitor.java | 31 ++++++++++--------- .../impl/matcher/GlobalMatchingVisitor.java | 6 ++-- .../StructuralReplaceTest.java | 24 ++++++++++++++ 4 files changed, 68 insertions(+), 24 deletions(-) diff --git a/java/structuralsearch-java/src/com/intellij/structuralsearch/JavaReplaceHandler.java b/java/structuralsearch-java/src/com/intellij/structuralsearch/JavaReplaceHandler.java index 503e2c8e8ad6..f33acd22c66d 100644 --- a/java/structuralsearch-java/src/com/intellij/structuralsearch/JavaReplaceHandler.java +++ b/java/structuralsearch-java/src/com/intellij/structuralsearch/JavaReplaceHandler.java @@ -5,7 +5,8 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.javadoc.PsiDocComment; -import com.intellij.structuralsearch.impl.matcher.JavaMatchingVisitor; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.structuralsearch.impl.matcher.GlobalMatchingVisitor; import com.intellij.structuralsearch.impl.matcher.MatcherImplUtil; import com.intellij.structuralsearch.impl.matcher.PatternTreeContext; import com.intellij.structuralsearch.plugin.replace.ReplaceOptions; @@ -282,14 +283,24 @@ public class JavaReplaceHandler extends StructuralReplaceHandler { replacement = handleSymbolReplacement(replacement, el); if (replacement instanceof PsiTryStatement) { - final List unmatchedCatchSections = el.getUserData(JavaMatchingVisitor.UNMATCHED_CATCH_SECTION_CONTENT_VAR_KEY); - if (unmatchedCatchSections != null) { - final PsiTryStatement tryStatement = (PsiTryStatement)replacement; + final PsiTryStatement tryStatement = (PsiTryStatement)replacement; + final List unmatchedElements = el.getUserData(GlobalMatchingVisitor.UNMATCHED_ELEMENTS_KEY); + if (unmatchedElements != null) { + final PsiElement firstElement = unmatchedElements.get(0); + if (firstElement instanceof PsiResourceList) addElementAfterAnchor(tryStatement, firstElement, tryStatement.getFirstChild()); 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) { - replacement.addAfter(unmatchedCatchSections.get(i), anchor); - replacement.addAfter(createWhiteSpace(replacement), anchor); + for (int i = unmatchedElements.size() - 1; i >= 0; i--) { + final PsiElement element = unmatchedElements.get(i); + if ((element instanceof PsiCatchSection)) addElementAfterAnchor(tryStatement, element, anchor); + } + final PsiElement lastElement = unmatchedElements.get(unmatchedElements.size() - 1); + if (lastElement instanceof PsiCodeBlock) { + final PsiElement finallyKeyword = PsiTreeUtil.skipSiblingsBackward(lastElement, PsiWhiteSpace.class); + assert finallyKeyword != null; + final PsiElement finallyAnchor = tryStatement.getLastChild(); + addElementAfterAnchor(tryStatement, lastElement, finallyAnchor); + addElementAfterAnchor(tryStatement, finallyKeyword, finallyAnchor); } } } @@ -436,6 +447,12 @@ public class JavaReplaceHandler extends StructuralReplaceHandler { } } + private static void addElementAfterAnchor(PsiElement parentElement, PsiElement element, PsiElement anchor) { + parentElement.addAfter(element, anchor); + final PsiElement sibling = element.getPrevSibling(); + if (sibling instanceof PsiWhiteSpace) parentElement.addAfter(sibling, anchor); // recycle whitespace + } + @Override public void postProcess(PsiElement affectedElement, ReplaceOptions options) { if (!affectedElement.isValid()) { diff --git a/java/structuralsearch-java/src/com/intellij/structuralsearch/impl/matcher/JavaMatchingVisitor.java b/java/structuralsearch-java/src/com/intellij/structuralsearch/impl/matcher/JavaMatchingVisitor.java index 40464e1d1d3a..d466f7525130 100644 --- a/java/structuralsearch-java/src/com/intellij/structuralsearch/impl/matcher/JavaMatchingVisitor.java +++ b/java/structuralsearch-java/src/com/intellij/structuralsearch/impl/matcher/JavaMatchingVisitor.java @@ -2,7 +2,6 @@ package com.intellij.structuralsearch.impl.matcher; import com.intellij.dupLocator.iterators.ArrayBackedNodeIterator; import com.intellij.dupLocator.iterators.NodeIterator; -import com.intellij.openapi.util.Key; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.javadoc.PsiDocComment; @@ -34,7 +33,6 @@ public class JavaMatchingVisitor extends JavaElementVisitor { PsiModifier.PUBLIC, PsiModifier.PROTECTED, PsiModifier.PRIVATE, PsiModifier.STATIC, PsiModifier.ABSTRACT, PsiModifier.FINAL, PsiModifier.NATIVE, PsiModifier.SYNCHRONIZED, PsiModifier.STRICTFP, PsiModifier.TRANSIENT, PsiModifier.VOLATILE, PsiModifier.DEFAULT }; - public static final Key> UNMATCHED_CATCH_SECTION_CONTENT_VAR_KEY = Key.create("UnmatchedCatchSection"); private final GlobalMatchingVisitor myMatchingVisitor; private PsiClass myClazz; @@ -1334,7 +1332,6 @@ public class JavaMatchingVisitor extends JavaElementVisitor { final PsiTryStatement try2 = (PsiTryStatement)myMatchingVisitor.getElement(); myMatchingVisitor.setResult(myMatchingVisitor.matchSons(try1.getTryBlock(), try2.getTryBlock())); - if (!myMatchingVisitor.getResult()) return; final PsiResourceList resourceList1 = try1.getResourceList(); @@ -1354,6 +1351,8 @@ public class JavaMatchingVisitor extends JavaElementVisitor { myMatchingVisitor.setResult(false); } else { + final List unmatchedElements = new ArrayList(); + if (resourceList1 != null) { if (resourceList2 == null) { myMatchingVisitor.setResult(false); @@ -1366,11 +1365,11 @@ public class JavaMatchingVisitor extends JavaElementVisitor { resourceVariables2.toArray(new PsiResourceVariable[resourceVariables2.size()]))); if (!myMatchingVisitor.getResult()) return; } + else if (resourceList2 != null){ + unmatchedElements.add(resourceList2); + } - final List unmatchedCatchSections = new ArrayList(); - - ContainerUtil.addAll(unmatchedCatchSections, catches2); - + ContainerUtil.addAll(unmatchedElements, catches2); for (PsiCatchSection catchSection : catches1) { final MatchingHandler handler = myMatchingVisitor.getMatchContext().getPattern().getHandler(catchSection); final PsiElement pinnedNode = handler.getPinnedNode(null); @@ -1380,15 +1379,15 @@ public class JavaMatchingVisitor extends JavaElementVisitor { if (!myMatchingVisitor.getResult()) return; } else { - int j; - for (j = 0; j < unmatchedCatchSections.size(); ++j) { - if (handler.match(catchSection, unmatchedCatchSections.get(j), myMatchingVisitor.getMatchContext())) { - unmatchedCatchSections.remove(j); + boolean matched = false; + for (int j = 0; j < unmatchedElements.size(); ++j) { + if (handler.match(catchSection, unmatchedElements.get(j), myMatchingVisitor.getMatchContext())) { + unmatchedElements.remove(j); + matched = true; break; } } - - if (j == catches2.length) { + if (!matched) { myMatchingVisitor.setResult(false); return; } @@ -1397,10 +1396,12 @@ public class JavaMatchingVisitor extends JavaElementVisitor { if (finally1 != null) { myMatchingVisitor.setResult(myMatchingVisitor.matchSons(finally1, finally2)); + } else if (finally2 != null) { + unmatchedElements.add(finally2); } - if (myMatchingVisitor.getResult() && unmatchedCatchSections.size() > 0) { - try2.putUserData(UNMATCHED_CATCH_SECTION_CONTENT_VAR_KEY, unmatchedCatchSections); + if (myMatchingVisitor.getResult() && unmatchedElements.size() > 0) { + try2.putUserData(GlobalMatchingVisitor.UNMATCHED_ELEMENTS_KEY, unmatchedElements); } } } diff --git a/platform/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/GlobalMatchingVisitor.java b/platform/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/GlobalMatchingVisitor.java index 36e3aec89bee..50e64b6457b9 100644 --- a/platform/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/GlobalMatchingVisitor.java +++ b/platform/structuralsearch/source/com/intellij/structuralsearch/impl/matcher/GlobalMatchingVisitor.java @@ -5,6 +5,7 @@ import com.intellij.dupLocator.iterators.NodeIterator; import com.intellij.dupLocator.util.NodeFilter; import com.intellij.lang.Language; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.Key; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.intellij.structuralsearch.MatchResult; @@ -29,6 +30,7 @@ import java.util.Map; @SuppressWarnings({"RefusedBequest"}) public class GlobalMatchingVisitor extends AbstractMatchingVisitor { private static final Logger LOG = Logger.getInstance("#com.intellij.structuralsearch.impl.matcher.GlobalMatchingVisitor"); + public static final Key> UNMATCHED_ELEMENTS_KEY = Key.create("UnmatchedElements"); // the pattern element for visitor check private PsiElement myElement; @@ -159,7 +161,7 @@ public class GlobalMatchingVisitor extends AbstractMatchingVisitor { */ public boolean matchSequentially(NodeIterator nodes, NodeIterator nodes2) { if (!nodes.hasNext()) { - return nodes.hasNext() == nodes2.hasNext(); + return !nodes2.hasNext(); } return matchContext.getPattern().getHandler(nodes.current()).matchSequentially( @@ -171,7 +173,7 @@ public class GlobalMatchingVisitor extends AbstractMatchingVisitor { public static boolean continueMatchingSequentially(final NodeIterator nodes, final NodeIterator nodes2, MatchContext matchContext) { if (!nodes.hasNext()) { - return nodes.hasNext() == nodes2.hasNext(); + return !nodes2.hasNext(); } return matchContext.getPattern().getHandler(nodes.current()).matchSequentially( diff --git a/platform/structuralsearch/testSource/com/intellij/structuralsearch/StructuralReplaceTest.java b/platform/structuralsearch/testSource/com/intellij/structuralsearch/StructuralReplaceTest.java index 63b095e70640..7ec889468374 100644 --- a/platform/structuralsearch/testSource/com/intellij/structuralsearch/StructuralReplaceTest.java +++ b/platform/structuralsearch/testSource/com/intellij/structuralsearch/StructuralReplaceTest.java @@ -2015,6 +2015,30 @@ public class StructuralReplaceTest extends StructuralReplaceTestCase { "}\n"; final String actualResult1 = replacer.testReplace(in1, what1, by1, options); assertEquals("Replacing try/finally should leave unmatched catch sections alone", expected1, actualResult1); + + final String in2 = "try (AutoCloseable a = null) {" + + " System.out.println(1);" + + "} catch (Exception e) {" + + " System.out.println(2);" + + "} finally {" + + " System.out.println(3);" + + "}"; + final String what2 = "try {" + + " '_Statement*;" + + "}"; + final String by2 = "try {" + + " /* comment */" + + " $Statement$;" + + "}"; + final String expected2 = "try (AutoCloseable a = null) {" + + " /* comment */ System.out.println(1);" + + "} catch (Exception e) {" + + " System.out.println(2);" + + "} finally {" + + " System.out.println(3);" + + "}"; + final String actualResult2 = replacer.testReplace(in2, what2, by2, options); + assertEquals("Replacing try/finally should also keep unmatched resource lists and finally blocks", expected2, actualResult2); } public void testReplaceExtraSemicolon() {