IDEA-188554 Infer method return value immutability from source

This commit is contained in:
Tagir Valeev
2018-03-20 13:18:14 +07:00
parent e2b404487b
commit 15b048e3b1
10 changed files with 215 additions and 51 deletions
@@ -21,7 +21,8 @@ import static com.intellij.codeInspection.dataFlow.ControlFlowAnalyzer.ORG_JETBR
public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager {
private static final Set<String> INFERRED_ANNOTATIONS =
ContainerUtil.set(NOT_NULL, NULLABLE, ORG_JETBRAINS_ANNOTATIONS_CONTRACT);
ContainerUtil.set(NOT_NULL, NULLABLE, ORG_JETBRAINS_ANNOTATIONS_CONTRACT, Mutability.UNMODIFIABLE_ANNOTATION,
Mutability.UNMODIFIABLE_VIEW_ANNOTATION);
private final Project myProject;
public InferredAnnotationsManagerImpl(Project project) {
@@ -64,6 +65,10 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager {
return anno == null ? null : annotationFQN.equals(anno.getQualifiedName()) ? anno : null;
}
if (Mutability.UNMODIFIABLE_ANNOTATION.equals(annotationFQN) || Mutability.UNMODIFIABLE_VIEW_ANNOTATION.equals(annotationFQN)) {
return getInferredMutabilityAnnotation(listOwner);
}
if (listOwner instanceof PsiMethodImpl && ORG_JETBRAINS_ANNOTATIONS_CONTRACT.equals(annotationFQN)) {
return getInferredContractAnnotation((PsiMethodImpl)listOwner);
}
@@ -97,6 +102,17 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager {
return false;
}
@Nullable
private PsiAnnotation getInferredMutabilityAnnotation(@NotNull PsiModifierListOwner owner) {
if (!(owner instanceof PsiMethod)) return null;
PsiModifierList modifiers = ((PsiMethod)owner).getModifierList();
if (modifiers.findAnnotation(Mutability.UNMODIFIABLE_ANNOTATION) != null ||
modifiers.findAnnotation(Mutability.UNMODIFIABLE_VIEW_ANNOTATION) != null) {
return null;
}
return Mutability.inferMutability(owner).asAnnotation(myProject);
}
@Nullable
private PsiAnnotation getInferredContractAnnotation(PsiMethodImpl method) {
if (method.getModifierList().findAnnotation(ORG_JETBRAINS_ANNOTATIONS_CONTRACT) != null) {
@@ -214,6 +230,8 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager {
ContainerUtil.addIfNotNull(result, getInferredNullityAnnotation((PsiParameter)listOwner));
}
ContainerUtil.addIfNotNull(result, getInferredMutabilityAnnotation(listOwner));
return result.toArray(PsiAnnotation.EMPTY_ARRAY);
}
@@ -30,7 +30,7 @@ import java.util.*
* @author peter
*/
private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 6, MethodDataExternalizer) { file ->
private val gist = GistManager.getInstance().newPsiFileGist("contractInference", 7, MethodDataExternalizer) { file ->
indexFile(file.node.lighterAST)
}
@@ -61,7 +61,7 @@ private fun calcData(tree: LighterAST, method: LighterASTNode): MethodData? {
val contracts = ContractInferenceInterpreter(tree, method, body).inferContracts(statements)
val nullityVisitor = NullityInference.NullityInferenceVisitor(tree, body)
val nullityVisitor = NullityInference.MethodReturnInferenceVisitor(tree, body)
val purityVisitor = PurityInference.PurityInferenceVisitor(tree, body)
for (statement in statements) {
walkMethodBody(tree, statement) { nullityVisitor.visitNode(it); purityVisitor.visitNode(it) }
@@ -85,12 +85,12 @@ private fun walkMethodBody(tree: LighterAST, root: LighterASTNode, processor: (L
private fun createData(body: LighterASTNode,
contracts: List<PreContract>,
nullity: NullityInferenceResult?,
methodReturn: MethodReturnInferenceResult?,
purity: PurityInferenceResult?,
notNullParams: BitSet): MethodData? {
if (nullity == null && purity == null && contracts.isEmpty() && notNullParams.isEmpty) return null
if (methodReturn == null && purity == null && contracts.isEmpty() && notNullParams.isEmpty) return null
return MethodData(nullity, purity, contracts, notNullParams, body.startOffset, body.endOffset)
return MethodData(methodReturn, purity, contracts, notNullParams, body.startOffset, body.endOffset)
}
fun getIndexedData(method: PsiMethodImpl): MethodData? = gist.getFileData(method.containingFile)?.get(JavaStubImplUtil.getMethodStubIndex(method))
@@ -84,12 +84,18 @@ public abstract class DfaFactType<T> extends Key<T> {
}
};
public static final DfaFactType<Mutability> MUTABILITY = new DfaFactType<Mutability>("Mutable") {
public static final DfaFactType<Mutability> MUTABILITY = new DfaFactType<Mutability>("Mutability") {
@Override
boolean isUnknown(@NotNull Mutability fact) {
return fact == Mutability.UNKNOWN;
}
@NotNull
@Override
Mutability unionFacts(@NotNull Mutability left, @NotNull Mutability right) {
return left.union(right);
}
@NotNull
@Override
Mutability calcFromVariable(@NotNull DfaVariableValue value) {
@@ -33,7 +33,7 @@ internal object MethodDataExternalizer : DataExternalizer<Map<Int, MethodData>>
override fun read(input: DataInput) = readSeq(input) { readINT(input) to readMethod(input) }.toMap()
private fun writeMethod(out: DataOutput, data: MethodData) {
writeNullable(out, data.nullity) { writeNullity(out, it) }
writeNullable(out, data.methodReturn) { writeNullity(out, it) }
writeNullable(out, data.purity) { writePurity(out, it) }
writeSeq(out, data.contracts) { writeContract(out, it) }
writeBitSet(out, data.notNullParameters)
@@ -65,14 +65,16 @@ internal object MethodDataExternalizer : DataExternalizer<Map<Int, MethodData>>
return BitSet.valueOf(bytes)
}
private fun writeNullity(out: DataOutput, nullity: NullityInferenceResult) = when (nullity) {
is NullityInferenceResult.Predefined -> { out.writeByte(0); out.writeByte(nullity.value.ordinal) }
is NullityInferenceResult.FromDelegate -> { out.writeByte(1); writeRanges(out, nullity.delegateCalls) }
else -> throw IllegalArgumentException(nullity.toString())
private fun writeNullity(out: DataOutput, methodReturn: MethodReturnInferenceResult) = when (methodReturn) {
is MethodReturnInferenceResult.Predefined -> { out.writeByte(0); out.writeByte(methodReturn.value.ordinal) }
is MethodReturnInferenceResult.FromDelegate -> {
out.writeByte(1); out.writeByte(methodReturn.value.ordinal); writeRanges(out, methodReturn.delegateCalls)
}
else -> throw IllegalArgumentException(methodReturn.toString())
}
private fun readNullity(input: DataInput): NullityInferenceResult = when (input.readByte().toInt()) {
0 -> NullityInferenceResult.Predefined(Nullness.values()[input.readByte().toInt()])
else -> NullityInferenceResult.FromDelegate(readRanges(input))
private fun readNullity(input: DataInput): MethodReturnInferenceResult = when (input.readByte().toInt()) {
0 -> MethodReturnInferenceResult.Predefined(Nullness.values()[input.readByte().toInt()])
else -> MethodReturnInferenceResult.FromDelegate(Nullness.values()[input.readByte().toInt()], readRanges(input))
}
private fun writeRanges(out: DataOutput, ranges: List<ExpressionRange>) = writeSeq(out, ranges) { writeRange(out, it) }
@@ -4,42 +4,85 @@
package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.AnnotationUtil;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.util.Key;
import com.intellij.openapi.util.ModificationTracker;
import com.intellij.openapi.util.RecursionManager;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.impl.source.PsiMethodImpl;
import com.intellij.psi.util.*;
import com.intellij.testFramework.LightVirtualFile;
import com.intellij.util.ObjectUtils;
import com.siyeh.ig.psiutils.ClassUtils;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import java.util.Collections;
import static com.intellij.codeInspection.bytecodeAnalysis.ProjectBytecodeAnalysis.INFERRED_ANNOTATION;
public enum Mutability {
/**
* Mutability is not known; probably value can be mutated
*/
UNKNOWN,
UNKNOWN("Unknown", null),
/**
* A value is known to be mutable (e.g. elements are sometimes added to the collection)
*/
MUTABLE,
MUTABLE("Modifiable", null),
/**
* A value is known to be immutable. For collection no elements could be added, removed or altered (though if collection
* contains mutable elements, they still could be mutated).
*/
UNMODIFIABLE,
UNMODIFIABLE("Unmodifiable", "org.jetbrains.annotations.Unmodifiable"),
/**
* A value is known to be an immutable view over a possibly mutable value: it cannot be mutated directly using this
* reference; however subsequent reads (e.g. {@link java.util.Collection#size}) may return different results if the
* underlying value is mutated by somebody else.
*/
UNMODIFIABLE_VIEW;
UNMODIFIABLE_VIEW("Unmodifiable view", "org.jetbrains.annotations.UnmodifiableView");
public static final String UNMODIFIABLE_ANNOTATION = "org.jetbrains.annotations.Unmodifiable";
public static final String UNMODIFIABLE_VIEW_ANNOTATION = "org.jetbrains.annotations.UnmodifiableView";
public static final @NotNull String UNMODIFIABLE_ANNOTATION = UNMODIFIABLE.myAnnotation;
public static final @NotNull String UNMODIFIABLE_VIEW_ANNOTATION = UNMODIFIABLE_VIEW.myAnnotation;
private final String myName;
private final String myAnnotation;
private final Key<CachedValue<PsiAnnotation>> myKey;
Mutability(String name, String annotation) {
myName = name;
myAnnotation = annotation;
myKey = annotation == null ? null : Key.create(annotation);
}
@Override
public String toString() {
return myName;
}
public boolean isUnmodifiable() {
return this == UNMODIFIABLE || this == UNMODIFIABLE_VIEW;
}
@NotNull
public Mutability union(Mutability other) {
if (this == other) return this;
if (this == MUTABLE || other == MUTABLE) return MUTABLE;
if (this == UNKNOWN || other == UNKNOWN) return UNKNOWN;
if (this == UNMODIFIABLE_VIEW || other == UNMODIFIABLE_VIEW) return UNMODIFIABLE_VIEW;
return UNMODIFIABLE;
}
@Nullable
public PsiAnnotation asAnnotation(Project project) {
if (myAnnotation == null) return null;
return CachedValuesManager.getManager(project).getCachedValue(project, myKey, () -> {
PsiAnnotation annotation = JavaPsiFacade.getElementFactory(project).createAnnotationFromText("@" + myAnnotation, null);
annotation.putUserData(INFERRED_ANNOTATION, Boolean.TRUE);
((LightVirtualFile)annotation.getContainingFile().getViewProvider().getVirtualFile()).setWritable(false);
return CachedValueProvider.Result.create(annotation, ModificationTracker.NEVER_CHANGED);
}, false);
}
/**
* Returns a mutability of the supplied element, if known. The element could be a method
* (in this case the return value mutability is returned), a method parameter
@@ -84,9 +127,35 @@ public enum Mutability {
PsiExpression initializer = PsiUtil.skipParenthesizedExprDown(((PsiField)owner).getInitializer());
if (initializer instanceof PsiMethodCallExpression) {
PsiMethod method = ((PsiMethodCallExpression)initializer).resolveMethod();
return method == null ? UNKNOWN : getMutability(method);
if (method == null) {
return UNKNOWN;
}
if (ClassUtils.isImmutable(method.getReturnType())) return UNMODIFIABLE;
return getMutability(method);
}
}
return UNKNOWN;
return inferMutability(owner);
}
public static Mutability inferMutability(PsiModifierListOwner owner) {
if (!(owner instanceof PsiMethodImpl)) return UNKNOWN;
PsiMethodImpl method = (PsiMethodImpl)owner;
if (!InferenceFromSourceUtil.shouldInferFromSource(method)) {
return UNKNOWN;
}
PsiType type = method.getReturnType();
if (type == null || ClassUtils.isImmutable(type)) {
return UNKNOWN;
}
return CachedValuesManager.getCachedValue(method, () -> {
MethodData data = ContractInferenceIndexKt.getIndexedData(method);
MethodReturnInferenceResult result = data == null ? null : data.getMethodReturn();
Mutability mutability = result == null ? null : RecursionManager
.doPreventingRecursion(method, true, () -> result.getMutability(method, data.methodBody(method)));
if (mutability == null) mutability = UNKNOWN;
return CachedValueProvider.Result.create(mutability, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
});
}
}
@@ -60,7 +60,7 @@ public class NullityInference {
return CachedValuesManager.getCachedValue(method, () -> {
MethodData data = ContractInferenceIndexKt.getIndexedData(method);
NullityInferenceResult result = data == null ? null : data.getNullity();
MethodReturnInferenceResult result = data == null ? null : data.getMethodReturn();
Nullness nullness = result == null ? null : RecursionManager.doPreventingRecursion(method, true, () -> result.getNullness(method, data.methodBody(method)));
if (nullness == null) nullness = Nullness.UNKNOWN;
return CachedValueProvider.Result.create(nullness, method, PsiModificationTracker.JAVA_STRUCTURE_MODIFICATION_COUNT);
@@ -90,7 +90,7 @@ public class NullityInference {
});
}
static class NullityInferenceVisitor {
static class MethodReturnInferenceVisitor {
private final LighterAST tree;
private final LighterASTNode body;
private boolean hasErrors;
@@ -101,7 +101,7 @@ public class NullityInference {
Set<String> assignments = ContainerUtil.newHashSet();
Set<String> returnedCheckedVars = ContainerUtil.newHashSet();
NullityInferenceVisitor(LighterAST tree, LighterASTNode body) {
MethodReturnInferenceVisitor(LighterAST tree, LighterASTNode body) {
this.tree = tree;
this.body = body;
}
@@ -193,15 +193,15 @@ public class NullityInference {
private boolean isNonNullCondition(@Nullable LighterASTNode expr, LighterASTNode var) {
expr = skipParenthesesCastsDown(tree, expr);
if (expr == null) return false;
IElementType type = expr.getTokenType();
if (type == BINARY_EXPRESSION || type == POLYADIC_EXPRESSION) {
List<LighterASTNode> operands = getExpressionChildren(tree, expr);
if (firstChildOfType(tree, expr, JavaTokenType.NE) != null) {
return operands.size() == 2 && isNullLiteral(operands.get(1)) && isReferenceTo(operands.get(0), var);
}
return firstChildOfType(tree, expr, JavaTokenType.ANDAND) != null && ContainerUtil.exists(operands, e -> isNonNullCondition(e, var));
return firstChildOfType(tree, expr, JavaTokenType.ANDAND) != null && ContainerUtil.exists(operands, e -> isNonNullCondition(e, var));
}
return type == INSTANCE_OF_EXPRESSION && isReferenceTo(expr, var);
@@ -209,8 +209,8 @@ public class NullityInference {
private boolean isReferenceTo(@NotNull LighterASTNode expr, @NotNull LighterASTNode var) {
LighterASTNode operand = skipParenthesesCastsDown(tree, findExpressionChild(tree, expr));
if (operand == null ||
operand.getTokenType() != REFERENCE_EXPRESSION ||
if (operand == null ||
operand.getTokenType() != REFERENCE_EXPRESSION ||
!Objects.equals(getNameIdentifierText(tree, operand), getNameIdentifierText(tree, operand))) {
return false;
}
@@ -222,7 +222,7 @@ public class NullityInference {
}
@Nullable
NullityInferenceResult getResult() {
MethodReturnInferenceResult getResult() {
if (!returnedCheckedVars.isEmpty()) {
if (ContainerUtil.exists(returnedCheckedVars, name -> !assignments.contains(name))) {
hasNotNulls = true;
@@ -230,19 +230,25 @@ public class NullityInference {
hasUnknowns = true;
}
}
List<ExpressionRange> delegateCalls = null;
if (delegates.size() == 1) {
delegateCalls = ContainerUtil.newArrayList(delegates.get(delegates.keySet().iterator().next()));
}
if (hasNulls) {
return new NullityInferenceResult.Predefined(Nullness.NULLABLE);
return delegateCalls == null || hasNotNulls || hasErrors || hasUnknowns
? new MethodReturnInferenceResult.Predefined(Nullness.NULLABLE)
: new MethodReturnInferenceResult.FromDelegate(Nullness.NULLABLE, delegateCalls);
}
if (hasErrors || hasUnknowns || delegates.size() > 1) {
return null;
}
if (delegates.size() == 1) {
return new NullityInferenceResult.FromDelegate(ContainerUtil.newArrayList(delegates.get(delegates.keySet().iterator().next())));
if (delegateCalls != null) {
return new MethodReturnInferenceResult.FromDelegate(hasNotNulls ? Nullness.NOT_NULL : Nullness.UNKNOWN, delegateCalls);
}
if (hasNotNulls) {
return new NullityInferenceResult.Predefined(Nullness.NOT_NULL);
return new MethodReturnInferenceResult.Predefined(Nullness.NOT_NULL);
}
return null;
}
@@ -25,6 +25,7 @@ import com.intellij.psi.util.CachedValueProvider
import com.intellij.psi.util.CachedValuesManager
import com.intellij.psi.util.PsiTreeUtil
import com.intellij.psi.util.PsiUtil
import com.siyeh.ig.psiutils.ClassUtils
import java.util.*
/**
@@ -89,11 +90,12 @@ data class PurityInferenceResult(internal val mutatedRefs: List<ExpressionRange>
}
interface NullityInferenceResult {
interface MethodReturnInferenceResult {
fun getNullness(method: PsiMethod, body: () -> PsiCodeBlock): Nullness
fun getMutability(method: PsiMethod, body: () -> PsiCodeBlock): Mutability = Mutability.UNKNOWN
@Suppress("EqualsOrHashCode")
data class Predefined(internal val value: Nullness) : NullityInferenceResult {
data class Predefined(internal val value: Nullness) : MethodReturnInferenceResult {
override fun hashCode() = value.ordinal
override fun getNullness(method: PsiMethod, body: () -> PsiCodeBlock) = when {
value == Nullness.NULLABLE && InferenceFromSourceUtil.suppressNullable(method) -> Nullness.UNKNOWN
@@ -101,10 +103,33 @@ interface NullityInferenceResult {
}
}
data class FromDelegate(internal val delegateCalls: List<ExpressionRange>) : NullityInferenceResult {
override fun getNullness(method: PsiMethod, body: () -> PsiCodeBlock) = when {
delegateCalls.all { range -> isNotNullCall(range, body()) } -> Nullness.NOT_NULL
else -> Nullness.UNKNOWN
data class FromDelegate(internal val value: Nullness, internal val delegateCalls: List<ExpressionRange>) : MethodReturnInferenceResult {
override fun getNullness(method: PsiMethod, body: () -> PsiCodeBlock): Nullness {
if (value == Nullness.NULLABLE) {
return if (InferenceFromSourceUtil.suppressNullable(method)) Nullness.UNKNOWN else Nullness.NULLABLE
}
return when {
delegateCalls.all { range -> isNotNullCall(range, body()) } -> Nullness.NOT_NULL
else -> Nullness.UNKNOWN
}
}
override fun getMutability(method: PsiMethod, body: () -> PsiCodeBlock): Mutability {
if (value == Nullness.NOT_NULL) {
return Mutability.UNKNOWN
}
return delegateCalls.stream().map { range -> getDelegateMutability(range, body()) }.reduce(Mutability::union).orElse(
Mutability.UNKNOWN)
}
private fun getDelegateMutability(delegate: ExpressionRange, body: PsiCodeBlock): Mutability {
val call = delegate.restoreExpression(body) as PsiMethodCallExpression
val target = call.resolveMethod()
return when {
target == null -> Mutability.UNKNOWN
ClassUtils.isImmutable(target.returnType) -> Mutability.UNMODIFIABLE
else -> Mutability.getMutability(target)
}
}
private fun isNotNullCall(delegate: ExpressionRange, body: PsiCodeBlock): Boolean {
@@ -118,7 +143,7 @@ interface NullityInferenceResult {
}
data class MethodData(
val nullity: NullityInferenceResult?,
val methodReturn: MethodReturnInferenceResult?,
val purity: PurityInferenceResult?,
val contracts: List<PreContract>,
val notNullParameters: BitSet,
@@ -0,0 +1,37 @@
import java.util.*;
import org.jetbrains.annotations.*;
public class MutabilityInferred {
@Nullable
static List<String> getListNullOrImmutable(int x) {
if(x == 0) return null;
return Collections.singletonList("foo");
}
static List<String> getListImmutable(int x) {
if(x == 0) return Collections.singletonList("bar");
return Collections.singletonList("foo");
}
static List<String> getListImmutableOrMutable(int x) {
if(x == 0) return new ArrayList<>();
return Collections.singletonList("foo");
}
static native List<String> getListUnknown(int x);
@Nullable
static List<String> getListVar(boolean b) {
List<String> list = getListUnknown(0);
if (list != null) return list;
if (b) return Collections.singletonList("xyz");
return null;
}
void test() {
getListNullOrImmutable(0).<warning descr="Immutable object is modified"><warning descr="Method invocation 'add' may produce 'java.lang.NullPointerException'">add</warning></warning>("a");
getListImmutable(0).<warning descr="Immutable object is modified">add</warning>("b");
getListImmutableOrMutable(0).add("c");
getListVar(false).<warning descr="Method invocation 'add' may produce 'java.lang.NullPointerException'">add</warning>("d");
}
}
@@ -213,9 +213,8 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase {
doTest();
}
public void testMutabilityJdk() {
doTest();
}
public void testMutabilityJdk() { doTest(); }
public void testMutabilityInferred() { doTest(); }
public void testPrimitiveGetters() { doTest(); }
public void testUnknownOnStack() { doTest(); }
@@ -116,7 +116,8 @@ public class ClassUtils {
return integralTypes.contains(type);
}
public static boolean isImmutable(PsiType type) {
@Contract("null -> false")
public static boolean isImmutable(@Nullable PsiType type) {
if (TypeConversionUtil.isPrimitiveAndNotNull(type)) {
return true;
}
@@ -124,7 +125,8 @@ public class ClassUtils {
if (aClass == null) {
return false;
}
if (immutableTypes.contains(aClass.getQualifiedName())) {
String qualifiedName = aClass.getQualifiedName();
if (immutableTypes.contains(qualifiedName) || (qualifiedName != null && qualifiedName.startsWith("com.google.common.collect.Immutable"))) {
return true;
}
return JCiPUtil.isImmutable(aClass);