From e21d6c984e2afeade3b1c4b252f69ea1c4cafa31 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Sun, 4 Feb 2018 18:10:11 +0700 Subject: [PATCH] IDEA-126310 Unnecessary toString() call fix breaks code --- .../afterConflictingFixes.java | 10 +++ .../unnecessaryTostring/afterSimple.java | 6 ++ .../unnecessaryTostring/afterUnqualified.java | 6 ++ .../beforeConflictingFixes.java | 10 +++ .../unnecessaryTostring/beforeSimple.java | 6 ++ .../beforeUnqualified.java | 6 ++ .../UnnecessaryToStringCallInspection.java | 83 +++++++------------ ...ecessaryToStringCallInspectionFixTest.java | 21 +++++ 8 files changed, 95 insertions(+), 53 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterConflictingFixes.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterUnqualified.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeConflictingFixes.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeSimple.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeUnqualified.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/UnnecessaryToStringCallInspectionFixTest.java diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterConflictingFixes.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterConflictingFixes.java new file mode 100644 index 000000000000..72748d36c130 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterConflictingFixes.java @@ -0,0 +1,10 @@ +// "Fix all 'Unnecessary call to 'toString()'' problems in file" "true" +public class MyFile { + interface UUID {} + interface InetAddress {} + + void test(UUID uuid, InetAddress address) { + // IDEA-126310 + String nodeString = uuid.toString() + address; + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterSimple.java new file mode 100644 index 000000000000..b8c4cb1ed8ac --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterSimple.java @@ -0,0 +1,6 @@ +// "Fix all 'Unnecessary call to 'toString()'' problems in file" "true" +class X { + void test(Object x) { + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterUnqualified.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterUnqualified.java new file mode 100644 index 000000000000..60a3af988d44 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/afterUnqualified.java @@ -0,0 +1,6 @@ +// "Replace with 'this'" "true" +class X { + void test(Object x) { + System.out.println(this); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeConflictingFixes.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeConflictingFixes.java new file mode 100644 index 000000000000..71be7eeddba7 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeConflictingFixes.java @@ -0,0 +1,10 @@ +// "Fix all 'Unnecessary call to 'toString()'' problems in file" "true" +public class MyFile { + interface UUID {} + interface InetAddress {} + + void test(UUID uuid, InetAddress address) { + // IDEA-126310 + String nodeString = uuid.toString() + address.toString(); + } +} diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeSimple.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeSimple.java new file mode 100644 index 000000000000..e95d58dc95d9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeSimple.java @@ -0,0 +1,6 @@ +// "Fix all 'Unnecessary call to 'toString()'' problems in file" "true" +class X { + void test(Object x) { + System.out.println(x.toString()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeUnqualified.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeUnqualified.java new file mode 100644 index 000000000000..1b89fbe8018d --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/unnecessaryTostring/beforeUnqualified.java @@ -0,0 +1,6 @@ +// "Replace with 'this'" "true" +class X { + void test(Object x) { + System.out.println(toString()); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessaryToStringCallInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessaryToStringCallInspection.java index e8fade5068bd..15f3cc073789 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessaryToStringCallInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/style/UnnecessaryToStringCallInspection.java @@ -20,17 +20,14 @@ import com.intellij.codeInspection.ProblemDescriptor; import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.openapi.project.Project; import com.intellij.psi.*; +import com.intellij.util.ObjectUtils; import com.siyeh.InspectionGadgetsBundle; import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.InspectionGadgetsFix; -import com.siyeh.ig.PsiReplacementUtil; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.TypeUtils; -import org.jetbrains.annotations.Nls; -import org.jetbrains.annotations.NonNls; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.*; public class UnnecessaryToStringCallInspection extends BaseInspection implements CleanupLocalInspectionTool { @@ -55,14 +52,6 @@ public class UnnecessaryToStringCallInspection extends BaseInspection implements return new UnnecessaryToStringCallFix(text); } - @NonNls - static String calculateReplacementText(PsiExpression expression) { - if (expression == null) { - return "this"; - } - return expression.getText(); - } - private static class UnnecessaryToStringCallFix extends InspectionGadgetsFix { private final String replacementText; @@ -85,15 +74,12 @@ public class UnnecessaryToStringCallInspection extends BaseInspection implements @Override protected void doFix(Project project, ProblemDescriptor descriptor) { - final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)descriptor.getPsiElement().getParent().getParent(); - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (qualifier == null) { - PsiReplacementUtil.replaceExpression(methodCallExpression, "this"); - } - else { - methodCallExpression.replace(qualifier); - } + final PsiMethodCallExpression call = + ObjectUtils.tryCast(descriptor.getPsiElement().getParent().getParent(), PsiMethodCallExpression.class); + if (!isRedundantToString(call)) return; + final PsiReferenceExpression methodExpression = call.getMethodExpression(); + final PsiExpression qualifier = ExpressionUtils.getQualifierOrThis(methodExpression); + call.replace(qualifier); } } @@ -105,38 +91,29 @@ public class UnnecessaryToStringCallInspection extends BaseInspection implements private static class UnnecessaryToStringCallVisitor extends BaseInspectionVisitor { @Override - public void visitMethodCallExpression(PsiMethodCallExpression expression) { - super.visitMethodCallExpression(expression); - final PsiReferenceExpression methodExpression = expression.getMethodExpression(); - @NonNls final String referenceName = methodExpression.getReferenceName(); - if (!"toString".equals(referenceName)) { - return; - } + public void visitMethodCallExpression(PsiMethodCallExpression call) { + if (!isRedundantToString(call)) return; + final PsiReferenceExpression methodExpression = call.getMethodExpression(); PsiElement referenceNameElement = methodExpression.getReferenceNameElement(); - if (referenceNameElement == null) { - return; - } - final PsiExpressionList argumentList = expression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 0) { - return; - } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (qualifier == null) { - return; - } - if (qualifier.getType() instanceof PsiArrayType) { - // do not warn on nonsensical code - return; - } - if (qualifier instanceof PsiSuperExpression) { - return; - } - final boolean throwable = TypeUtils.expressionHasTypeOrSubtype(qualifier, "java.lang.Throwable"); - if (ExpressionUtils.isConversionToStringNecessary(expression, throwable)) { - return; - } - registerError(referenceNameElement, ProblemHighlightType.LIKE_UNUSED_SYMBOL, calculateReplacementText(qualifier)); + if (referenceNameElement == null) return; + registerError(referenceNameElement, ProblemHighlightType.LIKE_UNUSED_SYMBOL, + ExpressionUtils.getQualifierOrThis(methodExpression).getText()); } } + + @Contract("null -> false") + private static boolean isRedundantToString(PsiMethodCallExpression call) { + if (call == null) return false; + PsiReferenceExpression methodExpression = call.getMethodExpression(); + @NonNls final String referenceName = methodExpression.getReferenceName(); + if (!"toString".equals(referenceName) || !call.getArgumentList().isEmpty()) return false; + final PsiExpression qualifier = ExpressionUtils.getQualifierOrThis(methodExpression); + if (qualifier.getType() instanceof PsiArrayType) { + // do not warn on nonsensical code + return false; + } + if (qualifier instanceof PsiSuperExpression) return false; + final boolean throwable = TypeUtils.expressionHasTypeOrSubtype(qualifier, "java.lang.Throwable"); + return !ExpressionUtils.isConversionToStringNecessary(call, throwable); + } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/UnnecessaryToStringCallInspectionFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/UnnecessaryToStringCallInspectionFixTest.java new file mode 100644 index 000000000000..4a999963f55f --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/style/UnnecessaryToStringCallInspectionFixTest.java @@ -0,0 +1,21 @@ +// Copyright 2000-2018 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.siyeh.ig.style; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + +public class UnnecessaryToStringCallInspectionFixTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new UnnecessaryToStringCallInspection()}; + } + + @Override + protected String getBasePath() { + return "/codeInsight/daemonCodeAnalyzer/unnecessaryTostring"; + } + + public void test() { doAllTests(); } +}