From 9dce85a4fef5e612fbe25c4f590a6d6e4535fa9a Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Fri, 29 Jun 2018 13:26:39 +0300 Subject: [PATCH] Java: Don't cast the argument to generic type because we already know that the actual type is assignable to it (IDEA-171284) --- .../refactoring/util/VariableData.java | 12 +++++----- .../extractMethod/ExtractMethodProcessor.java | 22 ++++++++++++------- .../AvoidGenericArgumentCast.java | 10 +++++++++ .../AvoidGenericArgumentCastLocalClass.java | 13 +++++++++++ ...idGenericArgumentCastLocalClass_after.java | 18 +++++++++++++++ .../AvoidGenericArgumentCast_after.java | 14 ++++++++++++ .../java/refactoring/ExtractMethodTest.java | 8 +++++++ 7 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java index 1acdac88e2b5..8b8e2b6271ea 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java @@ -15,10 +15,7 @@ */ package com.intellij.refactoring.util; -import com.intellij.psi.LambdaUtil; -import com.intellij.psi.PsiType; -import com.intellij.psi.PsiVariable; -import com.intellij.psi.SmartTypePointerManager; +import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -50,10 +47,11 @@ public class VariableData extends AbstractVariableData { if (var == null) { return this; } + PsiType type = JavaPsiFacade.getElementFactory(var.getProject()).createTypeFromText(this.type.getCanonicalText(), var); VariableData data = new VariableData(var, type); - data.name = name; - data.originalName = originalName; - data.passAsParameter = passAsParameter; + data.name = this.name; + data.originalName = this.originalName; + data.passAsParameter = this.passAsParameter; return data; } } diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java index f613e1b077a2..c33eb9d5e057 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -1339,14 +1339,11 @@ public class ExtractMethodProcessor implements MatchProvider { final List parameterValue = match.getParameterValues(data.variable); if (parameterValue != null) { for (PsiElement val : parameterValue) { - if (val instanceof PsiExpression) { - final PsiType exprType = ((PsiExpression)val).getType(); - if (exprType != null && !TypeConversionUtil.isAssignable(data.type, exprType)) { - final PsiTypeCastExpression cast = (PsiTypeCastExpression)elementFactory.createExpressionFromText("(A)a", val); - cast.getCastType().replace(elementFactory.createTypeElement(data.type)); - cast.getOperand().replace(val.copy()); - val = cast; - } + if (val instanceof PsiExpression && isCastRequired(data, (PsiExpression)val)) { + final PsiTypeCastExpression cast = (PsiTypeCastExpression)elementFactory.createExpressionFromText("(A)a", val); + cast.getCastType().replace(elementFactory.createTypeElement(data.type)); + cast.getOperand().replace(val.copy()); + val = cast; } methodCallExpression.getArgumentList().add(val); } @@ -1362,6 +1359,15 @@ public class ExtractMethodProcessor implements MatchProvider { return replacedMatch; } + private static boolean isCastRequired(@NotNull VariableData data, @NotNull PsiExpression val) { + final PsiType exprType = val.getType(); + if (exprType == null || TypeConversionUtil.isAssignable(data.type, exprType)) { + return false; + } + final PsiClass psiClass = PsiUtil.resolveClassInClassTypeOnly(data.type); + return !(psiClass instanceof PsiTypeParameter); + } + @NotNull private static List findReusedVariables(@NotNull Match match, @NotNull InputVariables inputVariables, diff --git a/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast.java b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast.java new file mode 100644 index 000000000000..8cb66b52dd68 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast.java @@ -0,0 +1,10 @@ +class C { + void f(K k) { + System.out.println(k); + } + + void g() { + Object o = ""; + System.out.println(o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass.java b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass.java new file mode 100644 index 000000000000..05c233036047 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass.java @@ -0,0 +1,13 @@ +class C { + void method() { + class Local { + void foo(K k) { + System.out.println(k); + } + void bar() { + Object o = new Object(); + System.out.println(o); + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass_after.java b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass_after.java new file mode 100644 index 000000000000..2c6718d8f6d7 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCastLocalClass_after.java @@ -0,0 +1,18 @@ +class C { + void method() { + class Local { + void foo(K k) { + newMethod(k); + } + + private void newMethod(K k) { + System.out.println(k); + } + + void bar() { + Object o = new Object(); + newMethod(o); + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast_after.java b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast_after.java new file mode 100644 index 000000000000..c98f87e1ce52 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/AvoidGenericArgumentCast_after.java @@ -0,0 +1,14 @@ +class C { + void f(K k) { + newMethod(k); + } + + private void newMethod(K k) { + System.out.println(k); + } + + void g() { + Object o = ""; + newMethod(o); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java index 3c780b600de8..db7327ea052e 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -1274,6 +1274,14 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testAvoidGenericArgumentCast() throws Exception { + doDuplicatesTest(); + } + + public void testAvoidGenericArgumentCastLocalClass() throws Exception { + doDuplicatesTest(); + } + public void testBeforeCommentAfterSelectedFragment() throws Exception { doTest(); }