From 4774b1e0e265950e29158c180f90e22624adc2d6 Mon Sep 17 00:00:00 2001 From: peter Date: Mon, 15 Jul 2013 16:28:54 +0200 Subject: [PATCH] suggest to replace with constant reference rather than its value --- .../dataFlow/DataFlowInspectionBase.java | 38 ++++++++++--------- .../dataFlow/DfaMemoryStateImpl.java | 2 +- .../dataFlow/StandardInstructionVisitor.java | 2 +- .../dataFlow/value/DfaConstValue.java | 37 ++++++++++-------- .../dataFlow/value/DfaValueFactory.java | 2 +- ...tConstantReferences_ReplaceWithString.java | 2 +- ...antReferences_ReplaceWithString_after.java | 2 +- .../DataFlowInspectionTest.java | 2 +- 8 files changed, 49 insertions(+), 38 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 1c7b7a9d2ef9..fc58969f5122 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -210,9 +210,10 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { } final Object value = pair.second.getValue(); - final String presentableName = value instanceof PsiNamedElement ? ((PsiNamedElement)value).getName() : String.valueOf(value); - final String exprText = getConstantValueText(value); - if (exprText == null) { + PsiVariable constant = pair.second.getConstant(); + final String presentableName = constant != null ? constant.getName() : String.valueOf(value); + final String exprText = getConstantValueText(value, constant); + if (presentableName == null || exprText == null) { continue; } @@ -231,26 +232,29 @@ public class DataFlowInspectionBase extends BaseJavaBatchLocalInspectionTool { @Override public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - PsiElement newElement = - descriptor.getPsiElement().replace(JavaPsiFacade.getElementFactory(project).createExpressionFromText(exprText, null)); - JavaCodeStyleManager.getInstance(project).shortenClassReferences(newElement); + JavaPsiFacade facade = JavaPsiFacade.getInstance(project); + PsiElement newElement = descriptor.getPsiElement().replace(facade.getElementFactory().createExpressionFromText(exprText, null)); + newElement = JavaCodeStyleManager.getInstance(project).shortenClassReferences(newElement); + if (newElement instanceof PsiJavaCodeReferenceElement) { + PsiJavaCodeReferenceElement ref = (PsiJavaCodeReferenceElement)newElement; + PsiElement target = ref.resolve(); + String shortName = ref.getReferenceName(); + if (target != null && shortName != null && ref.isQualified() && + facade.getResolveHelper().resolveReferencedVariable(shortName, newElement) == target) { + newElement.replace(facade.getElementFactory().createExpressionFromText(shortName, null)); + } + } } }); } } - private static String getConstantValueText(Object value) { - String exprText; - if (value instanceof String) { - exprText = "\"" + StringUtil.escapeStringCharacters((String)value) + "\""; - } else if (value instanceof PsiMember) { - exprText = PsiUtil.getMemberQualifiedName((PsiMember)value); - } else if (value instanceof PsiNamedElement) { - exprText = ((PsiNamedElement)value).getName(); - } else { - exprText = String.valueOf(value); + private static String getConstantValueText(Object value, @Nullable PsiVariable constant) { + if (constant != null) { + return constant instanceof PsiMember ? PsiUtil.getMemberQualifiedName((PsiMember)constant) : constant.getName(); } - return exprText; + + return value instanceof String ? "\"" + StringUtil.escapeStringCharacters((String)value) + "\"" : String.valueOf(value); } private void reportNullableArgumentsPassedToNonAnnotated(StandardDataFlowRunner runner, ProblemsHolder holder, Set reportedAnchors) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index e6ba5df7d842..73b7c8fb4bd7 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -711,7 +711,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaRight instanceof DfaConstValue) { Object constVal = ((DfaConstValue)dfaRight).getValue(); if (constVal instanceof Boolean) { - DfaConstValue negVal = myFactory.getConstFactory().createFromValue(!((Boolean)constVal).booleanValue(), PsiType.BOOLEAN); + DfaConstValue negVal = myFactory.getConstFactory().createFromValue(!((Boolean)constVal).booleanValue(), PsiType.BOOLEAN, null); if (!applyRelation(dfaLeft, negVal, !negated)) { return false; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index 793da08d135b..bd17eabcb801 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -271,7 +271,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (methodType == MethodCallInstruction.MethodType.CAST) { if (qualifierValue instanceof DfaConstValue) { - return factory.getConstFactory().createFromValue(castConstValue((DfaConstValue)qualifierValue), type); + return factory.getConstFactory().createFromValue(castConstValue((DfaConstValue)qualifierValue), type, null); } return qualifierValue; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java index 561475178372..5ed0c6b81dde 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaConstValue.java @@ -26,10 +26,10 @@ package com.intellij.codeInspection.dataFlow.value; import com.intellij.psi.*; import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.util.containers.HashMap; +import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NonNls; -import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.Map; @@ -39,14 +39,13 @@ public class DfaConstValue extends DfaValue { private final DfaConstValue dfaFalse; private final DfaConstValue dfaTrue; private final DfaValueFactory myFactory; - private final Map myValues; + private final Map myValues = ContainerUtil.newHashMap(); Factory(DfaValueFactory factory) { myFactory = factory; - myValues = new HashMap(); - dfaNull = new DfaConstValue(null, factory); - dfaFalse = new DfaConstValue(Boolean.FALSE, factory); - dfaTrue = new DfaConstValue(Boolean.TRUE, factory); + dfaNull = new DfaConstValue(null, factory, null); + dfaFalse = new DfaConstValue(Boolean.FALSE, factory, null); + dfaTrue = new DfaConstValue(Boolean.TRUE, factory, null); } @Nullable @@ -55,7 +54,7 @@ public class DfaConstValue extends DfaValue { if (PsiType.NULL.equals(type)) return dfaNull; Object value = expr.getValue(); if (value == null) return null; - return createFromValue(value, type); + return createFromValue(value, type, null); } @Nullable @@ -65,12 +64,12 @@ public class DfaConstValue extends DfaValue { if (value == null) { Boolean boo = computeJavaLangBooleanFieldReference(variable); if (boo != null) { - DfaConstValue unboxed = createFromValue(boo, PsiType.BOOLEAN); + DfaConstValue unboxed = createFromValue(boo, PsiType.BOOLEAN, variable); return myFactory.getBoxedFactory().createBoxed(unboxed); } return null; } - return createFromValue(value, type); + return createFromValue(value, type, variable); } @Nullable @@ -83,17 +82,18 @@ public class DfaConstValue extends DfaValue { } @NotNull - public DfaConstValue createFromValue(Object value, final PsiType type) { + public DfaConstValue createFromValue(Object value, final PsiType type, @Nullable PsiVariable constant) { if (value == Boolean.TRUE) return dfaTrue; if (value == Boolean.FALSE) return dfaFalse; if (TypeConversionUtil.isNumericType(type) && !TypeConversionUtil.isFloatOrDoubleType(type)) { value = TypeConversionUtil.computeCastTo(value, PsiType.LONG); } - DfaConstValue instance = myValues.get(value); + Object key = constant != null ? constant : value; + DfaConstValue instance = myValues.get(key); if (instance == null) { - instance = new DfaConstValue(value, myFactory); - myValues.put(value, instance); + instance = new DfaConstValue(value, myFactory, constant); + myValues.put(key, instance); } return instance; @@ -113,10 +113,12 @@ public class DfaConstValue extends DfaValue { } private final Object myValue; + @Nullable private final PsiVariable myConstant; - DfaConstValue(Object value, DfaValueFactory factory) { + private DfaConstValue(Object value, DfaValueFactory factory, @Nullable PsiVariable constant) { super(factory); myValue = value; + myConstant = constant; } @SuppressWarnings({"HardCodedStringLiteral"}) @@ -129,6 +131,11 @@ public class DfaConstValue extends DfaValue { return myValue; } + @Nullable + public PsiVariable getConstant() { + return myConstant; + } + @Override public DfaValue createNegated() { if (this == myFactory.getConstFactory().getTrue()) return myFactory.getConstFactory().getFalse(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java index 1ea146ec3ed6..ab05cf21dd5c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java @@ -89,7 +89,7 @@ public class DfaValueFactory { if (value instanceof String) { return getNotNullFactory().create(type); // Non-null string literal. } - return getConstFactory().createFromValue(value, type); + return getConstFactory().createFromValue(value, type, null); } return null; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString.java b/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString.java index 51b74fb5843c..fcff4846b378 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString.java @@ -2,7 +2,7 @@ class Test { public static final String CONST = "foo bar"; private void test() { String s = CONST; - System.out.println(s); + System.out.println(s); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString_after.java b/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString_after.java index b5075dd0c183..a462170b2b31 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString_after.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ReportConstantReferences_ReplaceWithString_after.java @@ -2,7 +2,7 @@ class Test { public static final String CONST = "foo bar"; private void test() { String s = CONST; - System.out.println("foo bar"); + System.out.println(CONST); } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index 17085495413b..941b9e431f65 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -156,7 +156,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { public void testReportConstantReferences_ReplaceWithString() { doTestReplaceConstantReferences(); - myFixture.launchAction(myFixture.findSingleIntention("Replace with 'foo bar'")); + myFixture.launchAction(myFixture.findSingleIntention("Replace with 'CONST'")); myFixture.checkResultByFile(getTestName(false) + "_after.java"); } public void testReportConstantReferences_ReplaceWithEnum() {