MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-09-27 10:03:11 +07:00
Code Issues Packages Projects Releases Wiki Activity

IDEA-175650 Simplify inspection breaks logic

Browse Source
This commit is contained in:
Tagir Valeev
2017-07-19 20:07:12 +07:00
parent 151955979f
commit 80744848c8
8 changed files with 244 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-impl/src/com/intellij/codeInspection/streamToLoop/StreamToLoopInspection.java
+2 -41
View File
@@ -85,7 +85,7 @@ public class StreamToLoopInspection extends BaseJavaBatchLocalInspectionTool {
super.visitMethodCallExpression(call);
PsiReferenceExpression expression = call.getMethodExpression();
PsiElement nameElement = expression.getReferenceNameElement();
if (nameElement == null || !SUPPORTED_TERMINALS.contains(nameElement.getText()) || !isSupportedCodeLocation(call)) return;
if (nameElement == null || !SUPPORTED_TERMINALS.contains(nameElement.getText()) || !ControlFlowUtils.canExtractStatement(call)) return;
PsiMethod method = call.resolveMethod();
if(method == null) return;
PsiClass aClass = method.getContainingClass();
@@ -112,45 +112,6 @@ public class StreamToLoopInspection extends BaseJavaBatchLocalInspectionTool {
};
}
private static boolean isSupportedCodeLocation(PsiMethodCallExpression call) {
PsiElement cur = call;
PsiElement parent = cur.getParent();
while(parent instanceof PsiExpression || parent instanceof PsiExpressionList) {
if(parent instanceof PsiLambdaExpression) {
return true;
}
if(parent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)parent;
IElementType type = polyadicExpression.getOperationTokenType();
if ((type.equals(JavaTokenType.ANDAND) || type.equals(JavaTokenType.OROR)) && polyadicExpression.getOperands()[0] != cur) {
// not the first in the &&/|| chain: we cannot properly generate code which would short-circuit as well
return false;
}
}
if(parent instanceof PsiConditionalExpression && ((PsiConditionalExpression)parent).getCondition() != cur) {
return false;
}
if(parent instanceof PsiMethodCallExpression) {
PsiReferenceExpression methodExpression = ((PsiMethodCallExpression)parent).getMethodExpression();
if(methodExpression.textMatches("this") || methodExpression.textMatches("super")) {
return false;
}
}
cur = parent;
parent = cur.getParent();
}
if(parent instanceof PsiReturnStatement || parent instanceof PsiExpressionStatement) return true;
if(parent instanceof PsiLocalVariable) {
PsiElement grandParent = parent.getParent();
if(grandParent instanceof PsiDeclarationStatement && ((PsiDeclarationStatement)grandParent).getDeclaredElements().length == 1) {
return true;
}
}
if(parent instanceof PsiForeachStatement && ((PsiForeachStatement)parent).getIteratedValue() == cur) return true;
if(parent instanceof PsiIfStatement && ((PsiIfStatement)parent).getCondition() == cur) return true;
return false;
}
@Nullable
static Operation createOperationFromCall(StreamVariable outVar, PsiMethodCallExpression call, boolean supportUnknownSources) {
PsiMethod method = call.resolveMethod();
@@ -304,7 +265,7 @@ public class StreamToLoopInspection extends BaseJavaBatchLocalInspectionTool {
PsiElement element = descriptor.getStartElement();
if(!(element instanceof PsiMethodCallExpression)) return;
PsiMethodCallExpression terminalCall = (PsiMethodCallExpression)element;
if(!isSupportedCodeLocation(terminalCall)) return;
if(!ControlFlowUtils.canExtractStatement(terminalCall)) return;
PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
terminalCall = RefactoringUtil.ensureCodeBlock(terminalCall);
if (terminalCall == null) return;
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties
+1
View File
@@ -1138,6 +1138,7 @@ conditional.expression.with.identical.branches.collapse.quickfix=Collapse condit
redundant.else.unwrap.quickfix=Remove redundant 'else'
constant.conditional.expression.problem.descriptor=<code>#ref</code> can be simplified to ''{0}'' #loc
constant.conditional.expression.simplify.quickfix=Simplify
constant.conditional.expression.simplify.quickfix.sideEffect=Extract side effects and simplify
enum.switch.statement.which.misses.cases.problem.descriptor=<code>#ref</code> statement on enumerated type ''{0}'' misses cases #loc
for.loop.replaceable.by.while.ignore.option=Ignore 'infinite' for loops without conditions
for.loop.replaceable.by.while.replace.quickfix=Replace with 'while'
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ControlFlowUtils.java
+44
View File
@@ -18,6 +18,7 @@ package com.siyeh.ig.psiutils;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.*;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ArrayUtil;
@@ -868,6 +869,49 @@ public class ControlFlowUtils {
return false;
}
/**
* @param expression expression to check
* @return true if given expression can be converted to statement without semantics change
*/
public static boolean canExtractStatement(PsiExpression expression) {
PsiElement cur = expression;
PsiElement parent = cur.getParent();
while(parent instanceof PsiExpression || parent instanceof PsiExpressionList) {
if(parent instanceof PsiLambdaExpression) {
return true;
}
if(parent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)parent;
IElementType type = polyadicExpression.getOperationTokenType();
if ((type.equals(JavaTokenType.ANDAND) || type.equals(JavaTokenType.OROR)) && polyadicExpression.getOperands()[0] != cur) {
// not the first in the &&/|| chain: we cannot properly generate code which would short-circuit as well
return false;
}
}
if(parent instanceof PsiConditionalExpression && ((PsiConditionalExpression)parent).getCondition() != cur) {
return false;
}
if(parent instanceof PsiMethodCallExpression) {
PsiReferenceExpression methodExpression = ((PsiMethodCallExpression)parent).getMethodExpression();
if(methodExpression.textMatches("this") || methodExpression.textMatches("super")) {
return false;
}
}
cur = parent;
parent = cur.getParent();
}
if(parent instanceof PsiReturnStatement || parent instanceof PsiExpressionStatement) return true;
if(parent instanceof PsiLocalVariable) {
PsiElement grandParent = parent.getParent();
if(grandParent instanceof PsiDeclarationStatement && ((PsiDeclarationStatement)grandParent).getDeclaredElements().length == 1) {
return true;
}
}
if(parent instanceof PsiForeachStatement && ((PsiForeachStatement)parent).getIteratedValue() == cur) return true;
if(parent instanceof PsiIfStatement && ((PsiIfStatement)parent).getCondition() == cur) return true;
return false;
}
public enum InitializerUsageStatus {
// Variable is declared just before the wanted place
DECLARED_JUST_BEFORE,
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java → plugins/InspectionGadgets/src/com/siyeh/ig/controlflow/PointlessBooleanExpressionInspection.java
+100 -23
View File
@@ -21,25 +21,26 @@ import com.intellij.openapi.project.Project;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.ConstantExpressionUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.refactoring.util.RefactoringUtil;
import com.intellij.util.IncorrectOperationException;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.InspectionGadgetsFix;
import com.siyeh.ig.PsiReplacementUtil;
import com.siyeh.ig.psiutils.ComparisonUtils;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.ParenthesesUtils;
import com.siyeh.ig.psiutils.*;
import org.jetbrains.annotations.Nls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import javax.swing.*;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.*;
public class PointlessBooleanExpressionInspection extends BaseInspection {
private enum BooleanExpressionKind {
USELESS, USELESS_WITH_SIDE_EFFECTS, UNKNOWN
}
private static final Set<IElementType> booleanTokens = new HashSet<>();
static {
@@ -261,10 +262,25 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
@Override
public InspectionGadgetsFix buildFix(Object... infos) {
return new PointlessBooleanExpressionFix();
boolean hasSideEffect = (boolean)infos[1];
return new PointlessBooleanExpressionFix(hasSideEffect);
}
private class PointlessBooleanExpressionFix extends InspectionGadgetsFix {
private boolean myHasSideEffect;
public PointlessBooleanExpressionFix(boolean hasSideEffect) {
myHasSideEffect = hasSideEffect;
}
@Nls
@NotNull
@Override
public String getName() {
return myHasSideEffect
? InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix.sideEffect")
: InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix");
}
@Override
@NotNull
@@ -278,8 +294,39 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
if (!(element instanceof PsiExpression)) {
return;
}
final PsiExpression expression = (PsiExpression)element;
PsiReplacementUtil.replaceExpression(expression, buildSimplifiedExpression(expression, new StringBuilder()).toString());
PsiExpression expression = (PsiExpression)element;
String simplifiedExpression = buildSimplifiedExpression(expression, new StringBuilder()).toString();
boolean isConstant = simplifiedExpression.equals("true") || simplifiedExpression.equals("false");
if (isConstant) {
expression = RefactoringUtil.ensureCodeBlock(expression);
if (expression == null) return;
PsiStatement anchor = PsiTreeUtil.getParentOfType(expression, PsiStatement.class);
if (anchor == null) return;
List<PsiExpression> sideEffects = extractSideEffects(expression);
PsiStatement[] statements = StatementExtractor.generateStatements(sideEffects, expression);
if (statements.length > 0) {
BlockUtils.addBefore(anchor, statements);
}
}
PsiReplacementUtil.replaceExpression(expression, simplifiedExpression);
}
private List<PsiExpression> extractSideEffects(PsiExpression expression) {
if (!(expression instanceof PsiPolyadicExpression)) {
return Collections.emptyList();
}
PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression;
IElementType sign = polyadicExpression.getOperationTokenType();
Boolean stopper = sign == JavaTokenType.ANDAND ? Boolean.FALSE : sign == JavaTokenType.OROR ? Boolean.TRUE : null;
PsiExpression[] operands = polyadicExpression.getOperands();
List<PsiExpression> sideEffects = new ArrayList<>();
for (PsiExpression operand : operands) {
if (stopper != null && stopper.equals(evaluate(operand))) {
break;
}
sideEffects.addAll(SideEffectChecker.extractSideEffectExpressions(operand));
}
return sideEffects;
}
}
@@ -303,41 +350,66 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
}
private void checkExpression(PsiExpression expression) {
if (!isPointlessBooleanExpression(expression)) {
BooleanExpressionKind kind = getExpressionKind(expression);
if (kind == BooleanExpressionKind.UNKNOWN) {
return;
}
final PsiElement parent = ParenthesesUtils.getParentSkipParentheses(expression);
if (parent instanceof PsiExpression && isPointlessBooleanExpression((PsiExpression)parent)) {
if (parent instanceof PsiExpression && getExpressionKind((PsiExpression)parent) != BooleanExpressionKind.UNKNOWN) {
return;
}
registerError(expression, expression);
registerError(expression, expression, kind == BooleanExpressionKind.USELESS_WITH_SIDE_EFFECTS);
}
private boolean isPointlessBooleanExpression(PsiExpression expression) {
@NotNull
private BooleanExpressionKind getExpressionKind(PsiExpression expression) {
if (expression instanceof PsiPrefixExpression) {
return evaluate(expression) != null;
return evaluate(expression) != null ? BooleanExpressionKind.USELESS : BooleanExpressionKind.UNKNOWN;
}
if (expression instanceof PsiPolyadicExpression) {
final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression;
final IElementType sign = polyadicExpression.getOperationTokenType();
if (!booleanTokens.contains(sign)) {
return false;
return BooleanExpressionKind.UNKNOWN;
}
final PsiExpression[] operands = polyadicExpression.getOperands();
boolean containsConstant = false;
boolean stopCheckingSideEffects = false;
boolean sideEffectMayBeRemoved = false;
boolean reducedToConstant = false;
for (PsiExpression operand : operands) {
if (operand == null) {
return false;
return BooleanExpressionKind.UNKNOWN;
}
final PsiType type = operand.getType();
if (type == null || !type.equals(PsiType.BOOLEAN) && !type.equalsToText(CommonClassNames.JAVA_LANG_BOOLEAN)) {
return false;
return BooleanExpressionKind.UNKNOWN;
}
if (!stopCheckingSideEffects && SideEffectChecker.mayHaveSideEffects(operand)) {
sideEffectMayBeRemoved = true;
}
Boolean value = evaluate(operand);
if (value != null) {
containsConstant = true;
if ((JavaTokenType.ANDAND.equals(sign) && !value) || (JavaTokenType.OROR.equals(sign) && value)) {
stopCheckingSideEffects = true;
reducedToConstant = true;
}
if ((JavaTokenType.AND.equals(sign) && !value) || (JavaTokenType.OR.equals(sign) && value)) {
reducedToConstant = true;
}
}
containsConstant |= evaluate(operand) != null;
}
return containsConstant;
if (containsConstant) {
if (sideEffectMayBeRemoved && reducedToConstant) {
return ControlFlowUtils.canExtractStatement(expression)
? BooleanExpressionKind.USELESS_WITH_SIDE_EFFECTS
: BooleanExpressionKind.UNKNOWN;
}
return BooleanExpressionKind.USELESS;
}
}
return false;
return BooleanExpressionKind.UNKNOWN;
}
}
@@ -356,6 +428,9 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
if (tokenType.equals(JavaTokenType.OROR)) {
final PsiExpression[] operands = polyadicExpression.getOperands();
for (PsiExpression operand : operands) {
if (SideEffectChecker.mayHaveSideEffects(operand)) {
return null;
}
if (evaluate(operand) == Boolean.TRUE) {
return Boolean.TRUE;
}
@@ -364,6 +439,9 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
else if (tokenType.equals(JavaTokenType.ANDAND)) {
final PsiExpression[] operands = polyadicExpression.getOperands();
for (PsiExpression operand : operands) {
if (SideEffectChecker.mayHaveSideEffects(operand)) {
return null;
}
if (evaluate(operand) == Boolean.FALSE) {
return Boolean.FALSE;
}
@@ -383,8 +461,7 @@ public class PointlessBooleanExpressionInspection extends BaseInspection {
}
}
}
final Boolean value = (Boolean)ConstantExpressionUtil.computeCastTo(expression, PsiType.BOOLEAN);
return value != null ? value : null;
return (Boolean)ConstantExpressionUtil.computeCastTo(expression, PsiType.BOOLEAN);
}
private static boolean containsReference(@Nullable PsiExpression expression) {
plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/SideEffects.after.java
+34
View File
@@ -0,0 +1,34 @@
/*
* 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.
*/
class C {
double sideEffect(int x) {
System.out.println("Side effect"+x);
return Math.random();
}
void m() {
if (sideEffect(1) > 0.5 && sideEffect(2) < 0.1) {
if (sideEffect(3) / sideEffect(4) < 1) {
sideEffect(5);
} else {
sideEffect(6);
}
}
if(false) {
System.out.println("oops");
}
}
}
plugins/InspectionGadgets/test/com/siyeh/igfixes/pointlessboolean/SideEffects.java
+29
View File
@@ -0,0 +1,29 @@
/*
* 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.
*/
class C {
double sideEffect(int x) {
System.out.println("Side effect"+x);
return Math.random();
}
void m() {
if(sideEffect(1) > 0.5 <caret>&& sideEffect(2) < 0.1
&& (sideEffect(3)/sideEffect(4) < 1 ? sideEffect(5) > 0.5 : sideEffect(6) < 0.6)
&& false && sideEffect(7) * sideEffect(8) < 0.2) {
System.out.println("oops");
}
}
}
plugins/InspectionGadgets/test/com/siyeh/igtest/controlflow/pointless_boolean_expression/PointlessBooleanExpression.java
+30
View File
@@ -28,6 +28,36 @@ class PointlessBooleanExpression {
boolean y = <warning descr="'false || c' can be simplified to 'c'">false || c</warning>;
boolean z = <warning descr="'b != true' can be simplified to '!b'">b != true</warning>;
}
boolean sideEffect() {
System.out.println("hello");
return Math.random() > 0.5;
}
// side-effect cannot be extracted from field declaration
boolean field = sideEffect() && false;
boolean field1 = false & sideEffect();
// no side-effect extraction necessary
boolean field2 = <warning descr="'sideEffect() && true' can be simplified to 'sideEffect()'">sideEffect() && true</warning>;
boolean field3 = <warning descr="'false && sideEffect()' can be simplified to 'false'">false && sideEffect()</warning>;
void method() {
if(<warning descr="'sideEffect() && false' can be simplified to 'false'">sideEffect() && false</warning>) {
System.out.println("ok");
}
if(<warning descr="'sideEffect() && true' can be simplified to 'sideEffect()'">sideEffect() && true</warning>) {
System.out.println("ooh");
}
// Do not warn as we cannot simplify w/o reordering calls which could be undesired
// this code is warned by DFA inspection (w/o quick-fix)
if(Math.random() > 0.5 && (sideEffect() || true)) {
System.out.println("well");
}
// Here side-effect can be extracted before loop
if((<warning descr="'sideEffect() || true' can be simplified to 'true'">sideEffect() || true</warning>) && Math.random() > 0.5) {
System.out.println("well");
}
}
}
class Presley {
void elvis(Object king) {
plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/PointlessBooleanExpressionFixTest.java
+4 -1
View File
@@ -26,10 +26,13 @@ public class PointlessBooleanExpressionFixTest extends IGQuickFixesTestCase {
super.setUp();
myFixture.enableInspections(new PointlessBooleanExpressionInspection());
myRelativePath = "pointlessboolean";
myDefaultHint = InspectionGadgetsBundle.message("pointless.bitwise.expression.simplify.quickfix");
myDefaultHint = InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix");
}
public void testNegation() { doTest(); }
public void testPolyadic() { doTest(); }
public void testBoxed() { doTest(); }
public void testSideEffects() {
doTest(InspectionGadgetsBundle.message("constant.conditional.expression.simplify.quickfix.sideEffect"));
}
}