From 165e2a527fedf8202e3617f40ff2a622844a10a6 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Sun, 15 Apr 2018 13:18:03 +0700 Subject: [PATCH] DFA: support of Class.isInstance, isAssignableFrom and class literal tracking Fixes IDEA-189865 Support class.isAssignableFrom and class.isInstance in DFA --- .../guess/impl/GuessManagerImpl.java | 4 +- .../codeInspection/dataFlow/CFGBuilder.java | 29 ++++ .../dataFlow/ControlFlowAnalyzer.java | 6 +- .../dataFlow/DataFlowInspectionBase.java | 2 +- .../dataFlow/InstructionVisitor.java | 16 +- .../dataFlow/StandardInstructionVisitor.java | 143 +++++++++++------- .../dataFlow/fix/RedundantInstanceofFix.java | 13 +- .../dataFlow/inliner/ClassMethodsInliner.java | 62 ++++++++ .../instructions/InstanceofInstruction.java | 27 +++- .../instructions/ObjectOfInstruction.java | 22 +++ .../afterInstanceOfClass.java | 10 ++ .../beforeInstanceOfClass.java | 10 ++ .../fixture/ClassMethodsInlining.java | 97 ++++++++++++ .../DataFlowInspectionTest.java | 1 + 14 files changed, 367 insertions(+), 75 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/ClassMethodsInliner.java create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ObjectOfInstruction.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfClass.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfClass.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ClassMethodsInlining.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java index ad3f24e8adb9..4216a763b32e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/guess/impl/GuessManagerImpl.java @@ -520,13 +520,13 @@ public class GuessManagerImpl extends GuessManager { @Override public DfaInstructionState[] visitInstanceof(InstanceofInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { PsiExpression psiOperand = instruction.getLeft(); - if (!isInteresting(psiOperand)) { + if (!isInteresting(psiOperand) || instruction.isClassObjectCheck()) { return super.visitInstanceof(instruction, runner, memState); } DfaValue type = memState.pop(); DfaValue operand = memState.pop(); DfaValue relation = runner.getFactory().createCondition(operand, DfaRelationValue.RelationType.IS, type); - memState.push(new DfaInstanceofValue(runner.getFactory(), psiOperand, instruction.getCastType(), relation, false)); + memState.push(new DfaInstanceofValue(runner.getFactory(), psiOperand, Objects.requireNonNull(instruction.getCastType()), relation, false)); return new DfaInstructionState[]{new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState)}; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java index a30e29021097..b569bf697bfb 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/CFGBuilder.java @@ -179,6 +179,35 @@ public class CFGBuilder { return this; } + /** + * Generate instructions to replace class value on top of stack with the corresponding object value + *

+ * Stack before: ... class_value + *

+ * Stack after: ... object_value + * + * @return this builder + */ + public CFGBuilder objectOf() { + myAnalyzer.addInstruction(new ObjectOfInstruction()); + return this; + } + + /** + * Generate instructions to perform an Class.isInstance operation + *

+ * Stack before: ... object class_object + *

+ * Stack after: ... result + * + * @param anchor element to bind this instruction to + * @return this builder + */ + public CFGBuilder isInstance(PsiMethodCallExpression anchor) { + myAnalyzer.addInstruction(new InstanceofInstruction(anchor)); + return this; + } + /** * Generate instructions to invoke the method associated with given method call assuming that method arguments and qualifier * are already on stack. If vararg call is specified, vararg arguments should be placed as is, without packing into array, diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 18b8c00bf8f0..b658607ea96f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -1511,7 +1511,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitClassObjectAccessExpression(PsiClassObjectAccessExpression expression) { startElement(expression); - addInstruction(new PushInstruction(myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), expression)); + PsiTypeElement operand = expression.getOperand(); + DfaConstValue classConstant = myFactory.getConstFactory().createFromValue(operand.getType(), expression.getType(), null); + addInstruction(new PushInstruction(classConstant, expression)); finishElement(expression); } @@ -2059,6 +2061,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } static final CallInliner[] INLINERS = {new OptionalChainInliner(), new LambdaInliner(), new CollectionFactoryInliner(), - new StreamChainInliner(), new MapUpdateInliner(), new AssumeInliner()}; + new StreamChainInliner(), new MapUpdateInliner(), new AssumeInliner(), new ClassMethodsInliner()}; } 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 46a203365a60..7465cdf40db7 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 @@ -638,7 +638,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool BranchingInstruction instruction) { PsiElement psiAnchor = instruction.getPsiAnchor(); if (instruction instanceof InstanceofInstruction && visitor.isInstanceofRedundant((InstanceofInstruction)instruction)) { - if (visitor.canBeNull((BinopInstruction)instruction)) { + if (visitor.canBeNull((InstanceofInstruction)instruction)) { holder.registerProblem(psiAnchor, InspectionsBundle.message("dataflow.message.redundant.instanceof"), new RedundantInstanceofFix()); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java index 6fe9a88ed924..7035d04de931 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/InstructionVisitor.java @@ -16,12 +16,10 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInspection.dataFlow.instructions.*; -import com.intellij.codeInspection.dataFlow.value.DfaUnknownValue; -import com.intellij.codeInspection.dataFlow.value.DfaValue; -import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; -import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; +import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.psi.PsiArrayAccessExpression; import com.intellij.psi.PsiExpression; +import com.intellij.psi.PsiType; import com.intellij.psi.util.PsiUtil; import com.intellij.util.ObjectUtils; import org.jetbrains.annotations.NotNull; @@ -99,6 +97,16 @@ public abstract class InstructionVisitor { return nextInstruction(instruction, runner, memState); } + public DfaInstructionState[] visitObjectOfInstruction(ObjectOfInstruction instruction, DataFlowRunner runner, DfaMemoryState state) { + DfaValue value = state.pop(); + DfaConstValue constant = value instanceof DfaConstValue ? (DfaConstValue)value : + value instanceof DfaVariableValue ? state.getConstantValue((DfaVariableValue)value) : + null; + PsiType type = constant == null ? null : ObjectUtils.tryCast(constant.getValue(), PsiType.class); + state.push(runner.getFactory().createTypeValue(type, Nullness.NOT_NULL)); + return nextInstruction(instruction, runner, state); + } + public DfaInstructionState[] visitCheckReturnValue(CheckReturnValueInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { memState.pop(); return nextInstruction(instruction, runner, memState); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index 7ecc43657eb6..d83f402a237e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -26,6 +26,7 @@ import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; import com.intellij.util.ObjectUtils; +import com.intellij.util.ThreeState; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.MethodUtils; import com.siyeh.ig.psiutils.TypeUtils; @@ -43,8 +44,8 @@ import java.util.stream.Stream; public class StandardInstructionVisitor extends InstructionVisitor { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.dataFlow.StandardInstructionVisitor"); - private final Set myReachable = new THashSet<>(); - private final Set myCanBeNullInInstanceof = new THashSet<>(); + private final Set myReachable = new THashSet<>(); + private final Set myCanBeNullInInstanceof = new THashSet<>(); private final Set myUsefulInstanceofs = new THashSet<>(); @Override @@ -496,10 +497,11 @@ public class StandardInstructionVisitor extends InstructionVisitor { private DfaValue dereference(DfaMemoryState memState, DfaValue value, @Nullable NullabilityProblemKind.NullabilityProblem problem) { - if (checkNotNullable(memState, value, problem)) return value; + boolean ok = checkNotNullable(memState, value, problem); if (value instanceof DfaFactMapValue) { return ((DfaFactMapValue)value).withFact(DfaFactType.CAN_BE_NULL, false); } + if (ok) return value; if (memState.isNull(value) && NullabilityProblemKind.nullableFunctionReturn.isMyProblem(problem)) { return value.getFactory().getFactValue(DfaFactType.CAN_BE_NULL, false); } @@ -613,8 +615,6 @@ public class StandardInstructionVisitor extends InstructionVisitor { @Override public DfaInstructionState[] visitBinop(BinopInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - myReachable.add(instruction); - DfaValue dfaRight = memState.pop(); DfaValue dfaLeft = memState.pop(); @@ -641,13 +641,8 @@ public class StandardInstructionVisitor extends InstructionVisitor { } } } - if (result == null) { - if (JavaTokenType.PLUS == opSign && TypeUtils.isJavaLangString(type)) { - result = runner.getFactory().createTypeValue(type, Nullness.NOT_NULL); - } - else if (instruction instanceof InstanceofInstruction) { - handleInstanceof((InstanceofInstruction)instruction, dfaRight, dfaLeft); - } + if (result == null && JavaTokenType.PLUS == opSign && TypeUtils.isJavaLangString(type)) { + result = runner.getFactory().createTypeValue(type, Nullness.NOT_NULL); } memState.push(result == null ? DfaUnknownValue.getInstance() : result); @@ -658,15 +653,13 @@ public class StandardInstructionVisitor extends InstructionVisitor { } @Nullable - private DfaInstructionState[] handleRelationBinop(BinopInstruction instruction, - DataFlowRunner runner, - DfaMemoryState memState, - DfaValue dfaRight, - DfaValue dfaLeft, - RelationType relationType) { + private static DfaInstructionState[] handleRelationBinop(BinopInstruction instruction, + DataFlowRunner runner, + DfaMemoryState memState, + DfaValue dfaRight, + DfaValue dfaLeft, + RelationType relationType) { DfaValueFactory factory = runner.getFactory(); - final Instruction next = runner.getInstruction(instruction.getIndex() + 1); - RelationType[] relations = splitRelation(relationType); ArrayList states = new ArrayList<>(relations.length); @@ -681,20 +674,9 @@ public class StandardInstructionVisitor extends InstructionVisitor { final DfaMemoryState copy = i == relations.length - 1 ? memState : memState.createCopy(); if (copy.applyCondition(condition)) { boolean isTrue = relationType.isSubRelation(relation); - copy.push(factory.getBoolean(isTrue)); - if (isTrue) { - instruction.setTrueReachable(); - } - else { - if (instruction instanceof InstanceofInstruction && !copy.isNull(dfaLeft)) { - myUsefulInstanceofs.add((InstanceofInstruction)instruction); - } - instruction.setFalseReachable(); - } - states.add(new DfaInstructionState(next, copy)); + states.add(makeBooleanResult(instruction, runner, copy, ThreeState.fromBoolean(isTrue))); } } - myCanBeNullInInstanceof.add(instruction); return states.toArray(DfaInstructionState.EMPTY_ARRAY); } @@ -712,20 +694,66 @@ public class StandardInstructionVisitor extends InstructionVisitor { } } - private void handleInstanceof(InstanceofInstruction instruction, DfaValue dfaRight, DfaValue dfaLeft) { - if (dfaLeft instanceof DfaFactMapValue && dfaRight instanceof DfaFactMapValue) { - DfaFactMapValue left = (DfaFactMapValue)dfaLeft; - DfaFactMapValue right = (DfaFactMapValue)dfaRight; + @Override + public DfaInstructionState[] visitInstanceof(InstanceofInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { + myReachable.add(instruction); - if (!Boolean.FALSE.equals(left.get(DfaFactType.CAN_BE_NULL))) { - myCanBeNullInInstanceof.add(instruction); - } - - if (right.getFacts().with(DfaFactType.CAN_BE_NULL, null).isSuperStateOf(left.getFacts())) { - return; + DfaValue dfaRight = memState.pop(); + DfaValue dfaLeft = memState.pop(); + DfaValueFactory factory = runner.getFactory(); + if (!memState.isNotNull(dfaLeft)) { + myCanBeNullInInstanceof.add(instruction); + } + boolean unknownTargetType = false; + DfaValue condition = null; + if (instruction.isClassObjectCheck()) { + DfaConstValue constant = dfaRight instanceof DfaConstValue ? (DfaConstValue)dfaRight : + dfaRight instanceof DfaVariableValue ? memState.getConstantValue((DfaVariableValue)dfaRight) : + null; + PsiType type = constant == null ? null : ObjectUtils.tryCast(constant.getValue(), PsiType.class); + if (type == null || type instanceof PsiPrimitiveType) { + // Unknown/primitive class: just execute contract "null -> false" + DfaConstValue aNull = factory.getConstFactory().getNull(); + condition = factory.createCondition(dfaLeft, RelationType.NE, aNull); + unknownTargetType = true; + } else { + dfaRight = factory.createTypeValue(type, Nullness.NOT_NULL); } } - myUsefulInstanceofs.add(instruction); + if (condition == null) { + condition = factory.createCondition(dfaLeft, RelationType.IS, dfaRight); + } + + boolean useful; + ArrayList states = new ArrayList<>(2); + if (condition instanceof DfaUnknownValue) { + if (dfaLeft instanceof DfaFactMapValue && dfaRight instanceof DfaFactMapValue) { + DfaFactMapValue left = (DfaFactMapValue)dfaLeft; + DfaFactMapValue right = (DfaFactMapValue)dfaRight; + useful = !right.getFacts().with(DfaFactType.CAN_BE_NULL, null).isSuperStateOf(left.getFacts()); + } else { + useful = true; + } + states.add(makeBooleanResult(instruction, runner, memState, ThreeState.UNSURE)); + } + else { + final DfaMemoryState trueState = memState.createCopy(); + useful = unknownTargetType; + if (trueState.applyCondition(condition)) { + states.add(makeBooleanResult(instruction, runner, trueState, unknownTargetType ? ThreeState.UNSURE : ThreeState.YES)); + } + if (memState.applyCondition(condition.createNegated())) { + if (unknownTargetType) { + memState.markEphemeral(); + } + states.add(makeBooleanResult(instruction, runner, memState, ThreeState.NO)); + useful |= !memState.isNull(dfaLeft); + } + } + if (useful) { + myUsefulInstanceofs.add(instruction); + } + return states.toArray(DfaInstructionState.EMPTY_ARRAY); } @Nullable @@ -764,10 +792,8 @@ public class StandardInstructionVisitor extends InstructionVisitor { dfaLeft == runner.getFactory().getConstFactory().getContractFail() || dfaRight == runner.getFactory().getConstFactory().getContractFail()) { boolean negated = (relationType == RelationType.NE) ^ (DfaMemoryStateImpl.isNaN(dfaLeft) || DfaMemoryStateImpl.isNaN(dfaRight)); - if (dfaLeft == dfaRight ^ negated) { - return alwaysTrue(instruction, runner, memState); - } - return alwaysFalse(instruction, runner, memState); + boolean result = dfaLeft == dfaRight ^ negated; + return makeBooleanResultArray(instruction, runner, memState, result); } return null; @@ -820,7 +846,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (result == null) { return null; } - return result ? alwaysTrue(instruction, runner, memState) : alwaysFalse(instruction, runner, memState); + return makeBooleanResultArray(instruction, runner, memState, result); } private static int compare(Number a, Number b) { @@ -831,23 +857,26 @@ public class StandardInstructionVisitor extends InstructionVisitor { return Double.compare(a.doubleValue(), b.doubleValue()); } - private static DfaInstructionState[] alwaysFalse(BinopInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - memState.push(runner.getFactory().getConstFactory().getFalse()); - instruction.setFalseReachable(); - return nextInstruction(instruction, runner, memState); + private static DfaInstructionState[] makeBooleanResultArray(BinopInstruction instruction, DataFlowRunner runner, DfaMemoryState memState, boolean result) { + return new DfaInstructionState[]{makeBooleanResult(instruction, runner, memState, ThreeState.fromBoolean(result))}; } - private static DfaInstructionState[] alwaysTrue(BinopInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) { - memState.push(runner.getFactory().getConstFactory().getTrue()); - instruction.setTrueReachable(); - return nextInstruction(instruction, runner, memState); + private static DfaInstructionState makeBooleanResult(BinopInstruction instruction, DataFlowRunner runner, DfaMemoryState memState, @NotNull ThreeState result) { + memState.push(result == ThreeState.UNSURE ? DfaUnknownValue.getInstance() : runner.getFactory().getBoolean(result.toBoolean())); + if (result != ThreeState.NO) { + instruction.setTrueReachable(); + } + if (result != ThreeState.YES) { + instruction.setFalseReachable(); + } + return new DfaInstructionState(runner.getInstruction(instruction.getIndex() + 1), memState); } public boolean isInstanceofRedundant(InstanceofInstruction instruction) { return !myUsefulInstanceofs.contains(instruction) && !instruction.isConditionConst() && myReachable.contains(instruction); } - public boolean canBeNull(BinopInstruction instruction) { + public boolean canBeNull(InstanceofInstruction instruction) { return myCanBeNullInInstanceof.contains(instruction); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java index 346794361365..96ac2bf01763 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/fix/RedundantInstanceofFix.java @@ -21,6 +21,8 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.util.ArrayUtil; +import com.siyeh.ig.psiutils.CommentTracker; import org.jetbrains.annotations.NotNull; /** @@ -37,8 +39,14 @@ public class RedundantInstanceofFix implements LocalQuickFix { public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { final PsiElement psiElement = descriptor.getPsiElement(); String replacement; + CommentTracker ct = new CommentTracker(); if (psiElement instanceof PsiInstanceOfExpression) { - replacement = ((PsiInstanceOfExpression)psiElement).getOperand().getText() + " != null"; + replacement = ct.text(((PsiInstanceOfExpression)psiElement).getOperand()) + " != null"; + } + else if (psiElement instanceof PsiMethodCallExpression) { + PsiExpression arg = ArrayUtil.getFirstElement(((PsiMethodCallExpression)psiElement).getArgumentList().getExpressions()); + if (arg == null) return; + replacement = ct.text(arg) + " != null"; } else if (psiElement instanceof PsiMethodReferenceExpression) { replacement = CommonClassNames.JAVA_UTIL_OBJECTS + "::nonNull"; @@ -46,7 +54,6 @@ public class RedundantInstanceofFix implements LocalQuickFix { else { return; } - PsiExpression compareToNull = JavaPsiFacade.getElementFactory(project).createExpressionFromText(replacement, psiElement.getParent()); - JavaCodeStyleManager.getInstance(project).shortenClassReferences(psiElement.replace(compareToNull)); + JavaCodeStyleManager.getInstance(project).shortenClassReferences(ct.replaceAndRestoreComments(psiElement, replacement)); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/ClassMethodsInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/ClassMethodsInliner.java new file mode 100644 index 000000000000..0187541c1276 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/ClassMethodsInliner.java @@ -0,0 +1,62 @@ +// Copyright 2000-2018 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.inliner; + +import com.intellij.codeInspection.dataFlow.CFGBuilder; +import com.intellij.codeInspection.dataFlow.NullabilityProblemKind; +import com.intellij.psi.PsiExpression; +import com.intellij.psi.PsiMethodCallExpression; +import com.intellij.psi.util.PsiUtil; +import com.siyeh.ig.callMatcher.CallMatcher; +import org.jetbrains.annotations.NotNull; + +import static com.intellij.psi.CommonClassNames.JAVA_LANG_CLASS; +import static com.intellij.psi.CommonClassNames.JAVA_LANG_OBJECT; +import static com.intellij.util.ObjectUtils.tryCast; + +/** + * Inline class.isInstance(obj) and class.isAssignableFrom(obj.getClass()) + */ +public class ClassMethodsInliner implements CallInliner { + private static final CallMatcher IS_ASSIGNABLE_FROM = CallMatcher.instanceCall(JAVA_LANG_CLASS, "isAssignableFrom").parameterTypes( + JAVA_LANG_CLASS); + private static final CallMatcher IS_INSTANCE = CallMatcher.instanceCall(JAVA_LANG_CLASS, "isInstance").parameterTypes( + JAVA_LANG_OBJECT); + private static final CallMatcher OBJECT_GET_CLASS = CallMatcher.instanceCall(JAVA_LANG_OBJECT, "getClass").parameterCount(0); + + + @Override + public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return false; + if (IS_ASSIGNABLE_FROM.matches(call)) { + PsiExpression arg = call.getArgumentList().getExpressions()[0]; + PsiMethodCallExpression nestedCall = tryCast(PsiUtil.skipParenthesizedExprDown(arg), PsiMethodCallExpression.class); + PsiExpression getClassQualifier = + OBJECT_GET_CLASS.matches(nestedCall) ? nestedCall.getMethodExpression().getQualifierExpression() : null; + if (getClassQualifier != null) { + builder.pushExpression(qualifier) + .pushExpression(getClassQualifier) + .checkNotNull(nestedCall, NullabilityProblemKind.callNPE); + } + else { + builder.pushExpression(qualifier) + .pushExpression(arg) + .checkNotNull(arg, NullabilityProblemKind.passingNullableToNotNullParameter) + .objectOf(); + } + builder.swap() + .checkNotNull(call, NullabilityProblemKind.callNPE) + .isInstance(call); + return true; + } + else if (IS_INSTANCE.matches(call)) { + builder.pushExpression(qualifier) + .pushExpression(call.getArgumentList().getExpressions()[0]) + .swap() + .checkNotNull(call, NullabilityProblemKind.callNPE) + .isInstance(call); + return true; + } + return false; + } +} diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java index 1b4a9a5446f0..2631084367e6 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/InstanceofInstruction.java @@ -19,10 +19,7 @@ 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.JavaTokenType; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiExpression; -import com.intellij.psi.PsiType; +import com.intellij.psi.*; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -31,7 +28,7 @@ import org.jetbrains.annotations.Nullable; */ public class InstanceofInstruction extends BinopInstruction { @Nullable private final PsiExpression myLeft; - @NotNull private final PsiType myCastType; + @Nullable private final PsiType myCastType; public InstanceofInstruction(PsiElement psiAnchor, @Nullable PsiExpression left, @NotNull PsiType castType) { super(JavaTokenType.INSTANCEOF_KEYWORD, psiAnchor, PsiType.BOOLEAN); @@ -39,6 +36,16 @@ public class InstanceofInstruction extends BinopInstruction { myCastType = castType; } + /** + * Construct a class object instanceof check (e.g. from Class.isInstance call); castType is not known + * @param psiAnchor anchor call + */ + public InstanceofInstruction(PsiMethodCallExpression psiAnchor) { + super(JavaTokenType.INSTANCEOF_KEYWORD, psiAnchor, PsiType.BOOLEAN); + myLeft = null; + myCastType = null; + } + @Override public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) { return visitor.visitInstanceof(this, runner, stateBefore); @@ -53,8 +60,16 @@ public class InstanceofInstruction extends BinopInstruction { return myLeft; } - @NotNull + @Nullable public PsiType getCastType() { return myCastType; } + + /** + * @return true if this instanceof instruction checks against Class object (e.g. Class.isInstance() call). In this case + * class object is located on the stack and cast type is not known + */ + public boolean isClassObjectCheck() { + return myCastType == null; + } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ObjectOfInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ObjectOfInstruction.java new file mode 100644 index 000000000000..638f3252e227 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/instructions/ObjectOfInstruction.java @@ -0,0 +1,22 @@ +// Copyright 2000-2018 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.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; + +/** + * Instruction which transforms class constant on top of stack to the object value which is instanceof that class + */ +public class ObjectOfInstruction extends Instruction { + @Override + public DfaInstructionState[] accept(DataFlowRunner runner, DfaMemoryState stateBefore, InstructionVisitor visitor) { + return visitor.visitObjectOfInstruction(this, runner, stateBefore); + } + + @Override + public String toString() { + return "OBJECT_OF"; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfClass.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfClass.java new file mode 100644 index 000000000000..46b2216afff8 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/afterInstanceOfClass.java @@ -0,0 +1,10 @@ +// "Replace with a null check" "true" +import java.util.stream.Stream; + +class Test { + void test(String s) { + if(s != null) { + System.out.println("not null s"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfClass.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfClass.java new file mode 100644 index 000000000000..cff05be9dba5 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/redundantInstanceOf/beforeInstanceOfClass.java @@ -0,0 +1,10 @@ +// "Replace with a null check" "true" +import java.util.stream.Stream; + +class Test { + void test(String s) { + if(String.class.isInstance(s)) { + System.out.println("not null s"); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ClassMethodsInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/ClassMethodsInlining.java new file mode 100644 index 000000000000..77932fc8cd2e --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ClassMethodsInlining.java @@ -0,0 +1,97 @@ +import java.util.*; +import org.jetbrains.annotations.*; + +class ClassMethodsInlining { + void assignableTest3(Class c, List list) { + if(c.isAssignableFrom(list.get(0).getClass())) { + System.out.println("possible"); + } + } + + void instanceTest(Class c, Object o) { + if(c.isInstance(o)) { + System.out.println(o.hashCode()); + } + } + + void instanceNotNullTest(Class c, Object o) { + if(o != null && c.isInstance(o)) { + System.out.println(o.hashCode()); + } + if(c.isInstance(new Object())) { + System.out.println("possible"); + } + } + + void assignableTest(Class c1, Class c2) { + if(c1.isAssignableFrom(c2)) { + System.out.println("possible"); + } + } + + void assignableTest2(Class c) { + if(c.isAssignableFrom(String.class)) { + System.out.println("possible"); + } + if(String.class.isAssignableFrom(c)) { + System.out.println("possible"); + } + } + + void testNullFalse(@Nullable Object s, Class c) { + if(c.isInstance(s)) { + System.out.println(s.hashCode()); + } + } + + void limitedClass(List list, Class c) { + assert c == Integer.class || c == String.class || c == Boolean.class; + + if(c.isInstance(list)) { + System.out.println("oops"); + } + } + + void primitiveClass(Object obj) { + // Not supported yet + if(int.class.isInstance(obj)) { + System.out.println("impossible"); + } + } + + void objectClass(Object obj, Object obj2) { + Class cls = Object.class; + if(cls.isInstance(obj)) { + System.out.println("for every non-null"); + } + if(cls.isAssignableFrom(obj2.getClass())) { + System.out.println("redundant"); + } + } + + void incompatibleClasses() { + Class c1 = Integer.class; + Class c2 = Double.class; + if(c1.isAssignableFrom(c2)) { + System.out.println("impossible"); + } + } + + void testEphemeral(Object obj, Class c) { + if(c.isInstance(obj)) { + System.out.println("yeah!"); + } + System.out.println(obj.hashCode()); + } + + native Object next(Object prev); + + void testLoop(Object o) { + while(o != null && !(o instanceof String)) { + o = next(o); + } + if(o instanceof String) { + System.out.println("String"); + } + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 87e8b5f5b995..2a7911d7abff 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -598,4 +598,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { doTest(); } public void testMergedInitializerAndConstructor() { doTest(); } + public void testClassMethodsInlining() { doTest(); } }