From f98cbb8d6ba33ba9cbe8ba8a8ea00bd114c48aab Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Thu, 1 Feb 2018 12:28:58 +0700 Subject: [PATCH] MismatchedCollectionQueryUpdate: support constructors with initial content (IDEA-175455); disable for public fields of private classes (follow-up for IDEA-161979) --- ...edCollectionQueryUpdateInspectionBase.java | 40 ++++++----- .../siyeh/ig/psiutils/ConstructionUtils.java | 67 ++++++++++++++----- .../MismatchedCollectionQueryUpdate.java | 20 +++++- 3 files changed, 88 insertions(+), 39 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspectionBase.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspectionBase.java index f2ff9f792ed1..23856cdb7cda 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspectionBase.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/MismatchedCollectionQueryUpdateInspectionBase.java @@ -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 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; } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ConstructionUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ConstructionUtils.java index 31a709a59707..38a14a903cbb 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ConstructionUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/ConstructionUtils.java @@ -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 * diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java index a982c16ab23e..99b2bb23cc6c 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/mismatched_collection_query_update/MismatchedCollectionQueryUpdate.java @@ -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 sourceMap = new HashMap<>(); + sourceMap.put("foo", "bar"); + + Map destMap = new HashMap<>(sourceMap); + destMap.put("hello", "world"); + + Collection sourceList = new ArrayList<>(); + sourceList.add("hello"); + + Collection destList = new ArrayList<>(sourceList); + destList.add("world"); + } + private class TestPrivateClass { - public List field = new ArrayList<>(); + List field = new ArrayList<>(); } class TestPackageClass { - public List field = new ArrayList<>(); + List field = new ArrayList<>(); } class Node{