Java: report implicit assignment or return of record components with mutable type (IDEA-325828)

GitOrigin-RevId: f7b7091c291db33468bb60b22cab01dc11aca586
This commit is contained in:
Bas Leijdekkers
2024-12-29 13:41:59 +00:00
committed by intellij-monorepo-bot
parent 24557b8338
commit 2d7e9aec9c
4 changed files with 74 additions and 20 deletions
@@ -2184,7 +2184,8 @@ redundant.explicit.var.type.display.name=Local variable type can be omitted
variable.type.can.be.explicit.display.name=Variable type can be explicit
assignment.or.return.of.field.with.mutable.type.display.name=Assignment or return of field with mutable type
assignment.of.field.with.mutable.type.problem.descriptor=Assignment to {0} field ''{1}'' from parameter <code>#ref</code> #loc
return.of.field.with.mutable.type.problem.descriptor=Return of {0} field <code>{1}</code> #loc
return.of.field.with.mutable.type.problem.descriptor=Return of {0} field <code>#ref</code> #loc
assignment.and.return.of.mutable.record.component=Implicit {0, choice, 1#assignment|2#return|3#assignment and return} of {1} record component <code>#ref</code> #loc
ignore.private.methods.option=Ignore assignments in and returns from private methods
inspection.replace.on.literal.display.name=Replacement operation has no effect
@@ -6,9 +6,12 @@ import com.intellij.codeInspection.dataFlow.Mutability;
import com.intellij.codeInspection.options.OptPane;
import com.intellij.psi.*;
import com.intellij.psi.tree.IElementType;
import com.intellij.psi.util.InheritanceUtil;
import com.intellij.psi.util.JavaPsiRecordUtil;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.intellij.util.ObjectUtils;
import com.intellij.util.containers.ContainerUtil;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
@@ -40,19 +43,28 @@ public final class AssignmentOrReturnOfFieldWithMutableTypeInspection extends Ba
@Override
public @NotNull String buildErrorString(Object... infos) {
final PsiField field = (PsiField)infos[0];
final PsiExpression rhs = (PsiExpression)infos[1];
final PsiType type = field.getType();
final boolean assignment = ((Boolean)infos[3]).booleanValue();
return assignment
? InspectionGadgetsBundle.message("assignment.of.field.with.mutable.type.problem.descriptor",
type.getPresentableText(), field.getName(), rhs.getText())
: InspectionGadgetsBundle.message("return.of.field.with.mutable.type.problem.descriptor",
type.getPresentableText(), field.getName());
if (infos[0] instanceof PsiRecordComponent component) {
final int reportAssignment = (boolean)infos[1] ? 0b01 : 0;
final int reportReturn = (boolean)infos[2] ? 0b10 : 0;
final String type = component.getType().getPresentableText();
return InspectionGadgetsBundle.message("assignment.and.return.of.mutable.record.component", reportAssignment | reportReturn, type);
}
else {
final PsiField field = (PsiField)infos[0];
final PsiExpression rhs = (PsiExpression)infos[1];
final PsiType type = field.getType();
final boolean assignment = ((Boolean)infos[3]).booleanValue();
return assignment
? InspectionGadgetsBundle.message("assignment.of.field.with.mutable.type.problem.descriptor",
type.getPresentableText(), field.getName(), rhs.getText())
: InspectionGadgetsBundle.message("return.of.field.with.mutable.type.problem.descriptor",
type.getPresentableText(), field.getName());
}
}
@Override
protected @Nullable LocalQuickFix buildFix(Object... infos) {
if (infos[0] instanceof PsiRecordComponent) return null;
final PsiReferenceExpression returnValue = (PsiReferenceExpression)infos[1];
final String type = (String)infos[2];
if (CommonClassNames.JAVA_UTIL_DATE.equals(type) ||
@@ -142,5 +154,25 @@ public final class AssignmentOrReturnOfFieldWithMutableTypeInspection extends Ba
Mutability.getMutability(field).isUnmodifiable()) return;
registerError(returnValue, field, returnValue, type, Boolean.FALSE);
}
@Override
public void visitRecordHeader(@NotNull PsiRecordHeader recordHeader) {
super.visitRecordHeader(recordHeader);
final PsiClass recordClass = recordHeader.getContainingClass();
if (recordClass == null) return;
boolean reportAssignment = !ContainerUtil.or(recordClass.getConstructors(), c -> JavaPsiRecordUtil.isExplicitCanonicalConstructor(c));
for (PsiRecordComponent component : recordHeader.getRecordComponents()) {
final PsiType type = component.getType();
final boolean mutable = type instanceof PsiArrayType ||
ContainerUtil.exists(MUTABLE_TYPES, typeName -> InheritanceUtil.isInheritor(type, typeName));
if (!mutable) continue;
final PsiMethod accessor = JavaPsiRecordUtil.getAccessorForRecordComponent(component);
final boolean reportReturn = accessor == null || accessor instanceof SyntheticElement;
if (!reportAssignment && !reportReturn) continue;
final PsiIdentifier identifier = component.getNameIdentifier();
if (identifier == null) continue;
registerError(identifier, component, reportAssignment, reportReturn);
}
}
}
}
@@ -203,4 +203,25 @@ class Demo {
public static List<Foo> getListTwo() {
return LIST_TWO;
}
}
record Data1(Collection<String> <warning descr="Implicit assignment and return of Collection<String> record component 'strings'">strings</warning>) {}
record Data2(Collection<String> <warning descr="Implicit assignment of Collection<String> record component 'strings'">strings</warning>) {
@Override
public Collection<String> strings() {
return <warning descr="Return of Collection<String> field 'strings'">strings</warning>;
}
}
record Data3(Collection<String> <warning descr="Implicit return of Collection<String> record component 'strings'">strings</warning>) {
Data3(Collection<String> strings) {
this.strings = <warning descr="Assignment to Collection<String> field 'strings' from parameter 'strings'">strings</warning>;
}
}
record Data4(Collection<String> strings) {
Data4(Collection<String> strings) {
this.strings = <warning descr="Assignment to Collection<String> field 'strings' from parameter 'strings'">strings</warning>;
}
@Override
public Collection<String> strings() {
return <warning descr="Return of Collection<String> field 'strings'">strings</warning>;
}
}
@@ -1,4 +1,4 @@
// 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.
// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package com.siyeh.ig.encapsulation;
import com.intellij.codeInspection.InspectionProfileEntry;
@@ -16,21 +16,21 @@ public class AssignmentOrReturnOfFieldWithMutableTypeInspectionTest extends Ligh
protected String[] getEnvironmentClasses() {
return new String[] {
"""
package com.google.common.collect;
import java.util.List;
public class ImmutableList<E> implements List<E> {
public static ImmutableList<?> of() {return new ImmutableList<>();}
public static <T> ImmutableList<T> copyOf(List<T> list) {return new ImmutableList<>();}
}"""
package com.google.common.collect;
import java.util.List;
public class ImmutableList<E> implements List<E> {
public static ImmutableList<?> of() {return new ImmutableList<>();}
public static <T> ImmutableList<T> copyOf(List<T> list) {return new ImmutableList<>();}
}"""
};
}
@NotNull
@Override
protected LightProjectDescriptor getProjectDescriptor() {
return JAVA_11_ANNOTATED;
return JAVA_17;
}
public void testAssignmentOrReturnOfFieldWithMutableType() {