From e6c2ee17915bbfac0c09030e3845382a9066c7a8 Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Wed, 1 Jul 2020 08:46:41 +0200 Subject: [PATCH] java: introduce variable: forbid extracting refExpr which resolves to class/package on explicit selection (IDEA-244925) GitOrigin-RevId: 32b84f9b74e508ae83eeaf46f19dfbce78833ae5 --- .../IntroduceVariableBase.java | 37 +++++++----- .../ClassSelectionInStaticMethodCall.java | 7 +++ .../refactoring/IntroduceVariableTest.java | 57 +++++-------------- 3 files changed, 44 insertions(+), 57 deletions(-) create mode 100644 java/java-tests/testData/refactoring/introduceVariable/ClassSelectionInStaticMethodCall.java diff --git a/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java b/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java index 5a18e8a52be6..8ed997b368a2 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java @@ -305,28 +305,35 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { while (expression != null) { if (!expressions.contains(expression) && !(expression instanceof PsiParenthesizedExpression) && !(expression instanceof PsiSuperExpression) && (acceptVoid || !PsiType.VOID.equals(expression.getType()))) { - if (expression instanceof PsiMethodReferenceExpression) { + if (isExtractable(expression)) { expressions.add(expression); } - else if (!(expression instanceof PsiAssignmentExpression)) { - if (!(expression instanceof PsiReferenceExpression)) { - expressions.add(expression); - } - else { - if (!(expression.getParent() instanceof PsiMethodCallExpression)) { - final PsiElement resolve = ((PsiReferenceExpression)expression).resolve(); - if (!(resolve instanceof PsiClass) && !(resolve instanceof PsiPackage)) { - expressions.add(expression); - } - } - } - } } expression = PsiTreeUtil.getParentOfType(expression, PsiExpression.class); } return expressions; } + public static boolean isExtractable(PsiExpression expression) { + if (expression instanceof PsiMethodReferenceExpression) { + return true; + } + else if (!(expression instanceof PsiAssignmentExpression)) { + if (!(expression instanceof PsiReferenceExpression)) { + return true; + } + else { + if (!(expression.getParent() instanceof PsiMethodCallExpression)) { + final PsiElement resolve = ((PsiReferenceExpression)expression).resolve(); + if (!(resolve instanceof PsiClass) && !(resolve instanceof PsiPackage)) { + return true; + } + } + } + } + return false; + } + public static PsiElement[] findStatementsAtOffset(final Editor editor, final PsiFile file, final int offset) { final Document document = editor.getDocument(); final int lineNumber = document.getLineNumber(offset); @@ -369,7 +376,7 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { if (tempExpr == null) { tempExpr = getSelectedExpression(project, file, startOffset, endOffset); } - return tempExpr; + return isExtractable(tempExpr) ? tempExpr : null; } /** diff --git a/java/java-tests/testData/refactoring/introduceVariable/ClassSelectionInStaticMethodCall.java b/java/java-tests/testData/refactoring/introduceVariable/ClassSelectionInStaticMethodCall.java new file mode 100644 index 000000000000..9b393f385be0 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/ClassSelectionInStaticMethodCall.java @@ -0,0 +1,7 @@ +class Foo { + static void foo() {} + + { + Foo.foo(); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java index 1f4386022a41..a5e0c93c04bc 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java @@ -175,14 +175,7 @@ public class IntroduceVariableTest extends LightJavaCodeInsightTestCase { public void testSubLiteral1() { doTest("str", false, false, false, JAVA_LANG_STRING); } public void testSubLiteralFailure() { - try { - doTest("str", false, false, false, "int"); - } - catch (Exception e) { - assertEquals("Error message:Cannot perform refactoring.\nSelected block should represent an expression", e.getMessage()); - return; - } - fail("Should not be able to perform refactoring"); + doTestWithFailure("str", "int"); } public void testSubLiteralFromExpression() { doTest("str", false, false, false, JAVA_LANG_STRING); } @@ -195,30 +188,20 @@ public class IntroduceVariableTest extends LightJavaCodeInsightTestCase { public void testFromFinalFieldOnAssignment() { doTest("strings", false, false, false, JAVA_LANG_STRING); } public void testNoArrayFromVarargs() { - try { - doTest("strings", false, false, false, "java.lang.String[]"); - } - catch (Exception e) { - assertEquals("Error message:Cannot perform refactoring.\nSelected block should represent an expression", e.getMessage()); - return; - } - fail("Should not be able to perform refactoring"); + doTestWithFailure("strings", "java.lang.String[]"); } public void testNoArrayFromVarargs1() { - try { - doTest("strings", false, false, false, "java.lang.String[]"); - } - catch (Exception e) { - assertEquals("Error message:Cannot perform refactoring.\nSelected block should represent an expression", e.getMessage()); - return; - } - fail("Should not be able to perform refactoring"); - } + doTestWithFailure("strings", "java.lang.String[]"); + } public void testNoArrayFromVarargsUntilComma() { + doTestWithFailure("strings", "java.lang.String[]"); + } + + public void doTestWithFailure(String newName, String expectedType) { try { - doTest("strings", false, false, false, "java.lang.String[]"); + doTest(newName, false, false, false, expectedType); } catch (Exception e) { assertEquals("Error message:Cannot perform refactoring.\nSelected block should represent an expression", e.getMessage()); @@ -292,25 +275,15 @@ public class IntroduceVariableTest extends LightJavaCodeInsightTestCase { } public void testIncorrectExpressionSelected() { - try { - doTest("toString", false, false, false, JAVA_LANG_STRING); - } - catch (Exception e) { - assertEquals("Error message:Cannot perform refactoring.\nSelected block should represent an expression", e.getMessage()); - return; - } - fail("Should not be able to perform refactoring"); + doTestWithFailure("toString", JAVA_LANG_STRING); } public void testIncompatibleTypesForSelectionSubExpression() { - try { - doTest("s", false, false, false, JAVA_LANG_STRING); - } - catch (Exception e) { - assertEquals("Error message:Cannot perform refactoring.\nSelected block should represent an expression", e.getMessage()); - return; - } - fail("Should not be able to perform refactoring"); + doTestWithFailure("s", JAVA_LANG_STRING); + } + + public void testClassSelectionInStaticMethodCall() { + doTestWithFailure("Foo", "Foo"); } public void testMultiCatchSimple() { doTest("e", true, true, false, "java.lang.Exception", true); }