IG: less "Map/Set replaceable with EnumMap/Set" false positives (IDEA-168988, IDEA-59407, IDEA-67813)

This commit is contained in:
Bas Leijdekkers
2017-03-03 13:56:57 +01:00
parent a8a299beaa
commit 1e1c15d6de
7 changed files with 234 additions and 114 deletions
@@ -0,0 +1,104 @@
/*
* 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.
*/
package com.siyeh.ig.performance;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ExpectedTypeUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.NotNull;
import java.util.List;
/**
* @author Bas Leijdekkers
*/
abstract class CollectionReplaceableByEnumCollectionVisitor extends BaseInspectionVisitor {
@Override
public final void visitNewExpression(@NotNull PsiNewExpression expression) {
super.visitNewExpression(expression);
final PsiType type = expression.getType();
if (!(type instanceof PsiClassType)) {
return;
}
PsiClassType classType = (PsiClassType)type;
final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false);
if (!(expectedType instanceof PsiClassType)) {
return;
}
if (!classType.hasParameters()) {
classType = (PsiClassType)expectedType;
}
final PsiType[] typeArguments = classType.getParameters();
if (typeArguments.length == 0) {
return;
}
final PsiType argumentType = typeArguments[0];
if (!(argumentType instanceof PsiClassType)) {
return;
}
if (!TypeUtils.expressionHasTypeOrSubtype(expression, getBaseCollectionName())) {
return;
}
if (TypeUtils.expressionHasTypeOrSubtype(expression, getReplacementCollectionName()) ||
TypeUtils.expressionHasTypeOrSubtype(expression, getUnreplaceableCollectionNames())) {
return;
}
final PsiClassType argumentClassType = (PsiClassType)argumentType;
final PsiClass argumentClass = argumentClassType.resolve();
if (argumentClass == null || !argumentClass.isEnum()) {
return;
}
final PsiClass aClass = PsiTreeUtil.getParentOfType(expression, PsiClass.class);
if (argumentClass.equals(aClass)) {
final PsiMember member = PsiTreeUtil.getParentOfType(expression, PsiMember.class);
if (member != null && !member.hasModifierProperty(PsiModifier.STATIC)) {
return;
}
}
final PsiExpressionList argumentList = expression.getArgumentList();
if (argumentList != null) {
final PsiExpression[] arguments = argumentList.getExpressions();
if (arguments.length > 0 && TypeUtils.expressionHasTypeOrSubtype(arguments[0], "java.util.Comparator")) {
return;
}
}
if (!expectedType.isAssignableFrom(TypeUtils.getType(getReplacementCollectionName(), expression)) &&
!isReplaceableType((PsiClassType)expectedType)) {
return;
}
registerNewExpressionError(expression);
}
private boolean isReplaceableType(PsiClassType classType) {
final PsiClassType rawType = classType.rawType();
return getReplaceableCollectionNames().stream().anyMatch(s -> rawType.equalsToText(s));
}
@NotNull
protected abstract List<String> getUnreplaceableCollectionNames();
@NotNull
protected abstract List<String> getReplaceableCollectionNames();
@NotNull
protected abstract String getReplacementCollectionName();
@NotNull
protected abstract String getBaseCollectionName();
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2012 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2017 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,15 +15,16 @@
*/
package com.siyeh.ig.performance;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.CommonClassNames;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.ExpectedTypeUtils;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.NotNull;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class MapReplaceableByEnumMapInspection extends BaseInspection {
@Override
@@ -40,61 +41,35 @@ public class MapReplaceableByEnumMapInspection extends BaseInspection {
@Override
public BaseInspectionVisitor buildVisitor() {
return new SetReplaceableByEnumSetVisitor();
return new MapReplaceableByEnumMapVisitor();
}
private static class SetReplaceableByEnumSetVisitor
extends BaseInspectionVisitor {
private static class MapReplaceableByEnumMapVisitor extends CollectionReplaceableByEnumCollectionVisitor {
@Override
public void visitNewExpression(@NotNull PsiNewExpression expression) {
super.visitNewExpression(expression);
final PsiType type = expression.getType();
if (!(type instanceof PsiClassType)) {
return;
}
PsiClassType classType = (PsiClassType)type;
if (!classType.hasParameters()) {
final PsiType expectedType = ExpectedTypeUtils.findExpectedType(expression, false);
if (!(expectedType instanceof PsiClassType)) {
return;
}
classType = (PsiClassType)expectedType;
}
final PsiType[] typeArguments = classType.getParameters();
if (typeArguments.length != 2) {
return;
}
final PsiType argumentType = typeArguments[0];
if (!(argumentType instanceof PsiClassType)) {
return;
}
if (!TypeUtils.expressionHasTypeOrSubtype(expression, CommonClassNames.JAVA_UTIL_MAP)) {
return;
}
if (null != TypeUtils.expressionHasTypeOrSubtype(expression, "java.util.EnumMap", "java.util.concurrent.ConcurrentMap")) {
return;
}
final PsiClassType argumentClassType = (PsiClassType)argumentType;
final PsiClass argumentClass = argumentClassType.resolve();
if (argumentClass == null || !argumentClass.isEnum()) {
return;
}
final PsiClass aClass = PsiTreeUtil.getParentOfType(expression, PsiClass.class);
if (argumentClass.equals(aClass)) {
final PsiMember member = PsiTreeUtil.getParentOfType(expression, PsiMember.class);
if (member != null && !member.hasModifierProperty(PsiModifier.STATIC)) {
return;
}
}
final PsiExpressionList argumentList = expression.getArgumentList();
if (argumentList != null) {
final PsiExpression[] arguments = argumentList.getExpressions();
if (arguments.length > 0 && TypeUtils.expressionHasTypeOrSubtype(arguments[0], "java.util.Comparator")) {
return;
}
}
registerNewExpressionError(expression);
@NotNull
protected List<String> getUnreplaceableCollectionNames() {
return Arrays.asList(CommonClassNames.JAVA_UTIL_CONCURRENT_HASH_MAP, "java.util.concurrent.ConcurrentSkipListMap",
"java.util.LinkedHashMap");
}
@NotNull
@Override
protected List<String> getReplaceableCollectionNames() {
return Collections.singletonList(CommonClassNames.JAVA_UTIL_HASH_MAP);
}
@Override
@NotNull
protected String getReplacementCollectionName() {
return "java.util.EnumMap";
}
@Override
@NotNull
protected String getBaseCollectionName() {
return CommonClassNames.JAVA_UTIL_MAP;
}
}
}
@@ -1,5 +1,5 @@
/*
* Copyright 2003-2010 Dave Griffith, Bas Leijdekkers
* Copyright 2003-2017 Dave Griffith, Bas Leijdekkers
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -15,27 +15,28 @@
*/
package com.siyeh.ig.performance;
import com.intellij.psi.*;
import com.intellij.psi.CommonClassNames;
import com.siyeh.InspectionGadgetsBundle;
import com.siyeh.ig.BaseInspection;
import com.siyeh.ig.BaseInspectionVisitor;
import com.siyeh.ig.psiutils.TypeUtils;
import org.jetbrains.annotations.NotNull;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
public class SetReplaceableByEnumSetInspection extends BaseInspection {
@Override
@NotNull
public String getDisplayName() {
return InspectionGadgetsBundle.message(
"set.replaceable.by.enum.set.display.name");
return InspectionGadgetsBundle.message("set.replaceable.by.enum.set.display.name");
}
@Override
@NotNull
protected String buildErrorString(Object... infos) {
return InspectionGadgetsBundle.message(
"set.replaceable.by.enum.set.problem.descriptor");
return InspectionGadgetsBundle.message("set.replaceable.by.enum.set.problem.descriptor");
}
@Override
@@ -43,46 +44,31 @@ public class SetReplaceableByEnumSetInspection extends BaseInspection {
return new SetReplaceableByEnumSetVisitor();
}
private static class SetReplaceableByEnumSetVisitor
extends BaseInspectionVisitor {
private static class SetReplaceableByEnumSetVisitor extends CollectionReplaceableByEnumCollectionVisitor {
@NotNull
@Override
public void visitNewExpression(
@NotNull PsiNewExpression expression) {
super.visitNewExpression(expression);
final PsiType type = expression.getType();
if (!(type instanceof PsiClassType)) {
return;
}
final PsiClassType classType = (PsiClassType)type;
if (!classType.hasParameters()) {
return;
}
final PsiType[] typeArguments = classType.getParameters();
if (typeArguments.length != 1) {
return;
}
final PsiType argumentType = typeArguments[0];
if (!(argumentType instanceof PsiClassType)) {
return;
}
if (!TypeUtils.expressionHasTypeOrSubtype(expression,
CommonClassNames.JAVA_UTIL_SET)) {
return;
}
if (TypeUtils.expressionHasTypeOrSubtype(expression,
"java.util.EnumSet")) {
return;
}
final PsiClassType argumentClassType = (PsiClassType)argumentType;
final PsiClass argumentClass = argumentClassType.resolve();
if (argumentClass == null) {
return;
}
if (!argumentClass.isEnum()) {
return;
}
registerNewExpressionError(expression);
protected List<String> getUnreplaceableCollectionNames() {
return Arrays.asList("java.util.concurrent.CopyOnWriteArraySet", "java.util.concurrent.ConcurrentSkipListSet",
"java.util.LinkedHashSet");
}
@NotNull
@Override
protected List<String> getReplaceableCollectionNames() {
return Collections.singletonList(CommonClassNames.JAVA_UTIL_HASH_SET);
}
@NotNull
@Override
protected String getReplacementCollectionName() {
return "java.util.EnumSet";
}
@NotNull
@Override
protected String getBaseCollectionName() {
return CommonClassNames.JAVA_UTIL_SET;
}
}
}
@@ -1,11 +0,0 @@
package com.siyeh.igtest.performance;
import com.siyeh.igtest.bugs.MyEnum;
import java.util.HashSet;
public class SetReplaceableByEnumSetInspection {
public static void main(String[] args) {
final HashSet<MyEnum> myEnums = new HashSet<MyEnum>();
}
}
@@ -1,7 +1,10 @@
package com.siyeh.igtest.performance.map_replaceable_by_enum_map;
import java.util.Collections; import java.util.HashMap;
import java.util.Map; import java.util.TreeMap;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;
import java.util.SortedMap;
public class MapReplaceableByEnumMap {
@@ -17,5 +20,6 @@ public class MapReplaceableByEnumMap {
void foo() {
final Map<MyEnum, Object> map = new TreeMap(Collections.reverseOrder());
SortedMap<MyEnum, String> schmap = new TreeMap<>();
}
}
@@ -0,0 +1,16 @@
package com.siyeh.igtest.performance;
import java.util.HashSet;
import java.util.Set;
public class SetReplaceableByEnumSet {
public static void main(String[] args) {
final HashSet<MyEnum> myEnums = new <warning descr="'HashSet<MyEnum>' replaceable with 'EnumSet'">HashSet<MyEnum></warning>();
}
enum MyEnum{
FOO, BAR, BAZ;
Set<MyEnum> enums = new HashSet();
// enum set here throws exception at runtime -> don't suggest it
}
}
@@ -0,0 +1,46 @@
/*
* 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.
*/
package com.siyeh.ig.performance;
import com.intellij.codeInspection.InspectionProfileEntry;
import com.siyeh.ig.LightInspectionTestCase;
import org.jetbrains.annotations.Nullable;
/**
* @author Bas Leijdekkers
*/
public class SetReplaceableByEnumSetInspectionTest extends LightInspectionTestCase {
public void testSetReplaceableByEnumSet() {
doTest();
}
@Nullable
@Override
protected InspectionProfileEntry getInspection() {
return new SetReplaceableByEnumSetInspection();
}
@Override
protected String[] getEnvironmentClasses() {
return new String[] {
"package java.util;" +
"public class HashSet<E> extends AbstractSet<E> implements Set<E>, Cloneable, java.io.Serializable {" +
" public boolean add(E e) { return false; }" +
"}"
};
}
}