From 7c9f5ce2c97016eef500d84ccdea52128e5fe879 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 22 Nov 2017 11:04:02 +0700 Subject: [PATCH] DataFlowInstructionVisitor extracted to separate file --- .../dataFlow/DataFlowInspectionBase.java | 234 +---------------- .../dataFlow/DataFlowInstructionVisitor.java | 243 ++++++++++++++++++ 2 files changed, 252 insertions(+), 225 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 90491bc92dc0..6d36624b7e9c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -16,9 +16,7 @@ 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.DfaUnknownValue; import com.intellij.codeInspection.dataFlow.value.DfaValue; -import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.codeInspection.nullable.NullableStuffInspectionBase; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; @@ -29,10 +27,15 @@ import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.util.*; +import com.intellij.util.ArrayUtil; +import com.intellij.util.ArrayUtilRt; +import com.intellij.util.IncorrectOperationException; +import com.intellij.util.ThreeState; import com.intellij.util.containers.ContainerUtil; -import com.intellij.util.containers.MultiMap; -import com.siyeh.ig.psiutils.*; +import com.siyeh.ig.psiutils.ComparisonUtils; +import com.siyeh.ig.psiutils.ControlFlowUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.TestUtils; import one.util.streamex.StreamEx; import org.jdom.Element; import org.jetbrains.annotations.NonNls; @@ -41,7 +44,6 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.*; -import java.util.stream.Stream; @SuppressWarnings("ConditionalExpressionWithIdenticalBranches") public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool { @@ -231,7 +233,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool ArrayList allProblems = new ArrayList<>(); allProblems.addAll(trueSet); allProblems.addAll(falseSet); - allProblems.addAll(visitor.myCCEInstructions); + allProblems.addAll(visitor.getClassCastExceptionInstructions()); allProblems.addAll(ContainerUtil.filter(runner.getInstructions(), instruction1 -> instruction1 instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction1))); HashSet reportedAnchors = new HashSet<>(); @@ -849,222 +851,4 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool public String getShortName() { return SHORT_NAME; } - - private static class DataFlowInstructionVisitor extends StandardInstructionVisitor { - private static final Object ANY_VALUE = new Object(); - private final Map, StateInfo> myStateInfos = new LinkedHashMap<>(); - private final Set myCCEInstructions = ContainerUtil.newHashSet(); - private final Map myFailingCalls = new HashMap<>(); - private final Map myOptionalCalls = new HashMap<>(); - private final Map myBooleanCalls = new HashMap<>(); - private final Map myOfNullableCalls = new HashMap<>(); - private final Map> myArrayStoreProblems = new HashMap<>(); - private final Map myMethodReferenceResults = new HashMap<>(); - private final Map myOutOfBoundsArrayAccesses = new HashMap<>(); - private final List myOptionalQualifiers = new ArrayList<>(); - private final MultiMap myPossibleVariableValues = MultiMap.createSet(); - private boolean myAlwaysReturnsNotNull = true; - - @Override - protected void onInstructionProducesCCE(TypeCastInstruction instruction) { - myCCEInstructions.add(instruction); - } - - StreamEx> problems() { - // non-ephemeral NPE should be reported - // ephemeral NPE should also be reported if only ephemeral states have reached a particular problematic instruction - // (e.g. if it's inside "if (var == null)" check after contract method invocation - return StreamEx.ofKeys(myStateInfos, info -> info.normalNpe || info.ephemeralNpe && !info.normalOk); - } - - public Map> getArrayStoreProblems() { - return myArrayStoreProblems; - } - - Map getOptionalCalls() { - return myOptionalCalls; - } - - Map getOfNullableCalls() { - return myOfNullableCalls; - } - - Map getBooleanCalls() { - return myBooleanCalls; - } - - Map getMethodReferenceResults() { - return myMethodReferenceResults; - } - - Stream outOfBoundsArrayAccesses() { - return StreamEx.ofKeys(myOutOfBoundsArrayAccesses, ThreeState.YES::equals); - } - - List getOptionalQualifiers() { - return myOptionalQualifiers; - } - - Map> getAlwaysFailingCalls() { - return StreamEx.ofKeys(myFailingCalls, v -> v) - .mapToEntry(MethodCallInstruction::getCallExpression, MethodCallInstruction::getContracts).toMap(); - } - - boolean isAlwaysReturnsNotNull(Instruction[] instructions) { - return myAlwaysReturnsNotNull && - ContainerUtil.exists(instructions, i -> i instanceof ReturnInstruction && ((ReturnInstruction)i).getAnchor() instanceof PsiReturnStatement); - } - - @Override - public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, - DataFlowRunner runner, - DfaMemoryState memState) { - PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class); - if (call != null) { - String methodName = call.getMethodExpression().getReferenceName(); - PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression()); - if (qualifier != null && TypeUtils.isOptional(qualifier.getType())) { - if ("isPresent".equals(methodName) && qualifier instanceof PsiMethodCallExpression) { - myOptionalQualifiers.add(qualifier); - } - else if (DfaOptionalSupport.isOptionalGetMethodName(methodName)) { - Boolean fact = memState.getValueFact(memState.peek(), DfaFactType.OPTIONAL_PRESENCE); - ThreeState state = fact == null ? ThreeState.UNSURE : ThreeState.fromBoolean(fact); - myOptionalCalls.merge(call, state, ThreeState::merge); - } - } - } - if (instruction.matches(DfaOptionalSupport.OPTIONAL_OF_NULLABLE)) { - DfaValue arg = memState.peek(); - ThreeState nullArg = memState.isNull(arg) ? ThreeState.YES : memState.isNotNull(arg) ? ThreeState.NO : ThreeState.UNSURE; - myOfNullableCalls.merge(instruction, nullArg, ThreeState::merge); - } - DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState); - if (hasNonTrivialFailingContracts(instruction)) { - DfaConstValue fail = runner.getFactory().getConstFactory().getContractFail(); - boolean allFail = Arrays.stream(states).allMatch(s -> s.getMemoryState().peek() == fail); - myFailingCalls.merge(instruction, allFail, Boolean::logicalAnd); - } - handleBooleanCalls(instruction, states); - return states; - } - - void handleBooleanCalls(MethodCallInstruction instruction, DfaInstructionState[] states) { - if (!hasNonTrivialBooleanContracts(instruction)) return; - PsiMethod method = instruction.getTargetMethod(); - if (method == null || !ControlFlowAnalyzer.isPure(method)) return; - PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class); - if (call == null || myBooleanCalls.get(call) == ThreeState.UNSURE) return; - PsiElement parent = call.getParent(); - if (parent instanceof PsiExpressionStatement) return; - if (parent instanceof PsiLambdaExpression && - PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType((PsiLambdaExpression)parent))) { - return; - } - for (DfaInstructionState s : states) { - DfaValue val = s.getMemoryState().peek(); - ThreeState state = ThreeState.UNSURE; - if (val instanceof DfaConstValue) { - Object value = ((DfaConstValue)val).getValue(); - if (value instanceof Boolean) { - state = ThreeState.fromBoolean((Boolean)value); - } - } - myBooleanCalls.merge(call, state, ThreeState::merge); - } - } - - @Override - protected void processArrayAccess(PsiArrayAccessExpression expression, boolean alwaysOutOfBounds) { - myOutOfBoundsArrayAccesses.merge(expression, ThreeState.fromBoolean(alwaysOutOfBounds), ThreeState::merge); - } - - @Override - protected void processArrayStoreTypeMismatch(PsiAssignmentExpression assignmentExpression, PsiType fromType, PsiType toType) { - if (assignmentExpression != null) { - myArrayStoreProblems.put(assignmentExpression, Pair.create(fromType, toType)); - } - } - - @Override - protected void processMethodReferenceResult(PsiMethodReferenceExpression methodRef, - List contracts, - DfaValue res) { - if(contracts.isEmpty() || !contracts.get(0).isTrivial()) { - // Do not track if method reference may have different results - myMethodReferenceResults.merge(methodRef, res, (a, b) -> a == b ? a : DfaUnknownValue.getInstance()); - } - } - - @Override - public DfaInstructionState[] visitPush(PushInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - PsiExpression place = instruction.getPlace(); - if (!instruction.isReferenceWrite() && place instanceof PsiReferenceExpression) { - DfaValue dfaValue = instruction.getValue(); - if (dfaValue instanceof DfaVariableValue) { - DfaConstValue constValue = memState.getConstantValue((DfaVariableValue)dfaValue); - boolean report = constValue != null && shouldReportConstValue(constValue.getValue(), place); - myPossibleVariableValues.putValue(instruction, report ? constValue : ANY_VALUE); - } - } - return super.visitPush(instruction, runner, memState); - } - - public List> getConstantReferenceValues() { - List> result = ContainerUtil.newArrayList(); - for (PushInstruction instruction : myPossibleVariableValues.keySet()) { - Collection values = myPossibleVariableValues.get(instruction); - if (values.size() == 1) { - Object singleValue = values.iterator().next(); - if (singleValue != ANY_VALUE) { - result.add(Pair.create((PsiReferenceExpression)instruction.getPlace(), (DfaConstValue)singleValue)); - } - } - } - return result; - } - - private static boolean hasNonTrivialFailingContracts(MethodCallInstruction instruction) { - List contracts = instruction.getContracts(); - return !contracts.isEmpty() && contracts.stream().anyMatch( - contract -> contract.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION && !contract.isTrivial()); - } - - private static boolean hasNonTrivialBooleanContracts(MethodCallInstruction instruction) { - if (CustomMethodHandlers.find(instruction) != null) return true; - List contracts = instruction.getContracts(); - return !contracts.isEmpty() && contracts.stream().anyMatch( - contract -> (contract.getReturnValue() == MethodContract.ValueConstraint.FALSE_VALUE || - contract.getReturnValue() == MethodContract.ValueConstraint.TRUE_VALUE) - && !contract.isTrivial()); - } - - @Override - protected boolean checkNotNullable(DfaMemoryState state, DfaValue value, @Nullable NullabilityProblem problem) { - if (NullabilityProblemKind.nullableReturn.isMyProblem(problem) && !state.isNotNull(value)) { - myAlwaysReturnsNotNull = false; - } - - boolean ok = super.checkNotNullable(state, value, problem); - if (problem == null) return ok; - StateInfo info = myStateInfos.computeIfAbsent(problem, k -> new StateInfo()); - if (state.isEphemeral() && !ok) { - info.ephemeralNpe = true; - } else if (!state.isEphemeral()) { - if (ok) info.normalOk = true; - else info.normalNpe = true; - } - return ok; - } - - private static boolean shouldReportConstValue(Object value, PsiElement place) { - return value == null || value instanceof Boolean; - } - - private static class StateInfo { - boolean ephemeralNpe; - boolean normalNpe; - boolean normalOk; - } - } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java new file mode 100644 index 000000000000..23a7de1aeb03 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -0,0 +1,243 @@ +// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.codeInspection.dataFlow; + +import com.intellij.codeInspection.dataFlow.instructions.*; +import com.intellij.codeInspection.dataFlow.value.DfaConstValue; +import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue; +import com.intellij.codeInspection.dataFlow.value.DfaValue; +import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; +import com.intellij.openapi.util.Pair; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ObjectUtils; +import com.intellij.util.ThreeState; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.MultiMap; +import com.siyeh.ig.psiutils.TypeUtils; +import one.util.streamex.StreamEx; +import org.jetbrains.annotations.Nullable; + +import java.util.*; +import java.util.stream.Stream; + +final class DataFlowInstructionVisitor extends StandardInstructionVisitor { + private static final Object ANY_VALUE = new Object(); + private final Map, StateInfo> myStateInfos = new LinkedHashMap<>(); + private final Set myCCEInstructions = ContainerUtil.newHashSet(); + private final Map myFailingCalls = new HashMap<>(); + private final Map myOptionalCalls = new HashMap<>(); + private final Map myBooleanCalls = new HashMap<>(); + private final Map myOfNullableCalls = new HashMap<>(); + private final Map> myArrayStoreProblems = new HashMap<>(); + private final Map myMethodReferenceResults = new HashMap<>(); + private final Map myOutOfBoundsArrayAccesses = new HashMap<>(); + private final List myOptionalQualifiers = new ArrayList<>(); + private final MultiMap myPossibleVariableValues = MultiMap.createSet(); + private boolean myAlwaysReturnsNotNull = true; + + @Override + protected void onInstructionProducesCCE(TypeCastInstruction instruction) { + myCCEInstructions.add(instruction); + } + + StreamEx> problems() { + // non-ephemeral NPE should be reported + // ephemeral NPE should also be reported if only ephemeral states have reached a particular problematic instruction + // (e.g. if it's inside "if (var == null)" check after contract method invocation + return StreamEx.ofKeys(myStateInfos, info -> info.normalNpe || info.ephemeralNpe && !info.normalOk); + } + + public Map> getArrayStoreProblems() { + return myArrayStoreProblems; + } + + Map getOptionalCalls() { + return myOptionalCalls; + } + + Map getOfNullableCalls() { + return myOfNullableCalls; + } + + Map getBooleanCalls() { + return myBooleanCalls; + } + + Map getMethodReferenceResults() { + return myMethodReferenceResults; + } + + Set getClassCastExceptionInstructions() { + return myCCEInstructions; + } + + Stream outOfBoundsArrayAccesses() { + return StreamEx.ofKeys(myOutOfBoundsArrayAccesses, ThreeState.YES::equals); + } + + List getOptionalQualifiers() { + return myOptionalQualifiers; + } + + Map> getAlwaysFailingCalls() { + return StreamEx.ofKeys(myFailingCalls, v -> v) + .mapToEntry(MethodCallInstruction::getCallExpression, MethodCallInstruction::getContracts).toMap(); + } + + boolean isAlwaysReturnsNotNull(Instruction[] instructions) { + return myAlwaysReturnsNotNull && + ContainerUtil.exists(instructions, i -> i instanceof ReturnInstruction && ((ReturnInstruction)i).getAnchor() instanceof PsiReturnStatement); + } + + @Override + public DfaInstructionState[] visitMethodCall(MethodCallInstruction instruction, + DataFlowRunner runner, + DfaMemoryState memState) { + PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class); + if (call != null) { + String methodName = call.getMethodExpression().getReferenceName(); + PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(call.getMethodExpression().getQualifierExpression()); + if (qualifier != null && TypeUtils.isOptional(qualifier.getType())) { + if ("isPresent".equals(methodName) && qualifier instanceof PsiMethodCallExpression) { + myOptionalQualifiers.add(qualifier); + } + else if (DfaOptionalSupport.isOptionalGetMethodName(methodName)) { + Boolean fact = memState.getValueFact(memState.peek(), DfaFactType.OPTIONAL_PRESENCE); + ThreeState state = fact == null ? ThreeState.UNSURE : ThreeState.fromBoolean(fact); + myOptionalCalls.merge(call, state, ThreeState::merge); + } + } + } + if (instruction.matches(DfaOptionalSupport.OPTIONAL_OF_NULLABLE)) { + DfaValue arg = memState.peek(); + ThreeState nullArg = memState.isNull(arg) ? ThreeState.YES : memState.isNotNull(arg) ? ThreeState.NO : ThreeState.UNSURE; + myOfNullableCalls.merge(instruction, nullArg, ThreeState::merge); + } + DfaInstructionState[] states = super.visitMethodCall(instruction, runner, memState); + if (hasNonTrivialFailingContracts(instruction)) { + DfaConstValue fail = runner.getFactory().getConstFactory().getContractFail(); + boolean allFail = Arrays.stream(states).allMatch(s -> s.getMemoryState().peek() == fail); + myFailingCalls.merge(instruction, allFail, Boolean::logicalAnd); + } + handleBooleanCalls(instruction, states); + return states; + } + + void handleBooleanCalls(MethodCallInstruction instruction, DfaInstructionState[] states) { + if (!hasNonTrivialBooleanContracts(instruction)) return; + PsiMethod method = instruction.getTargetMethod(); + if (method == null || !ControlFlowAnalyzer.isPure(method)) return; + PsiMethodCallExpression call = ObjectUtils.tryCast(instruction.getCallExpression(), PsiMethodCallExpression.class); + if (call == null || myBooleanCalls.get(call) == ThreeState.UNSURE) return; + PsiElement parent = call.getParent(); + if (parent instanceof PsiExpressionStatement) return; + if (parent instanceof PsiLambdaExpression && + PsiType.VOID.equals(LambdaUtil.getFunctionalInterfaceReturnType((PsiLambdaExpression)parent))) { + return; + } + for (DfaInstructionState s : states) { + DfaValue val = s.getMemoryState().peek(); + ThreeState state = ThreeState.UNSURE; + if (val instanceof DfaConstValue) { + Object value = ((DfaConstValue)val).getValue(); + if (value instanceof Boolean) { + state = ThreeState.fromBoolean((Boolean)value); + } + } + myBooleanCalls.merge(call, state, ThreeState::merge); + } + } + + @Override + protected void processArrayAccess(PsiArrayAccessExpression expression, boolean alwaysOutOfBounds) { + myOutOfBoundsArrayAccesses.merge(expression, ThreeState.fromBoolean(alwaysOutOfBounds), ThreeState::merge); + } + + @Override + protected void processArrayStoreTypeMismatch(PsiAssignmentExpression assignmentExpression, PsiType fromType, PsiType toType) { + if (assignmentExpression != null) { + myArrayStoreProblems.put(assignmentExpression, Pair.create(fromType, toType)); + } + } + + @Override + protected void processMethodReferenceResult(PsiMethodReferenceExpression methodRef, + List contracts, + DfaValue res) { + if(contracts.isEmpty() || !contracts.get(0).isTrivial()) { + // Do not track if method reference may have different results + myMethodReferenceResults.merge(methodRef, res, (a, b) -> a == b ? a : DfaUnknownValue.getInstance()); + } + } + + @Override + public DfaInstructionState[] visitPush(PushInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { + PsiExpression place = instruction.getPlace(); + if (!instruction.isReferenceWrite() && place instanceof PsiReferenceExpression) { + DfaValue dfaValue = instruction.getValue(); + if (dfaValue instanceof DfaVariableValue) { + DfaConstValue constValue = memState.getConstantValue((DfaVariableValue)dfaValue); + boolean report = constValue != null && shouldReportConstValue(constValue.getValue()); + myPossibleVariableValues.putValue(instruction, report ? constValue : ANY_VALUE); + } + } + return super.visitPush(instruction, runner, memState); + } + + public List> getConstantReferenceValues() { + List> result = ContainerUtil.newArrayList(); + for (PushInstruction instruction : myPossibleVariableValues.keySet()) { + Collection values = myPossibleVariableValues.get(instruction); + if (values.size() == 1) { + Object singleValue = values.iterator().next(); + if (singleValue != ANY_VALUE) { + result.add(Pair.create((PsiReferenceExpression)instruction.getPlace(), (DfaConstValue)singleValue)); + } + } + } + return result; + } + + private static boolean hasNonTrivialFailingContracts(MethodCallInstruction instruction) { + List contracts = instruction.getContracts(); + return !contracts.isEmpty() && contracts.stream().anyMatch( + contract -> contract.getReturnValue() == MethodContract.ValueConstraint.THROW_EXCEPTION && !contract.isTrivial()); + } + + private static boolean hasNonTrivialBooleanContracts(MethodCallInstruction instruction) { + if (CustomMethodHandlers.find(instruction) != null) return true; + List contracts = instruction.getContracts(); + return !contracts.isEmpty() && contracts.stream().anyMatch( + contract -> (contract.getReturnValue() == MethodContract.ValueConstraint.FALSE_VALUE || + contract.getReturnValue() == MethodContract.ValueConstraint.TRUE_VALUE) + && !contract.isTrivial()); + } + + @Override + protected boolean checkNotNullable(DfaMemoryState state, DfaValue value, @Nullable NullabilityProblemKind.NullabilityProblem problem) { + if (NullabilityProblemKind.nullableReturn.isMyProblem(problem) && !state.isNotNull(value)) { + myAlwaysReturnsNotNull = false; + } + + boolean ok = super.checkNotNullable(state, value, problem); + if (problem == null) return ok; + StateInfo info = myStateInfos.computeIfAbsent(problem, k -> new StateInfo()); + if (state.isEphemeral() && !ok) { + info.ephemeralNpe = true; + } else if (!state.isEphemeral()) { + if (ok) info.normalOk = true; + else info.normalNpe = true; + } + return ok; + } + + private static boolean shouldReportConstValue(Object value) { + return value == null || value instanceof Boolean; + } + + private static class StateInfo { + boolean ephemeralNpe; + boolean normalNpe; + boolean normalOk; + } +}