From 09f918bf2815c7335e7e04f97927dcf7b53242aa Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 27 Apr 2015 15:11:47 +0300 Subject: [PATCH] PY-12360 More robust way to handle trailing continuation backslashes Python lexer and parser can split whitespaces and attach them to different parents, so we have to scan backward through consequtive leaf elements to collect all of them. I also added several new tests for various such corner cases. --- ...TrailingBlankLinesPostFormatProcessor.java | 93 +++++++++++-------- ...ilingBlankLinesWithBackslashesAtFileEnd.py | 5 + ...lankLinesWithBackslashesAtFileEnd_after.py | 2 + ...gBlankLinesWithBackslashesAtFunctionEnd.py | 4 + ...esWithBackslashesAtFunctionEndNoNewLine.py | 4 + ...BackslashesAtFunctionEndNoNewLine_after.py | 2 + ...LinesWithBackslashesAtFunctionEnd_after.py | 2 + .../trailingBlankLinesWithBackslashesMixed.py | 6 ++ ...ingBlankLinesWithBackslashesMixed_after.py | 2 + .../formatter/wrapBeforeElse_after.py | 2 +- .../com/jetbrains/python/PyFormatterTest.java | 20 ++++ 11 files changed, 101 insertions(+), 41 deletions(-) create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd_after.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine_after.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd_after.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesMixed.py create mode 100644 python/testData/formatter/trailingBlankLinesWithBackslashesMixed_after.py diff --git a/python/src/com/jetbrains/python/formatter/PyTrailingBlankLinesPostFormatProcessor.java b/python/src/com/jetbrains/python/formatter/PyTrailingBlankLinesPostFormatProcessor.java index d6942835fb2c..31613335d732 100644 --- a/python/src/com/jetbrains/python/formatter/PyTrailingBlankLinesPostFormatProcessor.java +++ b/python/src/com/jetbrains/python/formatter/PyTrailingBlankLinesPostFormatProcessor.java @@ -19,18 +19,16 @@ import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.ex.EditorSettingsExternalizable; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Computable; -import com.intellij.openapi.util.Couple; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; -import com.intellij.psi.PsiDocumentManager; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiFile; -import com.intellij.psi.PsiWhiteSpace; +import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.CodeStyleSettings; import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.psi.impl.source.codeStyle.PostFormatProcessor; import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.containers.ContainerUtil; +import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.psi.LanguageLevel; import com.jetbrains.python.psi.PyElementGenerator; import com.jetbrains.python.psi.PyFile; @@ -38,7 +36,9 @@ import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import static com.jetbrains.python.psi.PyUtil.as; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; /** * Strip trailing extra blank lines at the end of the file and insert necessary line feed if corresponding whitespace element belongs to @@ -55,8 +55,10 @@ public class PyTrailingBlankLinesPostFormatProcessor implements PostFormatProces public PsiElement processElement(@NotNull PsiElement source, @NotNull CodeStyleSettings settings) { if (source instanceof PyFile) { final PyFile pyFile = (PyFile)source; - final Couple range = findTrailingWhitespaces(pyFile); - if (range != null && PsiTreeUtil.isAncestor(source, range.getFirst(), false)) { + final List range = findTrailingWhitespaces(pyFile); + if (!range.isEmpty() && + PsiTreeUtil.isAncestor(source, ContainerUtil.getFirstItem(range), false) && + PsiTreeUtil.isAncestor(source, ContainerUtil.getLastItem(range), false)) { replaceOrDeleteTrailingWhitespaces(pyFile, range); } } @@ -68,9 +70,8 @@ public class PyTrailingBlankLinesPostFormatProcessor implements PostFormatProces if (!(source instanceof PyFile)) { return rangeToReformat; } - final Couple range = findTrailingWhitespaces(source); - final TextRange oldWhitespaceRange; - oldWhitespaceRange = range != null ? unionRange(range) : TextRange.from(source.getTextLength(), 0); + final List range = findTrailingWhitespaces(source); + final TextRange oldWhitespaceRange = !range.isEmpty() ? unionRange(range) : TextRange.from(source.getTextLength(), 0); if (rangeToReformat.intersects(oldWhitespaceRange)) { final TextRange newWhitespaceRange = replaceOrDeleteTrailingWhitespaces((PyFile)source, range);; final int delta = newWhitespaceRange.getLength() - oldWhitespaceRange.getLength(); @@ -93,27 +94,46 @@ public class PyTrailingBlankLinesPostFormatProcessor implements PostFormatProces return rangeToReformat; } - @Nullable - private static Couple findTrailingWhitespaces(@NotNull PsiFile file) { - final PsiWhiteSpace lastWhitespace = as(PsiTreeUtil.lastChild(file), PsiWhiteSpace.class); - if (lastWhitespace == null) { - return null; + @NotNull + private static List findTrailingWhitespaces(@NotNull PsiFile file) { + final PsiElement lastLeaf = PsiTreeUtil.lastChild(file); + + if (isWhitespaceBackslashOrEmptyError(lastLeaf)) { + final List result = new ArrayList(); + boolean containsWhitespaces = false; + for (PsiElement prev = lastLeaf; isWhitespaceBackslashOrEmptyError(prev); prev = PsiTreeUtil.prevLeaf(prev)) { + containsWhitespaces |= prev instanceof PsiWhiteSpace; + result.add(prev); + } + if (containsWhitespaces) { + Collections.reverse(result); + return result; + } } - PsiWhiteSpace firstWhitespace = lastWhitespace; - for (PsiElement prev = lastWhitespace.getPrevSibling(); prev instanceof PsiWhiteSpace; prev = prev.getPrevSibling()) { - firstWhitespace = (PsiWhiteSpace)prev; + return Collections.emptyList(); + } + + private static boolean isWhitespaceBackslashOrEmptyError(@Nullable PsiElement elem) { + if (elem == null) { + return false; } - return Couple.of(firstWhitespace, lastWhitespace); + return elem instanceof PsiWhiteSpace || + elem.getNode().getElementType() == PyTokenTypes.BACKSLASH || + (elem instanceof PsiErrorElement && elem.getTextLength() == 0); } @NotNull - private static TextRange unionRange(@NotNull Couple range) { - return range.getFirst().getTextRange().union(range.getSecond().getTextRange()); + private static TextRange unionRange(@NotNull List range) { + if (range.isEmpty()) { + return TextRange.EMPTY_RANGE; + } + //noinspection ConstantConditions + return ContainerUtil.getFirstItem(range).getTextRange().union(ContainerUtil.getLastItem(range).getTextRange()); } @NotNull private static TextRange replaceOrDeleteTrailingWhitespaces(@NotNull final PyFile pyFile, - @Nullable final Couple whitespaces) { + @NotNull final List whitespaces) { final Project project = pyFile.getProject(); final PsiDocumentManager documentManager = PsiDocumentManager.getInstance(project); final Document document = documentManager.getDocument(pyFile); @@ -134,7 +154,7 @@ public class PyTrailingBlankLinesPostFormatProcessor implements PostFormatProces return codeStyleManager.performActionWithFormatterDisabled(new Computable() { @Override public TextRange compute() { - if (whitespaces != null) { + if (!whitespaces.isEmpty()) { return replaceOrDeletePsiRange(whitespaces, lineFeeds).getTextRange(); } else { @@ -143,7 +163,7 @@ public class PyTrailingBlankLinesPostFormatProcessor implements PostFormatProces } }); } - else if (whitespaces != null) { + else if (!whitespaces.isEmpty()) { return codeStyleManager.performActionWithFormatterDisabled(new Computable() { @Override public TextRange compute() { @@ -153,26 +173,19 @@ public class PyTrailingBlankLinesPostFormatProcessor implements PostFormatProces }); } } - return whitespaces == null ? TextRange.from(pyFile.getTextLength(), 0) : unionRange(whitespaces); + return whitespaces.isEmpty() ? TextRange.from(pyFile.getTextLength(), 0) : unionRange(whitespaces); } @Contract("_, null -> null; _, !null -> !null") @Nullable - private static PsiElement replaceOrDeletePsiRange(@NotNull Couple range, @Nullable PsiElement replacement) { - final PsiElement first = range.getFirst(); - final PsiElement last = range.getSecond(); - final PsiElement parent = first.getParent(); - final PsiElement beforeFirst = first.getPrevSibling(); - assert !(beforeFirst instanceof PsiWhiteSpace); - parent.deleteChildRange(first, last); - // chain `first.getParent().deleteRange(first.getNextSibling(), last); first.replace(replacement)` doesn't work + private static PsiElement replaceOrDeletePsiRange(@NotNull List range, @Nullable PsiElement replacement) { + final PsiElement file = range.get(0).getContainingFile(); + // If whitespaces span several parents, the safest option is to append new whitespace at the end of the file + for (PsiElement element : ContainerUtil.reverse(range)) { + element.delete(); + } if (replacement != null) { - if (beforeFirst != null) { - return parent.addAfter(replacement, beforeFirst); - } - else { - return parent.add(replacement); - } + return file.add(replacement); } return null; } diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd.py b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd.py new file mode 100644 index 000000000000..747e4f3091cd --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd.py @@ -0,0 +1,5 @@ +def foo(): + pass +\ +\ +\ diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd_after.py b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd_after.py new file mode 100644 index 000000000000..9332a2735b6a --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFileEnd_after.py @@ -0,0 +1,2 @@ +def foo(): + pass diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd.py b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd.py new file mode 100644 index 000000000000..3d1e801e99d1 --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd.py @@ -0,0 +1,4 @@ +def foo(): + pass \ +\ +\ diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine.py b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine.py new file mode 100644 index 000000000000..a1d931758582 --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine.py @@ -0,0 +1,4 @@ +def foo(): + pass \ +\ +\ \ No newline at end of file diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine_after.py b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine_after.py new file mode 100644 index 000000000000..9332a2735b6a --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEndNoNewLine_after.py @@ -0,0 +1,2 @@ +def foo(): + pass diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd_after.py b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd_after.py new file mode 100644 index 000000000000..9332a2735b6a --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesAtFunctionEnd_after.py @@ -0,0 +1,2 @@ +def foo(): + pass diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesMixed.py b/python/testData/formatter/trailingBlankLinesWithBackslashesMixed.py new file mode 100644 index 000000000000..94445101b6c7 --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesMixed.py @@ -0,0 +1,6 @@ +def foo(): + pass \ +\ +\ + + diff --git a/python/testData/formatter/trailingBlankLinesWithBackslashesMixed_after.py b/python/testData/formatter/trailingBlankLinesWithBackslashesMixed_after.py new file mode 100644 index 000000000000..9332a2735b6a --- /dev/null +++ b/python/testData/formatter/trailingBlankLinesWithBackslashesMixed_after.py @@ -0,0 +1,2 @@ +def foo(): + pass diff --git a/python/testData/formatter/wrapBeforeElse_after.py b/python/testData/formatter/wrapBeforeElse_after.py index b8185063fcba..ab628b73bfc7 100644 --- a/python/testData/formatter/wrapBeforeElse_after.py +++ b/python/testData/formatter/wrapBeforeElse_after.py @@ -1,2 +1,2 @@ id = 1 if looooooooooooooooooooooooong_vaaaaaaaaaaaaaaaar == 'loooooooooooooooong_vaaaaaaaaaaaaaaaaaaaaaaaaaalue' else \ -list('foo')[0] \ No newline at end of file +list('foo')[0] diff --git a/python/testSrc/com/jetbrains/python/PyFormatterTest.java b/python/testSrc/com/jetbrains/python/PyFormatterTest.java index dc04d530f20b..44a4c5eb3fda 100644 --- a/python/testSrc/com/jetbrains/python/PyFormatterTest.java +++ b/python/testSrc/com/jetbrains/python/PyFormatterTest.java @@ -542,6 +542,26 @@ public class PyFormatterTest extends PyTestCase { doTest(); } + // PY-11552 + public void testTrailingBlankLinesWithBackslashesAtFileEnd() { + doTest(); + } + + // PY-11552 + public void testTrailingBlankLinesWithBackslashesAtFunctionEnd() { + doTest(); + } + + // PY-11552 + public void testTrailingBlankLinesWithBackslashesAtFunctionEndNoNewLine() { + doTest(); + } + + // PY-11552 + public void testTrailingBlankLinesWithBackslashesMixed() { + doTest(); + } + // PY-15530 public void testAlignmentInArgumentListWhereFirstArgumentIsEmptyCall() { doTest();