From 8eaabb8c6f77d8802e64e9654dc4a9dd9e24d201 Mon Sep 17 00:00:00 2001 From: Denis Zhdanov Date: Mon, 6 Jul 2015 11:40:59 +0300 Subject: [PATCH 1/5] UP-4484 Improve performance of MessageBusImpl processing Minor refactoring to be able to conveniently use default implementation in tests --- .../util/messages/MessageBusFactory.java | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/platform/util/src/com/intellij/util/messages/MessageBusFactory.java b/platform/util/src/com/intellij/util/messages/MessageBusFactory.java index f91daed0dd2c..54496579bfca 100644 --- a/platform/util/src/com/intellij/util/messages/MessageBusFactory.java +++ b/platform/util/src/com/intellij/util/messages/MessageBusFactory.java @@ -27,26 +27,16 @@ import java.util.concurrent.atomic.AtomicReference; public class MessageBusFactory { - private static final AtomicReference ourImpl = new AtomicReference(new Impl() { - @NotNull - @Override - public MessageBus newMessageBus(@NotNull Object owner) { - return new MessageBusImpl.RootBus(owner); - } - - @NotNull - @Override - public MessageBus newMessageBus(@NotNull Object owner, @Nullable MessageBus parentBus) { - return parentBus == null ? newMessageBus(owner) : new MessageBusImpl(owner, parentBus); - } - }); + private static final AtomicReference ourImpl = new AtomicReference(Impl.DEFAULT); private MessageBusFactory() {} + @NotNull public static MessageBus newMessageBus(@NotNull Object owner) { return ourImpl.get().newMessageBus(owner); } + @NotNull public static MessageBus newMessageBus(@NotNull Object owner, @Nullable MessageBus parentBus) { return ourImpl.get().newMessageBus(owner, parentBus); } @@ -57,6 +47,20 @@ public class MessageBusFactory { public interface Impl { + Impl DEFAULT = new Impl() { + @NotNull + @Override + public MessageBus newMessageBus(@NotNull Object owner) { + return new MessageBusImpl.RootBus(owner); + } + + @NotNull + @Override + public MessageBus newMessageBus(@NotNull Object owner, @Nullable MessageBus parentBus) { + return parentBus == null ? newMessageBus(owner) : new MessageBusImpl(owner, parentBus); + } + }; + @NotNull MessageBus newMessageBus(@NotNull Object owner); From 1491d083e13f702191e93e5948f062b401ee97ff Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Mon, 6 Jul 2015 10:04:16 +0300 Subject: [PATCH 2/5] @NotNull --- platform/util/src/com/intellij/openapi/util/RecursionGuard.java | 2 +- .../util/src/com/intellij/openapi/util/RecursionManager.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/util/src/com/intellij/openapi/util/RecursionGuard.java b/platform/util/src/com/intellij/openapi/util/RecursionGuard.java index 9df4e10fdb7e..b016892de051 100644 --- a/platform/util/src/com/intellij/openapi/util/RecursionGuard.java +++ b/platform/util/src/com/intellij/openapi/util/RecursionGuard.java @@ -85,7 +85,7 @@ public abstract class RecursionGuard { * * @param since the id of a computation whose result is safe to cache whilst for more nested ones it's not. */ - public abstract void prohibitResultCaching(Object since); + public abstract void prohibitResultCaching(@NotNull Object since); public interface StackStamp { diff --git a/platform/util/src/com/intellij/openapi/util/RecursionManager.java b/platform/util/src/com/intellij/openapi/util/RecursionManager.java index 47d32a1389f5..b40d0bb7318e 100644 --- a/platform/util/src/com/intellij/openapi/util/RecursionManager.java +++ b/platform/util/src/com/intellij/openapi/util/RecursionManager.java @@ -156,7 +156,7 @@ public class RecursionManager { } @Override - public void prohibitResultCaching(Object since) { + public void prohibitResultCaching(@NotNull Object since) { MyKey realKey = new MyKey(id, since, false); final CalculationStack stack = ourStack.get(); stack.enableMemoization(realKey, stack.prohibitResultCaching(realKey)); From 2b7be736aa958a5727094c622c82698a7cb8f098 Mon Sep 17 00:00:00 2001 From: Vladimir Krivosheev Date: Mon, 6 Jul 2015 11:33:53 +0200 Subject: [PATCH 3/5] IDEA-CR-3497 it's better to use verb in method name to make it clearer what it does, i.e. it would be better to rename this method to 'runWriteAction'. add javadoc to avoid "exposes Function0 class from Kotlin stdlib to public API, it's forbidden for now" --- platform/built-in-server/testSrc/TestManager.kt | 10 +++++----- .../src/com/intellij/openapi/application/actions.kt | 8 +++++++- .../com/intellij/openapi/options/SchemeManagerImpl.kt | 7 ++++--- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/platform/built-in-server/testSrc/TestManager.kt b/platform/built-in-server/testSrc/TestManager.kt index be3fc2ef57e3..eb36f2a63d76 100644 --- a/platform/built-in-server/testSrc/TestManager.kt +++ b/platform/built-in-server/testSrc/TestManager.kt @@ -1,7 +1,7 @@ package org.jetbrains.ide import com.intellij.openapi.application.invokeAndWaitIfNeed -import com.intellij.openapi.application.writeAction +import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.roots.ModuleRootManager import com.intellij.openapi.roots.ModuleRootModificationUtil import com.intellij.openapi.util.io.FileUtilRt @@ -68,7 +68,7 @@ class TestManager(val projectFixture: IdeaProjectTestFixture) : TestWatcher() { val normalizedFilePath = FileUtilRt.toSystemIndependentName(filePath!!) if (annotation!!.relativeToProject) { val root = projectFixture.getProject().getBaseDir() - writeAction { + runWriteAction { fileToDelete = root.findOrCreateChildData(this@TestManager, normalizedFilePath) } } @@ -77,7 +77,7 @@ class TestManager(val projectFixture: IdeaProjectTestFixture) : TestWatcher() { ModuleRootModificationUtil.updateModel(projectFixture.getModule()) { model -> val contentEntry = model.getContentEntries()[0] val contentRoot = contentEntry.getFile()!! - writeAction { + runWriteAction { contentRoot.findChild(EXCLUDED_DIR_NAME)?.delete(this@TestManager) fileToDelete = contentRoot.createChildDirectory(this@TestManager, EXCLUDED_DIR_NAME) fileToDelete!!.createChildData(this@TestManager, normalizedFilePath) @@ -89,7 +89,7 @@ class TestManager(val projectFixture: IdeaProjectTestFixture) : TestWatcher() { } else { val root = ModuleRootManager.getInstance(projectFixture.getModule()).getSourceRoots()[0] - writeAction { + runWriteAction { fileToDelete = root.findOrCreateChildData(this@TestManager, normalizedFilePath) } } @@ -103,7 +103,7 @@ class TestManager(val projectFixture: IdeaProjectTestFixture) : TestWatcher() { } if (fileToDelete != null) { - invokeAndWaitIfNeed { writeAction { fileToDelete?.delete(this@TestManager) } } + invokeAndWaitIfNeed { runWriteAction { fileToDelete?.delete(this@TestManager) } } fileToDelete = null } diff --git a/platform/core-impl/src/com/intellij/openapi/application/actions.kt b/platform/core-impl/src/com/intellij/openapi/application/actions.kt index 4be236085b50..d7071412640c 100644 --- a/platform/core-impl/src/com/intellij/openapi/application/actions.kt +++ b/platform/core-impl/src/com/intellij/openapi/application/actions.kt @@ -17,7 +17,10 @@ package com.intellij.openapi.application import javax.swing.SwingUtilities -public inline fun writeAction(runnable: () -> Unit) { +/** + * @exclude Internal use only + */ +public inline fun runWriteAction(runnable: () -> Unit) { val token = WriteAction.start() try { runnable() @@ -27,6 +30,9 @@ public inline fun writeAction(runnable: () -> Unit) { } } +/** + * @exclude Internal use only + */ public fun invokeAndWaitIfNeed(runnable: () -> Unit) { val app = ApplicationManager.getApplication() if (app == null) { diff --git a/platform/platform-impl/src/com/intellij/openapi/options/SchemeManagerImpl.kt b/platform/platform-impl/src/com/intellij/openapi/options/SchemeManagerImpl.kt index 4c575727d86d..e887c56b3d57 100644 --- a/platform/platform-impl/src/com/intellij/openapi/options/SchemeManagerImpl.kt +++ b/platform/platform-impl/src/com/intellij/openapi/options/SchemeManagerImpl.kt @@ -19,7 +19,7 @@ import com.intellij.openapi.application.AccessToken import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.WriteAction import com.intellij.openapi.application.ex.DecodeDefaultsUtil -import com.intellij.openapi.application.writeAction +import com.intellij.openapi.application.runWriteAction import com.intellij.openapi.components.RoamingType import com.intellij.openapi.components.ServiceManager import com.intellij.openapi.components.impl.stores.DirectoryBasedStorage @@ -527,7 +527,8 @@ public class SchemeManagerImpl(private val if (renamed) { file = dir.findChild(externalInfo!!.fileName) if (file != null) { - writeAction { + runWriteAction { } + runWriteAction { file!!.rename(this, fileName) } } @@ -537,7 +538,7 @@ public class SchemeManagerImpl(private val file = DirectoryBasedStorage.getFile(fileName, dir, this) } - writeAction { + runWriteAction { file!!.getOutputStream(this).use { byteOut.writeTo(it) } From 736f2c0eac585b349c8047616917b65cd52857c3 Mon Sep 17 00:00:00 2001 From: "Egor.Ushakov" Date: Mon, 6 Jul 2015 12:42:39 +0300 Subject: [PATCH 4/5] IDEA-43728 Provide a way to step in a chosen thread while others remain suspended --- .../debugger/engine/DebugProcessEvents.java | 10 ++++- .../debugger/engine/DebugProcessImpl.java | 40 +++++++++++++++---- .../debugger/engine/SuspendManagerImpl.java | 2 +- .../util/resources/misc/registry.properties | 1 + 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java b/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java index 4e23d5a93f97..03814017244b 100644 --- a/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java +++ b/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessEvents.java @@ -193,8 +193,14 @@ public class DebugProcessEvents extends DebugProcessImpl { // check if there is already one request with policy SUSPEND_ALL for (SuspendContextImpl context : getSuspendManager().getEventContexts()) { if (context.getSuspendPolicy() == EventRequest.SUSPEND_ALL) { - eventSet.resume(); - return; + for (Event event : eventSet) { + if (event instanceof LocatableEvent && SuspendManagerUtil.isEvaluating(getSuspendManager(), + getVirtualMachineProxy().getThreadReferenceProxy( + ((LocatableEvent)event).thread()))) { + eventSet.resume(); + return; + } + } } } } diff --git a/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessImpl.java b/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessImpl.java index 9da93a15cda6..3215d726ad36 100644 --- a/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessImpl.java +++ b/java/debugger/impl/src/com/intellij/debugger/engine/DebugProcessImpl.java @@ -56,6 +56,7 @@ import com.intellij.openapi.ui.Messages; import com.intellij.openapi.util.Disposer; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.UserDataHolderBase; +import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.wm.ToolWindowId; import com.intellij.openapi.wm.impl.status.StatusBarUtil; @@ -406,7 +407,8 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb // suspend policy to match the suspend policy of the context: // if all threads were suspended, then during stepping all the threads must be suspended // if only event thread were suspended, then only this particular thread must be suspended during stepping - stepRequest.setSuspendPolicy(suspendContext.getSuspendPolicy() == EventRequest.SUSPEND_EVENT_THREAD? EventRequest.SUSPEND_EVENT_THREAD : EventRequest.SUSPEND_ALL); + stepRequest.setSuspendPolicy(Registry.is("debugger.step.resumes.one.thread") ? EventRequest.SUSPEND_EVENT_THREAD + : suspendContext.getSuspendPolicy()); if (hint != null) { //noinspection HardCodedStringLiteral @@ -1464,7 +1466,7 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb } } - private class StepOutCommand extends ResumeCommand { + private class StepOutCommand extends StepCommand { private final int myStepSize; public StepOutCommand(SuspendContextImpl suspendContext, int stepSize) { @@ -1489,7 +1491,7 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb } } - private class StepIntoCommand extends ResumeCommand { + private class StepIntoCommand extends StepCommand { private final boolean myForcedIgnoreFilters; private final MethodFilter mySmartStepFilter; @Nullable @@ -1536,7 +1538,7 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb } } - private class StepOverCommand extends ResumeCommand { + private class StepOverCommand extends StepCommand { private final boolean myIsIgnoreBreakpoints; private final int myStepSize; @@ -1574,7 +1576,7 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb } } - private class RunToCursorCommand extends ResumeCommand { + private class RunToCursorCommand extends StepCommand { private final RunToCursorBreakpoint myRunToCursorBreakpoint; private final boolean myIgnoreBreakpoints; @@ -1622,9 +1624,27 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb } } - public abstract class ResumeCommand extends SuspendContextCommandImpl { + private abstract class StepCommand extends ResumeCommand { + public StepCommand(SuspendContextImpl suspendContext) { + super(suspendContext); + } - private final ThreadReferenceProxyImpl myContextThread; + @Override + protected void resumeAction() { + SuspendContextImpl context = getSuspendContext(); + if (context != null + && Registry.is("debugger.step.resumes.one.thread") + && context.getSuspendPolicy() == EventRequest.SUSPEND_ALL) { + getSuspendManager().resumeThread(context, myContextThread); + } + else { + super.resumeAction(); + } + } + } + + public abstract class ResumeCommand extends SuspendContextCommandImpl { + protected final ThreadReferenceProxyImpl myContextThread; public ResumeCommand(SuspendContextImpl suspendContext) { super(suspendContext); @@ -1640,10 +1660,14 @@ public abstract class DebugProcessImpl extends UserDataHolderBase implements Deb @Override public void contextAction() { showStatusText(DebuggerBundle.message("status.process.resumed")); - getSuspendManager().resume(getSuspendContext()); + resumeAction(); myDebugProcessDispatcher.getMulticaster().resumed(getSuspendContext()); } + protected void resumeAction() { + getSuspendManager().resume(getSuspendContext()); + } + public ThreadReferenceProxyImpl getContextThread() { return myContextThread; } diff --git a/java/debugger/impl/src/com/intellij/debugger/engine/SuspendManagerImpl.java b/java/debugger/impl/src/com/intellij/debugger/engine/SuspendManagerImpl.java index 0717168a41c8..da2c8a1f3a08 100644 --- a/java/debugger/impl/src/com/intellij/debugger/engine/SuspendManagerImpl.java +++ b/java/debugger/impl/src/com/intellij/debugger/engine/SuspendManagerImpl.java @@ -274,7 +274,7 @@ public class SuspendManagerImpl implements SuspendManager { @Override public void resumeThread(SuspendContextImpl context, ThreadReferenceProxyImpl thread) { - LOG.assertTrue(thread != context.getThread(), "Use resume() instead of resuming breakpoint thread"); + //LOG.assertTrue(thread != context.getThread(), "Use resume() instead of resuming breakpoint thread"); LOG.assertTrue(!context.isExplicitlyResumed(thread)); if(context.myResumedThreads == null) { diff --git a/platform/util/resources/misc/registry.properties b/platform/util/resources/misc/registry.properties index ddbd89d3f954..f275df3a6506 100644 --- a/platform/util/resources/misc/registry.properties +++ b/platform/util/resources/misc/registry.properties @@ -194,6 +194,7 @@ debugger.batch.evaluation=false debugger.compiling.evaluator=true debugger.watches.in.variables=false debugger.auto.fetch.icons=true +debugger.step.resumes.one.thread=false analyze.exceptions.on.the.fly=false analyze.exceptions.on.the.fly.description=Automatically analyze clipboard on frame activation,\ From aed3a6e47da9cebc809d61fb9d92ab14091c72aa Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Fri, 3 Jul 2015 18:57:58 +0300 Subject: [PATCH 5/5] 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"; } }