From 3944be5659c6ce03623f5cd1bd64fc95118837aa Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Wed, 10 May 2023 15:53:33 +0200 Subject: [PATCH] Java: quick-fix should not make code uncompilable (IDEA-290348) for "Method parameter always has the same value" inspection GitOrigin-RevId: 114ab7d07d50298101555f3203b5161deea17816 --- .../reference/RefParameterImpl.java | 32 ++++++++++++++++--- .../SameParameterValueInspection.java | 32 ++++++++----------- .../CastedValue.after.java | 12 +++++++ .../CastedValue.java | 12 +++++++ .../LongValue.after.java | 10 ++++++ .../sameParameterValueQuickFix/LongValue.java | 10 ++++++ .../StringThatNeedsEscaping.after.java | 10 ++++++ .../StringThatNeedsEscaping.java | 10 ++++++ .../SameParameterValueQuickFixTest.java | 14 ++++---- 9 files changed, 112 insertions(+), 30 deletions(-) create mode 100644 java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.after.java create mode 100644 java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.java create mode 100644 java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.after.java create mode 100644 java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.java create mode 100644 java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.after.java create mode 100644 java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.java 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 633609dea791..c4d3ff6d29db 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 @@ -20,8 +20,8 @@ import java.util.Objects; import java.util.function.Supplier; public class RefParameterImpl extends RefJavaElementImpl implements RefParameter { - private static final int USED_FOR_READING_MASK = 0b1_00000000_00000000; - private static final int USED_FOR_WRITING_MASK = 0b10_00000000_00000000; + private static final int USED_FOR_READING_MASK = 0b01_00000000_00000000; // 17th bit + private static final int USED_FOR_WRITING_MASK = 0b10_00000000_00000000; // 18th bit private final short myIndex; private Object myActualValueTemplate; // guarded by this @@ -185,10 +185,32 @@ public class RefParameterImpl extends RefJavaElementImpl implements RefParameter } //don't unescape/escape to insert into the source file PsiElement sourcePsi = Objects.requireNonNull(expression.getSourcePsi()); - return value instanceof String ? ("\"" + StringUtil.unquoteString(sourcePsi.getText()) + "\"") : value; + return value instanceof String + ? ("\"" + StringUtil.unquoteString(sourcePsi.getText()) + "\"") + : convertToStringRepresentation(value); } - Object constValue = expression.evaluate(); //JavaConstantExpressionEvaluator.computeConstantExpression(expression, false); - return constValue == null ? VALUE_IS_NOT_CONST : constValue instanceof String ? "\"" + constValue + "\"" : constValue; + Object value = expression.evaluate(); + return value == null ? VALUE_IS_NOT_CONST : convertToStringRepresentation(value); + } + + @Nullable + private static Object convertToStringRepresentation(Object value) { + if (value instanceof Long) { + return value + "L"; + } + else if (value instanceof Short) { + return "(short)" + value; + } + else if (value instanceof Byte) { + return "(byte)" + value; + } + else if (value instanceof String string) { + return "\"" + StringUtil.escapeStringCharacters(string) + "\""; + } + else if (value instanceof Character character) { + return "'" + StringUtil.escapeCharCharacters(String.valueOf(character)) + "'"; + } + return value; } private static boolean isAccessible(@NotNull UField field, @NotNull PsiElement place) { 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 81954cca6222..ceb4bf5aca99 100644 --- a/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java @@ -175,37 +175,31 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool boolean suggestFix) { final String name = parameter.getName(); if (name == null || name.isEmpty()) return null; - String shortName; - String stringPresentation; + String presentableText; + String canonicalText; if (value instanceof PsiType) { - stringPresentation = ((PsiType)value).getCanonicalText() + ".class"; - shortName = ((PsiType)value).getPresentableText() + ".class"; + canonicalText = ((PsiType)value).getCanonicalText() + ".class"; + presentableText = ((PsiType)value).getPresentableText() + ".class"; } else { if (value instanceof PsiField) { - stringPresentation = PsiFormatUtil.formatVariable((PsiVariable)value, - PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_CONTAINING_CLASS | PsiFormatUtilBase.SHOW_FQ_NAME, - PsiSubstitutor.EMPTY); - shortName = PsiFormatUtil.formatVariable((PsiVariable)value, - PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_CONTAINING_CLASS, - PsiSubstitutor.EMPTY); - } - else if (value instanceof Character) { - stringPresentation = shortName = "'" + value + "'"; + canonicalText = PsiFormatUtil.formatVariable((PsiVariable)value, + PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_CONTAINING_CLASS | PsiFormatUtilBase.SHOW_FQ_NAME, + PsiSubstitutor.EMPTY); + presentableText = PsiFormatUtil.formatVariable((PsiVariable)value, + PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_CONTAINING_CLASS, + PsiSubstitutor.EMPTY); } else { - stringPresentation = shortName = String.valueOf(value); + canonicalText = presentableText = String.valueOf(value); } } PsiElement anchor = ObjectUtils.notNull(UDeclarationKt.getAnchorPsi(parameter), parameter); if (!anchor.isPhysical()) return null; - String value1 = stringPresentation.startsWith("\"\"") - ? stringPresentation - : StringUtil.escapeLineBreak(stringPresentation); return manager.createProblemDescriptor(anchor, JavaBundle.message("inspection.same.parameter.problem.descriptor", - StringUtil.unquoteString(shortName)), - suggestFix ? new InlineParameterValueFix(name, value1) : null, + StringUtil.unquoteString(presentableText)), + suggestFix ? new InlineParameterValueFix(name, canonicalText) : null, ProblemHighlightType.GENERIC_ERROR_OR_WARNING, false); } diff --git a/java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.after.java b/java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.after.java new file mode 100644 index 000000000000..4164a362865f --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.after.java @@ -0,0 +1,12 @@ +class SampleClazz { + private void handleTree() { + foo((short) 0); + } + + void foo(short sss){} + + + public static void main(String[] args) { + new SampleClazz().handleTree(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.java b/java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.java new file mode 100644 index 000000000000..15fcfeeea0ff --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValueQuickFix/CastedValue.java @@ -0,0 +1,12 @@ +class SampleClazz { + private void handleTree(Short simflag) { + foo(simflag); + } + + void foo(short sss){} + + + public static void main(String[] args) { + new SampleClazz().handleTree((short) 0); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.after.java b/java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.after.java new file mode 100644 index 000000000000..ba207492fd04 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.after.java @@ -0,0 +1,10 @@ +class LongValue { + + void x() { + System.out.println(10000000000L); + } + + void y() { + x(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.java b/java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.java new file mode 100644 index 000000000000..eee523205d92 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValueQuickFix/LongValue.java @@ -0,0 +1,10 @@ +class LongValue { + + void x(long l) { + System.out.println(l); + } + + void y() { + x(10000000000L); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.after.java b/java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.after.java new file mode 100644 index 000000000000..9a3bb8528765 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.after.java @@ -0,0 +1,10 @@ +class StringThatNeedsEscaping { + + void x() { + System.out.println("quote\""); + } + + void y() { + x(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.java b/java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.java new file mode 100644 index 000000000000..7f718f110f4f --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValueQuickFix/StringThatNeedsEscaping.java @@ -0,0 +1,10 @@ +class StringThatNeedsEscaping { + + void x(String s) { + System.out.println(s); + } + + void y() { + x("quote" + "\""); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueQuickFixTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueQuickFixTest.java index 17e7bacf2a79..42b7a085ccb2 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueQuickFixTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueQuickFixTest.java @@ -15,19 +15,21 @@ import java.util.List; */ public final class SameParameterValueQuickFixTest extends LightJavaCodeInsightFixtureTestCase { - public void testSimple() { - doTest(); - } + public void testSimple() { doTest(); } + public void testCastedValue() { doTest(); } + public void testStringThatNeedsEscaping() { doTest(false); } + public void testLongValue() { doTest(); } private void doTest() { - doNamedTest(getTestName(false)); + doTest(true); } - private void doNamedTest(String name) { + private void doTest(boolean testHighlighting) { + String name = getTestName(false); LocalInspectionTool inspection = new SameParameterValueInspection().getSharedLocalInspectionTool(); myFixture.enableInspections(inspection); myFixture.configureByFile(name + ".java"); - myFixture.testHighlighting(true, false, false); + if (testHighlighting) myFixture.testHighlighting(true, false, false); final @NotNull List intentions = myFixture.filterAvailableIntentions("Inline value"); assertEquals("intention not found", 1, intentions.size()); myFixture.checkPreviewAndLaunchAction(intentions.get(0));