diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaCallArguments.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaCallArguments.java index 1860ea35d7fb..96dacd4f0fe2 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaCallArguments.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaCallArguments.java @@ -38,6 +38,13 @@ public final class DfaCallArguments { return myArguments; } + /** + * @return pure equivalent of this + */ + public DfaCallArguments makeTransparent() { + return myMutation == MutationSignature.transparent() ? this : new DfaCallArguments(myQualifier, myArguments, MutationSignature.transparent()); + } + @Override public boolean equals(Object o) { if (this == o) return true; @@ -68,6 +75,7 @@ public final class DfaCallArguments { handler.handleSideEffect(factory, state, this); return; } + if (myMutation.isTransparent()) return; if (myMutation.isPure()) { if (myQualifier instanceof DfaVariableValue) { DfaValue qualifier; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/JavaMethodContractUtil.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/JavaMethodContractUtil.java index 20e91fc90f82..46a7cde13e5a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/JavaMethodContractUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/JavaMethodContractUtil.java @@ -109,7 +109,7 @@ public final class JavaMethodContractUtil { static class ContractInfo { static final ContractInfo EMPTY = new ContractInfo(Collections.emptyList(), false, false, MutationSignature.UNKNOWN); - static final ContractInfo PURE = new ContractInfo(Collections.emptyList(), true, false, MutationSignature.PURE); + static final ContractInfo PURE = new ContractInfo(Collections.emptyList(), true, false, MutationSignature.transparent()); private final @NotNull List myContracts; private final boolean myPure; @@ -154,7 +154,7 @@ public final class JavaMethodContractUtil { boolean pure = Boolean.TRUE.equals(AnnotationUtil.getBooleanAttributeValue(contractAnno, "pure")); MutationSignature mutationSignature = MutationSignature.UNKNOWN; if (pure) { - mutationSignature = MutationSignature.PURE; + mutationSignature = MutationSignature.pure(); } else { String mutationText = AnnotationUtil.getStringAttributeValue(contractAnno, MutationSignature.ATTR_MUTATES); if (mutationText != null) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java index e80a95377ba0..2047e9232e10 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java @@ -25,15 +25,27 @@ import java.util.stream.Stream; * Represents method mutation signature */ public final class MutationSignature { + private enum Kind { + TRANSPARENT, PURE, MUTATES_ANYTHING, OTHER + } + public static final String ATTR_MUTATES = "mutates"; - static final MutationSignature UNKNOWN = new MutationSignature(false, false, new boolean[0]); - static final MutationSignature PURE = new MutationSignature(false, false, new boolean[0]); - private static final MutationSignature MUTATES_THIS_ONLY = new MutationSignature(true, false, new boolean[0]); + static final MutationSignature UNKNOWN = new MutationSignature(Kind.MUTATES_ANYTHING, false, false, new boolean[0]); + private static final MutationSignature PURE = new MutationSignature(Kind.PURE, false, false, new boolean[0]); + private static final MutationSignature TRANSPARENT = new MutationSignature(Kind.TRANSPARENT, false, false, new boolean[0]); + private static final MutationSignature MUTATES_THIS_ONLY = new MutationSignature(Kind.OTHER, true, false, new boolean[0]); + private final @NotNull Kind myKind; private final boolean myThis; private final boolean myIo; private final boolean[] myParameters; - private MutationSignature(boolean mutatesThis, boolean io, boolean[] params) { + private MutationSignature(@NotNull Kind kind, boolean mutatesThis, boolean io, boolean[] params) { + if (kind == Kind.PURE || kind == Kind.TRANSPARENT || kind == Kind.MUTATES_ANYTHING) { + if (mutatesThis || io || params.length != 0) { + throw new IllegalArgumentException(); + } + } + myKind = kind; myThis = mutatesThis; myIo = io; myParameters = params; @@ -81,7 +93,7 @@ public final class MutationSignature { */ public MutationSignature alsoMutatesThis() { return this == UNKNOWN || myThis ? this : - isPure() ? MUTATES_THIS_ONLY : new MutationSignature(true, myIo, myParameters); + isPure() ? MUTATES_THIS_ONLY : new MutationSignature(Kind.OTHER, true, myIo, myParameters); } /** @@ -92,14 +104,21 @@ public final class MutationSignature { if (myParameters.length > n && myParameters[n]) return this; boolean[] params = Arrays.copyOf(myParameters, Math.max(n + 1, myParameters.length)); params[n] = true; - return new MutationSignature(myThis, myIo, params); + return new MutationSignature(Kind.OTHER, myThis, myIo, params); } /** * @return true if this signature represents a pure method */ public boolean isPure() { - return this == PURE; + return myKind == Kind.PURE || myKind == Kind.TRANSPARENT; + } + + /** + * @return true if this signature represents a pure method which doesn't mutate even the private state of the object + */ + public boolean isTransparent() { + return myKind == Kind.TRANSPARENT; } @Override @@ -110,17 +129,20 @@ public final class MutationSignature { @Override public boolean equals(Object obj) { if (obj == this) return true; - if ((this == UNKNOWN) != (obj == UNKNOWN)) return false; - return obj instanceof MutationSignature signature && signature.myThis == myThis && signature.myIo == myIo && + return obj instanceof MutationSignature signature && signature.myKind == myKind && + signature.myThis == myThis && signature.myIo == myIo && Arrays.equals(signature.myParameters, myParameters); } @Override public String toString() { - if (isPure()) return "(pure)"; - if (this == UNKNOWN) return "(unknown)"; - return IntStreamEx.range(myParameters.length).mapToEntry(idx -> "param" + (idx + 1), idx -> myParameters[idx]) - .prepend("this", myThis).prepend("io", myIo).filterValues(b -> b).keys().joining(","); + return switch (myKind) { + case TRANSPARENT -> "(transparent)"; + case PURE -> "(pure)"; + case MUTATES_ANYTHING -> "(unknown)"; + case OTHER -> IntStreamEx.range(myParameters.length).mapToEntry(idx -> "param" + (idx + 1), idx -> myParameters[idx]) + .prepend("this", myThis).prepend("io", myIo).filterValues(b -> b).keys().joining(","); + }; } /** @@ -197,7 +219,7 @@ public final class MutationSignature { throw new IllegalArgumentException(JavaAnalysisBundle.message("mutation.signature.problem.invalid.token", part)); } } - return new MutationSignature(mutatesThis, mutatesIO, args); + return new MutationSignature(Kind.OTHER, mutatesThis, mutatesIO, args); } /** @@ -289,6 +311,13 @@ public final class MutationSignature { return PURE; } + /** + * @return a signature of the pure method, which doesn't mutate anything, including private fields + */ + public static @NotNull MutationSignature transparent() { + return TRANSPARENT; + } + /** * @return a signature of the unknown method, which may mutate anything */ diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/inferenceResults.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/inferenceResults.kt index 985279ee7932..d8c5a3e563dd 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/inferenceResults.kt +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/inferenceResults.kt @@ -53,7 +53,7 @@ data class PurityInferenceResult(internal val mutatesThis: Boolean, } private fun fromCalls(currentMethod: PsiMethod, body: () -> PsiCodeBlock): MutationSignature { - if (singleCall == null) return MutationSignature.pure() + if (singleCall == null) return MutationSignature.transparent() val psiCall : PsiCallExpression = singleCall.restoreExpression(body()) val method = psiCall.resolveMethod() @@ -64,7 +64,7 @@ data class PurityInferenceResult(internal val mutatesThis: Boolean, return MutationSignature.unknown() } val signature = MutationSignature.fromCall(psiCall) - if (signature == MutationSignature.pure() || + if (signature.isPure || signature == MutationSignature.pure().alsoMutatesThis() && psiCall is PsiMethodCallExpression && ExpressionUtil.isEffectivelyUnqualified(psiCall.methodExpression)) { return if (currentMethod.isConstructor) MutationSignature.pure() else signature diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java index a2bfe59feba4..8b8493ff04f8 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/ControlFlowAnalyzer.java @@ -1408,7 +1408,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { addInstruction(new PopInstruction()); } } - addInstruction(new MethodCallInstruction(expression, null, List.of())); + addInstruction(new MethodCallInstruction(expression, List.of())); if (myTrapTracker.shouldHandleException()) { addThrows(ExceptionUtil.getOwnUnhandledExceptions(expression)); } @@ -2180,7 +2180,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { // Do not track contracts if return value is not used contracts = Collections.emptyList(); } - addInstruction(new MethodCallInstruction(expression, JavaDfaValueFactory.getExpressionDfaValue(myFactory, expression), contracts)); + addInstruction(new MethodCallInstruction(expression, contracts)); anchor = expression; } processFailResult(method, contracts, anchor); @@ -2210,7 +2210,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { pushUnknown(); pushConstructorArguments(enumConstant); - addInstruction(new MethodCallInstruction(enumConstant, null, Collections.emptyList())); + addInstruction(new MethodCallInstruction(enumConstant, Collections.emptyList())); addInstruction(new PopInstruction()); } @@ -2272,11 +2272,10 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { } addConditionalErrorThrow(); - DfaValue precalculatedNewValue = getPrecalculatedNewValue(expression); List contracts = constructor == null ? Collections.emptyList() : JavaMethodContractUtil.getMethodCallContracts(constructor, null); contracts = DfaUtil.addRangeContracts(constructor, contracts); - addInstruction(new MethodCallInstruction(expression, precalculatedNewValue, contracts)); + addInstruction(new MethodCallInstruction(expression, contracts)); processFailResult(constructor, contracts, expression); addMethodThrows(constructorOrClass); @@ -2285,17 +2284,6 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { finishElement(expression); } - private DfaValue getPrecalculatedNewValue(PsiNewExpression expression) { - PsiType type = expression.getType(); - if (type != null && ConstructionUtils.isEmptyCollectionInitializer(expression)) { - DfType dfType = SpecialField.COLLECTION_SIZE.asDfType(DfTypes.intValue(0)) - .meet(TypeConstraints.exact(type).asDfType()) - .meet(DfTypes.LOCAL_OBJECT); - return myFactory.fromDfType(dfType); - } - return null; - } - private void initializeSmallArray(PsiArrayType type, PsiExpression[] dimensions) { if (dimensions.length != 1) return; PsiType componentType = type.getComponentType(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java index 378ac8a51eaf..4976dbad6fbc 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/JavaDfaValueFactory.java @@ -1,9 +1,7 @@ // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection.dataFlow.java; -import com.intellij.codeInsight.AnnotationUtil; -import com.intellij.codeInsight.ConcurrencyAnnotationsManager; -import com.intellij.codeInsight.Nullability; +import com.intellij.codeInsight.*; import com.intellij.codeInspection.dataFlow.*; import com.intellij.codeInspection.dataFlow.jvm.SpecialField; import com.intellij.codeInspection.dataFlow.jvm.descriptors.ArrayElementDescriptor; @@ -19,10 +17,7 @@ import com.intellij.codeInspection.dataFlow.value.VariableDescriptor; import com.intellij.psi.*; import com.intellij.psi.impl.JavaConstantExpressionEvaluator; import com.intellij.psi.impl.source.PsiFieldImpl; -import com.intellij.psi.util.InheritanceUtil; -import com.intellij.psi.util.PropertyUtilBase; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.psi.util.PsiUtil; +import com.intellij.psi.util.*; import com.siyeh.ig.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.Contract; @@ -198,17 +193,36 @@ public final class JavaDfaValueFactory { @Contract("null -> null") @Nullable public static VariableDescriptor getAccessedVariableOrGetter(final PsiElement target) { + return getAccessedVariableOrGetter(target, false); + } + + /** + * @param target target element (variable or method) + * @param stable if true, it's known externally that the access to the element is stable, + * i.e., if the target element is a virtual method, we are definitely accessing the specified one, + * and not overridden one. + * @return the variable descriptor, describing the specified access; null if given element cannot be described as a dataflow variable + */ + @Contract("null, _ -> null") + @Nullable + public static VariableDescriptor getAccessedVariableOrGetter(@Nullable PsiElement target, boolean stable) { SpecialField sf = SpecialField.findSpecialField(target); if (sf != null) { return sf; } - if (target instanceof PsiVariable) { - return new PlainDescriptor((PsiVariable)target); + if (target instanceof PsiVariable variable) { + return new PlainDescriptor(variable); } if (target instanceof PsiMethod method) { // Assume that methods returning stream always return a new one if (InheritanceUtil.isInheritor(method.getReturnType(), JAVA_UTIL_STREAM_BASE_STREAM)) return null; - if (method.getParameterList().isEmpty() && + + PsiField targetField = getFieldForGetter(method, stable); + if (targetField != null) { + return new PlainDescriptor(targetField); + } + + if (!method.isConstructor() && method.getParameterList().isEmpty() && (PropertyUtilBase.isSimplePropertyGetter(method) || JavaMethodContractUtil.isPure(method) || isClassAnnotatedImmutable(method)) && isContractAllowedForGetter(method) && !UNSTABLE_METHODS.methodMatches(method)) { @@ -217,6 +231,20 @@ public final class JavaDfaValueFactory { } return null; } + + private static @Nullable PsiField getFieldForGetter(@NotNull PsiMethod method, boolean stable) { + if (!stable && PsiUtil.canBeOverridden(method)) return null; + if (GetterDescriptor.isKnownStableMethod(method)) return null; + PsiField field = PropertyUtil.getFieldOfGetter(method); + if (field == null) return null; + NullableNotNullManager manager = NullableNotNullManager.getInstance(method.getProject()); + if (manager.isNullable(method, true) && !manager.isNullable(field, true)) { + // Avoid inlining if getter is marked as nullable, while the field is not. + // In this rare case, we cannot preserve the nullability warning on the callsite. + return null; + } + return field; + } private static boolean isClassAnnotatedImmutable(PsiMethod method) { List annotations = ConcurrencyAnnotationsManager.getInstance(method.getProject()).getImmutableAnnotations(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java index 995487991ef0..5af2d75197ee 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inliner/AccessorInliner.java @@ -1,21 +1,14 @@ // Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection.dataFlow.java.inliner; -import com.intellij.codeInsight.Nullability; -import com.intellij.codeInsight.NullabilityAnnotationInfo; -import com.intellij.codeInsight.NullableNotNullManager; -import com.intellij.codeInspection.dataFlow.NullabilityProblemKind; import com.intellij.codeInspection.dataFlow.java.CFGBuilder; import com.intellij.codeInspection.dataFlow.java.JavaDfaValueFactory; -import com.intellij.codeInspection.dataFlow.jvm.descriptors.GetterDescriptor; import com.intellij.codeInspection.dataFlow.jvm.descriptors.PlainDescriptor; import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; -import com.intellij.codeInspection.util.OptionalUtil; import com.intellij.psi.*; import com.intellij.psi.util.PropertyUtil; import com.intellij.psi.util.PsiUtil; -import com.intellij.psi.util.TypeConversionUtil; import org.jetbrains.annotations.NotNull; /** @@ -27,8 +20,7 @@ public final class AccessorInliner implements CallInliner { PsiMethod method = call.resolveMethod(); if (method == null) return false; if (PsiUtil.canBeOverridden(method)) return false; - return tryInlineGetter(builder, call, method) || - tryInlineSetter(builder, call, method); + return tryInlineSetter(builder, call, method); } private static boolean tryInlineSetter(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call, PsiMethod method) { @@ -50,42 +42,4 @@ public final class AccessorInliner implements CallInliner { // so we can spare two instructions return true; } - - private static boolean tryInlineGetter(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call, PsiMethod method) { - PsiClass containingClass = method.getContainingClass(); - if (containingClass == null) return false; - String qualifiedName = containingClass.getQualifiedName(); - // Methods Enum.name() and Enum.ordinal() are handled especially - if (CommonClassNames.JAVA_LANG_ENUM.equals(qualifiedName)) return false; - // Unboxing calls like Boolean.booleanValue() are handled especially - if (qualifiedName != null && TypeConversionUtil.isPrimitiveWrapper(qualifiedName)) return false; - // Avoid inlining OptionalInt.isPresent(), etc. - if (OptionalUtil.isJdkOptionalClassName(qualifiedName)) return false; - // Known stable methods (like methods from reflection) may read non-final fields, - // so inlining them breaks the stability - if (GetterDescriptor.isKnownStableMethod(method)) return false; - PsiField field = PropertyUtil.getFieldOfGetter(method); - if (field == null) return false; - DfaValue value = JavaDfaValueFactory.getQualifierOrThisValue(builder.getFactory(), call.getMethodExpression()); - if (value == null) return false; - NullableNotNullManager manager = NullableNotNullManager.getInstance(method.getProject()); - NullabilityAnnotationInfo methodNullability = manager.findEffectiveNullabilityInfo(method); - NullabilityAnnotationInfo fieldNullability = manager.findEffectiveNullabilityInfo(field); - if (methodNullability != null && methodNullability.getNullability() == Nullability.NULLABLE && - (fieldNullability == null || fieldNullability.getNullability() != Nullability.NULLABLE)) { - // Avoid inlining if getter is marked as nullable, while the field is not. - // In this rare case, we cannot preserve the nullability warning on the callsite. - return false; - } - boolean nonNull = methodNullability != null && methodNullability.getNullability() == Nullability.NOT_NULL && !methodNullability.isInferred(); - PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); - if (qualifier != null && !(qualifier instanceof PsiReferenceExpression ref && ref.resolve() instanceof PsiClass)) { - builder.pushExpression(qualifier).pop(); - } - builder.push(new PlainDescriptor(field).createValue(builder.getFactory(), value), call); - if (nonNull) { - builder.nullCheck(NullabilityProblemKind.assumeNotNull.problem(call, call)); - } - return true; - } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java index 7441bdbfaa72..1b5c1c5fc944 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/java/inst/MethodCallInstruction.java @@ -11,21 +11,19 @@ import com.intellij.codeInspection.dataFlow.java.anchor.JavaExpressionAnchor; import com.intellij.codeInspection.dataFlow.java.anchor.JavaMethodReferenceReturnAnchor; import com.intellij.codeInspection.dataFlow.jvm.JvmPsiRangeSetUtil; import com.intellij.codeInspection.dataFlow.jvm.SpecialField; +import com.intellij.codeInspection.dataFlow.jvm.descriptors.PlainDescriptor; import com.intellij.codeInspection.dataFlow.jvm.problems.MutabilityProblem; import com.intellij.codeInspection.dataFlow.lang.ir.DfaInstructionState; import com.intellij.codeInspection.dataFlow.lang.ir.ExpressionPushingInstruction; -import com.intellij.codeInspection.dataFlow.lang.ir.Instruction; import com.intellij.codeInspection.dataFlow.memory.DfaMemoryState; import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; -import com.intellij.codeInspection.dataFlow.types.DfConstantType; -import com.intellij.codeInspection.dataFlow.types.DfReferenceType; -import com.intellij.codeInspection.dataFlow.types.DfStreamStateType; -import com.intellij.codeInspection.dataFlow.types.DfType; +import com.intellij.codeInspection.dataFlow.types.*; import com.intellij.codeInspection.dataFlow.value.*; import com.intellij.openapi.diagnostic.Logger; import com.intellij.psi.*; import com.intellij.psi.util.*; import com.intellij.util.ThreeState; +import com.siyeh.ig.psiutils.ConstructionUtils; import com.siyeh.ig.psiutils.MethodCallUtils; import com.siyeh.ig.psiutils.MethodUtils; import org.jetbrains.annotations.NotNull; @@ -36,6 +34,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import static com.intellij.codeInspection.dataFlow.jvm.SpecialField.COLLECTION_SIZE; import static com.intellij.codeInspection.dataFlow.jvm.SpecialField.CONSUMED_STREAM; import static com.intellij.codeInspection.dataFlow.types.DfTypes.*; import static com.intellij.psi.CommonClassNames.JAVA_UTIL_STREAM_BASE_STREAM; @@ -52,7 +51,6 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { private final @NotNull PsiElement myContext; // PsiCall or PsiMethodReferenceExpression private final @Nullable PsiMethod myTargetMethod; private final List myContracts; - private final @Nullable DfaValue myPrecalculatedReturnValue; private final Nullability[] myArgRequiredNullability; private final Nullability myReturnNullability; @@ -79,7 +77,6 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { myReturnNullability = DfaPsiUtil.getElementNullability(myType, myTargetMethod); } } - myPrecalculatedReturnValue = null; myArgRequiredNullability = myTargetMethod == null ? EMPTY_NULLABILITY_ARRAY : calcArgRequiredNullability(resolveResult.getSubstitutor(), @@ -87,7 +84,7 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { myMutation = MutationSignature.fromMethod(myTargetMethod); } - public MethodCallInstruction(@NotNull PsiCall call, @Nullable DfaValue precalculatedReturnValue, List contracts) { + public MethodCallInstruction(@NotNull PsiCall call, @NotNull List contracts) { super(call instanceof PsiExpression expr ? new JavaExpressionAnchor(expr) : null); myContext = call; myContracts = Collections.unmodifiableList(contracts); @@ -108,29 +105,9 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { } myMutation = MutationSignature.fromCall(call); - myPrecalculatedReturnValue = DfaTypeValue.isUnknown(precalculatedReturnValue) ? null : precalculatedReturnValue; myReturnNullability = call instanceof PsiNewExpression ? Nullability.NOT_NULL : DfaPsiUtil.getElementNullability(myType, myTargetMethod); } - private MethodCallInstruction(@NotNull MethodCallInstruction from, @NotNull DfaValue precalculatedReturnValue) { - super(from.getDfaAnchor()); - myPrecalculatedReturnValue = precalculatedReturnValue; - myContext = from.myContext; - myContracts = from.myContracts; - myArgCount = from.myArgCount; - myMutation = from.myMutation; - myType = from.myType; - myTargetMethod = from.myTargetMethod; - myArgRequiredNullability = from.myArgRequiredNullability; - myReturnNullability = from.myReturnNullability; - } - - @Override - public @NotNull Instruction bindToFactory(@NotNull DfaValueFactory factory) { - if (myPrecalculatedReturnValue == null) return this; - return new MethodCallInstruction(this, myPrecalculatedReturnValue.bindToFactory(factory)); - } - /** * Returns a PsiElement which at best represents an argument with given index * @@ -213,8 +190,12 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { Set finalStates = new LinkedHashSet<>(); - PsiType qualifierType = DfaPsiUtil.dfTypeToPsiType(factory.getProject(), stateBefore.getDfType(callArguments.getQualifier())); - PsiMethod realMethod = findSpecificMethod(qualifierType); + DfType qualifierDfType = stateBefore.getDfType(callArguments.getQualifier()); + PsiMethod realMethod = findSpecificMethod(DfaPsiUtil.dfTypeToPsiType(factory.getProject(), qualifierDfType)); + if (realMethod != null && (TypeConstraint.fromDfType(qualifierDfType).isExact() || !PsiUtil.canBeOverridden(realMethod)) && + PropertyUtil.getFieldOfGetter(realMethod) != null) { + callArguments = callArguments.makeTransparent(); + } DfaValue defaultResult = getMethodResultValue(callArguments, stateBefore, factory, realMethod); DfaCallState initialState = new DfaCallState(stateBefore, callArguments, defaultResult); Set currentStates = Collections.singleton(initialState); @@ -360,40 +341,51 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { @NotNull DfaMemoryState state, @NotNull DfaValueFactory factory, PsiMethod realMethod) { + DfaValue qualifierValue = callArguments.getQualifier(); + boolean stable = TypeConstraint.fromDfType(state.getDfType(qualifierValue)).isExact(); + VariableDescriptor descriptor = JavaDfaValueFactory.getAccessedVariableOrGetter(realMethod, stable); + DfaValue precomputedValue = descriptor == null ? null : descriptor.createValue(factory, qualifierValue); + if (callArguments.getArguments() != null && myTargetMethod != null) { CustomMethodHandlers.CustomMethodHandler handler = CustomMethodHandlers.find(myTargetMethod); if (handler != null) { DfaValue value = handler.getMethodResultValue(callArguments, state, factory, myTargetMethod); if (value != null) { - if (myPrecalculatedReturnValue != null) { - if (!state.applyCondition(myPrecalculatedReturnValue.eq(value))) { - throw new IllegalStateException("Precalculated value " + - myPrecalculatedReturnValue + - " mismatches with method handler result " + - value + - "; method = " + - PsiFormatUtil.formatMethod(myTargetMethod, PsiSubstitutor.EMPTY, - PsiFormatUtilBase.SHOW_CONTAINING_CLASS | - PsiFormatUtilBase.SHOW_NAME, PsiFormatUtilBase.SHOW_TYPE)); - } + if (precomputedValue != null && !state.applyCondition(precomputedValue.eq(value))) { + throw new IllegalStateException("Precalculated value " + + precomputedValue + + " mismatches with method handler result " + + value + + "; method = " + + PsiFormatUtil.formatMethod(myTargetMethod, PsiSubstitutor.EMPTY, + PsiFormatUtilBase.SHOW_CONTAINING_CLASS | + PsiFormatUtilBase.SHOW_NAME, PsiFormatUtilBase.SHOW_TYPE)); } - return myPrecalculatedReturnValue instanceof DfaVariableValue var && !var.isFlushableByCalls() - ? myPrecalculatedReturnValue - : value; + return precomputedValue instanceof DfaVariableValue var && !var.isFlushableByCalls() ? precomputedValue : value; } } } - DfaValue qualifierValue = callArguments.getQualifier(); + + if (precomputedValue != null) { + if (myReturnNullability == Nullability.NOT_NULL) { + if (precomputedValue instanceof DfaVariableValue) { + state.meetDfType(precomputedValue, DfaNullability.NOT_NULL.asDfType()); + } else { + precomputedValue = factory.fromDfType(precomputedValue.getDfType().meet(DfaNullability.NOT_NULL.asDfType())); + } + } + return precomputedValue; + } + + DfType dfType = getMethodResultType(state, factory, realMethod, qualifierValue); + return factory.fromDfType(dfType); + } + + private @NotNull DfType getMethodResultType(@NotNull DfaMemoryState state, + @NotNull DfaValueFactory factory, + PsiMethod realMethod, + DfaValue qualifierValue) { PsiType type = getResultType(); - - VariableDescriptor descriptor = JavaDfaValueFactory.getAccessedVariableOrGetter(realMethod); - if (descriptor instanceof SpecialField || descriptor != null && qualifierValue instanceof DfaVariableValue) { - return descriptor.createValue(factory, qualifierValue); - } - if (myPrecalculatedReturnValue != null) { - return myPrecalculatedReturnValue; - } - if (type != null && !(type instanceof PsiPrimitiveType)) { Nullability nullability = myReturnNullability; Mutability mutable = Mutability.UNKNOWN; @@ -413,12 +405,18 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { PsiType qualifierType = DfaPsiUtil.dfTypeToPsiType(factory.getProject(), state.getDfType(qualifierValue)); type = narrowReturnType(type, qualifierType, realMethod); } - DfType dfType = getContext() instanceof PsiNewExpression ? - TypeConstraints.exact(type).asDfType().meet(NOT_NULL_OBJECT) : - TypeConstraints.instanceOf(type).asDfType().meet(DfaNullability.fromNullability(nullability).asDfType()); - if (myMutation.isPure() && getContext() instanceof PsiNewExpression && - !TypeConstraint.fromDfType(dfType).isComparedByEquals()) { - dfType = dfType.meet(LOCAL_OBJECT); + DfType dfType; + if (getContext() instanceof PsiNewExpression newExpression) { + dfType = TypeConstraints.exact(type).asDfType().meet(NOT_NULL_OBJECT); + if (ConstructionUtils.isEmptyCollectionInitializer(newExpression)) { + dfType = dfType.meet(COLLECTION_SIZE.asDfType(intValue(0))); + } + if (myMutation.isPure() && !TypeConstraint.fromDfType(dfType).isComparedByEquals()) { + dfType = dfType.meet(LOCAL_OBJECT); + } + } + else { + dfType = TypeConstraints.instanceOf(type).asDfType().meet(DfaNullability.fromNullability(nullability).asDfType()); } if (InheritanceUtil.isInheritor(type, JAVA_UTIL_STREAM_BASE_STREAM)) { dfType = dfType.meet(((DfReferenceType)CONSUMED_STREAM.asDfType(DfStreamStateType.OPEN)).dropNullability()); @@ -427,16 +425,16 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { } } - return factory.fromDfType(dfType.meet(mutable.asDfType())); + return dfType.meet(mutable.asDfType()); } LongRangeSet range = JvmPsiRangeSetUtil.typeRange(type, true); if (range != null) { if (myTargetMethod != null) { range = range.meet(JvmPsiRangeSetUtil.fromPsiElement(myTargetMethod)); } - return factory.fromDfType(PsiTypes.longType().equals(type) ? longRange(range) : intRangeClamped(range)); + return PsiTypes.longType().equals(type) ? longRange(range) : intRangeClamped(range); } - return PsiTypes.voidType().equals(type) ? factory.getUnknown() : factory.fromDfType(typedObject(type, Nullability.UNKNOWN)); + return PsiTypes.voidType().equals(type) ? DfType.TOP : typedObject(type, Nullability.UNKNOWN); } private boolean mayLeakThis(@NotNull DfaMemoryState memState, DfaValue @Nullable [] argValues) { @@ -539,7 +537,12 @@ public class MethodCallInstruction extends ExpressionPushingInstruction { @Override public List getRequiredDescriptors(@NotNull DfaValueFactory factory) { - return myPrecalculatedReturnValue instanceof DfaVariableValue var ? - List.of(var.getDescriptor()) : List.of(); + if (myTargetMethod != null) { + PsiField field = PropertyUtil.getFieldOfGetter(myTargetMethod); + if (field != null) { + return List.of(new PlainDescriptor(field)); + } + } + return List.of(); } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java index 7e4db560a32c..db624519d215 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/GetterDescriptor.java @@ -30,6 +30,7 @@ public final class GetterDescriptor extends PsiVarDescriptor { private static final CallMatcher STABLE_METHODS = CallMatcher.anyOf( CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "getClass").parameterCount(0), CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_STRING, "trim", "stripLeading", "stripTrailing", "strip").parameterCount(0), + CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_ENUM, "name").parameterCount(0), CallMatcher.instanceCall("java.lang.reflect.Member", "getName", "getModifiers", "getDeclaringClass", "isSynthetic"), CallMatcher.instanceCall("java.lang.reflect.Executable", "getParameterCount", "isVarArgs"), CallMatcher.instanceCall("java.lang.reflect.Field", "getType"), diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java index 6cee18e9e086..7fdbc4cbdf23 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/jvm/descriptors/PlainDescriptor.java @@ -70,7 +70,7 @@ public final class PlainDescriptor extends PsiVarDescriptor { } if (PsiUtil.isJvmLocalVariable(myVariable) || (myVariable instanceof PsiField && myVariable.hasModifierProperty(PsiModifier.STATIC))) { - if (qualifier != null) return factory.getUnknown(); + if (qualifier != null && qualifier != factory.getUnknown()) return factory.getUnknown(); return factory.getVarFactory().createVariableValue(this); } if (qualifier instanceof DfaTypeValue typeValue) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/GetterVsDirectAccessNonFinal.java b/java/java-tests/testData/inspection/dataFlow/fixture/GetterVsDirectAccessNonFinal.java new file mode 100644 index 000000000000..c9b602a8396e --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/GetterVsDirectAccessNonFinal.java @@ -0,0 +1,38 @@ +import java.util.*; +import org.jetbrains.annotations.*; + +class Test { + private int x, y; + + int getX() { + return x; + } + + int getY() { + return y; + } + + static void test(Test t) { + if (t.getX() == t.getY()) { + if (t.x == t.y) { // Who knows, probably subclass + + } + } + if (t.getClass() == Test.class) { + if (t.getX() == t.getY()) { + if (t.x == t.y) { // Definitely not subclass + + } + } + } + } + + static void test2() { + Test t = new Test(); + if (t.getX() == t.getY()) { + if (t.x == t.y) { // Definitely not subclass + + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ParamContractBoolean.java b/java/java-tests/testData/inspection/dataFlow/fixture/ParamContractBoolean.java index 9df443abad12..c830aa15c6b5 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ParamContractBoolean.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ParamContractBoolean.java @@ -5,11 +5,15 @@ class Scratch public static void main(String[] args) { maybeNull = true; + if (!isTrue(getMaybeNull())) { } + unknown(); if (!isTrue(getMaybeNull())) { } if (!isTrue(null)) { } if (!isTrue(true)) { } if (!isTrue(false)) { } } + + static native void unknown(); static Boolean maybeNull = null; static Boolean getMaybeNull() diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java index bd210a9a9662..5cd18feb7f29 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection21Test.java @@ -147,5 +147,6 @@ public class DataFlowInspection21Test extends DataFlowInspectionTestCase { doTest(); } public void testGetterVsDirectAccess() { doTest(); } + public void testGetterVsDirectAccessNonFinal() { doTest(); } public void testSetterAndGetter() { doTest(); } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.java index e71184f5e986..7c98840ea04b 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.java @@ -11,7 +11,7 @@ import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase; public class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTestCase { public void test_getter() { - assertPure(""" + assertTransparent(""" Object getField() { return field; }"""); @@ -41,7 +41,7 @@ public class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTe } public void test_local_var_assignment() { - assertPure(""" + assertTransparent(""" int random(boolean b) { int i = 4; if (b) { @@ -104,13 +104,13 @@ public class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTe } public void test_empty_constructor() { - assertPure(""" + assertTransparent(""" public Foo() { }"""); } public void test_field_writes() { - assertPure(""" + assertTransparent(""" int x; int y; @@ -350,7 +350,7 @@ public class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTe } public void test_plain_field_read() { - assertPure(""" + assertTransparent(""" int x; int get() { @@ -443,6 +443,10 @@ public class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTe assertMutationSignature(classBody, MutationSignature.pure()); } + private void assertTransparent(String classBody) { + assertMutationSignature(classBody, MutationSignature.transparent()); + } + private void assertImpure(String classBody) { assertMutationSignature(classBody, MutationSignature.unknown()); } @@ -453,9 +457,9 @@ public class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTe private void assertMutationSignature(String classBody, MutationSignature expected) { PsiClass clazz = myFixture.addClass("final class Foo { " + classBody + " }"); - assert !((PsiFileImpl)clazz.getContainingFile()).isContentsLoaded(); + assertFalse(((PsiFileImpl)clazz.getContainingFile()).isContentsLoaded()); MutationSignature signature = JavaSourceInference.inferMutationSignature((PsiMethodImpl)clazz.getMethods()[0]); - assert !((PsiFileImpl)clazz.getContainingFile()).isContentsLoaded(); - assert expected.equals(signature); + assertFalse(((PsiFileImpl)clazz.getContainingFile()).isContentsLoaded()); + assertEquals(expected, signature); } }