diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/DefaultInferredAnnotationProvider.java b/java/java-analysis-impl/src/com/intellij/codeInsight/DefaultInferredAnnotationProvider.java index 6a9e7e28d27c..b3d4e37f00a4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/DefaultInferredAnnotationProvider.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/DefaultInferredAnnotationProvider.java @@ -2,10 +2,7 @@ package com.intellij.codeInsight; import com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis; -import com.intellij.codeInspection.dataFlow.HardcodedContracts; -import com.intellij.codeInspection.dataFlow.MethodContract; -import com.intellij.codeInspection.dataFlow.Mutability; -import com.intellij.codeInspection.dataFlow.StandardMethodContract; +import com.intellij.codeInspection.dataFlow.*; import com.intellij.codeInspection.dataFlow.inference.JavaSourceInference; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.text.StringUtil; @@ -102,10 +99,10 @@ public class DefaultInferredAnnotationProvider implements InferredAnnotationProv private PsiAnnotation getHardcodedContractAnnotation(PsiMethod method) { PsiClass aClass = method.getContainingClass(); if (aClass != null && aClass.getQualifiedName() != null && aClass.getQualifiedName().startsWith("org.assertj.core.api.")) { - return createContractAnnotation(Collections.emptyList(), true); + return createContractAnnotation(Collections.emptyList(), MutationSignature.pure()); } List contracts = HardcodedContracts.getHardcodedContracts(method, null); - return contracts.isEmpty() ? null : createContractAnnotation(contracts, HardcodedContracts.isHardcodedPure(method)); + return contracts.isEmpty() ? null : createContractAnnotation(contracts, HardcodedContracts.getHardcodedMutation(method)); } /** @@ -157,7 +154,7 @@ public class DefaultInferredAnnotationProvider implements InferredAnnotationProv return null; } - return createContractAnnotation(JavaSourceInference.inferContracts(method), JavaSourceInference.inferPurity(method)); + return createContractAnnotation(JavaSourceInference.inferContracts(method), JavaSourceInference.inferMutationSignature(method)); } @Nullable @@ -207,8 +204,10 @@ public class DefaultInferredAnnotationProvider implements InferredAnnotationProv } @Nullable - private PsiAnnotation createContractAnnotation(List contracts, boolean pure) { - return createContractAnnotation(myProject, pure, StreamEx.of(contracts).select(StandardMethodContract.class).joining("; "), ""); + private PsiAnnotation createContractAnnotation(List contracts, MutationSignature signature) { + return createContractAnnotation(myProject, signature.isPure(), + StreamEx.of(contracts).select(StandardMethodContract.class).joining("; "), + signature.isPure() || signature == MutationSignature.unknown() ? "" : signature.toString()); } @Nullable diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java index 82612d713422..8ba392948ac1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java @@ -462,34 +462,43 @@ public class HardcodedContracts { return Collections.singletonList(failContract); } - public static boolean isHardcodedPure(PsiMethod method) { + /** + * Returns the mutation signature for the methods that have hardcoded contracts + * + * @param method method that has hardcoded contracts (that is, {@link #getHardcodedContracts(PsiMethod, PsiMethodCallExpression)} + * returned non-empty list for this method) + * @return a mutation signature for the given method. Result is unspecified if method has no hardcoded contract. + */ + public static MutationSignature getHardcodedMutation(PsiMethod method) { PsiClass aClass = method.getContainingClass(); - if (aClass == null) return false; + if (aClass == null) return MutationSignature.unknown(); String className = aClass.getQualifiedName(); - if (className == null) return false; + if (className == null) return MutationSignature.unknown(); String name = method.getName(); if ("java.util.Objects".equals(className) && "requireNonNull".equals(name)) { PsiParameter[] parameters = method.getParameterList().getParameters(); if (parameters.length == 2 && parameters[1].getType().getCanonicalText().contains("Supplier")) { - return false; + return MutationSignature.unknown(); } } if ("remove".equals(name)) { - return false; + return MutationSignature.pure().alsoMutatesThis(); } if ("java.lang.System".equals(className)) { - return false; + return MutationSignature.unknown(); } if (JAVA_UTIL_ARRAYS.equals(className)) { - return name.equals("binarySearch") || name.equals("spliterator") || name.equals("stream"); + return name.equals("binarySearch") || name.equals("spliterator") || name.equals("stream") ? MutationSignature.pure() : + // else: fill, parallelPrefix, parallelSort, sort + MutationSignature.pure().alsoMutatesArg(0); } if (QUEUE_POLL.methodMatches(method)) { - return false; + return MutationSignature.pure().alsoMutatesThis(); } - return true; + return MutationSignature.pure(); } public static boolean hasHardcodedContracts(@Nullable PsiElement element) { 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 053ba8cffc13..6d0bce4218a6 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 @@ -23,6 +23,7 @@ public class MutationSignature { public static final String ATTR_MUTATES = "mutates"; static final MutationSignature UNKNOWN = new MutationSignature(false, new boolean[0]); static final MutationSignature PURE = new MutationSignature(false, new boolean[0]); + private static final MutationSignature MUTATES_THIS_ONLY = new MutationSignature(true, new boolean[0]); public static final String INVALID_TOKEN_MESSAGE = "Invalid token: %s; supported are 'this', 'param1', 'param2', etc."; private final boolean myThis; private final boolean[] myParameters; @@ -66,7 +67,8 @@ public class MutationSignature { * @return a signature that is equivalent to this signature but may also mutate this object */ public MutationSignature alsoMutatesThis() { - return myThis ? this : new MutationSignature(true, myParameters); + return this == UNKNOWN || myThis ? this : + isPure() ? MUTATES_THIS_ONLY : new MutationSignature(true, myParameters); } /** @@ -229,7 +231,7 @@ public class MutationSignature { PsiNewExpression newExpression = (PsiNewExpression)call; if (newExpression.isArrayCreation()) return PURE; if (newExpression.getArgumentList() == null || !newExpression.getArgumentList().isEmpty()) return UNKNOWN; - PsiJavaCodeReferenceElement classReference = newExpression.getClassReference(); + PsiJavaCodeReferenceElement classReference = newExpression.getClassOrAnonymousClassReference(); if (classReference == null) return UNKNOWN; PsiClass clazz = ObjectUtils.tryCast(classReference.resolve(), PsiClass.class); if (clazz == null) return UNKNOWN; 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 e6fbd1e07b18..93fa0ab5135b 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 @@ -466,9 +466,14 @@ public class StandardInstructionVisitor extends InstructionVisitor { } DfType dfType = memState.getDfType(value); if (instruction.getMutationSignature().mutatesThis() && Mutability.fromDfType(dfType).isUnmodifiable()) { - reportMutabilityViolation(true, instruction.getContext()); - if (dfType instanceof DfReferenceType) { - memState.setDfType(value, ((DfReferenceType)dfType).dropMutability().meet(Mutability.MUTABLE.asDfType())); + PsiMethod method = instruction.getTargetMethod(); + // Inferred mutation annotation may infer mutates="this" if invisible state is mutated (e.g. cached hashCode is stored). + // So let's conservatively skip the warning here. Such contract is still useful because it assures that nothing else is mutated. + if (method != null && JavaMethodContractUtil.getContractInfo(method).isExplicit()) { + reportMutabilityViolation(true, instruction.getContext()); + if (dfType instanceof DfReferenceType) { + memState.setDfType(value, ((DfReferenceType)dfType).dropMutability().meet(Mutability.MUTABLE.asDfType())); + } } } if (!(value.getType() instanceof PsiArrayType) && diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/ContractInferenceIndex.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/ContractInferenceIndex.kt index 5591bc724a22..4fdbcb57c5e1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/ContractInferenceIndex.kt +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/ContractInferenceIndex.kt @@ -22,7 +22,7 @@ import kotlin.collections.HashMap * @author peter */ -private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 12, MethodDataExternalizer) { file -> +private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 13, MethodDataExternalizer) { file -> indexFile(file.node.lighterAST) } @@ -118,12 +118,14 @@ private class InferenceVisitor(val tree : LighterAST) : RecursiveLighterASTNodeW val nullityVisitor = MethodReturnInferenceVisitor(tree, contractInference.parameters, body) val purityVisitor = PurityInferenceVisitor(tree, body, fieldMap, ctor) + var stopPurityAnalysis = maybeImpureCtor for (statement in statements) { walkMethodBody(statement) { nullityVisitor.visitNode(it) - if (!maybeImpureCtor) { - purityVisitor.visitNode(it) + if (!stopPurityAnalysis) { + stopPurityAnalysis = !purityVisitor.visitNode(it) } + true } } val notNullParams = inferNotNullParameters(tree, method, statements) @@ -131,13 +133,15 @@ private class InferenceVisitor(val tree : LighterAST) : RecursiveLighterASTNodeW return createData(body, contracts, nullityVisitor.result, if (maybeImpureCtor) null else purityVisitor.result, notNullParams) } - private fun walkMethodBody(root: LighterASTNode, processor: (LighterASTNode) -> Unit) { + private fun walkMethodBody(root: LighterASTNode, processor: (LighterASTNode) -> Boolean) { object : RecursiveLighterASTNodeWalkingVisitor(tree) { override fun visitNode(element: LighterASTNode) { val type = element.tokenType if (type === CLASS || type === FIELD || type === METHOD || type === ANNOTATION_METHOD || type === LAMBDA_EXPRESSION) return - processor(element) + if (!processor(element)) { + stopWalking() + } super.visitNode(element) } }.visitNode(root) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/JavaSourceInference.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/JavaSourceInference.java index bfa6c4104774..495b8e07666b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/JavaSourceInference.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/JavaSourceInference.java @@ -5,10 +5,7 @@ import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.Nullability; import com.intellij.codeInsight.NullabilityAnnotationInfo; import com.intellij.codeInsight.NullableNotNullManager; -import com.intellij.codeInspection.dataFlow.ContractReturnValue; -import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil; -import com.intellij.codeInspection.dataFlow.Mutability; -import com.intellij.codeInspection.dataFlow.StandardMethodContract; +import com.intellij.codeInspection.dataFlow.*; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.roots.FileIndexFacade; import com.intellij.openapi.util.RecursionManager; @@ -43,23 +40,23 @@ public class JavaSourceInference { private static class MethodInferenceData { static final MethodInferenceData UNKNOWN = - new MethodInferenceData(Mutability.UNKNOWN, Nullability.UNKNOWN, Collections.emptyList(), false, new BitSet()); + new MethodInferenceData(Mutability.UNKNOWN, Nullability.UNKNOWN, Collections.emptyList(), MutationSignature.unknown(), new BitSet()); final @NotNull Mutability myMutability; final @NotNull Nullability myNullability; final @NotNull List myContracts; - final boolean myPure; + final @NotNull MutationSignature myMutationSignature; final @NotNull BitSet myNotNullParameters; MethodInferenceData(@NotNull Mutability mutability, @NotNull Nullability nullability, @NotNull List contracts, - boolean pure, + @NotNull MutationSignature signature, @NotNull BitSet parameters) { myMutability = mutability; myNullability = nullability; myContracts = contracts; - myPure = pure; + myMutationSignature = signature; myNotNullParameters = parameters; } } @@ -78,12 +75,13 @@ public class JavaSourceInference { if (mode == InferenceMode.PARAMETERS) { // Infer parameters nullability only (for unstable methods) return notNullParameters.isEmpty() ? MethodInferenceData.UNKNOWN : - new MethodInferenceData(Mutability.UNKNOWN, Nullability.UNKNOWN, Collections.emptyList(), false, notNullParameters); + new MethodInferenceData(Mutability.UNKNOWN, Nullability.UNKNOWN, Collections.emptyList(), + MutationSignature.unknown(), notNullParameters); } Nullability nullability = findNullability(method, data); Mutability mutability = findMutability(method, data); - boolean pure = findPurity(method, data); + MutationSignature signature = findMutationSignature(method, data); IntPredicate isNotNullParameter = i -> { PsiParameter[] parameters = method.getParameterList().getParameters(); @@ -96,7 +94,7 @@ public class JavaSourceInference { nullability = Nullability.UNKNOWN; } - return new MethodInferenceData(mutability, nullability, contracts, pure, notNullParameters); + return new MethodInferenceData(mutability, nullability, contracts, signature, notNullParameters); } @NotNull @@ -123,10 +121,12 @@ public class JavaSourceInference { return mutability == null ? Mutability.UNKNOWN : mutability; } - private static boolean findPurity(@NotNull PsiMethodImpl method, @NotNull MethodData data) { + private static @NotNull MutationSignature findMutationSignature(@NotNull PsiMethodImpl method, @NotNull MethodData data) { PurityInferenceResult result = data.getPurity(); - if (result == null) return false; - return Boolean.TRUE.equals(RecursionManager.doPreventingRecursion(method, true, () -> result.isPure(method, data.methodBody(method)))); + if (result == null) return MutationSignature.unknown(); + MutationSignature signature = + RecursionManager.doPreventingRecursion(method, true, () -> result.getMutationSignature(method, data.methodBody(method))); + return signature == null ? MutationSignature.unknown() : signature; } @NotNull @@ -246,13 +246,13 @@ public class JavaSourceInference { } /** - * Infer method purity + * Infer method mutation signature * * @param method method to analyze - * @return true if method was inferred to be pure; false if method is not pure or cannot be analyzed + * @return method mutation signature; {@link MutationSignature#unknown()} if cannot be inferred */ - public static boolean inferPurity(@NotNull PsiMethodImpl method) { - return getInferenceData(method).myPure; + public static MutationSignature inferMutationSignature(@NotNull PsiMethodImpl method) { + return getInferenceData(method).myMutationSignature; } @NotNull diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/MethodDataExternalizer.kt b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/MethodDataExternalizer.kt index 68031a0538fe..4151aadb1e30 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/MethodDataExternalizer.kt +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/MethodDataExternalizer.kt @@ -90,10 +90,12 @@ internal object MethodDataExternalizer : DataExternalizer> private fun readRange(input: DataInput) = ExpressionRange(readINT(input), readINT(input)) private fun writePurity(out: DataOutput, purity: PurityInferenceResult) { + out.writeBoolean(purity.mutatesThis) writeRanges(out, purity.mutatedRefs) writeNullable(out, purity.singleCall) { writeRange(out, it) } } private fun readPurity(input: DataInput) = PurityInferenceResult( + input.readBoolean(), readRanges(input), readNullable(input) { readRange(input) }) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/PurityInferenceVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/PurityInferenceVisitor.java index 2c8b8617ac12..f87c602cc926 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/PurityInferenceVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inference/PurityInferenceVisitor.java @@ -24,6 +24,7 @@ class PurityInferenceVisitor { private final Map myFieldModifiers; private final List mutatedRefs = new ArrayList<>(); private final boolean constructor; + private boolean mutatesThis; private boolean hasVolatileReads; private final List calls = new ArrayList<>(); @@ -34,7 +35,7 @@ class PurityInferenceVisitor { myFieldModifiers = fieldModifiers; } - void visitNode(LighterASTNode element) { + boolean visitNode(LighterASTNode element) { IElementType type = element.getTokenType(); if (type == ASSIGNMENT_EXPRESSION) { addMutation(tree.getChildren(element).get(0)); @@ -58,6 +59,7 @@ class PurityInferenceVisitor { } } } + return !unknownPurity(); } private boolean isEffectivelyUnqualified(LighterASTNode element) { @@ -68,14 +70,19 @@ class PurityInferenceVisitor { private void addMutation(LighterASTNode mutated) { if (mutated == null) return; - if (constructor && !myFieldModifiers.isEmpty()) { + if (!myFieldModifiers.isEmpty()) { IElementType type = mutated.getTokenType(); - // writes to own fields in constructor do not count as mutations if (type == REFERENCE_EXPRESSION && isEffectivelyUnqualified(mutated)) { LighterASTNode modifiers = myFieldModifiers.get(JavaLightTreeUtil.getNameIdentifierText(tree, mutated)); if (modifiers != null) { boolean isStatic = LightTreeUtil.firstChildOfType(tree, modifiers, JavaTokenType.STATIC_KEYWORD) != null; - if (!isStatic) return; + if (!isStatic) { + if (!constructor) { + // writes to own fields in constructor do not count as mutations + mutatesThis = true; + } + return; + } } } } @@ -96,10 +103,16 @@ class PurityInferenceVisitor { @Nullable PurityInferenceResult getResult() { - if (calls.size() > 1 || (!constructor && hasVolatileReads)) return null; + if (unknownPurity()) return null; int bodyStart = body.getStartOffset(); - return new PurityInferenceResult(ContainerUtil.map(mutatedRefs, node -> ExpressionRange.create(node, bodyStart)), - calls.isEmpty() ? null : ExpressionRange.create(calls.get(0), bodyStart)); + return new PurityInferenceResult( + mutatesThis, + ContainerUtil.map(mutatedRefs, node -> ExpressionRange.create(node, bodyStart)), + calls.isEmpty() ? null : ExpressionRange.create(calls.get(0), bodyStart)); + } + + private boolean unknownPurity() { + return calls.size() > 1 || (!constructor && hasVolatileReads); } } 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 858e3b69ee6b..58fdb56dd8ef 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 @@ -1,10 +1,11 @@ // 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.inference +import com.intellij.codeInsight.ExpressionUtil import com.intellij.codeInsight.Nullability import com.intellij.codeInsight.NullableNotNullManager -import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil import com.intellij.codeInspection.dataFlow.Mutability +import com.intellij.codeInspection.dataFlow.MutationSignature import com.intellij.lang.LighterASTNode import com.intellij.psi.* import com.intellij.psi.impl.source.PsiMethodImpl @@ -35,29 +36,39 @@ data class ExpressionRange internal constructor (internal val startOffset: Int, } -data class PurityInferenceResult(internal val mutatedRefs: List, internal val singleCall: ExpressionRange?) { +data class PurityInferenceResult(internal val mutatesThis: Boolean, + internal val mutatedRefs: List, + internal val singleCall: ExpressionRange?) { - fun isPure(method: PsiMethod, body: () -> PsiCodeBlock): Boolean = !mutatesNonLocals(method, body) && callsOnlyPureMethods(method, body) + fun getMutationSignature(method: PsiMethod, body: () -> PsiCodeBlock): MutationSignature = + when { + mutatesNonLocals(method, body) -> MutationSignature.unknown() + mutatesThis -> fromCalls(method, body).alsoMutatesThis() + else -> fromCalls(method, body) + } private fun mutatesNonLocals(method: PsiMethod, body: () -> PsiCodeBlock): Boolean { return mutatedRefs.any { range -> !isLocalVarReference(range.restoreExpression(body()), method) } } - private fun callsOnlyPureMethods(currentMethod: PsiMethod, body: () -> PsiCodeBlock): Boolean { - if (singleCall == null) return true + private fun fromCalls(currentMethod: PsiMethod, body: () -> PsiCodeBlock): MutationSignature { + if (singleCall == null) return MutationSignature.pure() val psiCall = singleCall.restoreExpression(body()) as? PsiCall val method = psiCall?.resolveMethod() - if (method != null) { - return method == currentMethod || JavaMethodContractUtil.isPure(method) - } else if (psiCall is PsiNewExpression && psiCall.argumentList?.expressionCount == 0) { - val psiClass = psiCall.classOrAnonymousClassReference?.resolve() as? PsiClass - if (psiClass != null) { - val superClass = psiClass.superClass - return superClass == null || superClass.qualifiedName == CommonClassNames.JAVA_LANG_OBJECT + if (method == currentMethod) { + if (!mutatesThis || psiCall is PsiMethodCallExpression && ExpressionUtil.isEffectivelyUnqualified(psiCall.methodExpression)) { + return MutationSignature.pure() } + return MutationSignature.unknown() } - return false + val signature = MutationSignature.fromCall(psiCall) + if (signature == MutationSignature.pure() || + signature == MutationSignature.pure().alsoMutatesThis() && + psiCall is PsiMethodCallExpression && ExpressionUtil.isEffectivelyUnqualified(psiCall.methodExpression)) { + return if (currentMethod.isConstructor) MutationSignature.pure() else signature + } + return MutationSignature.unknown() } private fun isLocalVarReference(expression: PsiExpression?, scope: PsiMethod): Boolean { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutationContractFromSource.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutationContractFromSource.java new file mode 100644 index 000000000000..dd2f8424c8ad --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutationContractFromSource.java @@ -0,0 +1,16 @@ +import java.util.List; + +final class MainTest { + int x = 0; + + void increment() { + x++; + } + + void test(List list) { + if (list.isEmpty()) return; + increment(); // mutates this, so should not flush list + if (list.isEmpty()) return; + System.out.println("hello"); + } +} \ No newline at end of file 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 f2c4ca620bf5..17deda2ce3eb 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -670,6 +670,7 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testStringContains() { doTest(); } public void testSwitchLabelNull() { doTest(); } public void testMutationContractInFlush() { doTest(); } + public void testMutationContractFromSource() { doTest(); } public void testDefaultConstructor() { doTest(); } public void testInstanceOfUnresolved() { doTest(); } } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.groovy b/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.groovy index 81c6a1b4a5c4..8fbec6766703 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.groovy +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/PurityInferenceFromSourceTest.groovy @@ -16,6 +16,7 @@ package com.intellij.java.codeInspection import com.intellij.codeInspection.dataFlow.JavaMethodContractUtil +import com.intellij.codeInspection.dataFlow.MutationSignature import com.intellij.codeInspection.dataFlow.inference.JavaSourceInference import com.intellij.openapi.util.RecursionManager import com.intellij.psi.impl.source.PsiFileImpl @@ -29,7 +30,7 @@ import groovy.transform.CompileStatic class PurityInferenceFromSourceTest extends LightJavaCodeInsightFixtureTestCase { void "test getter"() { - assertPure true, """ + assertPure """ Object getField() { return field; } @@ -37,7 +38,7 @@ Object getField() { } void "test setter"() { - assertPure false, """ + assertImpure """ void setField(String s) { field = s; } @@ -45,7 +46,7 @@ void setField(String s) { } void "test unknown"() { - assertPure false, """ + assertImpure """ int random() { launchMissiles(); return 2; @@ -54,7 +55,7 @@ int random() { } void "test print"() { - assertPure false, """ + assertImpure """ int random() { System.out.println("hello"); return 2; @@ -63,7 +64,7 @@ int random() { } void "test local var assignment"() { - assertPure true, """ + assertPure """ int random(boolean b) { int i = 4; if (b) { @@ -77,7 +78,7 @@ int random(boolean b) { } void "test local array var assignment"() { - assertPure true, """ + assertPure """ int[] randomArray() { int[] i = new int[0]; i[0] = random(); @@ -88,7 +89,7 @@ int random() { return 2; } } void "test field array assignment"() { - assertPure false, """ + assertImpure """ int[] randomArray() { i[0] = random(); return i; @@ -99,7 +100,7 @@ int random() { return 2; } } void "test field array assignment as local var"() { - assertPure false, """ + assertImpure """ int[] randomArray() { int[] local = i; local[0] = random(); @@ -111,7 +112,7 @@ int random() { return 2; } } void "test use explicit pure contract"() { - assertPure true, """ + assertPure """ int method() { return smthPure(); } @@ -120,7 +121,7 @@ int method() { } void "test don't analyze more than one call"() { - assertPure false, """ + assertImpure """ int method() { return smthPure(smthPure2()); } @@ -130,14 +131,14 @@ int smthPure2() { return 42; } } void "test empty constructor"() { - assertPure true, """ + assertPure """ public Foo() { } """ } void "test field writes"() { - assertPure true, """ + assertPure """ int x; int y; @@ -150,7 +151,7 @@ public Foo() { void "test constructor calling"() { // IDEA-192251 - assertPure true, """ + assertPure """ private final int i; private final int j; private final Foo a; @@ -166,7 +167,7 @@ public Foo() { } void "test delegating field writes"() { - assertPure true, """ + assertPure """ int x; int y; @@ -182,7 +183,7 @@ Foo(int x, int y) { } void "test delegating unknown writes"() { - assertPure false, """ + assertImpure """ int x; int y; @@ -198,7 +199,7 @@ Foo(int x, int y) { } void "test static field writes"() { - assertPure false, """ + assertImpure """ int x; static int y; @@ -210,7 +211,7 @@ public Foo() { } void "test calling constructor with side effects"() { - assertPure false, """ + assertImpure """ Object newExample() { return new Example1(); } @@ -224,7 +225,7 @@ public Foo() { } void "test anonymous class initializer"() { - assertPure false, """ + assertImpure """ Object smth() { return new I(){{ created++; }}; } @@ -236,7 +237,7 @@ public Foo() { } void "test simple anonymous class creation"() { - assertPure true, """ + assertPure """ Object smth() { return new I(){}; } @@ -246,7 +247,7 @@ public Foo() { } void "test anonymous class with constructor side effect"() { - assertPure false, """ + assertImpure """ Object smth() { return new I(){}; } @@ -260,7 +261,7 @@ public Foo() { } void "test anonymous class with arguments"() { - assertPure false, """ + assertImpure """ Object smth() { return new I(unknown()){}; } @@ -272,7 +273,7 @@ public Foo() { } void "test class with impure initializer creation"() { - assertPure false, """ + assertImpure """ Object smth() { return new I(42); } @@ -287,7 +288,7 @@ public Foo() { } void "test class with impure static initializer creation"() { - assertPure true, """ + assertPure """ Object smth() { return new I(42); } @@ -302,7 +303,7 @@ public Foo() { } void "test class with pure field initializers"() { - assertPure true, """ + assertPure """ Object smth() { return new I(42); } @@ -315,7 +316,7 @@ public Foo() { } void "test class with impure field initializers"() { - assertPure false, """ + assertImpure """ Object smth() { return new I(42); } @@ -328,7 +329,7 @@ public Foo() { } void "test class with superclass"() { - assertPure false, """ + assertImpure """ Object smth() { return new I(42); } @@ -351,15 +352,53 @@ class Another { } } """) - assertPure false, """ + assertImpure """ Object smth() { return Another.method(); } """ } + + void "test increment field"() { + assertMutatesThis """ + int x = 0; + + private void increment() { + x++; + } +""" + } + + void "test delegate to setter"() { + assertMutatesThis """ + int x = 0; + + private void foo() { + setX(2); + } + + private void setX(int x) { + this.x = x; + } +""" + } + + void "test setter in ctor"() { + assertPure """ + int x = 0; + + public Foo() { + setX(2); + } + + private void setX(int x) { + this.x = x; + } +""" + } void "test plain field read"() { - assertPure true, """ + assertPure """ int x; int get() { @@ -369,7 +408,7 @@ int get() { } void "test volatile field read"() { - assertPure false, """ + assertImpure """ volatile int x; int get() { @@ -379,14 +418,14 @@ int get() { } void "test assertNotNull is pure"() { - assertPure true, """ + assertPure """ static void assertNotNull(Object val) { if(val == null) throw new AssertionError(); }""" } void "test recursive factorial"() { - assertPure true, """int factorial(int n) { return n == 1 ? 1 : factorial(n - 1) * n;}""" + assertPure """int factorial(int n) { return n == 1 ? 1 : factorial(n - 1) * n;}""" } void "test calling static method with the same signature in the subclass"() { @@ -425,12 +464,23 @@ class Super { assert JavaMethodContractUtil.isPure(clazz.superClass.methods[0]) } - private void assertPure(boolean expected, String classBody) { - def clazz = myFixture.addClass("final class Foo { $classBody }") - assert !((PsiFileImpl) clazz.containingFile).contentsLoaded - def purity = JavaSourceInference.inferPurity((PsiMethodImpl)clazz.methods[0]) - assert !((PsiFileImpl) clazz.containingFile).contentsLoaded - assert expected == purity + private void assertPure(String classBody) { + assertMutationSignature(classBody, MutationSignature.pure()) } + private void assertImpure(String classBody) { + assertMutationSignature(classBody, MutationSignature.unknown()) + } + + private void assertMutatesThis(String classBody) { + assertMutationSignature(classBody, MutationSignature.pure().alsoMutatesThis()) + } + + private void assertMutationSignature(String classBody, MutationSignature expected) { + def clazz = myFixture.addClass("final class Foo { $classBody }") + assert !((PsiFileImpl)clazz.containingFile).contentsLoaded + def signature = JavaSourceInference.inferMutationSignature((PsiMethodImpl)clazz.methods[0]) + assert !((PsiFileImpl)clazz.containingFile).contentsLoaded + assert expected == signature + } }