From d65bab611201a9618fdf7e55752747b2fd8dd332 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Mon, 27 Nov 2017 16:07:21 +0700 Subject: [PATCH] Mutability tracking support in dataflow analysis IDEA-182125 Mutability tracking support IDEA-167940 Inspection to warn of modification of immutable collections --- .../InferredAnnotationsManagerImpl.java | 48 +-- .../dataFlow/ControlFlowAnalyzer.java | 13 +- .../dataFlow/DataFlowInspectionBase.java | 16 + .../dataFlow/DataFlowInstructionVisitor.java | 24 ++ .../codeInspection/dataFlow/DfaFactType.java | 22 +- .../dataFlow/DfaMemoryStateImpl.java | 3 +- .../dataFlow/MutationSignature.java | 149 ++++++++ .../dataFlow/StandardInstructionVisitor.java | 36 +- .../inliner/CollectionFactoryInliner.java | 81 ++++- .../dataFlow/value/DfaValueFactory.java | 11 + .../dataFlow/EditContractIntention.java | 105 +++++- .../dataFlow/fixture/MutabilityBasics.java | 77 ++++ .../dataFlow/fixture/MutabilityJdk.java | 62 ++++ .../dataFlow/fixture/MutabilityJdk9.java | 50 +++ .../dataFlow/fixture/NullabilityJdk9.java | 15 + .../DataFlowInspection8Test.java | 9 + .../DataFlowInspection9Test.java | 33 ++ .../DataFlowInspectionTestSuite.java | 1 + .../dataFlow/MutationSignatureTest.java | 25 ++ java/jdkAnnotations/java/util/annotations.xml | 342 ++++++++++++++++++ .../org/jetbrains/annotations/Contract.java | 17 + .../src/messages/InspectionsBundle.properties | 2 + .../org/jetbrains/annotations/ReadOnly.java | 34 ++ 23 files changed, 1105 insertions(+), 70 deletions(-) create mode 100644 java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/MutabilityBasics.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk9.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NullabilityJdk9.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection9Test.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/dataFlow/MutationSignatureTest.java create mode 100644 platform/util/src/org/jetbrains/annotations/ReadOnly.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java b/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java index 41cb7c1f5238..e7296cfea9f4 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/InferredAnnotationsManagerImpl.java @@ -4,25 +4,24 @@ package com.intellij.codeInsight; import com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis; import com.intellij.codeInspection.dataFlow.*; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.impl.source.PsiMethodImpl; import com.intellij.psi.util.PsiUtil; import com.intellij.util.containers.ContainerUtil; +import one.util.streamex.EntryStream; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.List; -import java.util.Set; +import java.util.*; -import static com.intellij.codeInsight.AnnotationUtil.CHECK_EXTERNAL; -import static com.intellij.codeInsight.AnnotationUtil.CHECK_INFERRED; -import static com.intellij.codeInsight.AnnotationUtil.CHECK_TYPE; +import static com.intellij.codeInsight.AnnotationUtil.*; import static com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT; public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { private static final Set INFERRED_ANNOTATIONS = - ContainerUtil.set(AnnotationUtil.NOT_NULL, AnnotationUtil.NULLABLE, ORG_JETBRAINS_ANNOTATIONS_CONTRACT); + ContainerUtil.set(NOT_NULL, NULLABLE, ORG_JETBRAINS_ANNOTATIONS_CONTRACT); private final Project myProject; public InferredAnnotationsManagerImpl(Project project) { @@ -54,7 +53,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { return fromBytecode; } - if ((AnnotationUtil.NOT_NULL.equals(annotationFQN) || AnnotationUtil.NULLABLE.equals(annotationFQN))) { + if ((NOT_NULL.equals(annotationFQN) || NULLABLE.equals(annotationFQN))) { PsiAnnotation anno = null; if (listOwner instanceof PsiMethodImpl) { anno = getInferredNullityAnnotation((PsiMethodImpl)listOwner); @@ -86,9 +85,9 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { if (ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotationFQN) && HardcodedContracts.hasHardcodedContracts(owner)) { return true; } - if (AnnotationUtil.NOT_NULL.equals(annotationFQN) && owner instanceof PsiParameter && owner.getParent() != null) { + if (NOT_NULL.equals(annotationFQN) && owner instanceof PsiParameter && owner.getParent() != null) { List annotations = NullableNotNullManager.getInstance(owner.getProject()).getNullables(); - if (AnnotationUtil.isAnnotated(owner, annotations, CHECK_EXTERNAL | CHECK_INFERRED | CHECK_TYPE)) { + if (isAnnotated(owner, annotations, CHECK_EXTERNAL | CHECK_INFERRED | CHECK_TYPE)) { return true; } if (HardcodedContracts.hasHardcodedContracts(owner)) { @@ -110,7 +109,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { @Nullable private PsiAnnotation getInferredNullityAnnotation(PsiMethodImpl method) { NullableNotNullManager manager = NullableNotNullManager.getInstance(myProject); - if (AnnotationUtil.findAnnotation(method, manager.getNotNulls(), true) != null || AnnotationUtil.findAnnotation(method, manager.getNullables(), true) != null) { + if (findAnnotation(method, manager.getNotNulls(), true) != null || findAnnotation(method, manager.getNullables(), true) != null) { return null; } @@ -155,21 +154,26 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { @Nullable private PsiAnnotation createContractAnnotation(List contracts, boolean pure) { - return createContractAnnotation(myProject, pure, StreamEx.of(contracts).select(StandardMethodContract.class).joining("; ")); + return createContractAnnotation(myProject, pure, StreamEx.of(contracts).select(StandardMethodContract.class).joining("; "), ""); } @Nullable - public static PsiAnnotation createContractAnnotation(Project project, boolean pure, String contracts) { - final String attrs; - if (!contracts.isEmpty() && pure) { - attrs = "value = " + "\"" + contracts + "\", pure = true"; - } else if (pure) { - attrs = "pure = true"; - } else if (!contracts.isEmpty()) { - attrs = "\"" + contracts + "\""; - } else { + public static PsiAnnotation createContractAnnotation(Project project, boolean pure, String contracts, String mutates) { + Map attrMap = new LinkedHashMap<>(); + if (!contracts.isEmpty()) { + attrMap.put("value", StringUtil.wrapWithDoubleQuote(contracts)); + } + if (pure) { + attrMap.put("pure", "true"); + } + else if (!mutates.trim().isEmpty()) { + attrMap.put("mutates", StringUtil.wrapWithDoubleQuote(mutates)); + } + if (attrMap.isEmpty()) { return null; } + String attrs = attrMap.keySet().equals(Collections.singleton("value")) ? + attrMap.get("value") : EntryStream.of(attrMap).join(" = ").joining(", "); return ProjectBytecodeAnalysis.getInstance(project).createContractAnnotation(attrs); } @@ -193,7 +197,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { ContainerUtil.addIfNotNull(result, getInferredContractAnnotation((PsiMethodImpl)listOwner)); } - if (!ignoreInference(listOwner, AnnotationUtil.NOT_NULL) || !ignoreInference(listOwner, AnnotationUtil.NULLABLE)) { + if (!ignoreInference(listOwner, NOT_NULL) || !ignoreInference(listOwner, NULLABLE)) { PsiAnnotation annotation = getInferredNullityAnnotation((PsiMethodImpl)listOwner); if (annotation != null && !ignoreInference(listOwner, annotation.getQualifiedName())) { result.add(annotation); @@ -202,7 +206,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager { } } - if (listOwner instanceof PsiParameter && !ignoreInference(listOwner, AnnotationUtil.NOT_NULL)) { + if (listOwner instanceof PsiParameter && !ignoreInference(listOwner, NOT_NULL)) { ContainerUtil.addIfNotNull(result, getInferredNullityAnnotation((PsiParameter)listOwner)); } 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 1077a6e9a2c9..53c2810fb8b6 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 @@ -64,6 +64,7 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { private final ExceptionTransfer myError; private final PsiType myAssertionError; private InlinedBlockContext myInlinedBlockContext; + private final boolean myThisReadOnly; ControlFlowAnalyzer(final DfaValueFactory valueFactory, @NotNull PsiElement codeFragment, boolean ignoreAssertions, boolean inlining) { myInlining = inlining; @@ -75,6 +76,8 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { myRuntimeException = new ExceptionTransfer(myFactory.createDfaType(createClassType(scope, JAVA_LANG_RUNTIME_EXCEPTION))); myError = new ExceptionTransfer(myFactory.createDfaType(createClassType(scope, JAVA_LANG_ERROR))); myAssertionError = createClassType(scope, JAVA_LANG_ASSERTION_ERROR); + PsiElement member = PsiTreeUtil.getParentOfType(codeFragment, PsiMember.class, PsiLambdaExpression.class); + myThisReadOnly = member instanceof PsiMethod && MutationSignature.fromMethod((PsiMethod)member).preservesThis(); } private void buildClassInitializerFlow(PsiClass psiClass, boolean isStatic) { @@ -1486,7 +1489,9 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { if (qualifierExpression != null) { qualifierExpression.accept(this); } - else { + else if (myThisReadOnly) { + addInstruction(new PushInstruction(myFactory.getFactValue(DfaFactType.MUTABLE, false), null)); + } else { pushUnknown(); } @@ -1803,7 +1808,11 @@ public class ControlFlowAnalyzer extends JavaElementVisitor { @Override public void visitThisExpression(PsiThisExpression expression) { startElement(expression); - addInstruction(new PushInstruction(myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL), null)); + DfaValue value = myFactory.createTypeValue(expression.getType(), Nullness.NOT_NULL); + if (myThisReadOnly) { + value = myFactory.withFact(value, DfaFactType.MUTABLE, false); + } + addInstruction(new PushInstruction(value, null)); finishElement(expression); } 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 4d5682aa04ed..6d1bf08c8af8 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 @@ -281,6 +281,22 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool if (REPORT_NULLABLE_METHODS_RETURNING_NOT_NULL && visitor.isAlwaysReturnsNotNull(runner.getInstructions())) { reportAlwaysReturnsNotNull(holder, scope); } + + reportMutabilityViolations(holder, reportedAnchors, visitor.getMutabilityViolations(true), + InspectionsBundle.message("dataflow.message.immutable.modified")); + reportMutabilityViolations(holder, reportedAnchors, visitor.getMutabilityViolations(false), + InspectionsBundle.message("dataflow.message.immutable.passed")); + } + + private static void reportMutabilityViolations(ProblemsHolder holder, + Set reportedAnchors, + Set violations, + String message) { + for (PsiElement violation : violations) { + if (reportedAnchors.add(violation)) { + holder.registerProblem(violation, message); + } + } } private void reportNullabilityProblems(ProblemsHolder holder, diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java index 23a7de1aeb03..a83abedacb40 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -15,6 +15,7 @@ import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.MultiMap; import com.siyeh.ig.psiutils.TypeUtils; import one.util.streamex.StreamEx; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; @@ -33,6 +34,8 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { private final Map myOutOfBoundsArrayAccesses = new HashMap<>(); private final List myOptionalQualifiers = new ArrayList<>(); private final MultiMap myPossibleVariableValues = MultiMap.createSet(); + private final Set myReceiverMutabilityViolation = new HashSet<>(); + private final Set myArgumentMutabilityViolation = new HashSet<>(); private boolean myAlwaysReturnsNotNull = true; @Override @@ -71,6 +74,10 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { return myCCEInstructions; } + Set getMutabilityViolations(boolean receiver) { + return receiver ? myReceiverMutabilityViolation : myArgumentMutabilityViolation; + } + Stream outOfBoundsArrayAccesses() { return StreamEx.ofKeys(myOutOfBoundsArrayAccesses, ThreeState.YES::equals); } @@ -231,6 +238,23 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { return ok; } + @Override + protected void reportMutabilityViolation(boolean receiver, @NotNull PsiElement anchor) { + if (receiver) { + if (anchor instanceof PsiMethodReferenceExpression) { + anchor = ((PsiMethodReferenceExpression)anchor).getReferenceNameElement(); + } else if (anchor instanceof PsiMethodCallExpression) { + anchor = ((PsiMethodCallExpression)anchor).getMethodExpression().getReferenceNameElement(); + } + if (anchor != null) { + myReceiverMutabilityViolation.add(anchor); + } + } + else { + myArgumentMutabilityViolation.add(anchor); + } + } + private static boolean shouldReportConstValue(Object value) { return value == null || value instanceof Boolean; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java index 992295c8f030..fe0e2db262c0 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaFactType.java @@ -40,7 +40,7 @@ public abstract class DfaFactType extends Key { */ public static final DfaFactType CAN_BE_NULL = new DfaFactType("Can be null") { @Override - String toString(Boolean fact) { + String toString(@NotNull Boolean fact) { return fact ? "Nullable" : "NotNull"; } @@ -74,6 +74,20 @@ public abstract class DfaFactType extends Key { } }; + public static final DfaFactType MUTABLE = new DfaFactType("Mutable") { + @Override + String toString(@NotNull Boolean fact) { + return fact ? "Mutable" : "ReadOnly"; + } + + @Nullable + @Override + Boolean calcFromVariable(@NotNull DfaVariableValue value) { + PsiModifierListOwner variable = value.getPsiVariable(); + return MutationSignature.getMutabilityFact(variable); + } + }; + /** * This fact is applied to the Optional values (like {@link java.util.Optional} or Guava Optional). * When its value is true, then optional is known to be present. @@ -86,7 +100,7 @@ public abstract class DfaFactType extends Key { } @Override - String toString(Boolean fact) { + String toString(@NotNull Boolean fact) { return fact ? "present Optional" : "absent Optional"; } }; @@ -144,7 +158,7 @@ public abstract class DfaFactType extends Key { } @Override - String toString(LongRangeSet fact) { + String toString(@NotNull LongRangeSet fact) { return fact.toString(); } }; @@ -237,7 +251,7 @@ public abstract class DfaFactType extends Key { return left.equals(right) ? left : null; } - String toString(T fact) { + String toString(@NotNull T fact) { return fact.toString(); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index f6c5098f6f07..4debe52014c3 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -1239,7 +1239,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } } for (DfaVariableValue value : vars) { - if (value.isFlushableByCalls()) { + if (value.isFlushableByCalls() && (value.getQualifier() == null || + !Boolean.FALSE.equals(getValueFact(value.getQualifier(), DfaFactType.MUTABLE)))) { doFlush(value, shouldMarkUnknown(value)); } } 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 new file mode 100644 index 000000000000..29d30723dcc7 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/MutationSignature.java @@ -0,0 +1,149 @@ +// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.codeInspection.dataFlow; + +import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.ObjectUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Arrays; +import java.util.Collections; + +public class MutationSignature { + private static final String ATTR_MUTATES = "mutates"; + private static final String CONTRACT_ANNOTATION = "org.jetbrains.annotations.Contract"; + private static final MutationSignature UNKNOWN = new MutationSignature(false, new boolean[0]); + private static final MutationSignature PURE = new MutationSignature(false, new boolean[0]); + public static final String INVALID_TOKEN_MESSAGE = "Invalid token: %s; supported are 'this', 'arg1', 'arg2', etc."; + private final boolean myThis; + private final boolean[] myArgs; + + private MutationSignature(boolean mutatesThis, boolean[] args) { + myThis = mutatesThis; + myArgs = args; + } + + public boolean mutatesThis() { + return myThis; + } + + public boolean mutatesArg(int n) { + return n < myArgs.length && myArgs[n]; + } + + public boolean preservesThis() { + return this != UNKNOWN && !myThis; + } + + public boolean preservesArg(int n) { + return this != UNKNOWN && !mutatesArg(n); + } + + /** + * @param signature to parse + * @return a parsed mutation signature + * @throws IllegalArgumentException if signature is invalid + */ + public static MutationSignature parse(String signature) { + boolean mutatesThis = false; + boolean[] args = {}; + for (String part : signature.split(",")) { + part = part.trim(); + if (part.equals("this")) { + mutatesThis = true; + } + else if (part.equals("arg")) { + if (args.length == 0) { + args = new boolean[] {true}; + } else { + args[0] = true; + } + } + else if (part.startsWith("arg")) { + int argNum = Integer.parseInt(part.substring("arg".length())); + if (argNum < 0 || argNum > 255) { + throw new IllegalArgumentException(String.format(INVALID_TOKEN_MESSAGE, part)); + } + if(args.length < argNum) { + args = Arrays.copyOf(args, argNum); + } + args[argNum-1] = true; + } + else if (!part.isEmpty()) { + throw new IllegalArgumentException(String.format(INVALID_TOKEN_MESSAGE, part)); + } + } + return new MutationSignature(mutatesThis, args); + } + + /** + * Checks the mutation signature + * @param signature signature to check + * @param method a method to apply the signature + * @return error message or null if signature is valid + */ + @Nullable + public static String checkSignature(@NotNull String signature, @NotNull PsiMethod method) { + try { + MutationSignature ms = parse(signature); + if (ms.myThis && method.hasModifierProperty(PsiModifier.STATIC)) { + return "Static method cannot mutate 'this'"; + } + if (ms.myArgs.length > method.getParameterList().getParametersCount()) { + return "Reference to argument #" + ms.myArgs.length + " is invalid"; + } + } + catch (IllegalArgumentException ex) { + return ex.getMessage(); + } + return null; + } + + @NotNull + public static MutationSignature fromMethod(@Nullable PsiMethod method) { + if (method == null) return UNKNOWN; + PsiAnnotation annotation = AnnotationUtil.findAnnotation(method, CONTRACT_ANNOTATION); + if (annotation == null) return UNKNOWN; + PsiAnnotationMemberValue value = annotation.findAttributeValue(ATTR_MUTATES); + if (value instanceof PsiLiteralExpression) { + Object text = ((PsiLiteralExpression)value).getValue(); + if (text instanceof String) { + try { + return parse((String)text); + } + catch (IllegalArgumentException ignored) { } + } + } + if(ControlFlowAnalyzer.isPure(method)) { + return PURE; + } + return UNKNOWN; + } + + @Nullable + static Boolean getMutabilityFact(PsiModifierListOwner owner) { + if (owner instanceof PsiParameter && owner.getParent() instanceof PsiParameterList) { + PsiParameterList list = (PsiParameterList)owner.getParent(); + PsiMethod method = ObjectUtils.tryCast(list.getParent(), PsiMethod.class); + if (method != null) { + int index = list.getParameterIndex((PsiParameter)owner); + MutationSignature signature = fromMethod(method); + if (signature.mutatesArg(index)) { + return Boolean.TRUE; + } else if (signature.preservesArg(index) && + PsiTreeUtil.findChildOfAnyType(method.getBody(), PsiLambdaExpression.class, PsiClass.class) == null) { + // If method preserves argument, it still may return a lambda which captures an argument and changes it + // TODO: more precise check (at least differentiate parameters which are captured by lambdas or not) + return Boolean.FALSE; + } + return null; + } + } + return AnnotationUtil.isAnnotated(owner, Collections.singleton("org.jetbrains.annotations.ReadOnly"), + AnnotationUtil.CHECK_HIERARCHY | + AnnotationUtil.CHECK_EXTERNAL | + AnnotationUtil.CHECK_INFERRED) ? Boolean.FALSE : null; + } +} 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 b2513c2648e2..2c7f8529c102 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 @@ -340,8 +340,10 @@ public class StandardInstructionVisitor extends InstructionVisitor { DataFlowRunner runner, DfaMemoryState memState, boolean contractOnly) { - DfaValue[] argValues = popCallArguments(instruction, runner, memState, contractOnly); - final DfaValue qualifier = popQualifier(instruction, memState); + PsiMethod method = instruction.getTargetMethod(); + MutationSignature sig = MutationSignature.fromMethod(method); + DfaValue[] argValues = popCallArguments(instruction, runner, memState, contractOnly, sig); + final DfaValue qualifier = popQualifier(instruction, memState, sig); return new DfaCallArguments(qualifier, argValues); } @@ -349,7 +351,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { private DfaValue[] popCallArguments(MethodCallInstruction instruction, DataFlowRunner runner, DfaMemoryState memState, - boolean contractOnly) { + boolean contractOnly, MutationSignature sig) { final int argCount = instruction.getArgCount(); PsiMethod method = instruction.getTargetMethod(); @@ -382,6 +384,12 @@ public class StandardInstructionVisitor extends InstructionVisitor { else if (requiredNullability == Nullness.UNKNOWN) { checkNotNullable(memState, arg, NullabilityProblemKind.passingNullableArgumentToNonAnnotatedParameter.problem(anchor)); } + if (sig.mutatesArg(paramIndex) && !memState.applyFact(arg, DfaFactType.MUTABLE, true)) { + reportMutabilityViolation(false, anchor); + if (arg instanceof DfaVariableValue) { + memState.forceVariableFact((DfaVariableValue)arg, DfaFactType.MUTABLE, true); + } + } if (argValues != null && (paramIndex < argValues.length - 1 || !varargCall)) { argValues[paramIndex] = arg; } @@ -389,8 +397,20 @@ public class StandardInstructionVisitor extends InstructionVisitor { return argValues; } - private DfaValue popQualifier(MethodCallInstruction instruction, DfaMemoryState memState) { - return dereference(memState, memState.pop(), instruction.getQualifierNullabilityProblem()); + protected void reportMutabilityViolation(boolean receiver, @NotNull PsiElement anchor) { + } + + private DfaValue popQualifier(MethodCallInstruction instruction, + DfaMemoryState memState, + MutationSignature sig) { + DfaValue value = dereference(memState, memState.pop(), instruction.getQualifierNullabilityProblem()); + if (sig.mutatesThis() && !memState.applyFact(value, DfaFactType.MUTABLE, true)) { + reportMutabilityViolation(true, instruction.getContext()); + if (value instanceof DfaVariableValue) { + memState.forceVariableFact((DfaVariableValue)value, DfaFactType.MUTABLE, true); + } + } + return value; } private static LinkedHashSet addContractResults(DfaCallArguments callArguments, @@ -507,10 +527,13 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (type != null && !(type instanceof PsiPrimitiveType)) { Nullness nullability = instruction.getReturnNullability(); PsiMethod targetMethod = instruction.getTargetMethod(); + Boolean mutable = null; if (targetMethod != null) { + mutable = MutationSignature.getMutabilityFact(targetMethod); PsiMethod realMethod = findSpecificMethod(targetMethod, state, qualifierValue); if (realMethod != targetMethod) { nullability = DfaPsiUtil.getElementNullability(type, realMethod); + mutable = MutationSignature.getMutabilityFact(realMethod); PsiType returnType = realMethod.getReturnType(); if (returnType != null && TypeConversionUtil.erasure(type).isAssignableFrom(returnType)) { // possibly covariant return type @@ -521,7 +544,8 @@ public class StandardInstructionVisitor extends InstructionVisitor { nullability = factory.suggestNullabilityForNonAnnotatedMember(targetMethod); } } - return factory.createTypeValue(type, nullability); + DfaValue value = factory.createTypeValue(type, nullability); + return factory.withFact(value, DfaFactType.MUTABLE, mutable); } LongRangeSet range = LongRangeSet.fromType(type); if (range != null) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java index a4dd3ca9ffab..9288728d11f5 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/inliner/CollectionFactoryInliner.java @@ -15,30 +15,38 @@ */ package com.intellij.codeInspection.dataFlow.inliner; -import com.intellij.codeInspection.dataFlow.CFGBuilder; -import com.intellij.codeInspection.dataFlow.Nullness; -import com.intellij.codeInspection.dataFlow.SpecialField; +import com.intellij.codeInspection.dataFlow.*; +import com.intellij.codeInspection.dataFlow.value.DfaValue; import com.intellij.codeInspection.dataFlow.value.DfaValueFactory; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.psi.PsiExpression; import com.intellij.psi.PsiMethodCallExpression; import com.intellij.psi.PsiVariable; import com.siyeh.ig.callMatcher.CallMapper; +import com.siyeh.ig.callMatcher.CallMatcher; +import com.siyeh.ig.psiutils.MethodCallUtils; import org.jetbrains.annotations.NotNull; import static com.intellij.codeInspection.dataFlow.SpecialField.COLLECTION_SIZE; import static com.intellij.codeInspection.dataFlow.SpecialField.MAP_SIZE; -import static com.intellij.psi.CommonClassNames.JAVA_UTIL_COLLECTIONS; +import static com.intellij.psi.CommonClassNames.*; +import static com.siyeh.ig.callMatcher.CallMatcher.anyOf; import static com.siyeh.ig.callMatcher.CallMatcher.staticCall; public class CollectionFactoryInliner implements CallInliner { static final class FactoryInfo { - int mySize; - SpecialField mySizeField; + final boolean myNotNull; + final int mySize; + final SpecialField mySizeField; public FactoryInfo(int size, SpecialField sizeField) { + this(size, sizeField, false); + } + + public FactoryInfo(int size, SpecialField sizeField, boolean notNull) { mySize = size; mySizeField = sizeField; + myNotNull = notNull; } } @@ -48,24 +56,63 @@ public class CollectionFactoryInliner implements CallInliner { .register(staticCall(JAVA_UTIL_COLLECTIONS, "emptyMap").parameterCount(0), new FactoryInfo(0, MAP_SIZE)) .register(staticCall(JAVA_UTIL_COLLECTIONS, "singletonMap").parameterCount(2), new FactoryInfo(1, MAP_SIZE)); + private static final CallMatcher JDK9_MAP_FACTORIES = + staticCall(JAVA_UTIL_MAP, "of", "ofEntries"); + + private static final CallMatcher JDK9_FACTORIES = anyOf( + staticCall(JAVA_UTIL_LIST, "of"), + staticCall(JAVA_UTIL_SET, "of") + ); + + private static final CallMatcher JDK9_ARRAY_FACTORIES = anyOf( + staticCall(JAVA_UTIL_LIST, "of").parameterTypes("E..."), + staticCall(JAVA_UTIL_SET, "of").parameterTypes("E...") + ); + + private static FactoryInfo getFactoryInfo(@NotNull PsiMethodCallExpression call) { + FactoryInfo info = STATIC_FACTORIES.mapFirst(call); + if (info != null) return info; + if (JDK9_FACTORIES.test(call)) { + int size = + JDK9_ARRAY_FACTORIES.test(call) && !MethodCallUtils.isVarArgCall(call) ? -1 : call.getArgumentList().getExpressions().length; + return new FactoryInfo(size, COLLECTION_SIZE, true); + } + if (JDK9_MAP_FACTORIES.test(call)) { + boolean ofEntries = "ofEntries".equals(call.getMethodExpression().getReferenceName()); + int size = + ofEntries && !MethodCallUtils.isVarArgCall(call) ? -1 : call.getArgumentList().getExpressions().length / (ofEntries ? 1 : 2); + return new FactoryInfo(size, MAP_SIZE, true); + } + return null; + } + @Override public boolean tryInlineCall(@NotNull CFGBuilder builder, @NotNull PsiMethodCallExpression call) { - FactoryInfo factoryInfo = STATIC_FACTORIES.mapFirst(call); + FactoryInfo factoryInfo = getFactoryInfo(call); if (factoryInfo == null) return false; PsiExpression[] args = call.getArgumentList().getExpressions(); for (PsiExpression arg : args) { - builder.pushExpression(arg).pop(); + builder.pushExpression(arg); + if (factoryInfo.myNotNull) { + builder.checkNotNull(arg, NullabilityProblemKind.passingNullableToNotNullParameter); + } + builder.pop(); } - PsiVariable variable = builder.createTempVariable(call.getType()); DfaValueFactory factory = builder.getFactory(); - DfaVariableValue variableValue = factory.getVarFactory().createVariableValue(variable, false); - builder.pushVariable(variable) // tmpVar = - .push(factory.createTypeValue(call.getType(), Nullness.NOT_NULL)) - .assign() // leave tmpVar on stack: it's result of method call - .push(factoryInfo.mySizeField.createValue(factory, variableValue)) // tmpVar.size = - .push(factory.getInt(factoryInfo.mySize)) - .assign() - .pop(); + DfaValue result = factory.withFact(factory.createTypeValue(call.getType(), Nullness.NOT_NULL), DfaFactType.MUTABLE, false); + if (factoryInfo.mySize == -1) { + builder.push(result); + } else { + PsiVariable variable = builder.createTempVariable(call.getType()); + DfaVariableValue variableValue = factory.getVarFactory().createVariableValue(variable, false); + builder.pushVariable(variable) // tmpVar = + .push(result) + .assign() // leave tmpVar on stack: it's result of method call + .push(factoryInfo.mySizeField.createValue(factory, variableValue)) // tmpVar.size = + .push(factory.getInt(factoryInfo.mySize)) + .assign() + .pop(); + } return true; } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java index df36ea03d4f0..395958615e60 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java @@ -83,6 +83,17 @@ public class DfaValueFactory { return getFactFactory().createValue(facts); } + @NotNull + public DfaValue withFact(@NotNull DfaValue value, @NotNull DfaFactType factType, @Nullable T factValue) { + if(value instanceof DfaUnknownValue) { + return getFactFactory().createValue(DfaFactMap.EMPTY.with(factType, factValue)); + } + if(value instanceof DfaFactMapValue) { + return ((DfaFactMapValue)value).withFact(factType, factValue); + } + return DfaUnknownValue.getInstance(); + } + @NotNull public DfaPsiType createDfaType(@NotNull PsiType psiType) { int dimensions = psiType.getArrayDimensions(); diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/EditContractIntention.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/EditContractIntention.java index c8a48541c0c3..fd7d7c804b8e 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/EditContractIntention.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/EditContractIntention.java @@ -23,6 +23,7 @@ import com.intellij.codeInsight.daemon.DaemonCodeAnalyzer; import com.intellij.codeInsight.intention.AddAnnotationPsiFix; import com.intellij.codeInsight.intention.LowPriorityAction; import com.intellij.codeInsight.intention.impl.BaseIntentionAction; +import com.intellij.openapi.application.ex.ApplicationManagerEx; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.openapi.ui.DialogBuilder; @@ -33,6 +34,7 @@ import com.intellij.ui.DocumentAdapter; import com.intellij.ui.NonFocusableCheckBox; import com.intellij.ui.components.JBTextField; import com.intellij.util.IncorrectOperationException; +import com.intellij.util.ui.JBUI; import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -56,7 +58,7 @@ public class EditContractIntention extends BaseIntentionAction implements LowPri } @Nullable - private static PsiMethod getTargetMethod(@NotNull Project project, Editor editor, PsiFile file) { + private static PsiMethod getTargetMethod(Editor editor, PsiFile file) { final PsiModifierListOwner owner = AddAnnotationPsiFix.getContainer(file, editor.getCaretModel().getOffset()); if (owner instanceof PsiMethod && ExternalAnnotationsManagerImpl.areExternalAnnotationsApplicable(owner)) { PsiElement original = owner.getOriginalElement(); @@ -67,7 +69,7 @@ public class EditContractIntention extends BaseIntentionAction implements LowPri @Override public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) { - final PsiMethod method = getTargetMethod(project, editor, file); + final PsiMethod method = getTargetMethod(editor, file); if (method != null) { boolean hasContract = ControlFlowAnalyzer.findContractAnnotation(method) != null; setText(hasContract ? "Edit method contract of '" + method.getName() + "'" : "Add method contract to '" + method.getName() + "'"); @@ -78,33 +80,95 @@ public class EditContractIntention extends BaseIntentionAction implements LowPri @Override public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException { - final PsiMethod method = getTargetMethod(project, editor, file); + final PsiMethod method = getTargetMethod(editor, file); assert method != null; Contract existingAnno = AnnotationUtil.findAnnotationInHierarchy(method, Contract.class); String oldContract = existingAnno == null ? null : existingAnno.value(); boolean oldPure = existingAnno != null && existingAnno.pure(); + String oldMutates = existingAnno == null ? null : existingAnno.mutates(); JBTextField contractText = new JBTextField(oldContract); + JBTextField mutatesText = new JBTextField(oldMutates); JCheckBox pureCB = createPureCheckBox(oldPure); - DialogBuilder builder = createDialog(project, contractText, pureCB); - contractText.getDocument().addDocumentListener(new DocumentAdapter() { + DialogBuilder builder = createDialog(project, contractText, pureCB, mutatesText); + DocumentAdapter validator = new DocumentAdapter() { @Override protected void textChanged(DocumentEvent e) { - String error = getErrorMessage(contractText.getText(), method); - builder.setOkActionEnabled(error == null); - builder.setErrorText(error, contractText); + String contractError = getContractErrorMessage(contractText.getText(), method); + if (contractError != null) { + builder.setOkActionEnabled(false); + builder.setErrorText(contractError, contractText); + } + else { + String mutatesError = getMutatesErrorMessage(mutatesText.getText(), method); + if (mutatesError != null) { + builder.setOkActionEnabled(false); + builder.setErrorText(mutatesError, mutatesText); + } + else { + builder.setOkActionEnabled(true); + builder.setErrorText(null); + } + } } - }); + }; + Runnable updateControls = () -> { + if (pureCB.isSelected()) { + mutatesText.setText(""); + mutatesText.setEnabled(false); + } + else { + mutatesText.setEnabled(true); + } + }; + pureCB.addChangeListener(e -> updateControls.run()); + contractText.getDocument().addDocumentListener(validator); + mutatesText.getDocument().addDocumentListener(validator); + updateControls.run(); if (builder.showAndGet()) { - updateContract(method, contractText.getText(), pureCB.isSelected()); + updateContract(method, contractText.getText(), pureCB.isSelected(), mutatesText.getText()); } } - private static DialogBuilder createDialog(@NotNull Project project, JBTextField contractText, JCheckBox pureCB) { - JPanel panel = new JPanel(new BorderLayout()); - panel.add(Messages.configureMessagePaneUi(new JTextPane(), ourPrompt), BorderLayout.NORTH); - panel.add(contractText, BorderLayout.CENTER); - panel.add(pureCB, BorderLayout.SOUTH); + private static DialogBuilder createDialog(@NotNull Project project, + JBTextField contractText, + JCheckBox pureCB, + JBTextField mutatesText) { + JPanel panel = new JPanel(new GridBagLayout()); + + GridBagConstraints constraints = + new GridBagConstraints(0, 0, 2, 1, 4.0, 1.0, GridBagConstraints.WEST, GridBagConstraints.HORIZONTAL, JBUI.insets(2), 0, 0); + panel.add(Messages.configureMessagePaneUi(new JTextPane(), ourPrompt), constraints); + constraints.gridx = 0; + constraints.gridy = 1; + constraints.gridwidth = 1; + constraints.weightx = 1; + JLabel contractLabel = new JLabel("Contract:"); + contractLabel.setDisplayedMnemonic('c'); + contractLabel.setLabelFor(contractText); + panel.add(contractLabel, constraints); + constraints.gridx = 1; + constraints.weightx = 3; + panel.add(contractText, constraints); + constraints.gridx = 0; + constraints.gridy = 2; + constraints.gridwidth = 2; + constraints.weightx = 4; + panel.add(pureCB, constraints); + panel.add(pureCB, constraints); + if (ApplicationManagerEx.getApplicationEx().isInternal()) { + constraints.gridx = 0; + constraints.gridy = 3; + constraints.weightx = 1; + constraints.gridwidth = 1; + JLabel mutatesLabel = new JLabel("Mutates:"); + mutatesLabel.setDisplayedMnemonic('m'); + mutatesLabel.setLabelFor(mutatesText); + panel.add(mutatesLabel, constraints); + constraints.gridx = 1; + constraints.weightx = 3; + panel.add(mutatesText, constraints); + } DialogBuilder builder = new DialogBuilder(project).setNorthPanel(panel).title("Edit Method Contract"); builder.setPreferredFocusComponent(contractText); @@ -118,11 +182,11 @@ public class EditContractIntention extends BaseIntentionAction implements LowPri return pureCB; } - private static void updateContract(PsiMethod method, String contract, boolean pure) { + private static void updateContract(PsiMethod method, String contract, boolean pure, String mutates) { Project project = method.getProject(); ExternalAnnotationsManager manager = ExternalAnnotationsManager.getInstance(project); manager.deannotate(method, ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT); - PsiAnnotation mockAnno = InferredAnnotationsManagerImpl.createContractAnnotation(project, pure, contract); + PsiAnnotation mockAnno = InferredAnnotationsManagerImpl.createContractAnnotation(project, pure, contract, mutates); if (mockAnno != null) { try { manager.annotateExternally(method, ControlFlowAnalyzer.ORG_JETBRAINS_ANNOTATIONS_CONTRACT, method.getContainingFile(), @@ -134,7 +198,12 @@ public class EditContractIntention extends BaseIntentionAction implements LowPri } @Nullable - private static String getErrorMessage(String contract, PsiMethod method) { + private static String getMutatesErrorMessage(String mutates, PsiMethod method) { + return StringUtil.isEmpty(mutates) ? null : MutationSignature.checkSignature(mutates, method); + } + + @Nullable + private static String getContractErrorMessage(String contract, PsiMethod method) { return StringUtil.isEmpty(contract) ? null : ContractInspection.checkContract(method, contract); } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityBasics.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityBasics.java new file mode 100644 index 000000000000..c61458a8f1e2 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityBasics.java @@ -0,0 +1,77 @@ +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.ReadOnly; + +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +public class MutabilityBasics { + @ReadOnly + static List emptyList() { + return Collections.emptyList(); + } + + @Contract(mutates = "arg") + static > void sort(List collection) { + Collections.sort(collection); + } + + @Contract(mutates = "arg1") + static > void addAll(Collection collection, List other) { + sort(other); + collection.addAll(other); + } + + // Purity implies that no arguments should be changed + @Contract(pure = true) + static > T min(List list) { + sort(list); + return list.get(0); + } + + interface Point { + int get(); + + @Contract(mutates = "this") + void set(int x); + + @Contract(pure = true) + default void setZero() { + // cannot modify itself (call mutating method), because declared as pure + set(0); + } + } + + @ReadOnly + static Point getZero() { + return new Point() { + @Override + public int get() { + return 0; + } + + @Override + public void set(int x) { + throw new UnsupportedOperationException(); + } + }; + } + + // Differs from getZero as getZero() is considered as getter with predefined value + @ReadOnly + static Point zero() { + return getZero(); + } + + @ReadOnly List list = Arrays.asList("foo", "bar", "baz"); + + void test() { + List collection = emptyList(); + sort(collection); + sort(MutabilityBasics.emptyList()); + getZero().set(1); + zero().set(1); + sort(list); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java new file mode 100644 index 000000000000..86d4621e6854 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk.java @@ -0,0 +1,62 @@ +import java.util.*; +import java.io.*; + +public class MutabilityJdk { + + void testEmpty() { + List list = Collections.emptyList(); + Collections.sort(list); + } + + void testUnmodifiable(Collection collection) { + collection = Collections.unmodifiableCollection(collection); + collection.add("foo"); + } + + void testBranch(boolean b) { + List list; + if(b) { + list = Collections.emptyList(); + } else { + list = new ArrayList<>(); + } + if(!b) { + list.add("foo"); + } + list.add("bar"); + } + + void testArrayGtZero(File configRoot) { + final File[] files = configRoot.listFiles(); + + final Map templatesOnDisk = files != null && files.length > 0 ? new HashMap<>() : Collections.emptyMap(); + if (files != null) { + for (File file : files) { + if (!file.isDirectory()) { + final String name = file.getName(); + templatesOnDisk.put(name, file); + } + } + } + } + + // Purity is inferred + static Runnable testLambda(List list) { + return () -> list.add("foo"); + } + + interface X { + List get(); + } + + List getList(X x) { + List result = Collections.emptyList(); + for (Object obj : x.get()) { + if (obj instanceof String) { + if (result.isEmpty()) result = new ArrayList<>(); + result.add((String)obj); + } + } + return result; + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk9.java b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk9.java new file mode 100644 index 000000000000..d67cb35cba32 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/MutabilityJdk9.java @@ -0,0 +1,50 @@ +import java.util.*; + +public class MutabilityJdk9 { + + void testList() { + List list = List.of(1,2,3); + if(list.size() > 5) { + System.out.println("impossible"); + } + list.sort(null); + } + + void testSet() { + Set set = Set.of("foo", "bar", "baz", "qux"); + if(set.size() == 4) { + System.out.println("always"); + } + set.add("oops"); + } + + void testMap() { + Map map = Map.of("a", 1, "b", 2, "c", 3, "d", 4, "e", 5); + if(map.size() != 5) { + System.out.println("never"); + } + map.put("foo", 6); + } + + void testMapOfEntries() { + Map map = Map.ofEntries(Map.entry("x", 1), Map.entry("y", 2)); + if(map.size() == 2) { + System.out.println("you bet!"); + } + map.remove("x"); + } + + // IDEA-167940 + void testImmutable(String a, String b){ + List.of(a).add(b); //java 9 collection literals do not accept modification + + Collections.singletonList(a).add(b); //singleton should not be modified + } + + void testVarArg(String[] str) { + List list = List.of(str); + if(str.length > 2) { + list.add("foo"); + } + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NullabilityJdk9.java b/java/java-tests/testData/inspection/dataFlow/fixture/NullabilityJdk9.java new file mode 100644 index 000000000000..d6dd391e6204 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NullabilityJdk9.java @@ -0,0 +1,15 @@ +import java.util.*; + +public class NullabilityJdk9 { + + void test() { + List list = List.of(1,2,3,null); + Set set = Set.of("foo", "bar", null, "baz"); + Map map = Map.of("x", null, + "y", null, + null, 3); + Map map1 = Map.ofEntries(null, null); + Integer[] array = null; + List list1 = List.of(array); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index b3cb2f649e5f..7e2f87b0d4ae 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -207,4 +207,13 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { } public void testCastInstanceOf() { doTest(); } + + public void testMutabilityBasics() { + myFixture.addClass("package org.jetbrains.annotations;public @interface ReadOnly {}"); + doTest(); + } + + public void testMutabilityJdk() { + doTest(); + } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection9Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection9Test.java new file mode 100644 index 000000000000..9a27791d2dd3 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection9Test.java @@ -0,0 +1,33 @@ +// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.java.codeInspection; + +import com.intellij.JavaTestUtil; +import com.intellij.openapi.projectRoots.Sdk; +import com.intellij.testFramework.IdeaTestUtil; +import com.intellij.testFramework.LightProjectDescriptor; +import com.intellij.testFramework.PsiTestUtil; +import com.intellij.testFramework.fixtures.DefaultLightProjectDescriptor; +import org.jetbrains.annotations.NotNull; + +public class DataFlowInspection9Test extends DataFlowInspectionTestCase { + private static final DefaultLightProjectDescriptor PROJECT_DESCRIPTOR = new DefaultLightProjectDescriptor() { + @Override + public Sdk getSdk() { + return PsiTestUtil.addJdkAnnotations(IdeaTestUtil.getMockJdk9()); + } + }; + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return PROJECT_DESCRIPTOR; + } + + @Override + protected String getTestDataPath() { + return JavaTestUtil.getJavaTestDataPath() + "/inspection/dataFlow/fixture/"; + } + + public void testNullabilityJdk9() { doTest();} + public void testMutabilityJdk9() { doTest();} +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java index 3ef70c3fe9ee..3cc8e73df49b 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTestSuite.java @@ -29,6 +29,7 @@ public class DataFlowInspectionTestSuite { suite.addTestSuite(DataFlowInspectionTest.class); suite.addTestSuite(DataFlowInspection8Test.class); + suite.addTestSuite(DataFlowInspection9Test.class); suite.addTestSuite(DataFlowInspectionHeavyTest.class); suite.addTestSuite(DataFlowInspectionAncientTest.class); suite.addTestSuite(ContractCheckTest.class); diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/dataFlow/MutationSignatureTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/dataFlow/MutationSignatureTest.java new file mode 100644 index 000000000000..0a9bf2072295 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/dataFlow/MutationSignatureTest.java @@ -0,0 +1,25 @@ +// Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.java.codeInspection.dataFlow; + +import com.intellij.codeInspection.dataFlow.MutationSignature; +import org.junit.Test; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class MutationSignatureTest { + @Test + public void testParse() { + MutationSignature sig = MutationSignature.parse("this"); + assertTrue(sig.mutatesThis()); + assertFalse(sig.mutatesArg(0)); + sig = MutationSignature.parse("arg1 , arg2"); + assertFalse(sig.mutatesThis()); + assertTrue(sig.mutatesArg(0)); + assertTrue(sig.mutatesArg(1)); + assertFalse(sig.mutatesArg(2)); + sig = MutationSignature.parse(""); + assertFalse(sig.mutatesThis()); + assertFalse(sig.mutatesArg(0)); + } +} diff --git a/java/jdkAnnotations/java/util/annotations.xml b/java/jdkAnnotations/java/util/annotations.xml index cae8f755253f..db1c6ed3de0b 100644 --- a/java/jdkAnnotations/java/util/annotations.xml +++ b/java/jdkAnnotations/java/util/annotations.xml @@ -601,10 +601,23 @@ + + + + + + + + + + + + + @@ -615,9 +628,29 @@ + + + + + + + + + + + + + + + + + + + + @@ -631,6 +664,26 @@ + + + + + + + + + + + + + + + + + + + + @@ -649,6 +702,11 @@ + + + + + @@ -661,6 +719,11 @@ + + + + + @@ -679,6 +742,11 @@ + + + + + @@ -709,6 +777,10 @@ + + + + @@ -741,16 +813,23 @@ + + + + + + + @@ -768,13 +847,19 @@ + + + + + + @@ -793,18 +878,29 @@ + + + + + + + + + + + @@ -820,6 +916,10 @@ + + + + @@ -831,46 +931,96 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -961,8 +1111,14 @@ + + + + + + @@ -974,15 +1130,28 @@ + + + + + + + + + + + + + @@ -990,6 +1159,11 @@ + + + + + @@ -1000,9 +1174,24 @@ + + + + + + + + + + + + + + + @@ -1025,18 +1214,56 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1054,12 +1281,35 @@ + + + + + + + + + + + + + + + + + + + + + + + @@ -1067,6 +1317,9 @@ + + + @@ -1075,13 +1328,39 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1096,27 +1375,48 @@ + + + + + + + + + + + + + + + + + + + + + @@ -1151,6 +1451,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1326,6 +1651,11 @@ + + + + + @@ -1355,15 +1685,27 @@ + + + + + + + + + + + + diff --git a/platform/annotations/common/src/org/jetbrains/annotations/Contract.java b/platform/annotations/common/src/org/jetbrains/annotations/Contract.java index 662c347e5704..80b6257b4ec6 100644 --- a/platform/annotations/common/src/org/jetbrains/annotations/Contract.java +++ b/platform/annotations/common/src/org/jetbrains/annotations/Contract.java @@ -64,4 +64,21 @@ public @interface Contract { * to check that the method's return value is actually used in the call place. */ boolean pure() default false; + + /** + * Contains a specifier which describes which method parameters can be mutated during the method call. + *

+ * The following values are possible: + * + * Method mutates the receiver object, and doesn't mutates any objects passed as arguments (cannot be applied for static method or constructor) + * Method mutates the sole argument and doesn't mutate the receiver object (if applicable) + * Method mutates the N-th argument + * Method mutates the receiver and first argument and doesn't mutate any other arguments + *
"this"
"arg"
"arg1", "arg2", ...
"this,arg1"
+ *

+ * + * @return a mutation specifier string + * Warning: This annotation parameter is experimental and may be changed or removed without further notice! + */ + String mutates() default ""; } diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index cc944871b5fd..95ebd082675e 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -92,6 +92,8 @@ dataflow.method.fails.with.null.argument=Method will throw an exception when par dataflow.message.optional.get.without.is.present={0}.#ref() without ''isPresent()'' check dataflow.message.constant.method.reference=Method reference result is always ''{0}'' dataflow.message.array.index.out.of.bounds=Array index is out of bounds +dataflow.message.immutable.modified=Immutable object is modified +dataflow.message.immutable.passed=Immutable object is passed where mutable is expected #deprecated inspection.deprecated.display.name=Deprecated API usage diff --git a/platform/util/src/org/jetbrains/annotations/ReadOnly.java b/platform/util/src/org/jetbrains/annotations/ReadOnly.java new file mode 100644 index 000000000000..29bb4b380876 --- /dev/null +++ b/platform/util/src/org/jetbrains/annotations/ReadOnly.java @@ -0,0 +1,34 @@ +// Copyright 2000-2017 JetBrains s.r.o. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package org.jetbrains.annotations; + +import java.lang.annotation.*; + +/** + * An annotation which depicts that method returns a read-only value or a variable + * contains a read-only value. Read-only value means that calling methods which may + * mutate this value (alter visible behavior) either don't have any effect or throw + * an exception. This does not mean that value cannot be altered at all. For example, + * a value could be a read-only wrapper over a mutable value. + *

+ * This annotation is experimental and may be changed/removed in future + * without additional notice! + *

+ */ +@Documented +@Retention(RetentionPolicy.CLASS) +@Target({ElementType.METHOD, ElementType.FIELD, ElementType.LOCAL_VARIABLE}) +@ApiStatus.Experimental +public @interface ReadOnly { +} \ No newline at end of file