From b525e80a879c866d514b4b092a8d86a1a3551615 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 20 Jul 2016 18:50:05 +0300 Subject: [PATCH] Java inspection: Returned back RemoveRedundantElseAction intention, because it turns out it's useful in some cases (IDEA-157727) --- .../quickfix/RemoveRedundantElseAction.java | 109 ++++++++++++++++++ .../quickFix/removeRedundantElse/after1.java | 13 +++ .../quickFix/removeRedundantElse/before1.java | 14 +++ .../beforeCanThrowException.java | 15 +++ .../beforeIfElseChain.java | 17 +++ .../RemoveRedundantElseActionTest.java | 32 +++++ .../after.java.template | 8 ++ .../before.java.template | 10 ++ .../description.html | 7 ++ resources/src/META-INF/IdeaPlugin.xml | 4 + 10 files changed, 229 insertions(+) create mode 100644 java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveRedundantElseAction.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/after1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/before1.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeCanThrowException.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeIfElseChain.java create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseActionTest.java create mode 100644 resources-en/src/intentionDescriptions/RemoveRedundantElseAction/after.java.template create mode 100644 resources-en/src/intentionDescriptions/RemoveRedundantElseAction/before.java.template create mode 100644 resources-en/src/intentionDescriptions/RemoveRedundantElseAction/description.html diff --git a/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveRedundantElseAction.java b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveRedundantElseAction.java new file mode 100644 index 000000000000..28db235e1e1b --- /dev/null +++ b/java/java-impl/src/com/intellij/codeInsight/daemon/impl/quickfix/RemoveRedundantElseAction.java @@ -0,0 +1,109 @@ +/* + * Copyright 2000-2009 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInsight.daemon.impl.quickfix; + +import com.intellij.codeInsight.FileModificationService; +import com.intellij.codeInsight.daemon.QuickFixBundle; +import com.intellij.codeInsight.intention.PsiElementBaseIntentionAction; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.controlFlow.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.BitUtil; +import com.intellij.util.IncorrectOperationException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * @author ven + */ +public class RemoveRedundantElseAction extends PsiElementBaseIntentionAction { + private static final Logger LOG = Logger.getInstance("#com.intellij.codeInsight.daemon.impl.quickfix.RemoveRedundantElseAction"); + + @Override + @NotNull + public String getText() { + return QuickFixBundle.message("remove.redundant.else.fix"); + } + + @Override + @NotNull + public String getFamilyName() { + return QuickFixBundle.message("remove.redundant.else.fix"); + } + + @Override + public boolean isAvailable(@NotNull Project project, Editor editor, @NotNull PsiElement element) { + if (element instanceof PsiKeyword && + element.getParent() instanceof PsiIfStatement && + PsiKeyword.ELSE.equals(element.getText())) { + PsiIfStatement ifStatement = (PsiIfStatement)element.getParent(); + if (ifStatement.getElseBranch() == null) return false; + PsiStatement thenBranch = ifStatement.getThenBranch(); + if (thenBranch == null) return false; + PsiElement block = PsiTreeUtil.getParentOfType(ifStatement, PsiCodeBlock.class); + if (block != null) { + while (cantCompleteNormally(thenBranch, block)) { + thenBranch = getPrevThenBranch(thenBranch); + if (thenBranch == null) return true; + } + return false; + } + } + return false; + } + + @Nullable + private static PsiStatement getPrevThenBranch(@NotNull PsiElement thenBranch) { + final PsiElement ifStatement = thenBranch.getParent(); + final PsiElement parent = ifStatement.getParent(); + if (parent instanceof PsiIfStatement && ((PsiIfStatement)parent).getElseBranch() == ifStatement) { + return ((PsiIfStatement)parent).getThenBranch(); + } + return null; + } + + private static boolean cantCompleteNormally(@NotNull PsiStatement thenBranch, PsiElement block) { + try { + ControlFlow controlFlow = ControlFlowFactory.getInstance(thenBranch.getProject()).getControlFlow(block, LocalsOrMyInstanceFieldsControlFlowPolicy.getInstance()); + int startOffset = controlFlow.getStartOffset(thenBranch); + int endOffset = controlFlow.getEndOffset(thenBranch); + return startOffset != -1 && endOffset != -1 && !BitUtil.isSet(ControlFlowUtil.getCompletionReasons(controlFlow, startOffset, endOffset), ControlFlowUtil.NORMAL_COMPLETION_REASON); + } + catch (AnalysisCanceledException e) { + return false; + } + } + + @Override + public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException { + if (!FileModificationService.getInstance().preparePsiElementForWrite(element)) return; + PsiIfStatement ifStatement = (PsiIfStatement)element.getParent(); + LOG.assertTrue(ifStatement != null && ifStatement.getElseBranch() != null); + PsiStatement elseBranch = ifStatement.getElseBranch(); + if (elseBranch instanceof PsiBlockStatement) { + PsiElement[] statements = ((PsiBlockStatement)elseBranch).getCodeBlock().getStatements(); + if (statements.length > 0) { + ifStatement.getParent().addRangeAfter(statements[0], statements[statements.length-1], ifStatement); + } + } else { + ifStatement.getParent().addAfter(elseBranch, ifStatement); + } + ifStatement.getElseBranch().delete(); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/after1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/after1.java new file mode 100644 index 000000000000..d0d55761f11b --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/after1.java @@ -0,0 +1,13 @@ +// "Remove redundant 'else'" "true" +class a { + void foo() { + int a = 0; + int b = 0; + if (a != b) { + return; + } + a = b; + a++; + } +} + diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/before1.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/before1.java new file mode 100644 index 000000000000..ea4609835464 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/before1.java @@ -0,0 +1,14 @@ +// "Remove redundant 'else'" "true" +class a { + void foo() { + int a = 0; + int b = 0; + if (a != b) { + return; + } else { + a = b; + } + a++; + } +} + diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeCanThrowException.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeCanThrowException.java new file mode 100644 index 000000000000..8fd59ef094f3 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeCanThrowException.java @@ -0,0 +1,15 @@ +// "Remove redundant 'else'" "false" +import java.io.IOException; +class a { + void foo(boolean condition) throws IOException{ + if (condition) { + tMethod(); + } + else { + System.out.println("else"); + } + } + + void tMethod() throws IOException {} +} + diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeIfElseChain.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeIfElseChain.java new file mode 100644 index 000000000000..f702178d781f --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse/beforeIfElseChain.java @@ -0,0 +1,17 @@ +// "Remove redundant 'else'" "false" +class a { + void foo() { + int a = 0; + int b = 0; + if (a != b) { + a = 10; + } else if (a + 1 == b) { + return; + } + else { + a = b; + } + a++; + } +} + diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseActionTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseActionTest.java new file mode 100644 index 000000000000..0a33ada748f8 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/quickFix/RemoveRedundantElseActionTest.java @@ -0,0 +1,32 @@ +/* + * Copyright 2000-2010 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInsight.daemon.quickFix; + +/** + * User: anna + * Date: Aug 30, 2010 + */ +public class RemoveRedundantElseActionTest extends LightQuickFixParameterizedTestCase { + + public void test() throws Exception { doAllTests(); } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/quickFix/removeRedundantElse"; + } + +} + diff --git a/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/after.java.template b/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/after.java.template new file mode 100644 index 000000000000..6bbd9d56135a --- /dev/null +++ b/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/after.java.template @@ -0,0 +1,8 @@ +public class X { + void f(int i) { + if (i==0) { + return; + } + int j = 0; + } +} \ No newline at end of file diff --git a/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/before.java.template b/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/before.java.template new file mode 100644 index 000000000000..edad99d14f45 --- /dev/null +++ b/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/before.java.template @@ -0,0 +1,10 @@ +public class X { + void f(int i) { + if (i==0) { + return; + } + else { + int j = 0; + } + } +} \ No newline at end of file diff --git a/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/description.html b/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/description.html new file mode 100644 index 000000000000..8992a7b29b2c --- /dev/null +++ b/resources-en/src/intentionDescriptions/RemoveRedundantElseAction/description.html @@ -0,0 +1,7 @@ + + +This intention detaches else clause from the if statement, +if corresponding then clause never completes normally. + + + diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index ad80bf08d27d..02c0b3840c5c 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -832,6 +832,10 @@ com.intellij.codeInsight.intention.impl.ExtractIfConditionAction Java/Control Flow + + com.intellij.codeInsight.daemon.impl.quickfix.RemoveRedundantElseAction + Java/Control Flow + com.intellij.codeInsight.intention.impl.AddNotNullAnnotationIntention Java/Annotations