From 9cbf19e193ad8f01f11835f1e9497f2fcab0b3c5 Mon Sep 17 00:00:00 2001 From: anna Date: Mon, 11 Jul 2011 17:51:21 +0400 Subject: [PATCH] introduce: pass error message if non-physical expression can't be extracted --- .../extractMethod/ExtractMethodHandler.java | 2 +- .../introduceField/ElementToWorkOn.java | 17 ++++++++--- .../IntroduceVariableBase.java | 29 +++++++++++++++++-- .../ExpressionOccurenceManager.java | 2 +- .../refactoring/IntroduceVariableTest.java | 3 +- 5 files changed, 44 insertions(+), 9 deletions(-) diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodHandler.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodHandler.java index f543ebcc32b1..cc0c597e308c 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodHandler.java @@ -110,7 +110,7 @@ public class ExtractMethodHandler implements RefactoringActionHandler { elements = CodeInsightUtil.findStatementsInRange(file, startOffset, endOffset); if (elements.length == 0) { final PsiExpression expression = IntroduceVariableBase.getSelectedExpression(project, file, startOffset, endOffset); - if (expression != null) { + if (expression != null && IntroduceVariableBase.getErrorMessage(expression) == null) { final PsiType originalType = RefactoringUtil.getTypeByExpressionWithExpectedType(expression); if (originalType != null) { elements = new PsiElement[]{expression}; diff --git a/java/java-impl/src/com/intellij/refactoring/introduceField/ElementToWorkOn.java b/java/java-impl/src/com/intellij/refactoring/introduceField/ElementToWorkOn.java index f685a3e53806..3dfc4c470276 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceField/ElementToWorkOn.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceField/ElementToWorkOn.java @@ -163,10 +163,19 @@ public class ElementToWorkOn { expr = IntroduceVariableBase.getSelectedExpression(project, file, startOffset, endOffset); } - if (localVar == null && expr == null) { - String message = RefactoringBundle.getCannotRefactorMessage(RefactoringBundle.message("error.wrong.caret.position.local.or.expression.name")); - CommonRefactoringUtil.showErrorHint(project, editor, message, refactoringName, helpId); - return null; + if (localVar == null) { + if (expr != null) { + final String errorMessage = IntroduceVariableBase.getErrorMessage(expr); + if (errorMessage != null) { + CommonRefactoringUtil.showErrorHint(project, editor, errorMessage, refactoringName, helpId); + return null; + } + } + if (expr == null) { + String message = RefactoringBundle.getCannotRefactorMessage(RefactoringBundle.message("error.wrong.caret.position.local.or.expression.name")); + CommonRefactoringUtil.showErrorHint(project, editor, message, refactoringName, helpId); + return null; + } } return new ElementToWorkOn(localVar, expr); } 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 682cd8b228b7..7236cb19437f 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java @@ -35,6 +35,7 @@ import com.intellij.openapi.editor.colors.EditorColorsManager; import com.intellij.openapi.editor.markup.TextAttributes; import com.intellij.openapi.fileEditor.FileDocumentManager; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Key; import com.intellij.openapi.util.Pass; import com.intellij.openapi.util.Ref; import com.intellij.openapi.util.TextRange; @@ -74,6 +75,7 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase impleme @NonNls private static final String PREFER_STATEMENTS_OPTION = "introduce.variable.prefer.statements"; protected static final String REFACTORING_NAME = RefactoringBundle.message("introduce.variable.title"); + public static final Key NEED_PARENTHESIS = Key.create("NEED_PARENTHESIS"); public static SuggestedNameInfo getSuggestedName(PsiType type, final PsiExpression expression) { final JavaCodeStyleManager codeStyleManager = JavaCodeStyleManager.getInstance(expression.getProject()); @@ -97,7 +99,9 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase impleme statementsInRange[0].getTextRange().getEndOffset() <= offset || isPreferStatements())) { selectionModel.selectLineAtCaret(); - if (findExpressionInRange(project, file, selectionModel.getSelectionStart(), selectionModel.getSelectionEnd()) == null) { + final PsiExpression expressionInRange = + findExpressionInRange(project, file, selectionModel.getSelectionStart(), selectionModel.getSelectionEnd()); + if (expressionInRange == null || getErrorMessage(expressionInRange) != null) { selectionModel.removeSelection(); } } @@ -214,6 +218,9 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase impleme return tempExpr; } + /** + * @return can return NotNull value thought extraction should fail: reason could be retrieved from {@link #getErrorMessage(PsiExpression)} + */ public static PsiExpression getSelectedExpression(final Project project, final PsiFile file, int startOffset, int endOffset) { PsiElement elementAtStart = file.findElementAt(startOffset); @@ -349,7 +356,8 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase impleme final PsiReferenceExpression refExpr = PsiTreeUtil.getParentOfType(toBeExpression.findElementAt(refIdx[0]), PsiReferenceExpression.class); assert refExpr != null; if (ReplaceExpressionUtil.isNeedParenthesis(refExpr.getNode(), tempExpr.getNode())) { - return null; + tempExpr.putCopyableUserData(NEED_PARENTHESIS, Boolean.TRUE); + return tempExpr; } } catch (IncorrectOperationException e) { @@ -359,6 +367,15 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase impleme return tempExpr; } + @Nullable + public static String getErrorMessage(PsiExpression expr) { + final Boolean needParenthesis = expr.getCopyableUserData(NEED_PARENTHESIS); + if (needParenthesis != null && needParenthesis.booleanValue()) { + return "Extracting selected expression would change the semantic of the whole expression."; + } + return null; + } + private static PsiExpression createArrayCreationExpression(String text, int startOffset, int endOffset, PsiMethodCallExpression parent) { if (text == null || parent == null) return null; final String[] varargsExpressions = text.split("s*,s*"); @@ -411,6 +428,14 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase impleme protected boolean invokeImpl(final Project project, final PsiExpression expr, final Editor editor) { + if (expr != null) { + final String errorMessage = getErrorMessage(expr); + if (errorMessage != null) { + showErrorMessage(project, editor, RefactoringBundle.getCannotRefactorMessage(errorMessage)); + return false; + } + } + if (expr != null && expr.getParent() instanceof PsiExpressionStatement) { FeatureUsageTracker.getInstance().triggerFeatureUsed("refactoring.introduceVariable.incompleteStatement"); } diff --git a/java/java-impl/src/com/intellij/refactoring/util/occurences/ExpressionOccurenceManager.java b/java/java-impl/src/com/intellij/refactoring/util/occurences/ExpressionOccurenceManager.java index 6e38ed8b8622..0beb43d086c4 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/occurences/ExpressionOccurenceManager.java +++ b/java/java-impl/src/com/intellij/refactoring/util/occurences/ExpressionOccurenceManager.java @@ -103,7 +103,7 @@ public class ExpressionOccurenceManager extends BaseOccurenceManager { if (literalExpression != null && !literals.contains(literalExpression)) { //enum. occurrences inside string literals final PsiExpression expression = IntroduceVariableBase.getSelectedExpression(file.getProject(), file, startOffset, offset + endOffset); - if (expression != null) { + if (expression != null && IntroduceVariableBase.getErrorMessage(expression) == null) { results.add(expression); literals.add(literalExpression); } diff --git a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceVariableTest.java b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceVariableTest.java index 694a1eadfaa3..c8b2cf6f808f 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/IntroduceVariableTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/IntroduceVariableTest.java @@ -271,7 +271,8 @@ public class IntroduceVariableTest extends LightCodeInsightTestCase { doTest(new MockIntroduceVariableHandler("sum", true, true, false, "int"){ @Override protected void showErrorMessage(Project project, Editor editor, String message) { - assertEquals("Cannot perform refactoring.\n" + "Selected block should represent an expression.", message); + assertEquals("Cannot perform refactoring.\n" + + "Extracting selected expression would change the semantic of the whole expression.", message); } }); }