From 5f70aac2f898957758b9f51ca5929059a5d1bcb5 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Fri, 9 Jul 2021 12:52:21 +0300 Subject: [PATCH] IDEA-259356 inspection 'same parameter value': add option to ignore cases without quick-fix GitOrigin-RevId: b9f41d65fd21be0052fdae3b2b01027043778ba9 --- .../SameParameterValueInspection.java | 36 +++++++++++++------ .../SameParameterValue.html | 5 +++ .../fixAvailable/expected.xml | 10 ++++++ .../fixAvailable/src/Test.java | 13 +++++++ .../fixNotAvailable/expected.xml | 3 ++ .../fixNotAvailable/src/Test.java | 13 +++++++ .../fixNotAvailableIsShown/expected.xml | 10 ++++++ .../fixNotAvailableIsShown/src/Test.java | 13 +++++++ .../SameParameterValueLocalTest.java | 14 ++++++++ .../SameParameterValueTest.java | 8 ++++- .../resources/messages/JavaBundle.properties | 1 + 11 files changed, 115 insertions(+), 11 deletions(-) create mode 100644 java/java-tests/testData/inspection/sameParameterValue/fixAvailable/expected.xml create mode 100644 java/java-tests/testData/inspection/sameParameterValue/fixAvailable/src/Test.java create mode 100644 java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/expected.xml create mode 100644 java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/src/Test.java create mode 100644 java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/expected.xml create mode 100644 java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/src/Test.java 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 51c8c50042e7..202882b42176 100644 --- a/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/sameParameterValue/SameParameterValueInspection.java @@ -31,6 +31,7 @@ import com.intellij.refactoring.safeDelete.JavaSafeDeleteProcessor; import com.intellij.refactoring.util.CommonRefactoringUtil; import com.intellij.refactoring.util.InlineUtil; import com.intellij.uast.UastHintedVisitorAdapter; +import com.intellij.ui.components.JBCheckBox; import com.intellij.ui.components.fields.IntegerField; import com.intellij.ui.components.fields.valueEditors.ValueEditor; import com.intellij.util.IncorrectOperationException; @@ -57,11 +58,17 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool @PsiModifier.ModifierConstant public String highestModifier = DEFAULT_HIGHEST_MODIFIER; public int minimalUsageCount = 1; + public boolean ignoreWhenRefactoringIsComplicated = true; @Nullable @Override public JComponent createOptionsPanel() { JPanel panel = new InspectionOptionsPanel(); + + final JBCheckBox checkBox = new JBCheckBox(JavaBundle.message("label.ignore.complicated.fix"), ignoreWhenRefactoringIsComplicated); + checkBox.addChangeListener((e) -> ignoreWhenRefactoringIsComplicated = checkBox.isSelected()); + panel.add(checkBox); + LabeledComponent component = LabeledComponent.create(new VisibilityModifierChooser(() -> true, highestModifier, (newModifier) -> highestModifier = newModifier), @@ -111,7 +118,9 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool if (problems == null) problems = new ArrayList<>(1); UParameter parameter = refParameter.getUastElement(); if (parameter == null) continue; - problems.add(registerProblem(manager, parameter, value, refParameter.isUsedForWriting())); + Boolean isFixAvailable = isFixAvailable(parameter, value, refParameter.isUsedForWriting()); + if (Boolean.FALSE.equals(isFixAvailable) && ignoreWhenRefactoringIsComplicated) return null; + problems.add(registerProblem(manager, parameter, value, Boolean.TRUE.equals(isFixAvailable))); } } } @@ -181,20 +190,17 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool private ProblemDescriptor registerProblem(@NotNull InspectionManager manager, UParameter parameter, Object value, - boolean usedForWriting) { + boolean suggestFix) { final String name = parameter.getName(); String shortName; String stringPresentation; - boolean accessible = true; - PsiParameter javaParameter = ObjectUtils.tryCast(parameter.getSourcePsi(), PsiParameter.class); if (value instanceof PsiType) { stringPresentation = ((PsiType)value).getCanonicalText() + ".class"; shortName = ((PsiType)value).getPresentableText() + ".class"; } else { if (value instanceof PsiField) { - accessible = javaParameter != null && PsiUtil.isMemberAccessibleAt((PsiMember)value, javaParameter); stringPresentation = PsiFormatUtil.formatVariable((PsiVariable)value, PsiFormatUtilBase.SHOW_NAME | PsiFormatUtilBase.SHOW_CONTAINING_CLASS | PsiFormatUtilBase.SHOW_FQ_NAME, PsiSubstitutor.EMPTY); @@ -209,10 +215,6 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool stringPresentation = shortName = String.valueOf(value); } } - boolean suggestFix = false; - if (javaParameter != null) { - suggestFix = !javaParameter.isVarArgs() && !usedForWriting && accessible ; - } return manager.createProblemDescriptor(ObjectUtils.notNull(UDeclarationKt.getAnchorPsi(parameter), parameter), JavaBundle.message("inspection.same.parameter.problem.descriptor", name, @@ -221,6 +223,17 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool ProblemHighlightType.GENERIC_ERROR_OR_WARNING, false); } + protected static @Nullable Boolean isFixAvailable(UParameter parameter, Object value, boolean usedForWriting) { + if (usedForWriting) return false; + PsiParameter javaParameter = ObjectUtils.tryCast(parameter.getSourcePsi(), PsiParameter.class); + if (javaParameter == null) return null; + if (javaParameter.isVarArgs()) return false; + if (value instanceof PsiField && !PsiUtil.isMemberAccessibleAt((PsiMember)value, javaParameter)) { + return false; + } + return true; + } + public static final class InlineParameterValueFix implements LocalQuickFix { private final String myValue; private final String myParameterName; @@ -456,7 +469,10 @@ public class SameParameterValueInspection extends GlobalJavaBatchInspectionTool for (int i = 0, length = paramValues.length; i < length; i++) { Object value = paramValues[i]; if (value != VALUE_UNDEFINED && value != VALUE_IS_NOT_CONST) { - holder.registerProblem(registerProblem(holder.getManager(), parameters.get(i), value, false)); + final UParameter parameter = parameters.get(i); + Boolean isFixAvailable = isFixAvailable(parameter, value, false); + if (Boolean.FALSE.equals(isFixAvailable) && ignoreWhenRefactoringIsComplicated) return true; + holder.registerProblem(registerProblem(holder.getManager(), parameter, value, Boolean.TRUE.equals(isFixAvailable))); } } } diff --git a/java/java-impl/src/inspectionDescriptions/SameParameterValue.html b/java/java-impl/src/inspectionDescriptions/SameParameterValue.html index 11b4e06f4032..514ab911664b 100644 --- a/java/java-impl/src/inspectionDescriptions/SameParameterValue.html +++ b/java/java-impl/src/inspectionDescriptions/SameParameterValue.html @@ -15,5 +15,10 @@ public static void main(String[] args) {

The quick-fix inlines the constant value. This may simplify the method implementation.

+ +

+ Use the checkbox below to ignore cases when complicated refactoring may be required. + For example, when the reported parameter is modified inside the method or when the passed parameter value is a reference to inaccessible field. +

diff --git a/java/java-tests/testData/inspection/sameParameterValue/fixAvailable/expected.xml b/java/java-tests/testData/inspection/sameParameterValue/fixAvailable/expected.xml new file mode 100644 index 000000000000..fa7c0df16d34 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/fixAvailable/expected.xml @@ -0,0 +1,10 @@ + + + + Test.java + 10 + Actual value of parameter 'obj' is always 'A.OBJ' + + + + diff --git a/java/java-tests/testData/inspection/sameParameterValue/fixAvailable/src/Test.java b/java/java-tests/testData/inspection/sameParameterValue/fixAvailable/src/Test.java new file mode 100644 index 000000000000..d7c8382bb69f --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/fixAvailable/src/Test.java @@ -0,0 +1,13 @@ +class A { + public static final Object OBJ = new Object(); + + void test() { + B.use(OBJ); + } +} + +class B { + static void use(Object obj) { + System.out.println(obj); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/expected.xml b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/expected.xml new file mode 100644 index 000000000000..ec272abeaa3a --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/expected.xml @@ -0,0 +1,3 @@ + + + diff --git a/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/src/Test.java b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/src/Test.java new file mode 100644 index 000000000000..a8461292c9a0 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailable/src/Test.java @@ -0,0 +1,13 @@ +class A { + private static final Object OBJ = new Object(); + + void test() { + B.use(OBJ); + } +} + +class B { + static void use(Object obj) { + System.out.println(obj); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/expected.xml b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/expected.xml new file mode 100644 index 000000000000..fa7c0df16d34 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/expected.xml @@ -0,0 +1,10 @@ + + + + Test.java + 10 + Actual value of parameter 'obj' is always 'A.OBJ' + + + + diff --git a/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/src/Test.java b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/src/Test.java new file mode 100644 index 000000000000..a8461292c9a0 --- /dev/null +++ b/java/java-tests/testData/inspection/sameParameterValue/fixNotAvailableIsShown/src/Test.java @@ -0,0 +1,13 @@ +class A { + private static final Object OBJ = new Object(); + + void test() { + B.use(OBJ); + } +} + +class B { + static void use(Object obj) { + System.out.println(obj); + } +} \ 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 a06ad7722e68..a0515bef7203 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueLocalTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueLocalTest.java @@ -80,6 +80,20 @@ public class SameParameterValueLocalTest extends JavaInspectionTestCase { doTest(getGlobalTestDir(), myTool); } + public void testFixAvailable() { doTest(getGlobalTestDir(), myTool); } + + public void testFixNotAvailable() { doTest(getGlobalTestDir(), myTool); } + + public void testFixNotAvailableIsShown() { + boolean previous = myGlobalTool.ignoreWhenRefactoringIsComplicated; + try { + myGlobalTool.ignoreWhenRefactoringIsComplicated = false; + doTest(getGlobalTestDir(), myTool); + } finally { + myGlobalTool.ignoreWhenRefactoringIsComplicated = previous; + } + } + public void testUsageCount() { int previous = myGlobalTool.minimalUsageCount; try { 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 28e2b6cf57b7..6d412c48f059 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/SameParameterValueTest.java @@ -62,7 +62,13 @@ public class SameParameterValueTest extends JavaInspectionTestCase { } public void testSimpleVararg() { - doTest(getTestDir(), myTool, false, true); + boolean previous = myTool.ignoreWhenRefactoringIsComplicated; + try { + myTool.ignoreWhenRefactoringIsComplicated = false; + doTest(getTestDir(), myTool, false, true); + } finally { + myTool.ignoreWhenRefactoringIsComplicated = previous; + } } public void testMethodWithSuper() { diff --git a/java/openapi/resources/messages/JavaBundle.properties b/java/openapi/resources/messages/JavaBundle.properties index 42426036d817..12f293a6c68e 100644 --- a/java/openapi/resources/messages/JavaBundle.properties +++ b/java/openapi/resources/messages/JavaBundle.properties @@ -959,6 +959,7 @@ label.implements.method.of_class_or_interface.name=implements method of {0, choi label.implements.method.of_interfaces=implements methods of the following classes/interfaces: label.maximal.reported.method.visibility=Maximal reported method visibility: label.method=Method ''{0}'' +label.ignore.complicated.fix=Ignore when complicated refactoring may be required label.minimal.reported.method.usage.count=Minimal reported method usage count: label.minimal.reported.method.visibility=Minimal reported method visibility: label.mutates=Mutates: