MismatchedCollectionQueryUpdate: support constructors with initial content (IDEA-175455); disable for public fields of private classes (follow-up for IDEA-161979)

2026-09-27 10:03:11 +07:00 · 2018-02-01 12:28:58 +07:00
parent 0621ee18d3
commit f98cbb8d6b
3 changed files with 88 additions and 39 deletions
@@ -1,18 +1,4 @@
-/*
- * 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.
- */
+// Copyright 2000-2018 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.bugs;

 import com.intellij.codeInsight.daemon.impl.UnusedSymbolUtil;
@@ -107,6 +93,10 @@ public class MismatchedCollectionQueryUpdateInspectionBase extends BaseInspectio
    return true;
  }

+  static boolean isCollectionInitializer(PsiExpression initializer) {
+    return isEmptyCollectionInitializer(initializer) || ConstructionUtils.isPrepopulatedCollectionInitializer(initializer);
+  }
+
  @Pattern(VALID_ID_PATTERN)
  @Override
  @NotNull
@@ -153,8 +143,8 @@ public class MismatchedCollectionQueryUpdateInspectionBase extends BaseInspectio
        PsiExpression initializer = variable.getInitializer();
        if (initializer != null) {
          List<PsiExpression> expressions = ExpressionUtils.nonStructuralChildren(initializer).collect(Collectors.toList());
-          if (!expressions.stream().allMatch(MismatchedCollectionQueryUpdateInspectionBase::isEmptyCollectionInitializer)) {
-            expressions.stream().filter(MismatchedCollectionQueryUpdateInspectionBase::isEmptyCollectionInitializer)
+          if (!expressions.stream().allMatch(MismatchedCollectionQueryUpdateInspectionBase::isCollectionInitializer)) {
+            expressions.stream().filter(MismatchedCollectionQueryUpdateInspectionBase::isCollectionInitializer)
                       .forEach(emptyCollection -> registerError(emptyCollection, Boolean.TRUE));
            return;
          }
@@ -168,7 +158,9 @@ public class MismatchedCollectionQueryUpdateInspectionBase extends BaseInspectio
      super.visitField(field);
      if (!field.hasModifierProperty(PsiModifier.PRIVATE)) {
        PsiClass aClass = field.getContainingClass();
-        if (aClass == null || !aClass.hasModifierProperty(PsiModifier.PRIVATE)) {
+        if (aClass == null || !aClass.hasModifierProperty(PsiModifier.PRIVATE) || field.hasModifierProperty(PsiModifier.PUBLIC)) {
+          // Public field within private class can be written/read via reflection even without setAccessible hacks
+          // so we don't analyze such fields to reduce false-positives
          return;
        }
      }
@@ -240,7 +232,7 @@ public class MismatchedCollectionQueryUpdateInspectionBase extends BaseInspectio
      final PsiExpression initializer = variable.getInitializer();
      return initializer != null &&
             ExpressionUtils.nonStructuralChildren(initializer)
-                            .noneMatch(MismatchedCollectionQueryUpdateInspectionBase::isEmptyCollectionInitializer);
+                            .noneMatch(MismatchedCollectionQueryUpdateInspectionBase::isCollectionInitializer);
    }
  }

@@ -314,8 +306,14 @@ public class MismatchedCollectionQueryUpdateInspectionBase extends BaseInspectio
        }
        if (parent instanceof PsiAssignmentExpression && ((PsiAssignmentExpression)parent).getLExpression() == reference) {
          PsiExpression rValue = ((PsiAssignmentExpression)parent).getRExpression();
-          if (rValue == null || ExpressionUtils.nonStructuralChildren(rValue)
-                                               .allMatch(MismatchedCollectionQueryUpdateInspectionBase::isEmptyCollectionInitializer)) {
+          if (rValue == null) return;
+          if (ExpressionUtils.nonStructuralChildren(rValue)
+                             .allMatch(MismatchedCollectionQueryUpdateInspectionBase::isEmptyCollectionInitializer)) {
+            return;
+          }
+          if (ExpressionUtils.nonStructuralChildren(rValue)
+                             .allMatch(MismatchedCollectionQueryUpdateInspectionBase::isCollectionInitializer)) {
+            makeUpdated();
            return;
          }
        }
@@ -1,18 +1,4 @@
-/*
- * 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.
- */
+// Copyright 2000-2018 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.psiutils;

 import com.intellij.openapi.util.text.StringUtil;
@@ -111,6 +97,57 @@ public class ConstructionUtils {
    return isCustomizedEmptyCollectionInitializer(expression);
  }

+  public static boolean isPrepopulatedCollectionInitializer(PsiExpression expression) {
+    expression = PsiUtil.skipParenthesizedExprDown(expression);
+    if (expression instanceof PsiNewExpression) {
+      PsiExpressionList args = ((PsiNewExpression)expression).getArgumentList();
+      if (args == null || args.isEmpty()) return false;
+      PsiMethod ctor = ((PsiNewExpression)expression).resolveMethod();
+      if (ctor == null) return false;
+      PsiClass aClass = ctor.getContainingClass();
+      if (aClass == null) return false;
+      String name = aClass.getQualifiedName();
+      if (name == null || !name.startsWith("java.util.")) return false;
+      for (PsiParameter parameter : ctor.getParameterList().getParameters()) {
+        PsiType type = parameter.getType();
+        if (type instanceof PsiClassType) {
+          PsiClassType rawType = ((PsiClassType)type).rawType();
+          if(rawType.equalsToText(CommonClassNames.JAVA_UTIL_COLLECTION) ||
+             rawType.equalsToText(CommonClassNames.JAVA_UTIL_MAP)) {
+            return true;
+          }
+        }
+      }
+    }
+    if (expression instanceof PsiMethodCallExpression) {
+      PsiMethodCallExpression call = (PsiMethodCallExpression)expression;
+      String name = call.getMethodExpression().getReferenceName();
+      PsiExpressionList argumentList = call.getArgumentList();
+      if(name != null && name.startsWith("new") && !argumentList.isEmpty()) {
+        PsiMethod method = call.resolveMethod();
+        if (method == null) return false;
+        PsiClass aClass = method.getContainingClass();
+        if (aClass == null) return false;
+        String qualifiedName = aClass.getQualifiedName();
+        if (!GUAVA_UTILITY_CLASSES.contains(qualifiedName)) return false;
+        for (PsiParameter parameter : method.getParameterList().getParameters()) {
+          PsiType type = parameter.getType();
+          if (type instanceof PsiEllipsisType) {
+            return true;
+          }
+          if (type instanceof PsiClassType) {
+            PsiClassType rawType = ((PsiClassType)type).rawType();
+            if(rawType.equalsToText(CommonClassNames.JAVA_LANG_ITERABLE) ||
+               rawType.equalsToText(CommonClassNames.JAVA_UTIL_ITERATOR)) {
+              return true;
+            }
+          }
+        }
+      }
+    }
+    return false;
+  }
+
  /**
   * Checks that given expression initializes empty Collection or Map with custom initial capacity or load factor
   *
@@ -15,7 +15,6 @@
 */
 package com.siyeh.igtest.bugs.mismatched_collection_query_update;

-import java.util.*;
 import java.io.FileInputStream;
 import java.util.concurrent.BlockingQueue;

@@ -182,12 +181,27 @@ public class MismatchedCollectionQueryUpdate {
      tpc.field.add("foo");
    }

+    void copyConstructors() {
+      // IDEA-175455
+      Map<String, String> sourceMap = new HashMap<>();
+      sourceMap.put("foo", "bar");
+
+      Map<String, String> <warning descr="Contents of collection 'destMap' are updated, but never queried">destMap</warning> = new HashMap<>(sourceMap);
+      destMap.put("hello", "world");
+
+      Collection<String> sourceList = new ArrayList<>();
+      sourceList.add("hello");
+
+      Collection<String> <warning descr="Contents of collection 'destList' are updated, but never queried">destList</warning> = new ArrayList<>(sourceList);
+      destList.add("world");
+    }
+
    private class TestPrivateClass {
-      public List<String> <warning descr="Contents of collection 'field' are updated, but never queried">field</warning> = new ArrayList<>();
+      List<String> <warning descr="Contents of collection 'field' are updated, but never queried">field</warning> = new ArrayList<>();
    }

    class TestPackageClass {
-      public List<String> field = new ArrayList<>();
+      List<String> field = new ArrayList<>();
    }

    class Node{