From aed3a6e47da9cebc809d61fb9d92ab14091c72aa Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Fri, 3 Jul 2015 18:57:58 +0300 Subject: [PATCH] PY-16351 Ignore not inline comments to detect proper spacing between declarations --- .../imports/PyImportOptimizer.java | 10 ++- .../jetbrains/python/formatter/PyBlock.java | 70 +++++++++++-------- .../noExtraBlankLineAfterImportBlock/m1.py | 2 + .../main.after.py | 7 ++ .../noExtraBlankLineAfterImportBlock/main.py | 7 ++ .../python/PyOptimizeImportsTest.java | 20 +++++- 6 files changed, 81 insertions(+), 35 deletions(-) create mode 100644 python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/m1.py create mode 100644 python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.after.py create mode 100644 python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.py diff --git a/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java b/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java index 2b5461fe102f..be747a858ade 100644 --- a/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java +++ b/python/src/com/jetbrains/python/codeInsight/imports/PyImportOptimizer.java @@ -35,6 +35,8 @@ import java.util.List; * @author yole */ public class PyImportOptimizer implements ImportOptimizer { + private static final boolean SORT_IMPORTS = true; + @Override public boolean supports(PsiFile file) { return true; @@ -148,9 +150,11 @@ public class PyImportOptimizer implements ImportOptimizer { } private void applyResults() { - Collections.sort(myBuiltinImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); - Collections.sort(myThirdPartyImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); - Collections.sort(myProjectImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); + if (SORT_IMPORTS) { + Collections.sort(myBuiltinImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); + Collections.sort(myThirdPartyImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); + Collections.sort(myProjectImports, AddImportHelper.IMPORT_BY_NAME_COMPARATOR); + } markGroupBegin(myThirdPartyImports); markGroupBegin(myProjectImports); diff --git a/python/src/com/jetbrains/python/formatter/PyBlock.java b/python/src/com/jetbrains/python/formatter/PyBlock.java index cd90ce4ad4f7..16ce9caeb2f9 100644 --- a/python/src/com/jetbrains/python/formatter/PyBlock.java +++ b/python/src/com/jetbrains/python/formatter/PyBlock.java @@ -32,13 +32,11 @@ import com.jetbrains.python.PyElementTypes; import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.PythonDialectsTokenSetProvider; import com.jetbrains.python.psi.*; +import com.jetbrains.python.psi.impl.PyPsiUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; +import java.util.*; import static com.jetbrains.python.formatter.PyCodeStyleSettings.DICT_ALIGNMENT_ON_COLON; import static com.jetbrains.python.formatter.PyCodeStyleSettings.DICT_ALIGNMENT_ON_VALUE; @@ -88,6 +86,7 @@ public class PyBlock implements ASTBlock { private final Wrap myWrap; private final PyBlockContext myContext; private List mySubBlocks = null; + private Map mySubBlockByNode = null; private Alignment myChildAlignment; private final Alignment myDictAlignment; private final Wrap myDictWrapping; @@ -137,16 +136,28 @@ public class PyBlock implements ASTBlock { @NotNull public List getSubBlocks() { if (mySubBlocks == null) { - mySubBlocks = buildSubBlocks(); + mySubBlockByNode = buildSubBlocks(); + mySubBlocks = new ArrayList(mySubBlockByNode.values()); if (DUMP_FORMATTING_BLOCKS) { dumpSubBlocks(); } } - return new ArrayList(mySubBlocks); + return Collections.unmodifiableList(mySubBlocks); } - private List buildSubBlocks() { - final List blocks = new ArrayList(); + @Nullable + private PyBlock getSubBlockByNode(@NotNull ASTNode node) { + return mySubBlockByNode.get(node); + } + + @Nullable + private PyBlock getSubBlockByIndex(int index) { + return mySubBlocks.get(index); + } + + @NotNull + private Map buildSubBlocks() { + final Map blocks = new LinkedHashMap(); for (ASTNode child = myNode.getFirstChildNode(); child != null; child = child.getTreeNext()) { final IElementType childType = child.getElementType(); @@ -157,9 +168,9 @@ public class PyBlock implements ASTBlock { continue; } - blocks.add(buildSubBlock(child)); + blocks.put(child, buildSubBlock(child)); } - return Collections.unmodifiableList(blocks); + return Collections.unmodifiableMap(blocks); } private PyBlock buildSubBlock(ASTNode child) { @@ -685,9 +696,22 @@ public class PyBlock implements ASTBlock { public Spacing getSpacing(Block child1, @NotNull Block child2) { if (child1 instanceof ASTBlock && child2 instanceof ASTBlock) { final ASTNode node1 = ((ASTBlock)child1).getNode(); - final PsiElement psi1 = node1.getPsi(); - final PsiElement psi2 = ((ASTBlock)child2).getNode().getPsi(); + ASTNode node2 = ((ASTBlock)child2).getNode(); final IElementType childType1 = node1.getElementType(); + final PsiElement psi1 = node1.getPsi(); + + PsiElement psi2 = node2.getPsi(); + // skip not inline comments to handles blank lines between various declarations + if (psi2 instanceof PsiComment && hasLineBreaksBefore(node2, 1)) { + final PsiElement nonCommentAfter = PyPsiUtils.getNextNonCommentSibling(psi2, true); + if (nonCommentAfter != null) { + psi2 = nonCommentAfter; + } + } + node2 = psi2.getNode(); + final IElementType childType2 = psi2.getNode().getElementType(); + //noinspection ConstantConditions + child2 = getSubBlockByNode(node2); final CommonCodeStyleSettings settings = myContext.getSettings(); if (childType1 == PyTokenTypes.COLON && psi2 instanceof PyStatementList) { @@ -696,8 +720,8 @@ public class PyBlock implements ASTBlock { } } - if ((PyElementTypes.CLASS_OR_FUNCTION.contains(childType1) && hasTypeIgnoringPrecedingComments(psi2, STATEMENT_OR_DECLARATION)) || - STATEMENT_OR_DECLARATION.contains(childType1) && hasTypeIgnoringPrecedingComments(psi2, PyElementTypes.CLASS_OR_FUNCTION)) { + if ((PyElementTypes.CLASS_OR_FUNCTION.contains(childType1) && STATEMENT_OR_DECLARATION.contains(childType2)) || + STATEMENT_OR_DECLARATION.contains(childType1) && PyElementTypes.CLASS_OR_FUNCTION.contains(childType2)) { if (PyUtil.isTopLevel(psi1)) { return getBlankLinesForOption(myContext.getPySettings().BLANK_LINES_AROUND_TOP_LEVEL_CLASSES_FUNCTIONS); } @@ -725,17 +749,6 @@ public class PyBlock implements ASTBlock { return myContext.getSpacingBuilder().getSpacing(this, child1, child2); } - private static boolean hasTypeIgnoringPrecedingComments(@NotNull PsiElement element, @NotNull TokenSet types) { - if (element instanceof PsiComment) { - final PsiElement psi3 = PsiTreeUtil.getNextSiblingOfType(element, PyElement.class); - if (psi3 != null) { - final IElementType type3 = psi3.getNode().getElementType(); - return types.contains(type3); - } - } - return types.contains(element.getNode().getElementType()); - } - private Spacing getBlankLinesForOption(final int option) { final int blankLines = option + 1; return Spacing.createSpacing(0, 0, blankLines, @@ -762,7 +775,7 @@ public class PyBlock implements ASTBlock { return ChildAttributes.DELEGATE_TO_PREV_CHILD; } - final PyBlock insertAfterBlock = mySubBlocks.get(newChildIndex - 1); + final PyBlock insertAfterBlock = getSubBlockByIndex(newChildIndex - 1); final ASTNode prevNode = insertAfterBlock.getNode(); final PsiElement prevElt = prevNode.getPsi(); @@ -950,11 +963,10 @@ public class PyBlock implements ASTBlock { return null; } int prevIndex = newChildIndex - 1; - while (prevIndex > 0 && mySubBlocks.get(prevIndex).getNode().getElementType() == PyTokenTypes.END_OF_LINE_COMMENT) { + while (prevIndex > 0 && getSubBlockByIndex(prevIndex).getNode().getElementType() == PyTokenTypes.END_OF_LINE_COMMENT) { prevIndex--; } - final PyBlock insertAfterBlock = mySubBlocks.get(prevIndex); - return insertAfterBlock.getNode(); + return getSubBlockByIndex(prevIndex).getNode(); } private static ASTNode getLastNonSpaceChild(ASTNode node, boolean acceptError) { diff --git a/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/m1.py b/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/m1.py new file mode 100644 index 000000000000..1dea43b79188 --- /dev/null +++ b/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/m1.py @@ -0,0 +1,2 @@ +class MyClass: + pass \ No newline at end of file diff --git a/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.after.py b/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.after.py new file mode 100644 index 000000000000..26f0fa7e708a --- /dev/null +++ b/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.after.py @@ -0,0 +1,7 @@ +from collections import OrderedDict +import sys + +from m1 import MyClass + +# comment +print(sys, OrderedDict, MyClass) diff --git a/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.py b/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.py new file mode 100644 index 000000000000..26f0fa7e708a --- /dev/null +++ b/python/testData/optimizeImports/noExtraBlankLineAfterImportBlock/main.py @@ -0,0 +1,7 @@ +from collections import OrderedDict +import sys + +from m1 import MyClass + +# comment +print(sys, OrderedDict, MyClass) diff --git a/python/testSrc/com/jetbrains/python/PyOptimizeImportsTest.java b/python/testSrc/com/jetbrains/python/PyOptimizeImportsTest.java index 1e3765d12e6c..40fa5081fc2a 100644 --- a/python/testSrc/com/jetbrains/python/PyOptimizeImportsTest.java +++ b/python/testSrc/com/jetbrains/python/PyOptimizeImportsTest.java @@ -72,9 +72,23 @@ public class PyOptimizeImportsTest extends PyTestCase { doTest(); } - private void doTest() { - myFixture.configureByFile("optimizeImports/" + getTestName(true) + ".py"); + // PY-16351 + public void testNoExtraBlankLineAfterImportBlock() { + final String testName = getTestName(true); + myFixture.copyDirectoryToProject(testName, ""); + myFixture.configureByFile("main.py"); OptimizeImportsAction.actionPerformedImpl(DataManager.getInstance().getDataContext(myFixture.getEditor().getContentComponent())); - myFixture.checkResultByFile("optimizeImports/" + getTestName(true) + ".after.py"); + myFixture.checkResultByFile(testName + "/main.after.py"); + } + + private void doTest() { + myFixture.configureByFile(getTestName(true) + ".py"); + OptimizeImportsAction.actionPerformedImpl(DataManager.getInstance().getDataContext(myFixture.getEditor().getContentComponent())); + myFixture.checkResultByFile(getTestName(true) + ".after.py"); + } + + @Override + protected String getTestDataPath() { + return super.getTestDataPath() + "/optimizeImports"; } }