From db33a7203a77044f295c561d56ead58b959c28f6 Mon Sep 17 00:00:00 2001 From: Dmitry Batkovich Date: Mon, 5 Feb 2018 17:45:32 +0300 Subject: [PATCH] actual method parameter is the same const inspection should work for negative numbers (IDEA-185830) --- .../reference/RefParameter.java | 7 ++- .../reference/RefParameterImpl.java | 56 +++++++++++-------- .../SameParameterValueInspectionBase.java | 41 +++++++------- .../SameParameterValueInspection.java | 15 ++--- .../negativeDouble/expected.xml | 10 ++++ .../negativeDouble/src/Test.java | 10 ++++ .../SameParameterValueLocalTest.java | 4 ++ .../SameParameterValueTest.java | 4 ++ 8 files changed, 93 insertions(+), 54 deletions(-) create mode 100644 java/java-tests/testData/inspection/sameParameterValue/negativeDouble/expected.xml create mode 100644 java/java-tests/testData/inspection/sameParameterValue/negativeDouble/src/Test.java diff --git a/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefParameter.java b/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefParameter.java index 21c62f2f20bc..c85340c29fd1 100644 --- a/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefParameter.java +++ b/java/java-analysis-api/src/com/intellij/codeInspection/reference/RefParameter.java @@ -25,6 +25,9 @@ import org.jetbrains.annotations.Nullable; * @since 6.0 */ public interface RefParameter extends RefJavaElement { + Object VALUE_IS_NOT_CONST = new Object(); + Object VALUE_UNDEFINED = new Object(); + /** * Checks if the parameter is used for reading. * @@ -49,11 +52,11 @@ public interface RefParameter extends RefJavaElement { /** * If all invocations of the method pass the same value to the parameter, returns * that value (the name of a static final field or the text of a literal expression). - * Otherwise, returns null. + * Otherwise, returns {@link RefParameter#VALUE_IS_NOT_CONST}. * * @return the parameter value or null if it's different or impossible to determine. */ - @Nullable String getActualValueIfSame(); + @Nullable Object getActualValueIfSame(); /** * Marks the parameter as referenced for reading or writing. diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefParameterImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefParameterImpl.java index 6fb756ef524a..f2ee9a0450d6 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefParameterImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/reference/RefParameterImpl.java @@ -19,6 +19,7 @@ package com.intellij.codeInspection.reference; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.util.Comparing; import com.intellij.psi.*; +import com.intellij.psi.impl.JavaConstantExpressionEvaluator; import com.intellij.psi.util.PsiFormatUtil; import com.intellij.psi.util.PsiFormatUtilBase; import com.intellij.psi.util.PsiTreeUtil; @@ -28,10 +29,10 @@ import org.jetbrains.annotations.Nullable; public class RefParameterImpl extends RefJavaElementImpl implements RefParameter { private static final int USED_FOR_READING_MASK = 0x10000; private static final int USED_FOR_WRITING_MASK = 0x20000; - private static final String VALUE_UNDEFINED = "#"; + private final short myIndex; - private String myActualValueTemplate; + private Object myActualValueTemplate; RefParameterImpl(PsiParameter parameter, int index, RefManager manager) { super(parameter, manager); @@ -100,37 +101,20 @@ public class RefParameterImpl extends RefJavaElementImpl implements RefParameter } void updateTemplateValue(PsiExpression expression) { - if (myActualValueTemplate == null) return; - - String newTemplate = null; - if (expression instanceof PsiLiteralExpression) { - PsiLiteralExpression psiLiteralExpression = (PsiLiteralExpression) expression; - newTemplate = psiLiteralExpression.getText(); - } else if (expression instanceof PsiReferenceExpression) { - PsiReferenceExpression referenceExpression = (PsiReferenceExpression) expression; - PsiElement resolved = referenceExpression.resolve(); - if (resolved instanceof PsiField) { - PsiField psiField = (PsiField) resolved; - if (psiField.hasModifierProperty(PsiModifier.STATIC) && - psiField.hasModifierProperty(PsiModifier.FINAL) && - psiField.getContainingClass().getQualifiedName() != null) { - newTemplate = PsiFormatUtil.formatVariable(psiField, PsiFormatUtilBase.SHOW_NAME | - PsiFormatUtilBase.SHOW_CONTAINING_CLASS | PsiFormatUtilBase.SHOW_FQ_NAME, PsiSubstitutor.EMPTY); - } - } - } + if (myActualValueTemplate == VALUE_IS_NOT_CONST) return; + Object newTemplate = getExpressionValue(expression); if (myActualValueTemplate == VALUE_UNDEFINED) { myActualValueTemplate = newTemplate; } else if (!Comparing.equal(myActualValueTemplate, newTemplate)) { - myActualValueTemplate = null; + myActualValueTemplate = VALUE_IS_NOT_CONST; } } + @Nullable @Override - public String getActualValueIfSame() { - if (myActualValueTemplate == VALUE_UNDEFINED) return null; + public Object getActualValueIfSame() { return myActualValueTemplate; } @@ -152,6 +136,30 @@ public class RefParameterImpl extends RefJavaElementImpl implements RefParameter return result[0]; } + @Nullable + public static Object getExpressionValue(PsiExpression expression) { + if (expression instanceof PsiReferenceExpression) { + PsiReferenceExpression referenceExpression = (PsiReferenceExpression) expression; + PsiElement resolved = referenceExpression.resolve(); + if (resolved instanceof PsiField) { + PsiField psiField = (PsiField) resolved; + if (psiField.hasModifierProperty(PsiModifier.STATIC) && + psiField.hasModifierProperty(PsiModifier.FINAL) && + psiField.getContainingClass().getQualifiedName() != null) { + return PsiFormatUtil.formatVariable(psiField, PsiFormatUtilBase.SHOW_NAME | + PsiFormatUtilBase.SHOW_CONTAINING_CLASS | + PsiFormatUtilBase.SHOW_FQ_NAME, + PsiSubstitutor.EMPTY); + } + } + } + if (expression instanceof PsiLiteralExpression && ((PsiLiteralExpression)expression).getValue() == null) { + return null; + } + Object constValue = JavaConstantExpressionEvaluator.computeConstantExpression(expression, false); + return constValue == null ? VALUE_IS_NOT_CONST : constValue; + } + @Nullable static RefElement parameterFromExternalName(final RefManager manager, final String fqName) { final int idx = fqName.lastIndexOf(' '); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspectionBase.java index 2b8e54c96360..73aa0101536e 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspectionBase.java @@ -16,8 +16,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import static com.intellij.codeInspection.reference.RefParameter.VALUE_IS_NOT_CONST; +import static com.intellij.codeInspection.reference.RefParameter.VALUE_UNDEFINED; + /** * @author max */ @@ -44,8 +48,8 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT RefParameter[] parameters = refMethod.getParameters(); for (RefParameter refParameter : parameters) { - String value = refParameter.getActualValueIfSame(); - if (value != null) { + Object value = refParameter.getActualValueIfSame(); + if (value != VALUE_IS_NOT_CONST && value != RefParameter.VALUE_UNDEFINED) { if (!globalContext.shouldCheck(refParameter, this)) continue; if (problems == null) problems = new ArrayList<>(1); problems.add(registerProblem(manager, refParameter.getElement(), value, refParameter.isUsedForWriting())); @@ -109,7 +113,7 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT return createFix(paramName, value); } - protected LocalQuickFix createFix(String paramName, String value) { + protected LocalQuickFix createFix(String paramName, Object value) { return null; } @@ -126,8 +130,6 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT } private class LocalSameParameterValueInspection extends AbstractBaseJavaLocalInspectionTool { - private static final String NOT_CONST = "_NOT_CONST"; - private final SameParameterValueInspectionBase myGlobal; private LocalSameParameterValueInspection(SameParameterValueInspectionBase global) { @@ -178,14 +180,15 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT if (!method.getHierarchicalMethodSignature().getSuperSignatures().isEmpty()) return; PsiParameter lastParameter = parameters[parameters.length - 1]; - final String[] paramValues; + final Object[] paramValues; final boolean hasVarArg = lastParameter.getType() instanceof PsiEllipsisType; if (hasVarArg) { if (parameters.length == 1) return; - paramValues = new String[parameters.length - 1]; + paramValues = new Object[parameters.length - 1]; } else { - paramValues = new String[parameters.length]; + paramValues = new Object[parameters.length]; } + Arrays.fill(paramValues, VALUE_UNDEFINED); if (UnusedSymbolUtil.processUsages(holder.getProject(), method.getContainingFile(), method, new EmptyProgressIndicator(), null, info -> { PsiElement element = info.getElement(); @@ -204,15 +207,15 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT boolean needFurtherProcess = false; for (int i = 0; i < paramValues.length; i++) { Object value = paramValues[i]; - final String currentArg = getArgValue(arguments[i]); - if (value == null) { + final Object currentArg = getArgValue(arguments[i]); + if (value == VALUE_UNDEFINED) { paramValues[i] = currentArg; - if (currentArg != NOT_CONST) { + if (currentArg != VALUE_IS_NOT_CONST) { needFurtherProcess = true; } - } else if (value != NOT_CONST) { + } else if (value != VALUE_IS_NOT_CONST) { if (!paramValues[i].equals(currentArg)) { - paramValues[i] = NOT_CONST; + paramValues[i] = VALUE_IS_NOT_CONST; } else { needFurtherProcess = true; } @@ -222,8 +225,8 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT return needFurtherProcess; })) { for (int i = 0, length = paramValues.length; i < length; i++) { - String value = paramValues[i]; - if (value != null && value != NOT_CONST) { + Object value = paramValues[i]; + if (value != VALUE_UNDEFINED && value != VALUE_IS_NOT_CONST) { holder.registerProblem(registerProblem(holder.getManager(), parameters[i], value, false)); } } @@ -232,20 +235,20 @@ public class SameParameterValueInspectionBase extends GlobalJavaBatchInspectionT }; } - private String getArgValue(PsiExpression arg) { - return arg instanceof PsiLiteralExpression ? arg.getText() : NOT_CONST; + private Object getArgValue(PsiExpression arg) { + return RefParameterImpl.getExpressionValue(arg); } } private ProblemDescriptor registerProblem(@NotNull InspectionManager manager, PsiParameter parameter, - String value, + Object value, boolean usedForWriting) { final String name = parameter.getName(); return manager.createProblemDescriptor(ObjectUtils.notNull(parameter.getNameIdentifier(), parameter), InspectionsBundle.message("inspection.same.parameter.problem.descriptor", name, - StringUtil.unquoteString(value)), + StringUtil.unquoteString(String.valueOf(value))), usedForWriting ? null : createFix(name, value), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, false); } diff --git a/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java b/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java index 6f44e2c1ae97..a6953f00e9f2 100644 --- a/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java @@ -75,23 +75,24 @@ public class SameParameterValueInspection extends SameParameterValueInspectionBa } public static class InlineParameterValueFix implements LocalQuickFix { - private final String myValue; + private final Object myValue; private final String myParameterName; - private InlineParameterValueFix(final String parameterName, final String value) { + private InlineParameterValueFix(String parameterName, Object value) { myValue = value; myParameterName = parameterName; } @Override public String toString() { - return getParamName() + " " + getValue(); + return getParamName() + " " + myValue; } @Override @NotNull public String getName() { - return InspectionsBundle.message("inspection.same.parameter.fix.name", myParameterName, StringUtil.unquoteString(myValue)); + return InspectionsBundle + .message("inspection.same.parameter.fix.name", myParameterName, StringUtil.unquoteString(String.valueOf(myValue))); } @Override @@ -120,7 +121,7 @@ public class SameParameterValueInspection extends SameParameterValueInspectionBa final PsiExpression defToInline; try { - defToInline = JavaPsiFacade.getInstance(project).getElementFactory().createExpressionFromText(myValue, parameter); + defToInline = JavaPsiFacade.getInstance(project).getElementFactory().createExpressionFromText(String.valueOf(myValue), parameter); } catch (IncorrectOperationException e) { return; @@ -213,10 +214,6 @@ public class SameParameterValueInspection extends SameParameterValueInspectionBa psiParameters.toArray(new ParameterInfoImpl[0])).run(); } - public String getValue() { - return myValue; - } - public String getParamName() { return myParameterName; } diff --git a/java/java-tests/testData/inspection/sameParameterValue/negativeDouble/expected.xml b/java/java-tests/testData/inspection/sameParameterValue/negativeDouble/expected.xml new file mode 100644 index 000000000000..4ef35873e2ba --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/negativeDouble/expected.xml @@ -0,0 +1,10 @@ + + + + Test.java + 3 + Actual method parameter is the same constant + Actual value of parameter 'val' is always '-1.111' + + + diff --git a/java/java-tests/testData/inspection/sameParameterValue/negativeDouble/src/Test.java b/java/java-tests/testData/inspection/sameParameterValue/negativeDouble/src/Test.java new file mode 100644 index 000000000000..5f178ba98264 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/negativeDouble/src/Test.java @@ -0,0 +1,10 @@ +public class Test { + + static void someMethod(double val) { + + } + + public static void main(String[] args) { + someMethod(-1.111); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueLocalTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueLocalTest.java index a0e4afcee489..7a80754a075e 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueLocalTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueLocalTest.java @@ -71,4 +71,8 @@ public class SameParameterValueLocalTest extends InspectionTestCase { public void testNativeMethod() { doTest(getGlobalTestDir(), myTool); } + + public void testNegativeDouble() { + doTest(getGlobalTestDir(), myTool); + } } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueTest.java index 8f160396db0d..8795d0a1fba6 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueTest.java @@ -78,4 +78,8 @@ public class SameParameterValueTest extends InspectionTestCase { public void testNotReportedDueToHighVisibility() { doTest(getTestDir(), myTool, false, false); } + + public void testNegativeDouble() { + doTest(getTestDir(), myTool, false, true); + } }