IDEA-179128 Quick-fix for "'if' statement has empty body"

This commit is contained in:
Tagir Valeev
2017-09-19 13:04:21 +07:00
parent 271d62ad19
commit 36999ebfc4
20 changed files with 208 additions and 28 deletions

View File

@@ -16,28 +16,33 @@
package com.intellij.codeInsight.daemon.impl.quickfix;
import com.intellij.codeInsight.daemon.QuickFixBundle;
import com.intellij.codeInsight.intention.IntentionAction;
import com.intellij.codeInsight.intention.LowPriorityAction;
import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement;
import com.intellij.openapi.editor.Editor;
import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.IncorrectOperationException;
import com.siyeh.ig.psiutils.BlockUtils;
import com.siyeh.ig.psiutils.SideEffectChecker;
import com.siyeh.ig.psiutils.StatementExtractor;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.List;
import java.util.Objects;
public class DeleteSideEffectsAwareFix implements IntentionAction, LowPriorityAction {
private final SmartPsiElementPointer<PsiExpressionStatement> myPointer;
public class DeleteSideEffectsAwareFix extends LocalQuickFixAndIntentionActionOnPsiElement implements LowPriorityAction {
private final SmartPsiElementPointer<PsiStatement> myStatementPtr;
private final SmartPsiElementPointer<PsiExpression> myExpressionPtr;
private final String myMessage;
public DeleteSideEffectsAwareFix(PsiExpressionStatement statement) {
myPointer = SmartPointerManager.getInstance(statement.getProject()).createSmartPsiElementPointer(statement);
PsiExpression expression = statement.getExpression();
public DeleteSideEffectsAwareFix(@NotNull PsiStatement statement, PsiExpression expression) {
super(statement);
SmartPointerManager manager = SmartPointerManager.getInstance(statement.getProject());
myStatementPtr = manager.createSmartPsiElementPointer(statement);
myExpressionPtr = manager.createSmartPsiElementPointer(expression);
List<PsiExpression> sideEffects = SideEffectChecker.extractSideEffectExpressions(expression);
if (sideEffects.isEmpty()) {
myMessage = QuickFixBundle.message("delete.element.fix.text");
@@ -72,19 +77,28 @@ public class DeleteSideEffectsAwareFix implements IntentionAction, LowPriorityAc
}
@Override
public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
public boolean isAvailable(@NotNull Project project,
@NotNull PsiFile file,
@NotNull PsiElement startElement,
@NotNull PsiElement endElement) {
return !myMessage.isEmpty();
}
@Override
public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException {
PsiExpressionStatement statement = myPointer.getElement();
public void invoke(@NotNull Project project,
@NotNull PsiFile file,
@Nullable Editor editor,
@NotNull PsiElement startElement,
@NotNull PsiElement endElement) {
PsiStatement statement = myStatementPtr.getElement();
if (statement == null) return;
PsiExpression expression = statement.getExpression();
PsiExpression expression = myExpressionPtr.getElement();
if (expression == null) return;
List<PsiExpression> sideEffects = SideEffectChecker.extractSideEffectExpressions(expression);
PsiStatement[] statements = StatementExtractor.generateStatements(sideEffects, expression);
if (statements.length > 0) {
BlockUtils.addBefore(statement, statements);
PsiStatement lastAdded = BlockUtils.addBefore(statement, statements);
statement = Objects.requireNonNull(PsiTreeUtil.getNextSiblingOfType(lastAdded, PsiStatement.class));
}
statement.delete();
}

View File

@@ -893,6 +893,6 @@ public class QuickFixFactoryImpl extends QuickFixFactory {
@NotNull
@Override
public IntentionAction createDeleteSideEffectAwareFix(@NotNull PsiExpressionStatement statement) {
return new DeleteSideEffectsAwareFix(statement);
return new DeleteSideEffectsAwareFix(statement, statement.getExpression());
}
}

View File

@@ -1,4 +1,4 @@
// "Convert to 'if' statement" "true"
// "Extract side effects as an 'if' statement" "true"
import java.io.File;
public class Main {

View File

@@ -1,4 +1,4 @@
// "Convert to 'if' statement" "true"
// "Extract side effects as an 'if' statement" "true"
import java.io.File;
public class Main {

View File

@@ -0,0 +1,5 @@
// "Delete element" "true"
class Test {
void test(int x) {
}
}

View File

@@ -0,0 +1,8 @@
// "Delete element" "true"
class Test {
void test(int x) {
if(x > -5) {
System.out.println("ok");
}
}
}

View File

@@ -0,0 +1,16 @@
// "Extract side effects as an 'if' statement" "true"
class Test {
void test(int x) {
if (x > 0) {
foo(x, 1);
foo(x, 2);
} else {
foo(x, 3);
}
}
boolean foo(int x, int y) {
System.out.println(x);
return x > y;
}
}

View File

@@ -0,0 +1,14 @@
// "Extract side effects" "true"
class Test {
void test(int x) {
if(x > 0) {
foo(x, 1);
foo(x, 2);
}
}
boolean foo(int x, int y) {
System.out.println(x);
return x > y;
}
}

View File

@@ -0,0 +1,5 @@
// "Delete element" "true"
class Test {
void test(int x) {
}
}

View File

@@ -0,0 +1,6 @@
// "Delete element" "true"
class Test {
void test(int x) {
i<caret>f(x > 0) {}
}
}

View File

@@ -0,0 +1,8 @@
// "Delete element" "true"
class Test {
void test(int x) {
if(x > -5) {
System.out.println("ok");
} else i<caret>f(x > 0) {} else {}
}
}

View File

@@ -0,0 +1,10 @@
// "Delete element" "false"
class Test {
void test(int x) {
if(x > -5) {
System.out.println("ok");
} else i<caret>f(x > 0) {} else {
System.out.println("not ok");
}
}
}

View File

@@ -0,0 +1,13 @@
// "Extract side effects as an 'if' statement" "true"
class Test {
void test(int x) {
i<caret>f(x > 0 ? !foo(x, 1) ^ foo(x, 2) : foo(x, 3)) {
}
}
boolean foo(int x, int y) {
System.out.println(x);
return x > y;
}
}

View File

@@ -0,0 +1,14 @@
// "Extract side effects" "true"
class Test {
void test(int x) {
if(x > 0)
i<caret>f(!foo(x, 1) ^ foo(x, 2)) {
}
}
boolean foo(int x, int y) {
System.out.println(x);
return x > y;
}
}

View File

@@ -0,0 +1,6 @@
// "Delete element" "true"
class Test {
void test(int x) {
s<caret>witch(x*2) {}
}
}

View File

@@ -18,6 +18,7 @@ package com.siyeh.ig;
import com.intellij.codeInsight.daemon.HighlightDisplayKey;
import com.intellij.codeInspection.BaseJavaBatchLocalInspectionTool;
import com.intellij.codeInspection.InspectionProfileEntry;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ProblemsHolder;
import com.intellij.codeInspection.ex.InspectionProfileImpl;
import com.intellij.openapi.util.DefaultJDOMExternalizer;
@@ -81,12 +82,12 @@ public abstract class BaseInspection extends BaseJavaBatchLocalInspectionTool {
}
@Nullable
protected InspectionGadgetsFix buildFix(Object... infos) {
protected LocalQuickFix buildFix(Object... infos) {
return null;
}
@NotNull
protected InspectionGadgetsFix[] buildFixes(Object... infos) {
protected LocalQuickFix[] buildFixes(Object... infos) {
return InspectionGadgetsFix.EMPTY_ARRAY;
}

View File

@@ -191,27 +191,29 @@ public abstract class BaseInspectionVisitor extends JavaElementVisitor {
@NotNull
private LocalQuickFix[] createAndInitFixes(Object[] infos) {
final InspectionGadgetsFix[] fixes = createFixes(infos);
for (InspectionGadgetsFix fix : fixes) {
fix.setOnTheFly(onTheFly);
final LocalQuickFix[] fixes = createFixes(infos);
for (LocalQuickFix fix : fixes) {
if(fix instanceof InspectionGadgetsFix) {
((InspectionGadgetsFix)fix).setOnTheFly(onTheFly);
}
}
return fixes;
}
@NotNull
private InspectionGadgetsFix[] createFixes(Object... infos) {
private LocalQuickFix[] createFixes(Object... infos) {
if (!onTheFly && inspection.buildQuickFixesOnlyForOnTheFlyErrors()) {
return InspectionGadgetsFix.EMPTY_ARRAY;
}
final InspectionGadgetsFix[] fixes = inspection.buildFixes(infos);
final LocalQuickFix[] fixes = inspection.buildFixes(infos);
if (fixes.length > 0) {
return fixes;
}
final InspectionGadgetsFix fix = inspection.buildFix(infos);
final LocalQuickFix fix = inspection.buildFix(infos);
if (fix == null) {
return InspectionGadgetsFix.EMPTY_ARRAY;
}
return new InspectionGadgetsFix[]{fix};
return new LocalQuickFix[]{fix};
}
@Override

View File

@@ -15,16 +15,23 @@
*/
package com.siyeh.ig.bugs;
import com.intellij.codeInsight.daemon.impl.quickfix.DeleteElementFix;
import com.intellij.codeInsight.daemon.impl.quickfix.DeleteSideEffectsAwareFix;
import com.intellij.codeInspection.LocalQuickFix;
import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel;
import com.intellij.openapi.util.WriteExternalException;
import com.intellij.psi.*;
import com.intellij.psi.util.FileTypeUtils;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.ObjectUtils;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import org.intellij.lang.annotations.Pattern;
import org.jdom.Element;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
@@ -44,6 +51,7 @@ public class EmptyStatementBodyInspection extends BaseInspection {
}
}
@Pattern(VALID_ID_PATTERN)
@Override
@NotNull
public String getID() {
@@ -80,6 +88,12 @@ public class EmptyStatementBodyInspection extends BaseInspection {
return !FileTypeUtils.isInServerPageFile(file);
}
@Nullable
@Override
protected LocalQuickFix buildFix(Object... infos) {
return ObjectUtils.tryCast(ArrayUtil.getFirstElement(infos), LocalQuickFix.class);
}
@Override
public BaseInspectionVisitor buildVisitor() {
return new EmptyStatementVisitor();
@@ -123,11 +137,12 @@ public class EmptyStatementBodyInspection extends BaseInspection {
public void visitIfStatement(@NotNull PsiIfStatement statement) {
super.visitIfStatement(statement);
final PsiStatement thenBranch = statement.getThenBranch();
final PsiStatement elseBranch = statement.getElseBranch();
if (thenBranch != null && isEmpty(thenBranch)) {
registerStatementError(statement);
LocalQuickFix fix = elseBranch == null || isEmpty(elseBranch) ? createFix(statement, statement.getCondition()) : null;
registerStatementError(statement, fix);
return;
}
final PsiStatement elseBranch = statement.getElseBranch();
if (elseBranch != null && isEmpty(elseBranch)) {
final PsiElement elseToken = statement.getElseElement();
if (elseToken == null) {
@@ -144,7 +159,15 @@ public class EmptyStatementBodyInspection extends BaseInspection {
if (body == null || !isEmpty(body)) {
return;
}
registerStatementError(statement);
registerStatementError(statement, createFix(statement, statement.getExpression()));
}
@NotNull
private LocalQuickFix createFix(@NotNull PsiStatement statement, PsiExpression expression) {
if (expression == null) {
return new DeleteElementFix(statement);
}
return new DeleteSideEffectsAwareFix(statement, expression);
}
private boolean isEmpty(PsiElement element) {

View File

@@ -0,0 +1,35 @@
/*
* Copyright 2000-2017 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.siyeh.ig.bugs;
import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase;
import com.intellij.codeInspection.LocalInspectionTool;
import org.jetbrains.annotations.NotNull;
public class EmptyStatementBodyInspectionFixTest extends LightQuickFixParameterizedTestCase {
public void test() { doAllTests(); }
@NotNull
@Override
protected LocalInspectionTool[] configureLocalInspectionTools() {
return new LocalInspectionTool[] {new EmptyStatementBodyInspection()};
}
@Override
protected String getBasePath() {
return "/codeInsight/daemonCodeAnalyzer/quickFix/emptyStatement";
}
}

View File

@@ -300,7 +300,7 @@ delete.element.fix.text=Delete element
delete.reference.fix.text=Delete reference
delete.unreachable.statement.fix.text=Delete unreachable statement
extract.side.effects.convert.to.if=Convert to 'if' statement
extract.side.effects.convert.to.if=Extract side effects as an 'if' statement
extract.side.effects=Extract side {0, choice, 1#effect|2#effects}
extract.side.effects.family.name=Delete statement extracting side effects