From 96dd88677e4a3cf70cbde658061fc95cad5249d5 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Wed, 24 Sep 2014 16:47:30 +0400 Subject: [PATCH] PY-5475 Don't attempt to inline expression if it contains comments --- .../introduce/IntroduceHandler.java | 42 +++++++++++-------- ...functionCallWithCommentNotInlined.after.py | 8 ++++ .../functionCallWithCommentNotInlined.py | 7 ++++ ...orExpressionWithCommentNotInlined.after.py | 8 ++++ ...eneratorExpressionWithCommentNotInlined.py | 7 ++++ ...enthesisAroundGeneratorExpression.after.py | 2 + ...antParenthesisAroundGeneratorExpression.py | 2 + .../refactoring/PyIntroduceVariableTest.java | 20 ++++++++- 8 files changed, 77 insertions(+), 19 deletions(-) create mode 100644 python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.after.py create mode 100644 python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.py create mode 100644 python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.after.py create mode 100644 python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.py create mode 100644 python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.after.py create mode 100644 python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.py diff --git a/python/src/com/jetbrains/python/refactoring/introduce/IntroduceHandler.java b/python/src/com/jetbrains/python/refactoring/introduce/IntroduceHandler.java index 00448c8b69bd..069f0e596197 100644 --- a/python/src/com/jetbrains/python/refactoring/introduce/IntroduceHandler.java +++ b/python/src/com/jetbrains/python/refactoring/introduce/IntroduceHandler.java @@ -31,10 +31,7 @@ import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.Pass; import com.intellij.openapi.util.TextRange; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiFile; -import com.intellij.psi.PsiWhiteSpace; -import com.intellij.psi.TokenType; +import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.IntroduceTargetChooser; import com.intellij.refactoring.RefactoringActionHandler; @@ -511,9 +508,7 @@ abstract public class IntroduceHandler implements RefactoringActionHandler { public PyAssignmentStatement createDeclaration(IntroduceOperation operation) { final Project project = operation.getProject(); final PyExpression initializer = operation.getInitializer(); - InitializerTextBuilder builder = new InitializerTextBuilder(); - initializer.accept(builder); - String assignmentText = operation.getName() + " = " + builder.result(); + String assignmentText = operation.getName() + " = " + new InitializerTextBuilder(initializer).result(); PsiElement anchor = operation.isReplaceAll() ? findAnchor(operation.getOccurrences()) : PsiTreeUtil.getParentOfType(initializer, PyStatement.class); @@ -523,6 +518,18 @@ abstract public class IntroduceHandler implements RefactoringActionHandler { private static class InitializerTextBuilder extends PyRecursiveElementVisitor { private final StringBuilder myResult = new StringBuilder(); + public InitializerTextBuilder(@NotNull PyExpression expression) { + if (PsiTreeUtil.findChildOfType(expression, PsiComment.class) != null) { + myResult.append(expression.getText()); + } + else { + expression.accept(this); + } + if (needToWrapTopLevelExpressionInParenthesis(expression)) { + myResult.insert(0, "(").append(")"); + } + } + @Override public void visitWhiteSpace(PsiWhiteSpace space) { myResult.append(space.getText().replace('\n', ' ').replace("\\", "")); @@ -560,17 +567,6 @@ abstract public class IntroduceHandler implements RefactoringActionHandler { } } - @Override - public void visitPyGeneratorExpression(PyGeneratorExpression node) { - final PsiElement firstChild = node.getFirstChild(); - if (firstChild != null && firstChild.getNode().getElementType() != PyTokenTypes.LPAR) { - myResult.append("(").append(node.getText()).append(")"); - } - else { - super.visitPyGeneratorExpression(node); - } - } - @Override public void visitElement(PsiElement element) { if (element.getChildren().length == 0) { @@ -581,6 +577,16 @@ abstract public class IntroduceHandler implements RefactoringActionHandler { } } + private boolean needToWrapTopLevelExpressionInParenthesis(@NotNull PyExpression node) { + if (node instanceof PyGeneratorExpression) { + final PsiElement firstChild = node.getFirstChild(); + if (firstChild != null && firstChild.getNode().getElementType() != PyTokenTypes.LPAR) { + return true; + } + } + return false; + } + public String result() { return myResult.toString(); } diff --git a/python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.after.py b/python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.after.py new file mode 100644 index 000000000000..3140c9b2ab96 --- /dev/null +++ b/python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.after.py @@ -0,0 +1,8 @@ +import subprocess as sp + +a = sp.check_output( + args=['python', '-c', 'print("Spam")'], + # read errors too + stderr=sp.STDOUT +) +print(a) \ No newline at end of file diff --git a/python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.py b/python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.py new file mode 100644 index 000000000000..a3c16cedbcd8 --- /dev/null +++ b/python/testData/refactoring/introduceVariable/functionCallWithCommentNotInlined.py @@ -0,0 +1,7 @@ +import subprocess as sp + +print(sp.check_output( + args=['python', '-c', 'print("Spam")'], + # read errors too + stderr=sp.STDOUT +)) \ No newline at end of file diff --git a/python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.after.py b/python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.after.py new file mode 100644 index 000000000000..c768fe153c93 --- /dev/null +++ b/python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.after.py @@ -0,0 +1,8 @@ +baz = [1, 2] +a = ( + el # comment + if el >= 0 + else -el + for el in baz +) +foo = bar(*a) \ No newline at end of file diff --git a/python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.py b/python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.py new file mode 100644 index 000000000000..50a6fe107b51 --- /dev/null +++ b/python/testData/refactoring/introduceVariable/generatorExpressionWithCommentNotInlined.py @@ -0,0 +1,7 @@ +baz = [1, 2] +foo = bar(*( + el # comment + if el >= 0 + else -el + for el in baz +)) \ No newline at end of file diff --git a/python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.after.py b/python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.after.py new file mode 100644 index 000000000000..3ff77fd24b6b --- /dev/null +++ b/python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.after.py @@ -0,0 +1,2 @@ +a = list(i for in range(100) if x % 3 == 0) +xs = a \ No newline at end of file diff --git a/python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.py b/python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.py new file mode 100644 index 000000000000..ea8e32c10897 --- /dev/null +++ b/python/testData/refactoring/introduceVariable/noRedundantParenthesisAroundGeneratorExpression.py @@ -0,0 +1,2 @@ +xs = list(i for in range(100) + if x % 3 == 0) \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceVariableTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceVariableTest.java index 33a191f2b9a0..5a5cd3b89ff6 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceVariableTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyIntroduceVariableTest.java @@ -224,10 +224,16 @@ public class PyIntroduceVariableTest extends PyIntroduceTestCase { doTest(); } + // PY-11909 public void testGeneratorParameter() { doTest(); } + // PY-11909 + public void testNoRedundantParenthesisAroundGeneratorExpression() { + doTest(); + } + // PY-10964 public void testMultiReference() { myFixture.configureByFile(getTestName(true) + ".py"); @@ -245,7 +251,19 @@ public class PyIntroduceVariableTest extends PyIntroduceTestCase { } } - public void testSelectionBreaksBinaryOperator() {doTest();} + // PY-5475 + public void testGeneratorExpressionWithCommentNotInlined() { + doTest(); + } + + // PY-5475 + public void testFunctionCallWithCommentNotInlined() { + doTest(); + } + + public void testSelectionBreaksBinaryOperator() { + doTest(); + } private void doTestCannotPerform() { boolean thrownExpectedException = false;