From cfe311a41dad7ecaee89ed6a244ed775ad8e1186 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Sat, 26 May 2018 16:43:55 +0700 Subject: [PATCH] AssignmentOrReturnOfFieldWithMutableType: do not warn if field actually assigned to immutable value (IDEA-191042) --- .../codeInspection/dataFlow/Mutability.java | 7 ++-- ...eturnOfFieldWithMutableTypeInspection.java | 35 +++++++------------ ...ignmentOrReturnOfFieldWithMutableType.java | 14 +++++--- ...nOfFieldWithMutableTypeInspectionTest.java | 18 ++++++++++ 4 files changed, 41 insertions(+), 33 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Mutability.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Mutability.java index 646c506b7754..c765baa69b23 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Mutability.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Mutability.java @@ -125,13 +125,10 @@ public enum Mutability { } if (owner instanceof PsiField && owner.hasModifierProperty(PsiModifier.FINAL)) { PsiExpression initializer = PsiUtil.skipParenthesizedExprDown(((PsiField)owner).getInitializer()); + if (initializer != null && ClassUtils.isImmutable(initializer.getType())) return UNMODIFIABLE; if (initializer instanceof PsiMethodCallExpression) { PsiMethod method = ((PsiMethodCallExpression)initializer).resolveMethod(); - if (method == null) { - return UNKNOWN; - } - if (ClassUtils.isImmutable(method.getReturnType())) return UNMODIFIABLE; - return getMutability(method); + return method == null ? UNKNOWN : getMutability(method); } } return owner instanceof PsiMethodImpl ? JavaSourceInference.inferMutability((PsiMethodImpl)owner) : UNKNOWN; diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspection.java b/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspection.java index 5b7001574c25..7943192505cf 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspection.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspection.java @@ -1,15 +1,18 @@ // 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.siyeh.ig.encapsulation; +import com.intellij.codeInspection.dataFlow.Mutability; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.psi.*; import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; +import com.intellij.util.ObjectUtils; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; +import com.siyeh.ig.psiutils.ClassUtils; import com.siyeh.ig.psiutils.CollectionUtils; import com.siyeh.ig.psiutils.TypeUtils; import org.jetbrains.annotations.Nls; @@ -106,16 +109,10 @@ public class AssignmentOrReturnOfFieldWithMutableTypeInspection extends BaseInsp if (!(rhs instanceof PsiReferenceExpression)) { return; } - final PsiElement lhsReferent = ((PsiReference)lhs).resolve(); - if (!(lhsReferent instanceof PsiField)) { - return; - } - final PsiElement rhsReferent = ((PsiReference)rhs).resolve(); - if (!(rhsReferent instanceof PsiParameter)) { - return; - } - final PsiParameter parameter = (PsiParameter)rhsReferent; - if (!(parameter.getDeclarationScope() instanceof PsiMethod)) { + final PsiField field = ObjectUtils.tryCast(((PsiReference)lhs).resolve(), PsiField.class); + if (field == null) return; + final PsiParameter parameter = ObjectUtils.tryCast(((PsiReference)rhs).resolve(), PsiParameter.class); + if (parameter == null || !(parameter.getDeclarationScope() instanceof PsiMethod) || ClassUtils.isImmutable(parameter.getType())) { return; } if (ignorePrivateMethods) { @@ -130,7 +127,7 @@ public class AssignmentOrReturnOfFieldWithMutableTypeInspection extends BaseInsp } } } - registerError(rhs, lhsReferent, rhs, type, Boolean.TRUE); + registerError(rhs, field, rhs, type, Boolean.TRUE); } @Override @@ -144,19 +141,11 @@ public class AssignmentOrReturnOfFieldWithMutableTypeInspection extends BaseInsp if (ignorePrivateMethods && element instanceof PsiMethod && ((PsiMethod)element).hasModifierProperty(PsiModifier.PRIVATE)) { return; } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)returnValue; - final PsiElement referent = referenceExpression.resolve(); - if (!(referent instanceof PsiField)) { - return; - } + final PsiField field = ObjectUtils.tryCast(((PsiReferenceExpression)returnValue).resolve(), PsiField.class); + if (field == null) return; final String type = TypeUtils.expressionHasTypeOrSubtype(returnValue, MUTABLE_TYPES); - if (type == null && !(returnValue.getType() instanceof PsiArrayType)) { - return; - } - final PsiField field = (PsiField)referent; - if (CollectionUtils.isConstantEmptyArray(field)) { - return; - } + if (type == null && !(returnValue.getType() instanceof PsiArrayType)) return; + if (CollectionUtils.isConstantEmptyArray(field) || Mutability.getMutability(field).isUnmodifiable()) return; registerError(returnValue, field, returnValue, type, Boolean.FALSE); } } diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/encapsulation/assignment_or_return_of_field_with_mutable_type/AssignmentOrReturnOfFieldWithMutableType.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/encapsulation/assignment_or_return_of_field_with_mutable_type/AssignmentOrReturnOfFieldWithMutableType.java index 17db53775e96..2ca5bace4cf2 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/encapsulation/assignment_or_return_of_field_with_mutable_type/AssignmentOrReturnOfFieldWithMutableType.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/encapsulation/assignment_or_return_of_field_with_mutable_type/AssignmentOrReturnOfFieldWithMutableType.java @@ -1,10 +1,6 @@ package com.siyeh.igtest.encapsulation; -import java.util.Calendar; -import java.util.Collections; -import java.util.Date; -import java.util.Set; -import java.util.List; +import java.util.*; public class AssignmentOrReturnOfFieldWithMutableType { @@ -147,4 +143,12 @@ class Test { return field; }; } +} + +class ImmutableTest { + final List list = Collections.unmodifiableList(Arrays.asList("foo", "bar", "baz")); + + public List getList() { + return list; + } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspectionTest.java index abc99605939c..93daae40c4ce 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/encapsulation/AssignmentOrReturnOfFieldWithMutableTypeInspectionTest.java @@ -2,13 +2,31 @@ package com.siyeh.ig.encapsulation; import com.intellij.codeInspection.InspectionProfileEntry; +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 com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; /** * @author Bas Leijdekkers */ public class AssignmentOrReturnOfFieldWithMutableTypeInspectionTest extends LightInspectionTestCase { + private static final DefaultLightProjectDescriptor PROJECT_DESCRIPTOR = new DefaultLightProjectDescriptor() { + @Override + public Sdk getSdk() { + return PsiTestUtil.addJdkAnnotations(IdeaTestUtil.getMockJdk18()); + } + }; + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return PROJECT_DESCRIPTOR; + } public void testAssignmentOrReturnOfFieldWithMutableType() { doTest();