Java: better mutability check for record components (IDEA-371018)

GitOrigin-RevId: e4f351bcc297d41857406d177df00ff2696a7bfd
This commit is contained in:
Bas Leijdekkers
2025-06-09 14:24:43 +00:00
committed by intellij-monorepo-bot
parent d461d596f7
commit 9ae32a8adb
4 changed files with 90 additions and 54 deletions
@@ -1,4 +1,4 @@
// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.AnnotationUtil;
@@ -37,6 +37,8 @@ import com.intellij.openapi.util.Ref;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.psi.*;
import com.intellij.psi.controlFlow.ControlFlowUtil;
import com.intellij.psi.impl.light.LightCompactConstructorParameter;
import com.intellij.psi.impl.light.LightRecordField;
import com.intellij.psi.search.LocalSearchScope;
import com.intellij.psi.search.searches.DeepestSuperMethodsSearch;
import com.intellij.psi.search.searches.ReferencesSearch;
@@ -470,32 +472,41 @@ public final class DfaPsiUtil {
return result;
}
private static MultiMap<PsiField, PsiExpression> getAllConstructorFieldInitializers(final PsiClass psiClass) {
public static List<PsiExpression> findAllConstructorInitializers(PsiRecordComponent component) {
PsiClass containingClass = component.getContainingClass();
return containingClass == null || containingClass instanceof PsiCompiledElement
? Collections.emptyList()
: new ArrayList<>(getAllConstructorFieldInitializers(containingClass).get(component));
}
private static MultiMap<PsiVariable, PsiExpression> getAllConstructorFieldInitializers(PsiClass psiClass) {
if (psiClass instanceof PsiCompiledElement) {
return MultiMap.empty();
}
return CachedValuesManager.getCachedValue(psiClass, new CachedValueProvider<>() {
@Override
public @NotNull Result<MultiMap<PsiField, PsiExpression>> compute() {
public @NotNull Result<MultiMap<PsiVariable, PsiExpression>> compute() {
final Set<String> fieldNames = new HashSet<>();
for (PsiField field : psiClass.getFields()) {
ContainerUtil.addIfNotNull(fieldNames, field.getName());
}
final MultiMap<PsiField, PsiExpression> result = new MultiMap<>();
final MultiMap<PsiVariable, PsiExpression> result = new MultiMap<>();
JavaRecursiveElementWalkingVisitor visitor = new JavaRecursiveElementWalkingVisitor() {
@Override
public void visitAssignmentExpression(@NotNull PsiAssignmentExpression assignment) {
super.visitAssignmentExpression(assignment);
PsiExpression lExpression = assignment.getLExpression();
PsiExpression rExpression = assignment.getRExpression();
if (rExpression != null &&
lExpression instanceof PsiReferenceExpression &&
fieldNames.contains(((PsiReferenceExpression)lExpression).getReferenceName())) {
PsiElement target = ((PsiReferenceExpression)lExpression).resolve();
if (target instanceof PsiField && ((PsiField)target).getContainingClass() == psiClass) {
result.putValue((PsiField)target, rExpression);
assignment.getLExpression() instanceof PsiReferenceExpression ref &&
fieldNames.contains(ref.getReferenceName())) {
PsiElement target = ref.resolve();
if (target instanceof PsiField field && field.getContainingClass() == psiClass) {
result.putValue(field instanceof LightRecordField f ? f.getRecordComponent() : field, rExpression);
}
else if (target instanceof LightCompactConstructorParameter parameter) {
result.putValue(parameter.getRecordComponent(), rExpression);
}
}
}
@@ -1,4 +1,4 @@
// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
// Copyright 2000-2025 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.intellij.codeInspection.dataFlow;
import com.intellij.codeInsight.AnnotationUtil;
@@ -19,7 +19,6 @@ import com.intellij.psi.util.CachedValueProvider;
import com.intellij.psi.util.CachedValuesManager;
import com.intellij.psi.util.PsiModificationTracker;
import com.intellij.testFramework.LightVirtualFile;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.ClassUtils;
@@ -153,21 +152,21 @@ public enum Mutability {
}
private static @NotNull Mutability calcMutability(@NotNull PsiModifierListOwner owner) {
if (owner instanceof PsiParameter && owner.getParent() instanceof PsiParameterList list) {
PsiMethod method = ObjectUtils.tryCast(list.getParent(), PsiMethod.class);
if (method != null) {
int index = list.getParameterIndex((PsiParameter)owner);
JavaMethodContractUtil.ContractInfo contractInfo = JavaMethodContractUtil.getContractInfo(method);
if (contractInfo.isExplicit()) {
MutationSignature signature = contractInfo.getMutationSignature();
if (signature.mutatesArg(index)) {
return MUTABLE;
} else if (signature.preservesArg(index)) {
return MUST_NOT_MODIFY;
}
if (owner instanceof PsiParameter parameter
&& owner.getParent() instanceof PsiParameterList list
&& list.getParent() instanceof PsiMethod method) {
JavaMethodContractUtil.ContractInfo contractInfo = JavaMethodContractUtil.getContractInfo(method);
if (contractInfo.isExplicit()) {
int index = list.getParameterIndex(parameter);
MutationSignature signature = contractInfo.getMutationSignature();
if (signature.mutatesArg(index)) {
return MUTABLE;
}
else if (signature.preservesArg(index)) {
return MUST_NOT_MODIFY;
}
return UNKNOWN;
}
return UNKNOWN;
}
if (AnnotationUtil.isAnnotated(owner, Collections.singleton(UNMODIFIABLE_ANNOTATION),
AnnotationUtil.CHECK_HIERARCHY |
@@ -186,33 +185,40 @@ public enum Mutability {
if (initializers.isEmpty() && !owner.hasModifierProperty(PsiModifier.STATIC)) {
initializers = DfaPsiUtil.findAllConstructorInitializers(field);
}
initializers = StreamEx.of(initializers).flatMap(ExpressionUtils::nonStructuralChildren).toList();
if (initializers.isEmpty()) return UNKNOWN;
Mutability mutability = UNMODIFIABLE;
for (PsiExpression initializer : initializers) {
Mutability newMutability = UNKNOWN;
if (ClassUtils.isImmutable(initializer.getType())) {
newMutability = UNMODIFIABLE;
} else if (initializer instanceof PsiMethodCallExpression call) {
if (STREAM_COLLECT.test(call)) {
PsiExpression collector = call.getArgumentList().getExpressions()[0];
newMutability = UNMODIFIABLE_COLLECTORS.matches(collector) ? UNMODIFIABLE : UNKNOWN;
} else if (STREAM_TO_LIST.test(call)) {
newMutability = UNMODIFIABLE;
} else {
PsiMethod method = call.resolveMethod();
newMutability = method == null ? UNKNOWN : getMutability(method);
}
}
mutability = mutability.join(newMutability);
if (!mutability.isUnmodifiable()) break;
}
return mutability;
return calcMutability(initializers);
}
return owner instanceof PsiMethodImpl ? JavaSourceInference.inferMutability((PsiMethodImpl)owner) : UNKNOWN;
else if (owner instanceof PsiRecordComponent component) {
return calcMutability(DfaPsiUtil.findAllConstructorInitializers(component));
}
return owner instanceof PsiMethodImpl method ? JavaSourceInference.inferMutability(method) : UNKNOWN;
}
private static @NotNull Mutability calcMutability(List<PsiExpression> expressions) {
expressions = StreamEx.of(expressions).flatMap(ExpressionUtils::nonStructuralChildren).toList();
if (expressions.isEmpty()) return UNKNOWN;
Mutability mutability = UNMODIFIABLE;
for (PsiExpression initializer : expressions) {
Mutability newMutability = UNKNOWN;
if (ClassUtils.isImmutable(initializer.getType())) {
newMutability = UNMODIFIABLE;
} else if (initializer instanceof PsiMethodCallExpression call) {
if (STREAM_COLLECT.test(call)) {
PsiExpression collector = call.getArgumentList().getExpressions()[0];
newMutability = UNMODIFIABLE_COLLECTORS.matches(collector) ? UNMODIFIABLE : UNKNOWN;
} else if (STREAM_TO_LIST.test(call)) {
newMutability = UNMODIFIABLE;
} else {
PsiMethod method = call.resolveMethod();
newMutability = method == null ? UNKNOWN : getMutability(method);
}
}
mutability = mutability.join(newMutability);
if (!mutability.isUnmodifiable()) break;
}
return mutability;
}
public static Mutability fromDfType(DfType dfType) {
return dfType instanceof DfReferenceType ? ((DfReferenceType)dfType).getMutability() : UNKNOWN;
return dfType instanceof DfReferenceType referenceType ? referenceType.getMutability() : UNKNOWN;
}
}
@@ -147,10 +147,19 @@ public final class AssignmentOrReturnOfFieldWithMutableTypeInspection extends Ba
final String type = TypeUtils.expressionHasTypeOrSubtype(returnValue, MUTABLE_TYPES);
if (type == null && !(returnValue.getType() instanceof PsiArrayType)) return;
if (CollectionUtils.isConstantEmptyArray(field) ||
field.hasModifierProperty(PsiModifier.FINAL) && field.hasModifierProperty(PsiModifier.STATIC) && field.getType() instanceof PsiArrayType && field.getInitializer() == null ||
ClassUtils.isImmutable(field.getType()) ||
Mutability.getMutability(field).isUnmodifiable()) return;
registerError(returnValue, field, returnValue, type, Boolean.FALSE);
Mutability.getMutability(field).isUnmodifiable()) {
return;
}
if (field.hasModifierProperty(PsiModifier.FINAL)
&& field.hasModifierProperty(PsiModifier.STATIC)
&& field.getType() instanceof PsiArrayType
&& field.getInitializer() == null) {
return;
}
PsiElement nameElement = ref.getReferenceNameElement();
if (nameElement == null) return;
registerError(nameElement, field, returnValue, type, Boolean.FALSE);
}
@Override
@@ -161,7 +170,7 @@ public final class AssignmentOrReturnOfFieldWithMutableTypeInspection extends Ba
boolean reportAssignment = !ContainerUtil.or(recordClass.getConstructors(), c -> JavaPsiRecordUtil.isExplicitCanonicalConstructor(c));
for (PsiRecordComponent component : recordHeader.getRecordComponents()) {
final PsiType type = component.getType();
if (ClassUtils.isImmutable(type)) continue;
if (ClassUtils.isImmutable(type) || Mutability.getMutability(component).isUnmodifiable()) continue;
final boolean mutable = type instanceof PsiArrayType ||
ContainerUtil.exists(MUTABLE_TYPES, typeName -> InheritanceUtil.isInheritor(type, typeName));
if (!mutable) continue;
@@ -68,7 +68,7 @@ class ReturnOfCollectionFieldInspection
public Set foo()
{
return <warning descr="Return of Set field 'm_foo'">m_foo</warning>;
return this.<warning descr="Return of Set field 'm_foo'">m_foo</warning>;
}
public List<String> fooBar()
@@ -228,4 +228,14 @@ record Data4(Collection<String> strings) {
}
}
record Fooz(ImmutableList<Integer> ints) {
}
record Safe1(List<Integer> values) {
Safe1 {
values = List.copyOf(values);
}
}
record Safe2(List<Integer> values) {
Safe2(List<Integer> values) {
this.values = List.copyOf(values);
}
}