From 113706d5006f7354310a0102280f39ce18eb69e3 Mon Sep 17 00:00:00 2001 From: Maxim Medvedev Date: Wed, 14 Jul 2010 15:39:36 +0400 Subject: [PATCH] IDEA-56460 'Convert to for-in' removes too much code --- .../intentions/closure/EachToForIntention.java | 9 +++++---- .../psi/api/statements/blocks/GrClosableBlock.java | 4 ++++ .../impl/statements/blocks/GrClosableBlockImpl.java | 11 ++++++++--- .../groovy/intentions/GrIntentionTestCase.java | 2 +- .../closure/eachToFor/EachToForIntentionTest.java | 12 ++++++++---- .../intentions/EachToFor/WithClosureInBody.groovy | 10 ++++++++++ .../EachToFor/WithClosureInBody_after.groovy | 10 ++++++++++ 7 files changed, 46 insertions(+), 12 deletions(-) create mode 100644 plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody.groovy create mode 100644 plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody_after.groovy diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/EachToForIntention.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/EachToForIntention.java index 17333c0a9fdb..9656453f486c 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/EachToForIntention.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/intentions/closure/EachToForIntention.java @@ -69,12 +69,13 @@ public class EachToForIntention extends Intention { StringBuilder builder = new StringBuilder(); builder.append("for (").append(var).append(" in ").append(qualifier.getText()).append(") {\n"); String text = block.getText(); - int index = text.indexOf("->"); - if (index == -1) { - index = 1; + final PsiElement blockArrow = block.getArrow(); + int index; + if (blockArrow != null) { + index = blockArrow.getStartOffsetInParent() + blockArrow.getTextLength(); } else { - index += 2; + index = 1; } while (index < text.length() && Character.isWhitespace(text.charAt(index))) index++; text = text.substring(index, text.length() - 1); diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/blocks/GrClosableBlock.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/blocks/GrClosableBlock.java index 1a11d3775c5f..66ad117e2e61 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/blocks/GrClosableBlock.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/api/statements/blocks/GrClosableBlock.java @@ -16,6 +16,7 @@ package org.jetbrains.plugins.groovy.lang.psi.api.statements.blocks; +import com.intellij.psi.PsiElement; import com.intellij.psi.PsiParameter; import com.intellij.psi.PsiType; import org.jetbrains.annotations.Nullable; @@ -44,4 +45,7 @@ public interface GrClosableBlock extends GrExpression, GrCodeBlock, GrParameters PsiType getReturnType(); PsiParameter[] getAllParameters(); + + @Nullable + PsiElement getArrow(); } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrClosableBlockImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrClosableBlockImpl.java index 3ca2ff237f3b..0aa8f7ae378e 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrClosableBlockImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/statements/blocks/GrClosableBlockImpl.java @@ -24,7 +24,6 @@ import com.intellij.util.Function; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.plugins.groovy.lang.lexer.GroovyTokenTypes; -import org.jetbrains.plugins.groovy.lang.parser.GroovyElementTypes; import org.jetbrains.plugins.groovy.lang.psi.GroovyElementVisitor; import org.jetbrains.plugins.groovy.lang.psi.GroovyFile; import org.jetbrains.plugins.groovy.lang.psi.GroovyPsiElement; @@ -114,13 +113,19 @@ public class GrClosableBlockImpl extends GrBlockImpl implements GrClosableBlock return new PsiParameter[]{getSyntheticItParameter()}; } + @Override + @Nullable + public PsiElement getArrow() { + return findChildByType(GroovyTokenTypes.mCLOSABLE_BLOCK_OP); + } + public GrParameterListImpl getParameterList() { return findChildByClass(GrParameterListImpl.class); } public void addParameter(GrParameter parameter) { GrParameterList parameterList = getParameterList(); - if (findChildByType(GroovyTokenTypes.mCLOSABLE_BLOCK_OP) == null) { + if (getArrow() == null) { ASTNode next = parameterList.getNode().getTreeNext(); getNode().addLeaf(GroovyTokenTypes.mCLOSABLE_BLOCK_OP, "->", next); getNode().addLeaf(GroovyTokenTypes.mNLS, "\n", next); @@ -130,7 +135,7 @@ public class GrClosableBlockImpl extends GrBlockImpl implements GrClosableBlock } public boolean hasParametersSection() { - return findChildByType(GroovyElementTypes.mCLOSABLE_BLOCK_OP) != null; + return getArrow() != null; } public PsiType getType() { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.java index 79838774e574..2c12cbc405dc 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/GrIntentionTestCase.java @@ -26,7 +26,7 @@ import java.util.List; * @author Maxim.Medvedev */ public abstract class GrIntentionTestCase extends LightCodeInsightFixtureTestCase { - protected void doTest(String hint, boolean intentionExists) throws Exception { + protected void doTest(String hint, boolean intentionExists) { myFixture.configureByFile(getTestName(false) + ".groovy"); final List list = myFixture.filterAvailableIntentions(hint); if (intentionExists) { diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/closure/eachToFor/EachToForIntentionTest.java b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/closure/eachToFor/EachToForIntentionTest.java index 7bfb56d79b0c..30e4ae8beb36 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/closure/eachToFor/EachToForIntentionTest.java +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/intentions/closure/eachToFor/EachToForIntentionTest.java @@ -28,19 +28,23 @@ public class EachToForIntentionTest extends GrIntentionTestCase { return TestUtils.getTestDataPath() + "intentions/EachToFor/"; } - public void testEachToFor() throws Throwable { + public void testEachToFor() { doTest("Replace with For-In", true); } - public void testEachToForWithFinal() throws Throwable { + public void testEachToForWithFinal() { doTest("Replace with For-In", true); } - public void testEachToForWithDefaultVariable() throws Throwable { + public void testEachToForWithDefaultVariable() { doTest("Replace with For-In", true); } - public void testEachForInWithNoQualifier () throws Throwable { + public void testEachForInWithNoQualifier() { + doTest("Replace with For-In", true); + } + + public void testWithClosureInBody() { doTest("Replace with For-In", true); } } diff --git a/plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody.groovy b/plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody.groovy new file mode 100644 index 000000000000..b5b7bb6c3aa5 --- /dev/null +++ b/plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody.groovy @@ -0,0 +1,10 @@ +[].each { + if (it == 2) { + println 2 + } + if (it == 3) { + println { String s -> + println s + } + } +} \ No newline at end of file diff --git a/plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody_after.groovy b/plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody_after.groovy new file mode 100644 index 000000000000..d63dbe71b6ce --- /dev/null +++ b/plugins/groovy/testdata/intentions/EachToFor/WithClosureInBody_after.groovy @@ -0,0 +1,10 @@ +for (it in []) { + if (it == 2) { + println 2 + } + if (it == 3) { + println { String s -> + println s + } + } +} \ No newline at end of file