MDS/openide
1
0
Fork 0
mirror of https://gitflic.ru/project/openide/openide.git synced 2026-08-26 15:27:45 +07:00
Code Issues Packages Projects Releases Wiki Activity

DFA instruction visitor refactoring wave#5

Browse Source 
CheckReturnValueInstruction replaced with checkReturnValue call (PSI-based)
beforeExpressionPush for typecast result
Control flow: &&/|| handling unified and simplified (less jumps, less states)
ContractChecker rewritten (now visitor-based)
This commit is contained in:
Tagir Valeev
2018-06-25 13:49:39 +07:00
parent 664b9a4d4c
commit fd66719ce0
24 changed files with 391 additions and 477 deletions
Download Patch File Download Diff File Expand all files Collapse all files
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ContractChecker.java
+100 -103
View File
@@ -3,8 +3,7 @@ package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.NullableNotNullManager;
import com.intellij.codeInspection.dataFlow.StandardMethodContract.ValueConstraint;
import com.intellij.codeInspection.dataFlow.instructions.CheckReturnValueInstruction;
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
import com.intellij.codeInspection.dataFlow.instructions.ControlTransferInstruction;
import com.intellij.codeInspection.dataFlow.instructions.MethodCallInstruction;
import com.intellij.codeInspection.dataFlow.instructions.ReturnInstruction;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
@@ -26,32 +25,111 @@ import java.util.Set;
/**
* @author peter
*/
class ContractChecker extends DataFlowRunner {
private final PsiMethod myMethod;
private final StandardMethodContract myContract;
private final boolean myOwnContract;
private final Set<PsiElement> myViolations = ContainerUtil.newHashSet();
private final Set<PsiElement> myNonViolations = ContainerUtil.newHashSet();
private final Set<PsiElement> myFailures = ContainerUtil.newHashSet();
private boolean myMayReturnNormally = false;
class ContractChecker {
private static class ContractCheckerVisitor extends StandardInstructionVisitor {
private final PsiMethod myMethod;
private final StandardMethodContract myContract;
private final boolean myOwnContract;
private final Set<PsiElement> myViolations = ContainerUtil.newHashSet();
private final Set<PsiElement> myNonViolations = ContainerUtil.newHashSet();
private final Set<PsiElement> myFailures = ContainerUtil.newHashSet();
private boolean myMayReturnNormally = false;
private ContractChecker(PsiMethod method, StandardMethodContract contract, boolean ownContract) {
super(false, null);
myMethod = method;
myContract = contract;
myOwnContract = ownContract;
ContractCheckerVisitor(PsiMethod method, StandardMethodContract contract, boolean ownContract) {
myMethod = method;
myContract = contract;
myOwnContract = ownContract;
}
@Override
protected void checkReturnValue(@NotNull DfaValue value,
@NotNull PsiExpression expression,
@NotNull PsiParameterListOwner context,
@NotNull DfaMemoryState state) {
if (context != myMethod || state.isEphemeral()) return;
if (!myContract.getReturnValue().isValueCompatible(state, value)) {
myViolations.add(expression);
} else {
myNonViolations.add(expression);
}
}
@Override
public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
if (!memState.isEphemeral() && instruction.getMethodType() == MethodCallInstruction.MethodType.REGULAR_METHOD_CALL) {
if (myContract.getReturnValue().isFail()) {
ContainerUtil.addIfNotNull(myFailures, instruction.getCallExpression());
return DfaInstructionState.EMPTY_ARRAY;
}
if (weCannotInferAnythingAboutMethodReturnValue(instruction)) {
DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState);
for (DfaInstructionState state: states) {
state.getMemoryState().markEphemeral();
}
return states;
}
}
return super.visitMethodCall(instruction, runner, memState);
}
@NotNull
@Override
public DfaInstructionState[] visitControlTransfer(@NotNull ControlTransferInstruction instruction,
@NotNull DataFlowRunner runner,
@NotNull DfaMemoryState state) {
if (!state.isEphemeral()) {
if (instruction instanceof ReturnInstruction && ((ReturnInstruction)instruction).isViaException()) {
ContainerUtil.addIfNotNull(myFailures, ((ReturnInstruction)instruction).getAnchor());
}
else {
myMayReturnNormally = true;
}
}
return super.visitControlTransfer(instruction, runner, state);
}
private Map<PsiElement, String> getErrors() {
HashMap<PsiElement, String> errors = ContainerUtil.newHashMap();
for (PsiElement element : myViolations) {
if (!myNonViolations.contains(element)) {
errors.put(element, "Contract clause '" + myContract + "' is violated");
}
}
if (!myContract.getReturnValue().isFail()) {
if (myOwnContract && !myMayReturnNormally &&
!(PsiUtil.canBeOverridden(myMethod) && ControlFlowUtils.methodAlwaysThrowsException(myMethod))) {
for (PsiElement element : myFailures) {
errors.put(element, "Return value of clause '" + myContract + "' could be replaced with 'fail' as method always fails"+
(myContract.isTrivial() ? "" : " in this case"));
}
}
} else if (myFailures.isEmpty() && errors.isEmpty()) {
PsiIdentifier nameIdentifier = myMethod.getNameIdentifier();
errors.put(nameIdentifier != null ? nameIdentifier : myMethod,
"Contract clause '" + myContract + "' is violated: no exception is thrown");
}
return errors;
}
private static boolean weCannotInferAnythingAboutMethodReturnValue(MethodCallInstruction instruction) {
PsiMethod target = instruction.getTargetMethod();
return instruction.getContracts().isEmpty() && target != null && !target.isConstructor() && !NullableNotNullManager.isNotNull(target);
}
}
static Map<PsiElement, String> checkContractClause(PsiMethod method, StandardMethodContract contract, boolean ownContract) {
PsiCodeBlock body = method.getBody();
if (body == null) return Collections.emptyMap();
ContractChecker checker = new ContractChecker(method, contract, ownContract);
DataFlowRunner runner = new StandardDataFlowRunner(false, null);
PsiParameter[] parameters = method.getParameterList().getParameters();
final DfaMemoryState initialState = checker.createMemoryState();
final DfaValueFactory factory = checker.getFactory();
final DfaMemoryState initialState = runner.createMemoryState();
final DfaValueFactory factory = runner.getFactory();
for (int i = 0; i < contract.getParameterCount(); i++) {
ValueConstraint constraint = contract.getParameterConstraint(i);
DfaConstValue comparisonValue = constraint.getComparisonValue(factory);
@@ -62,89 +140,8 @@ class ContractChecker extends DataFlowRunner {
}
}
checker.analyzeMethod(body, new StandardInstructionVisitor(), false, Collections.singletonList(initialState));
return checker.getErrors();
}
@NotNull
@Override
protected DfaInstructionState[] acceptInstruction(@NotNull InstructionVisitor visitor, @NotNull DfaInstructionState instructionState) {
DfaMemoryState memState = instructionState.getMemoryState();
if (memState.isEphemeral()) {
return super.acceptInstruction(visitor, instructionState);
}
Instruction instruction = instructionState.getInstruction();
if (instruction instanceof CheckReturnValueInstruction) {
PsiElement anchor = ((CheckReturnValueInstruction)instruction).getReturn();
DfaValue retValue = memState.pop();
if (!myContract.getReturnValue().isValueCompatible(memState, retValue)) {
myViolations.add(anchor);
} else {
myNonViolations.add(anchor);
}
return InstructionVisitor.nextInstruction(instruction, this, memState);
}
if (instruction instanceof ReturnInstruction) {
if (((ReturnInstruction)instruction).isViaException()) {
ContainerUtil.addIfNotNull(myFailures, ((ReturnInstruction)instruction).getAnchor());
} else {
myMayReturnNormally = true;
}
}
if (instruction instanceof MethodCallInstruction &&
((MethodCallInstruction)instruction).getMethodType() == MethodCallInstruction.MethodType.REGULAR_METHOD_CALL) {
if (myContract.getReturnValue().isFail()) {
ContainerUtil.addIfNotNull(myFailures, ((MethodCallInstruction)instruction).getCallExpression());
return DfaInstructionState.EMPTY_ARRAY;
}
if (weCannotInferAnythingAboutMethodReturnValue((MethodCallInstruction)instruction)) {
return markEverythingEphemeral(visitor, instructionState);
}
}
return super.acceptInstruction(visitor, instructionState);
}
private static boolean weCannotInferAnythingAboutMethodReturnValue(MethodCallInstruction instruction) {
PsiMethod target = instruction.getTargetMethod();
return instruction.getContracts().isEmpty() && target != null && !target.isConstructor() && !NullableNotNullManager.isNotNull(target);
}
@NotNull
private DfaInstructionState[] markEverythingEphemeral(@NotNull InstructionVisitor visitor,
@NotNull DfaInstructionState instructionState) {
DfaInstructionState[] result = super.acceptInstruction(visitor, instructionState);
for (DfaInstructionState state : result) {
state.getMemoryState().markEphemeral();
}
return result;
}
private Map<PsiElement, String> getErrors() {
HashMap<PsiElement, String> errors = ContainerUtil.newHashMap();
for (PsiElement element : myViolations) {
if (!myNonViolations.contains(element)) {
errors.put(element, "Contract clause '" + myContract + "' is violated");
}
}
if (!myContract.getReturnValue().isFail()) {
if (myOwnContract && !myMayReturnNormally &&
!(PsiUtil.canBeOverridden(myMethod) && ControlFlowUtils.methodAlwaysThrowsException(myMethod))) {
for (PsiElement element : myFailures) {
errors.put(element, "Return value of clause '" + myContract + "' could be replaced with 'fail' as method always fails"+
(myContract.isTrivial() ? "" : " in this case"));
}
}
} else if (myFailures.isEmpty() && errors.isEmpty()) {
PsiIdentifier nameIdentifier = myMethod.getNameIdentifier();
errors.put(nameIdentifier != null ? nameIdentifier : myMethod,
"Contract clause '" + myContract + "' is violated: no exception is thrown");
}
return errors;
ContractCheckerVisitor visitor = new ContractCheckerVisitor(method, contract, ownContract);
runner.analyzeMethod(body, visitor, false, Collections.singletonList(initialState));
return visitor.getErrors();
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java
+20 -49
View File
@@ -140,7 +140,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
if (parent instanceof PsiLambdaExpression && myCodeFragment instanceof PsiExpression) {
generateBoxingUnboxingInstructionFor((PsiExpression)myCodeFragment,
LambdaUtil.getFunctionalInterfaceReturnType((PsiLambdaExpression)parent));
addInstruction(new CheckReturnValueInstruction((PsiExpression)myCodeFragment));
addInstruction(new CheckNotNullInstruction(NullabilityProblemKind.nullableReturn.problem((PsiExpression)myCodeFragment)));
addInstruction(new PopInstruction());
}
addInstruction(new ReturnInstruction(myFactory.controlTransfer(ReturnTransfer.INSTANCE, FList.emptyList()), null));
@@ -831,7 +832,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
generateBoxingUnboxingInstructionFor(returnValue, LambdaUtil.getFunctionalInterfaceReturnType(lambdaExpression));
}
}
addInstruction(new CheckReturnValueInstruction(returnValue));
addInstruction(new CheckNotNullInstruction(NullabilityProblemKind.nullableReturn.problem(returnValue)));
addInstruction(new PopInstruction());
}
addInstruction(new ReturnInstruction(myFactory.controlTransfer(ReturnTransfer.INSTANCE, myTrapStack), statement));
@@ -1292,19 +1294,19 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
PsiType type = expression.getType();
if (op == JavaTokenType.ANDAND) {
generateAndExpression(operands, type, true);
generateAndOrExpression(expression, operands, type, true, true);
}
else if (op == JavaTokenType.OROR) {
generateOrExpression(operands, type, true);
generateAndOrExpression(expression, operands, type, false, true);
}
else if (op == JavaTokenType.XOR && PsiType.BOOLEAN.equals(type)) {
generateXorExpression(expression, operands, type, false);
}
else if (op == JavaTokenType.AND && PsiType.BOOLEAN.equals(type)) {
generateAndExpression(operands, type, false);
generateAndOrExpression(expression, operands, type, true, false);
}
else if (op == JavaTokenType.OR && PsiType.BOOLEAN.equals(type)) {
generateOrExpression(operands, type, false);
generateAndOrExpression(expression, operands, type, false, false);
}
else if (isBinaryDivision(op) && operands.length == 2 &&
type != null && PsiType.LONG.isAssignableFrom(type)) {
@@ -1459,27 +1461,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
}
}
private void generateOrExpression(PsiExpression[] operands, final PsiType exprType, boolean shortCircuit) {
for (int i = 0; i < operands.length; i++) {
PsiExpression operand = operands[i];
operand.accept(this);
generateBoxingUnboxingInstructionFor(operand, exprType);
if (!shortCircuit) {
if (i > 0) {
combineStackBooleans(false, operand);
}
continue;
}
PsiExpression nextOperand = i == operands.length - 1 ? null : operands[i + 1];
if (nextOperand != null) {
addInstruction(new ConditionalGotoInstruction(getStartOffset(nextOperand), true, operand));
addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null));
addInstruction(new GotoInstruction(getEndOffset(operands[operands.length - 1])));
}
}
}
private void generateBooleanAssignmentExpression(boolean and, PsiExpression lExpression, PsiExpression rExpression, PsiType exprType) {
lExpression.accept(this);
generateBoxingUnboxingInstructionFor(lExpression, exprType);
@@ -1508,39 +1489,29 @@ public class ControlFlowAnalyzer extends JavaElementVisitor {
overPushSuccess.setOffset(pushSuccess.getIndex() + 1);
}
private void generateAndExpression(PsiExpression[] operands, final PsiType exprType, boolean shortCircuit) {
List<ConditionalGotoInstruction> branchToFail = new ArrayList<>();
private void generateAndOrExpression(PsiExpression expression,
PsiExpression[] operands,
final PsiType exprType,
boolean and,
boolean shortCircuit) {
for (int i = 0; i < operands.length; i++) {
PsiExpression operand = operands[i];
operand.accept(this);
generateBoxingUnboxingInstructionFor(operand, exprType);
if (!shortCircuit) {
if (i > 0) {
combineStackBooleans(true, operand);
combineStackBooleans(and, operand);
}
continue;
}
ConditionalGotoInstruction onFail = new ConditionalGotoInstruction(null, true, operand);
branchToFail.add(onFail);
addInstruction(onFail);
PsiExpression nextOperand = i == operands.length - 1 ? null : operands[i + 1];
if (nextOperand != null) {
addInstruction(new ConditionalGotoInstruction(getStartOffset(nextOperand), !and, operand));
addInstruction(new PushInstruction(myFactory.getBoolean(!and), expression));
addInstruction(new GotoInstruction(getEndOffset(operands[operands.length - 1])));
}
}
if (!shortCircuit) {
return;
}
addInstruction(new PushInstruction(myFactory.getConstFactory().getTrue(), null));
GotoInstruction toSuccess = new GotoInstruction(null);
addInstruction(toSuccess);
PushInstruction pushFalse = new PushInstruction(myFactory.getConstFactory().getFalse(), null);
addInstruction(pushFalse);
for (ConditionalGotoInstruction toFail : branchToFail) {
toFail.setOffset(pushFalse.getIndex());
}
toSuccess.setOffset(pushFalse.getIndex()+1);
}
@Override public void visitClassObjectAccessExpression(PsiClassObjectAccessExpression expression) {
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java
+6 -24
View File
@@ -14,7 +14,6 @@ import com.intellij.codeInspection.dataFlow.fix.ReplaceWithObjectsEqualsFix;
import com.intellij.codeInspection.dataFlow.fix.SimplifyToAssignmentFix;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.DfaConstValue;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.codeInspection.nullable.NullableStuffInspectionBase;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.project.Project;
@@ -254,7 +253,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
for (Instruction instruction : allProblems) {
if (instruction instanceof TypeCastInstruction &&
reportedAnchors.add(((TypeCastInstruction)instruction).getCastExpression().getCastType())) {
reportedAnchors.add(((TypeCastInstruction)instruction).getExpression().getCastType())) {
reportCastMayFail(holder, (TypeCastInstruction)instruction);
}
else if (instruction instanceof BranchingInstruction) {
@@ -264,8 +263,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
reportAlwaysFailingCalls(holder, visitor, reportedAnchors);
reportConstantPushes(runner, holder, reportedAnchors);
reportNullabilityProblems(holder, visitor, reportedAnchors);
reportNullableReturns(visitor, holder, reportedAnchors, scope);
if (SUGGEST_NULLABLE_ANNOTATIONS) {
@@ -275,9 +272,9 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
reportOptionalOfNullableImprovements(holder, reportedAnchors, visitor.getOfNullableCalls());
visitor.getBooleanCalls().forEach((call, state) -> {
if (state != ThreeState.UNSURE && reportedAnchors.add(call)) {
reportConstantCondition(holder, call, state.toBoolean());
visitor.getBooleanExpressions().forEach((expression, state) -> {
if (state != ThreeState.UNSURE && reportedAnchors.add(expression)) {
reportConstantBoolean(holder, expression, state.toBoolean());
}
});
@@ -471,21 +468,6 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
return call;
}
private void reportConstantPushes(StandardDataFlowRunner runner,
ProblemsHolder holder,
Set<PsiElement> reportedAnchors) {
for (Instruction instruction : runner.getInstructions()) {
if (instruction instanceof PushInstruction) {
PsiExpression place = ((PushInstruction)instruction).getExpression();
DfaValue value = ((PushInstruction)instruction).getValue();
Object constant = value instanceof DfaConstValue ? ((DfaConstValue)value).getValue() : null;
if (place instanceof PsiPolyadicExpression && constant instanceof Boolean && !isFlagCheck(place) && reportedAnchors.add(place)) {
reportConstantCondition(holder, place, (Boolean)constant);
}
}
}
}
private static void reportOptionalOfNullableImprovements(ProblemsHolder holder,
Set<PsiElement> reportedAnchors,
Map<PsiElement, ThreeState> nullArgs) {
@@ -611,7 +593,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
}
private static void reportCastMayFail(ProblemsHolder holder, TypeCastInstruction instruction) {
PsiTypeCastExpression typeCast = instruction.getCastExpression();
PsiTypeCastExpression typeCast = instruction.getExpression();
PsiExpression operand = typeCast.getOperand();
PsiTypeElement castType = typeCast.getCastType();
assert castType != null;
@@ -882,7 +864,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
return false;
}
private static boolean isFlagCheck(PsiElement element) {
static boolean isFlagCheck(PsiElement element) {
PsiElement scope = PsiTreeUtil.getParentOfType(element, PsiStatement.class, PsiVariable.class);
PsiExpression topExpression = scope instanceof PsiIfStatement ? ((PsiIfStatement)scope).getCondition() :
scope instanceof PsiVariable ? ((PsiVariable)scope).getInitializer() :
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java
+46 -29
View File
@@ -9,6 +9,7 @@ import com.intellij.openapi.util.TextRange;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiTypesUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ThreeState;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.psiutils.ExpressionUtils;
@@ -26,7 +27,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
private final Map<NullabilityProblemKind.NullabilityProblem<?>, StateInfo> myStateInfos = new LinkedHashMap<>();
private final Set<Instruction> myCCEInstructions = ContainerUtil.newHashSet();
private final Map<PsiCallExpression, Boolean> myFailingCalls = new HashMap<>();
private final Map<PsiMethodCallExpression, ThreeState> myBooleanCalls = new HashMap<>();
private final Map<PsiExpression, ThreeState> myBooleanExpressions = new HashMap<>();
private final Map<PsiElement, ThreeState> myOfNullableCalls = new HashMap<>();
private final Map<PsiAssignmentExpression, Pair<PsiType, PsiType>> myArrayStoreProblems = new HashMap<>();
private final Map<PsiMethodReferenceExpression, DfaValue> myMethodReferenceResults = new HashMap<>();
@@ -127,8 +128,8 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
return myOfNullableCalls;
}
Map<PsiMethodCallExpression, ThreeState> getBooleanCalls() {
return myBooleanCalls;
Map<PsiExpression, ThreeState> getBooleanExpressions() {
return myBooleanExpressions;
}
Map<PsiMethodReferenceExpression, DfaValue> getMethodReferenceResults() {
@@ -166,6 +167,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
@Nullable TextRange range,
@NotNull DfaMemoryState memState) {
expression.accept(new ExpressionVisitor(value, memState));
handleBooleanResults(value, memState, expression);
}
@Override
@@ -221,13 +223,47 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
contracts.stream().anyMatch(contract -> contract.getReturnValue().isFail() && !contract.isTrivial());
}
private static boolean shouldCollectBooleanCallResult(PsiMethodCallExpression call) {
if (ExpressionUtils.isVoidContext(call)) return false;
PsiMethod method = call.resolveMethod();
if (method == null || !PsiType.BOOLEAN.equals(method.getReturnType()) || !JavaMethodContractUtil.isPure(method)) return false;
List<? extends MethodContract> contracts = JavaMethodContractUtil.getMethodCallContracts(method, call);
return CustomMethodHandlers.find(method) != null ||
!contracts.isEmpty() && contracts.stream().anyMatch(contract -> contract.getReturnValue().isBoolean() && !contract.isTrivial());
private void handleBooleanResults(DfaValue value, DfaMemoryState memState, PsiExpression expression) {
ThreeState curState = myBooleanExpressions.get(expression);
if (curState == ThreeState.UNSURE) return;
ThreeState nextState = ThreeState.UNSURE;
value = value instanceof DfaVariableValue ? memState.getConstantValue((DfaVariableValue)value) : value;
if (value instanceof DfaConstValue) {
Object val = ((DfaConstValue)value).getValue();
if (val instanceof Boolean) {
nextState = ThreeState.fromBoolean((Boolean)val);
if (curState != null && curState != nextState) {
nextState = ThreeState.UNSURE;
}
}
}
if (curState != null || shouldCollectBooleanResult(expression)) {
myBooleanExpressions.put(expression, nextState);
}
}
private static boolean shouldCollectBooleanResult(PsiExpression expression) {
PsiType type = expression.getType();
if (type == null || !PsiType.BOOLEAN.isAssignableFrom(type)) return false;
if (expression instanceof PsiPrefixExpression || expression instanceof PsiPolyadicExpression) {
return !DataFlowInspectionBase.isFlagCheck(expression);
}
PsiPolyadicExpression polyadic = tryCast(PsiUtil.skipParenthesizedExprUp(expression.getParent()), PsiPolyadicExpression.class);
if (polyadic != null) {
if ((polyadic.getOperationTokenType().equals(JavaTokenType.ANDAND) || polyadic.getOperationTokenType().equals(JavaTokenType.OROR)) &&
!DataFlowInspectionBase.isFlagCheck(expression)) return true;
}
if (expression instanceof PsiMethodCallExpression) {
PsiMethodCallExpression call = (PsiMethodCallExpression)expression;
if (ExpressionUtils.isVoidContext(call)) return false;
PsiMethod method = call.resolveMethod();
if (method == null || !JavaMethodContractUtil.isPure(method)) return false;
List<? extends MethodContract> contracts = JavaMethodContractUtil.getMethodCallContracts(method, call);
return CustomMethodHandlers.find(method) != null ||
!contracts.isEmpty() &&
contracts.stream().anyMatch(contract -> contract.getReturnValue().isBoolean() && !contract.isTrivial());
}
return false;
}
@Override
@@ -290,7 +326,6 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
if (DfaOptionalSupport.OPTIONAL_OF_NULLABLE.test(call)) {
processOfNullableResult(myValue, myMemState, call.getArgumentList().getExpressions()[0]);
}
handleBooleanCalls(call);
}
@Override
@@ -319,23 +354,5 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
myValues.put(expression, newValue);
}
}
private void handleBooleanCalls(PsiMethodCallExpression call) {
ThreeState curState = myBooleanCalls.get(call);
if (curState == ThreeState.UNSURE) return;
ThreeState nextState = ThreeState.UNSURE;
if (myValue instanceof DfaConstValue) {
Object val = ((DfaConstValue)myValue).getValue();
if (val instanceof Boolean) {
nextState = ThreeState.fromBoolean((Boolean)val);
if (curState != null && curState != nextState) {
nextState = ThreeState.UNSURE;
}
}
}
if (curState != null || shouldCollectBooleanCallResult(call)) {
myBooleanCalls.put(call, nextState);
}
}
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaUtil.java
+28 -30
View File
@@ -14,10 +14,7 @@ import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.impl.source.resolve.JavaResolveUtil;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.CachedValueProvider;
import com.intellij.psi.util.CachedValuesManager;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.*;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.containers.ContainerUtil;
import com.intellij.util.containers.FList;
@@ -27,7 +24,6 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.*;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Predicate;
/**
@@ -146,57 +142,59 @@ public class DfaUtil {
@NotNull
public static Nullability inferMethodNullability(PsiMethod method) {
final PsiCodeBlock body = method.getBody();
if (body == null || PsiUtil.resolveClassInType(method.getReturnType()) == null) {
if (PsiUtil.resolveClassInType(method.getReturnType()) == null) {
return Nullability.UNKNOWN;
}
return inferBlockNullability(body, InferenceFromSourceUtil.suppressNullable(method));
return inferBlockNullability(method, InferenceFromSourceUtil.suppressNullable(method));
}
@NotNull
public static Nullability inferLambdaNullability(PsiLambdaExpression lambda) {
final PsiElement body = lambda.getBody();
if (body == null || LambdaUtil.getFunctionalInterfaceReturnType(lambda) == null) {
if (LambdaUtil.getFunctionalInterfaceReturnType(lambda) == null) {
return Nullability.UNKNOWN;
}
return inferBlockNullability(body, false);
return inferBlockNullability(lambda, false);
}
@NotNull
private static Nullability inferBlockNullability(PsiElement body, boolean suppressNullable) {
final AtomicBoolean hasNulls = new AtomicBoolean();
final AtomicBoolean hasNotNulls = new AtomicBoolean();
final AtomicBoolean hasUnknowns = new AtomicBoolean();
private static Nullability inferBlockNullability(PsiParameterListOwner owner, boolean suppressNullable) {
PsiElement body = owner.getBody();
if (body == null) return Nullability.UNKNOWN;
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner();
final RunnerResult rc = dfaRunner.analyzeMethod(body, new StandardInstructionVisitor() {
class BlockNullabilityVisitor extends StandardInstructionVisitor {
boolean hasNulls = false;
boolean hasNotNulls = false;
boolean hasUnknowns = false;
@Override
public DfaInstructionState[] visitCheckReturnValue(CheckReturnValueInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
if(PsiTreeUtil.isAncestor(body, instruction.getReturn(), false)) {
DfaValue returned = memState.peek();
if (memState.isNull(returned)) {
hasNulls.set(true);
protected void checkReturnValue(@NotNull DfaValue value,
@NotNull PsiExpression expression,
@NotNull PsiParameterListOwner context,
@NotNull DfaMemoryState state) {
if (context == owner) {
if (TypeConversionUtil.isPrimitiveAndNotNull(expression.getType()) || state.isNotNull(value)) {
hasNotNulls = true;
}
else if (memState.isNotNull(returned)) {
hasNotNulls.set(true);
else if (state.isNull(value)) {
hasNulls = true;
}
else {
hasUnknowns.set(true);
hasUnknowns = true;
}
}
return super.visitCheckReturnValue(instruction, runner, memState);
}
});
}
BlockNullabilityVisitor visitor = new BlockNullabilityVisitor();
final RunnerResult rc = dfaRunner.analyzeMethod(body, visitor);
if (rc == RunnerResult.OK) {
if (hasNulls.get()) {
if (visitor.hasNulls) {
return suppressNullable ? Nullability.UNKNOWN : Nullability.NULLABLE;
}
if (hasNotNulls.get() && !hasUnknowns.get()) {
if (visitor.hasNotNulls && !visitor.hasUnknowns) {
return Nullability.NOT_NULL;
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java
+40 -10
View File
@@ -19,16 +19,16 @@ import com.intellij.codeInsight.Nullability;
import com.intellij.codeInspection.dataFlow.instructions.*;
import com.intellij.codeInspection.dataFlow.value.*;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.PsiArrayAccessExpression;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiMethodReferenceExpression;
import com.intellij.psi.PsiType;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ArrayUtil;
import com.intellij.util.ObjectUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.ArrayList;
import java.util.Objects;
/**
* @author peter
@@ -48,6 +48,13 @@ public abstract class InstructionVisitor {
}
protected void checkReturnValue(@NotNull DfaValue value,
@NotNull PsiExpression expression,
@NotNull PsiParameterListOwner context,
@NotNull DfaMemoryState state) {
}
void pushExpressionResult(@NotNull DfaValue value,
@NotNull ExpressionPushingInstruction instruction,
@NotNull DfaMemoryState state) {
@@ -61,12 +68,39 @@ public abstract class InstructionVisitor {
beforeMethodReferenceResultPush(value, (PsiMethodReferenceExpression)anchor, state);
}
else {
beforeExpressionPush(value, anchor, instruction.getExpressionRange(), state);
callBeforeExpressionPush(value, instruction, state, anchor);
}
}
state.push(value);
}
private void callBeforeExpressionPush(@NotNull DfaValue value,
@NotNull ExpressionPushingInstruction instruction,
@NotNull DfaMemoryState state, PsiExpression anchor) {
beforeExpressionPush(value, anchor, instruction.getExpressionRange(), state);
PsiElement parent = PsiUtil.skipParenthesizedExprUp(anchor.getParent());
if (parent instanceof PsiLambdaExpression) {
checkReturnValue(value, Objects.requireNonNull(instruction.getExpression()), (PsiLambdaExpression)parent, state);
}
else if (parent instanceof PsiReturnStatement) {
PsiParameterListOwner context = PsiTreeUtil.getParentOfType(parent, PsiMethod.class, PsiLambdaExpression.class);
if (context != null) {
checkReturnValue(value, Objects.requireNonNull(instruction.getExpression()), context, state);
}
}
else if (parent instanceof PsiConditionalExpression &&
!PsiTreeUtil.isAncestor(((PsiConditionalExpression)parent).getCondition(), anchor, false)) {
callBeforeExpressionPush(value, instruction, state, (PsiConditionalExpression)parent);
}
else if (parent instanceof PsiPolyadicExpression) {
PsiPolyadicExpression polyadic = (PsiPolyadicExpression)parent;
if ((polyadic.getOperationTokenType().equals(JavaTokenType.ANDAND) || polyadic.getOperationTokenType().equals(JavaTokenType.OROR)) &&
PsiTreeUtil.isAncestor(ArrayUtil.getLastElement(polyadic.getOperands()), anchor, false)) {
callBeforeExpressionPush(value, instruction, state, polyadic);
}
}
}
public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
memState.pop();
DfaValue dest = memState.pop();
@@ -138,11 +172,6 @@ public abstract class InstructionVisitor {
return nextInstruction(instruction, runner, state);
}
public DfaInstructionState[] visitCheckReturnValue(CheckReturnValueInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
memState.pop();
return nextInstruction(instruction, runner, memState);
}
public DfaInstructionState[] visitLambdaExpression(LambdaInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
return nextInstruction(instruction, runner, memState);
}
@@ -247,6 +276,7 @@ public abstract class InstructionVisitor {
}
public DfaInstructionState[] visitTypeCast(TypeCastInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
pushExpressionResult(memState.pop(), instruction, memState);
return nextInstruction(instruction, runner, memState);
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java
+9 -12
View File
@@ -155,15 +155,6 @@ public class StandardInstructionVisitor extends InstructionVisitor {
}
}
@Override
public DfaInstructionState[] visitCheckReturnValue(CheckReturnValueInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
final DfaValue retValue = memState.pop();
checkNotNullable(memState, retValue, NullabilityProblemKind.nullableReturn.problem(instruction.getReturn()));
return nextInstruction(instruction, runner, memState);
}
@Override
public DfaInstructionState[] visitArrayAccess(ArrayAccessInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
PsiArrayAccessExpression arrayExpression = instruction.getExpression();
@@ -280,9 +271,11 @@ public class StandardInstructionVisitor extends InstructionVisitor {
onInstructionProducesCCE(instruction);
}
DfaValue value = memState.pop();
if (type instanceof PsiPrimitiveType) {
memState.push(factory.getBoxedFactory().createUnboxed(memState.pop()));
value = factory.getBoxedFactory().createUnboxed(value);
}
pushExpressionResult(value, instruction, memState);
return nextInstruction(instruction, runner, memState);
}
@@ -595,8 +588,12 @@ public class StandardInstructionVisitor extends InstructionVisitor {
@Override
public DfaInstructionState[] visitCheckNotNull(CheckNotNullInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
DfaValue result = dereference(memState, memState.pop(), instruction.getProblem());
memState.push(result);
NullabilityProblemKind.NullabilityProblem<?> problem = instruction.getProblem();
if (NullabilityProblemKind.nullableReturn.isMyProblem(problem)) {
checkNotNullable(memState, memState.peek(), problem);
} else {
memState.push(dereference(memState, memState.pop(), problem));
}
return super.visitCheckNotNull(instruction, runner, memState);
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/CheckReturnValueInstruction.java
-48
View File
@@ -1,48 +0,0 @@
/*
* 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.codeInspection.dataFlow.instructions;
import com.intellij.codeInspection.dataFlow.DataFlowRunner;
import com.intellij.codeInspection.dataFlow.DfaInstructionState;
import com.intellij.codeInspection.dataFlow.DfaMemoryState;
import com.intellij.codeInspection.dataFlow.InstructionVisitor;
import com.intellij.psi.PsiExpression;
import org.jetbrains.annotations.NotNull;
/**
* @author max
*/
public class CheckReturnValueInstruction extends Instruction {
private final @NotNull PsiExpression myReturnValue;
public CheckReturnValueInstruction(@NotNull PsiExpression returnValue) {
myReturnValue = returnValue;
}
@Override
public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) {
return visitor.visitCheckReturnValue(this, runner, stateBefore);
}
@NotNull
public PsiExpression getReturn() {
return myReturnValue;
}
public String toString() {
return "CheckReturnValue";
}
}
java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/TypeCastInstruction.java
+8 -5
View File
@@ -23,8 +23,9 @@ import com.intellij.codeInspection.dataFlow.InstructionVisitor;
import com.intellij.psi.PsiExpression;
import com.intellij.psi.PsiType;
import com.intellij.psi.PsiTypeCastExpression;
import org.jetbrains.annotations.NotNull;
public class TypeCastInstruction extends Instruction {
public class TypeCastInstruction extends Instruction implements ExpressionPushingInstruction {
private final PsiTypeCastExpression myCastExpression;
private final PsiExpression myCasted;
private final PsiType myCastTo;
@@ -35,10 +36,6 @@ public class TypeCastInstruction extends Instruction {
myCastTo = castTo;
}
public PsiTypeCastExpression getCastExpression() {
return myCastExpression;
}
public PsiExpression getCasted() {
return myCasted;
}
@@ -56,4 +53,10 @@ public class TypeCastInstruction extends Instruction {
public String toString() {
return "CAST_TO "+myCastTo.getCanonicalText();
}
@NotNull
@Override
public PsiTypeCastExpression getExpression() {
return myCastExpression;
}
}
java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java
+19 -24
View File
@@ -11,8 +11,8 @@ import com.intellij.codeInsight.intention.AddAnnotationPsiFix;
import com.intellij.codeInsight.navigation.NavigationUtil;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.codeInspection.dataFlow.instructions.BranchingInstruction;
import com.intellij.codeInspection.dataFlow.instructions.CheckReturnValueInstruction;
import com.intellij.codeInspection.dataFlow.instructions.Instruction;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
import com.intellij.ide.DataManager;
import com.intellij.ide.util.PropertiesComponent;
import com.intellij.ide.util.PsiClassListCellRenderer;
@@ -399,11 +399,8 @@ public class ExtractMethodProcessor implements MatchProvider {
*/
private boolean getReturnsNullability(boolean nullsExpected) {
PsiElement body = null;
if (myCodeFragmentMember instanceof PsiMethod) {
body = ((PsiMethod)myCodeFragmentMember).getBody();
}
else if (myCodeFragmentMember instanceof PsiLambdaExpression) {
body = ((PsiLambdaExpression)myCodeFragmentMember).getBody();
if (myCodeFragmentMember instanceof PsiParameterListOwner) {
body = ((PsiParameterListOwner)myCodeFragmentMember).getBody();
}
if (body == null) return false;
@@ -434,25 +431,23 @@ public class ExtractMethodProcessor implements MatchProvider {
}
if (returnedExpressions.isEmpty()) return true;
class ReturnChecker extends StandardInstructionVisitor {
boolean myResult = true;
@Override
public DfaInstructionState[] visitCheckReturnValue(CheckReturnValueInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
if (returnedExpressions.contains(instruction.getReturn())) {
myResult &= nullsExpected ? memState.isNull(memState.peek()) : memState.isNotNull(memState.peek());
}
return super.visitCheckReturnValue(instruction, runner, memState);
}
}
final StandardDataFlowRunner dfaRunner = new StandardDataFlowRunner();
final ReturnChecker returnChecker = new ReturnChecker();
if (dfaRunner.analyzeMethod(body, returnChecker) == RunnerResult.OK) {
return returnChecker.myResult;
}
return false;
final StandardInstructionVisitor returnChecker = new StandardInstructionVisitor() {
@Override
protected void checkReturnValue(@NotNull DfaValue value,
@NotNull PsiExpression expression,
@NotNull PsiParameterListOwner context,
@NotNull DfaMemoryState state) {
if (context == myCodeFragmentMember &&
returnedExpressions.stream().anyMatch(ret -> PsiTreeUtil.isAncestor(ret, expression, false))) {
boolean result = nullsExpected ? state.isNull(value) : state.isNotNull(value);
if (!result) {
dfaRunner.cancel();
}
}
}
};
return dfaRunner.analyzeMethod(body, returnChecker) == RunnerResult.OK;
}
protected boolean insertNotNullCheckIfPossible() {
java/java-tests/testData/inspection/dataFlow/boxingBoolean/expected.xml
-45
View File
@@ -1,45 +0,0 @@
<?xml version="1.0" encoding="UTF-8"?>
<problems>
<problem>
<file>Test.java</file>
<line>14</line>
<description>Condition &lt;code&gt;c&lt;/code&gt; is always &lt;code&gt;false&lt;/code&gt;.</description>
</problem>
<problem>
<file>Test.java</file>
<line>20</line>
<description>Condition &lt;code&gt;c&lt;/code&gt; is always &lt;code&gt;true&lt;/code&gt;.</description>
</problem>
<problem>
<file>Test.java</file>
<line>33</line>
<description>Condition &lt;code&gt;o&lt;/code&gt; is always &lt;code&gt;true&lt;/code&gt;.</description>
</problem>
<problem>
<file>Test.java</file>
<line>39</line>
<description>Condition &lt;code&gt;o&lt;/code&gt; is always &lt;code&gt;false&lt;/code&gt;.</description>
</problem>
<problem>
<file>Test.java</file>
<line>45</line>
<description>Condition &lt;code&gt;o&lt;/code&gt; is always &lt;code&gt;true&lt;/code&gt;.</description>
</problem>
<problem>
<file>Test.java</file>
<line>51</line>
<description>Condition &lt;code&gt;o&lt;/code&gt; at the left side of assignment expression is always &lt;code&gt;false&lt;/code&gt;. Can be simplified.</description>
</problem>
<problem>
<file>Test.java</file>
<line>62</line>
<description>Condition &lt;code&gt;o&lt;/code&gt; is always &lt;code&gt;true&lt;/code&gt;</description>
</problem>
</problems>
java/java-tests/testData/inspection/dataFlow/boxingBoolean/src/Test.java
-65
View File
@@ -1,65 +0,0 @@
public class S {
void f(Boolean override) {
if (override == null) {
//doSomething();
} else if (override) { // always false?
//doOverride();
}
}
public void te0(boolean b){
Boolean c = false;
// if (b) c = true;
if (c) {
}
}
public void te1(boolean b){
Boolean c = true;
// if (b) c = true;
if (c) {
}
}
public void te2(boolean b){
Boolean c = false;
if (b) c = true;
if (c) {
}
}
public void te3(boolean b){
Boolean c = Boolean.FALSE;
boolean o = !c;
if (o) {
}
}
public void te4(boolean b){
Boolean c = Boolean.FALSE;
boolean o = c;
if (o) {
}
}
public void te5(boolean b){
Boolean c = Boolean.TRUE;
boolean o = b||c;
if (o) {
}
}
public void te6(boolean b){
Boolean c = Boolean.TRUE;
boolean o = !c;
o |= c&b;
if (o) {
}
}
public void flushOriginal(boolean b){
boolean o;
{
Boolean c = Boolean.FALSE;
o = !c;
}
if (o) {
}
}
}
java/java-tests/testData/inspection/dataFlow/fixture/AndEquals.java
+1 -1
View File
@@ -3,7 +3,7 @@ import java.util.List;
class Some {
public static void appendTokenTypes(StringBuilder sb, List<String> tokenTypes) {
for (int count = 0, line = 0, size = tokenTypes.size(); count < size; count++) {
boolean newLine = count == 2 || <warning descr="Condition 'line > 0' is always 'false' when reached">line > 0</warning> && (count - 2) % 6 == 0;
boolean newLine = count == 2 || <warning descr="Condition 'line > 0 && (count - 2) % 6 == 0' is always 'false' when reached"><warning descr="Condition 'line > 0' is always 'false' when reached">line > 0</warning> && (count - 2) % 6 == 0</warning>;
newLine &= (size - count) > 2;
}
}
java/java-tests/testData/inspection/dataFlow/fixture/BoxingBoolean.java
+65
View File
@@ -0,0 +1,65 @@
class S {
void f(Boolean override) {
if (override == null) {
//doSomething();
} else if (override) { // always false?
//doOverride();
}
}
public void te0(boolean b){
Boolean c = false;
// if (b) c = true;
if (<warning descr="Condition 'c' is always 'false'">c</warning>) {
}
}
public void te1(boolean b){
Boolean c = true;
// if (b) c = true;
if (<warning descr="Condition 'c' is always 'true'">c</warning>) {
}
}
public void te2(boolean b){
Boolean c = false;
if (b) c = true;
if (c) {
}
}
public void te3(boolean b){
Boolean c = Boolean.FALSE;
boolean o = !c;
if (<warning descr="Condition 'o' is always 'true'">o</warning>) {
}
}
public void te4(boolean b){
Boolean c = Boolean.FALSE;
boolean o = c;
if (<warning descr="Condition 'o' is always 'false'">o</warning>) {
}
}
public void te5(boolean b){
Boolean c = Boolean.TRUE;
boolean o = <warning descr="Condition 'b||c' is always 'true'">b||<warning descr="Condition 'c' is always 'true' when reached">c</warning></warning>;
if (<warning descr="Condition 'o' is always 'true'">o</warning>) {
}
}
public void te6(boolean b){
Boolean c = Boolean.TRUE;
boolean o = !c;
<warning descr="Condition 'o' at the left side of assignment expression is always 'false'. Can be simplified">o</warning> |= c&b;
if (o) {
}
}
public void flushOriginal(boolean b){
boolean o;
{
Boolean c = Boolean.FALSE;
o = !c;
}
if (<warning descr="Condition 'o' is always 'true'">o</warning>) {
}
}
}
java/java-tests/testData/inspection/dataFlow/fixture/BuildRegexpNotComplex.java
+1 -1
View File
@@ -8,7 +8,7 @@ class X {
final char c = pattern.charAt(i);
if (c == '*') { }
else if (c == ' ') { }
else if (c == ':' || prevIsUppercase) { }
else if (c == ':' || <warning descr="Condition 'prevIsUppercase' is always 'false' when reached">prevIsUppercase</warning>) { }
}
System.out.println(forCompletion);
System.out.println(exactPrefixLen);
java/java-tests/testData/inspection/dataFlow/fixture/DoubleNaN.java
+1 -1
View File
@@ -32,6 +32,6 @@ public class DoubleNaN {
void test2() {
System.out.println(<warning descr="Condition '1.0 == Double.NaN' is always 'false'">1.0 == Double.NaN</warning>);
System.out.println(!(<warning descr="Condition '1.0 < Double.NaN' is always 'false'">1.0 < Double.NaN</warning>));
System.out.println(<warning descr="Condition '!(1.0 < Double.NaN)' is always 'true'">!(<warning descr="Condition '1.0 < Double.NaN' is always 'false'">1.0 < Double.NaN</warning>)</warning>);
}
}
java/java-tests/testData/inspection/dataFlow/fixture/EqualsWithItself.java
+1 -1
View File
@@ -11,7 +11,7 @@ class Test {
System.out.println("never");
}
y = x;
if(x.equals(y)) {
if(<warning descr="Condition 'x.equals(y)' is always 'true'">x.equals(y)</warning>) {
System.out.println("always");
}
}
java/java-tests/testData/inspection/dataFlow/fixture/OptionalIsPresent.java
+2 -2
View File
@@ -29,8 +29,8 @@ class Test {
maybe = Optional.empty();
System.out.println(maybe.<warning descr="The call to 'get' always fails, according to its method contracts">get</warning>());
}
boolean b = <warning descr="Condition '((maybe.isPresent()))' is always 'false'">((<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>))</warning> && maybe.get() == 1;
boolean c = <warning descr="Condition '(!maybe.isPresent())' is always 'true'">(!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>)</warning> || maybe.get() == 1;
boolean b = <warning descr="Condition '((maybe.isPresent())) && maybe.get() == 1' is always 'false'"><warning descr="Condition '((maybe.isPresent()))' is always 'false'">((<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning>))</warning> && maybe.get() == 1</warning>;
boolean c = <warning descr="Condition '(!maybe.isPresent()) || maybe.get() == 1' is always 'true'"><warning descr="Condition '(!maybe.isPresent())' is always 'true'">(<warning descr="Condition '!maybe.isPresent()' is always 'true'">!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning></warning>)</warning> || maybe.get() == 1</warning>;
Integer value = <warning descr="Condition '!maybe.isPresent()' is always 'true'">!<warning descr="Condition 'maybe.isPresent()' is always 'false'">maybe.isPresent()</warning></warning> ? 0 : maybe.get();
}
java/java-tests/testData/inspection/dataFlow/fixture/ReturningConstantExpression.java
+1 -1
View File
@@ -2,7 +2,7 @@ class BrokenAlignment {
boolean smth() {
if (<warning descr="Condition '2 == 2' is always 'true'">2 == 2</warning>) {
return true;
System.out.println("True");
}
boolean b = <warning descr="Condition '3 == 3' is always 'true'">3 == 3</warning>;
java/java-tests/testData/inspection/dataFlow/fixture/SkipAssertions.java
+1 -1
View File
@@ -38,7 +38,7 @@ class Test {
private static void testOrNotFail(boolean a, boolean b, boolean c) {
if(b) {
assert <warning descr="Condition '!(a || b || c)' is always 'false'">!(a || <warning descr="Condition 'b' is always 'true'">b</warning> || c)</warning>;
assert <warning descr="Condition '!(a || b || c)' is always 'false'">!(<warning descr="Condition 'a || b || c' is always 'true'">a || <warning descr="Condition 'b' is always 'true'">b</warning> || c</warning>)</warning>;
}
}
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionAncientTest.java
-1
View File
@@ -105,7 +105,6 @@ public class DataFlowInspectionAncientTest extends InspectionTestCase {
public void testIDEADEV2605() { doTest15(); }
public void testConstantsDifferentTypes() { doTest15(); }
public void testBoxingNaN() { doTest15(); }
public void testBoxingBoolean() { doTest15(true); }
public void testCheckedExceptionDominance() { doTest15(); }
public void testIDEADEV10489() { doTest15(); }
public void testPlusOnStrings() { doTest15(); }
java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java
+1
View File
@@ -621,4 +621,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testPolyadicEquality() { doTest(); }
public void testEqualsInLoopNotTooComplex() { doTest(); }
public void testEqualsWithItself() { doTest(); }
public void testBoxingBoolean() { doTest(); }
}
plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/SuspiciousComparatorCompareInspection.java
+33 -22
View File
@@ -16,7 +16,6 @@
package com.siyeh.ig.bugs;
import com.intellij.codeInspection.dataFlow.*;
import com.intellij.codeInspection.dataFlow.instructions.CheckReturnValueInstruction;
import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet;
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue;
import com.intellij.codeInspection.dataFlow.value.DfaValue;
@@ -76,7 +75,7 @@ public class SuspiciousComparatorCompareInspection extends BaseInspection {
if (!MethodUtils.isComparatorCompare(method) || ControlFlowUtils.methodAlwaysThrowsException(method)) {
return;
}
check(method.getParameterList(), method.getBody());
check(method);
}
@Override
@@ -87,10 +86,12 @@ public class SuspiciousComparatorCompareInspection extends BaseInspection {
ControlFlowUtils.lambdaExpressionAlwaysThrowsException(lambda)) {
return;
}
check(lambda.getParameterList(), lambda.getBody());
check(lambda);
}
private void check(PsiParameterList parameterList, PsiElement body) {
private void check(PsiParameterListOwner owner) {
PsiParameterList parameterList = owner.getParameterList();
PsiElement body = owner.getBody();
if (body == null || parameterList.getParametersCount() != 2) return;
// comparator like "(a, b) -> 0" fulfills the comparator contract, so no need to warn its parameters are not used
if (body instanceof PsiExpression && ExpressionUtils.isZero((PsiExpression)body)) return;
@@ -105,7 +106,7 @@ public class SuspiciousComparatorCompareInspection extends BaseInspection {
}
PsiParameter[] parameters = parameterList.getParameters();
checkParameterList(parameters, body);
checkReflexivity(parameters, body);
checkReflexivity(owner, parameters, body);
}
private void checkParameterList(PsiParameter[] parameters, PsiElement context) {
@@ -117,7 +118,7 @@ public class SuspiciousComparatorCompareInspection extends BaseInspection {
}
}
private void checkReflexivity(PsiParameter[] parameters, PsiElement body) {
private void checkReflexivity(PsiParameterListOwner owner, PsiParameter[] parameters, PsiElement body) {
StandardDataFlowRunner runner = new StandardDataFlowRunner(false, body) {
@NotNull
@Override
@@ -130,20 +131,25 @@ public class SuspiciousComparatorCompareInspection extends BaseInspection {
return state;
}
};
ComparatorVisitor visitor = new ComparatorVisitor();
ComparatorVisitor visitor = new ComparatorVisitor(owner);
if (runner.analyzeMethod(body, visitor) != RunnerResult.OK) return;
if (visitor.myRange.contains(0)) return;
if (visitor.myRange.contains(0) || visitor.myContexts.isEmpty()) return;
PsiElement context = null;
if (visitor.myContexts.size() == 1) {
context = visitor.myContexts.iterator().next();
}
else {
PsiElement parent = PsiTreeUtil.getParentOfType(body, PsiMethod.class, PsiLambdaExpression.class);
if (parent instanceof PsiMethod) {
context = ((PsiMethod)parent).getNameIdentifier();
}
else if (parent instanceof PsiLambdaExpression) {
context = ((PsiLambdaExpression)parent).getParameterList();
PsiElement commonParent = PsiTreeUtil.findCommonParent(visitor.myContexts.toArray(PsiElement.EMPTY_ARRAY));
if (commonParent instanceof PsiExpression) {
context = commonParent;
} else {
PsiParameterListOwner parent = PsiTreeUtil.getParentOfType(body, PsiMethod.class, PsiLambdaExpression.class);
if (parent instanceof PsiMethod) {
context = ((PsiMethod)parent).getNameIdentifier();
}
else if (parent instanceof PsiLambdaExpression) {
context = parent.getParameterList();
}
}
}
registerError(context != null ? context : body,
@@ -151,18 +157,23 @@ public class SuspiciousComparatorCompareInspection extends BaseInspection {
}
private static class ComparatorVisitor extends StandardInstructionVisitor {
private final PsiParameterListOwner myOwner;
private final Set<PsiElement> myContexts = new HashSet<>();
LongRangeSet myRange = LongRangeSet.empty();
Set<PsiElement> myContexts = new HashSet<>();
public ComparatorVisitor(PsiParameterListOwner owner) {
myOwner = owner;
}
@Override
public DfaInstructionState[] visitCheckReturnValue(CheckReturnValueInstruction instruction,
DataFlowRunner runner,
DfaMemoryState memState) {
myContexts.add(instruction.getReturn());
DfaValue value = memState.peek();
LongRangeSet range = memState.getValueFact(value, DfaFactType.RANGE);
protected void checkReturnValue(@NotNull DfaValue value,
@NotNull PsiExpression expression,
@NotNull PsiParameterListOwner owner,
@NotNull DfaMemoryState state) {
if (owner != myOwner) return;
myContexts.add(expression);
LongRangeSet range = state.getValueFact(value, DfaFactType.RANGE);
myRange = range == null ? LongRangeSet.all() : myRange.union(range);
return super.visitCheckReturnValue(instruction, runner, memState);
}
}
plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/suspicious_comparator_compare/ComparatorIsNotReflexive.java
+8 -2
View File
@@ -21,11 +21,17 @@ class ComparatorIsNotReflexive implements Comparator<Integer> {
return <warning descr="Comparator does not return 0 for equal elements">-1</warning>;
}
Comparator<String> lambda = (a, b) -> <warning descr="Comparator does not return 0 for equal elements">a.length() > b.length() ? 1 : -1</warning>;
Comparator<String> lambda = (a, b) -> a.length() > b.length() ? 1 : <warning descr="Comparator does not return 0 for equal elements">-1</warning>;
Comparator<String> lambda2 = (a, b) -> a.length() > b.length() ? 1 :
(a.length() < b.length() ? 0 : <warning descr="Comparator does not return 0 for equal elements">-1</warning>);
Comparator<String> lambda3 = (a, b) -> (a.length() > b.length() ? 0 :
<warning descr="Comparator does not return 0 for equal elements">Math.random() > 0.5 ? (-1) : (1)</warning>);
Comparator<byte[]> arrayComparator = (b1, b2) -> {
if(b1.length != b2.length) return 0; // typo: == was intended
return <warning descr="Comparator does not return 0 for equal elements">b1.length > b2.length ? 1 : -1</warning>;
return b1.length > b2.length ? 1 : <warning descr="Comparator does not return 0 for equal elements">-1</warning>;
};
Comparator<String> cmp = (a,b) -> test();