ConstantExpressionVisitor: for class access expression return the operand type

Before for String.class the "Class<String> type was returned (instead of simply "String" type). This was usually fine, but for int.class and Integer.class the same "Class<Integer>" type was returned, despite the constants are different.
This commit is contained in:
Tagir Valeev
2017-09-28 13:15:14 +07:00
parent cac7576b37
commit 389e6a828d
5 changed files with 124 additions and 1 deletions

View File

@@ -0,0 +1,100 @@
// 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.intellij.codeInspection;
import com.intellij.codeInsight.PsiEquivalenceUtil;
import com.intellij.psi.*;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.psi.util.PsiUtil;
import com.siyeh.ig.callMatcher.CallMatcher;
import com.siyeh.ig.psiutils.ExpressionUtils;
import com.siyeh.ig.psiutils.VariableAccessUtils;
import org.jetbrains.annotations.NotNull;
import java.util.*;
import static com.intellij.util.ObjectUtils.tryCast;
public class OverwrittenKeyInspection extends BaseJavaBatchLocalInspectionTool {
private static final CallMatcher SET_ADD =
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_SET, "add").parameterCount(1);
private static final CallMatcher MAP_PUT =
CallMatcher.instanceCall(CommonClassNames.JAVA_UTIL_MAP, "put").parameterCount(2);
@NotNull
@Override
public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) {
return new JavaElementVisitor() {
Set<PsiMethodCallExpression> analyzed = new HashSet<>();
@Override
public void visitMethodCallExpression(PsiMethodCallExpression call) {
PsiExpressionStatement statement = tryCast(call.getParent(), PsiExpressionStatement.class);
if (statement == null) return;
CallMatcher myMatcher;
if (SET_ADD.test(call)) {
myMatcher = SET_ADD;
}
else if (MAP_PUT.test(call)) {
myMatcher = MAP_PUT;
}
else {
return;
}
if (!analyzed.add(call)) return;
Object key = getKey(call);
if (key == null) return;
PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(ExpressionUtils.getQualifierOrThis(call.getMethodExpression()));
if (qualifier == null) return;
PsiVariable qualifierVar =
qualifier instanceof PsiReferenceExpression ? tryCast(((PsiReferenceExpression)qualifier).resolve(), PsiVariable.class) : null;
Map<Object, List<PsiMethodCallExpression>> map = new HashMap<>();
map.computeIfAbsent(key, k -> new ArrayList<>()).add(call);
while (true) {
PsiExpressionStatement nextStatement =
tryCast(PsiTreeUtil.getNextSiblingOfType(statement, PsiStatement.class), PsiExpressionStatement.class);
if (nextStatement == null) break;
PsiMethodCallExpression nextCall = tryCast(nextStatement.getExpression(), PsiMethodCallExpression.class);
if (!myMatcher.test(nextCall)) break;
PsiExpression nextQualifier =
PsiUtil.skipParenthesizedExprDown(ExpressionUtils.getQualifierOrThis(nextCall.getMethodExpression()));
if (nextQualifier == null || !PsiEquivalenceUtil.areElementsEquivalent(qualifier, nextQualifier)) break;
analyzed.add(nextCall);
if(qualifierVar != null && VariableAccessUtils.variableIsUsed(qualifierVar, nextCall.getArgumentList())) break;
Object nextKey = getKey(nextCall);
if (nextKey != null) {
map.computeIfAbsent(nextKey, k -> new ArrayList<>()).add(nextCall);
}
statement = nextStatement;
}
for (List<PsiMethodCallExpression> calls : map.values()) {
if (calls.size() < 2) continue;
for (PsiMethodCallExpression dup : calls) {
PsiExpression arg = dup.getArgumentList().getExpressions()[0];
holder.registerProblem(arg, myMatcher == SET_ADD ? "Duplicating Set element" : "Duplicating Map key");
}
}
}
private Object getKey(PsiMethodCallExpression call) {
PsiExpression key = call.getArgumentList().getExpressions()[0];
Object constant = ExpressionUtils.computeConstantExpression(key);
if (constant != null) {
return constant;
}
if (key instanceof PsiReferenceExpression) {
PsiField field = tryCast(((PsiReferenceExpression)key).resolve(), PsiField.class);
if (field instanceof PsiEnumConstant ||
field != null && field.hasModifierProperty(PsiModifier.FINAL) &&
field.hasModifierProperty(PsiModifier.STATIC) &&
PsiUtil.skipParenthesizedExprDown(field.getInitializer()) instanceof PsiNewExpression) {
return field;
}
}
return null;
}
};
}
}

View File

@@ -508,7 +508,7 @@ class ConstantExpressionVisitor extends JavaElementVisitor implements PsiConstan
@Override
public void visitClassObjectAccessExpression(PsiClassObjectAccessExpression expression) {
myResult = expression.getType();
myResult = expression.getOperand().getType();
}
@Override public void visitReferenceExpression(PsiReferenceExpression expression) {

View File

@@ -0,0 +1,10 @@
// "Remove redundant parameter" "false"
@interface Anno {
Class foo() default void.class;
}
@Anno(foo = Vo<caret>id.class)
class Foo {
}

View File

@@ -0,0 +1,8 @@
<html>
<body>
Warns if <code>Map</code> key or <code>Set</code> element was overwritten in the sequence of add/put calls. This usually
occurs due to copy-paste error.
<!-- tooltip end -->
<p><small>New in 2017.3</small></p>
</body>
</html>

View File

@@ -883,6 +883,11 @@
groupKey="group.names.code.style.issues" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.CollectionAddAllCanBeReplacedWithConstructorInspection"
displayName="Redundant 'Collection.addAll()' call"/>
<localInspection groupPath="Java" language="JAVA" shortName="OverwrittenKey"
groupBundle="messages.InspectionsBundle"
groupKey="group.names.probable.bugs" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.OverwrittenKeyInspection"
displayName="Overwritten Map key or Set element"/>
<localInspection groupPath="Java,Java language level migration aids" language="JAVA" shortName="AnonymousHasLambdaAlternative" displayName="Anonymous type has shorter lambda alternative"
groupKey="group.names.language.level.specific.issues.and.migration.aids8" groupBundle="messages.InspectionsBundle" enabledByDefault="true" level="WARNING"
implementationClass="com.intellij.codeInspection.AnonymousHasLambdaAlternativeInspection" />