From 54563b5b5b0b20d53bc751229d9cd1da2ffc49e6 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Fri, 12 Jul 2019 21:10:13 +0300 Subject: [PATCH] PY-4311 Don't display method separators for nested functions and classes I also changed the implementation so that only the very first tokens of declarations are considered as anchors for method separators. Otherwise, they start to flicker as their corresponding class and function elements are often not re-evaluated by LineMarkerProviders on changes in a document. GitOrigin-RevId: 90488c4bd1c662634c4b6681ce78b1b64c503782 --- .../codeInsight/PyLineMarkerProvider.java | 35 ++++-- .../LineMarkersOnDecoratedDeclarations.py | 12 ++ .../SeparatorsNotDisplayedForNestedClasses.py | 23 ++++ ...eparatorsNotDisplayedForNestedFunctions.py | 27 +++++ .../overriding}/__init__.py | 0 .../overriding}/eggs/__init__.py | 0 .../overriding}/eggs/spam/__init__.py | 0 .../overriding}/eggs/spam/eggs.py | 0 .../overriding}/spam.py | 0 .../codeInsight/PyLineMarkerProviderTest.java | 103 +++++++++++++++++- 10 files changed, 189 insertions(+), 11 deletions(-) create mode 100644 python/testData/lineMarkers/LineMarkersOnDecoratedDeclarations.py create mode 100644 python/testData/lineMarkers/SeparatorsNotDisplayedForNestedClasses.py create mode 100644 python/testData/lineMarkers/SeparatorsNotDisplayedForNestedFunctions.py rename python/testData/{lineMarkerTest => lineMarkers/overriding}/__init__.py (100%) rename python/testData/{lineMarkerTest => lineMarkers/overriding}/eggs/__init__.py (100%) rename python/testData/{lineMarkerTest => lineMarkers/overriding}/eggs/spam/__init__.py (100%) rename python/testData/{lineMarkerTest => lineMarkers/overriding}/eggs/spam/eggs.py (100%) rename python/testData/{lineMarkerTest => lineMarkers/overriding}/spam.py (100%) diff --git a/python/src/com/jetbrains/python/codeInsight/PyLineMarkerProvider.java b/python/src/com/jetbrains/python/codeInsight/PyLineMarkerProvider.java index bb204818569c..f98d97fc584f 100644 --- a/python/src/com/jetbrains/python/codeInsight/PyLineMarkerProvider.java +++ b/python/src/com/jetbrains/python/codeInsight/PyLineMarkerProvider.java @@ -5,13 +5,13 @@ import com.intellij.codeInsight.daemon.DaemonCodeAnalyzerSettings; import com.intellij.codeInsight.daemon.LineMarkerInfo; import com.intellij.codeInsight.daemon.LineMarkerProvider; import com.intellij.icons.AllIcons; -import com.intellij.lang.ASTNode; import com.intellij.notebook.editor.BackedVirtualFile; import com.intellij.openapi.editor.markup.GutterIconRenderer; import com.intellij.openapi.util.NlsContexts; import com.intellij.openapi.util.NlsContexts.PopupTitle; import com.intellij.openapi.util.text.HtmlBuilder; import com.intellij.psi.PsiElement; +import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.CollectionQuery; import com.intellij.util.Function; @@ -19,10 +19,7 @@ import com.intellij.util.Query; import com.intellij.util.containers.MultiMap; import com.jetbrains.python.PyBundle; import com.jetbrains.python.PyTokenTypes; -import com.jetbrains.python.psi.PyClass; -import com.jetbrains.python.psi.PyFunction; -import com.jetbrains.python.psi.PyTargetExpression; -import com.jetbrains.python.psi.PyUtil; +import com.jetbrains.python.psi.*; import com.jetbrains.python.psi.search.PyClassInheritorsSearch; import com.jetbrains.python.psi.search.PyOverridingMethodsSearch; import com.jetbrains.python.psi.search.PySuperMethodsSearch; @@ -168,16 +165,25 @@ public class PyLineMarkerProvider implements LineMarkerProvider, PyLineSeparator @Override public LineMarkerInfo getLineMarkerInfo(final @NotNull PsiElement element) { - final ASTNode node = element.getNode(); - if (node != null && node.getElementType() == PyTokenTypes.IDENTIFIER && element.getParent() instanceof PyFunction) { + IElementType elementType = element.getNode().getElementType(); + if (elementType == PyTokenTypes.IDENTIFIER && element.getParent() instanceof PyFunction) { final PyFunction function = (PyFunction)element.getParent(); return getMethodMarker(element, function); } if (element instanceof PyTargetExpression && PyUtil.isClassAttribute(element)) { return getAttributeMarker((PyTargetExpression)element); } - if (DaemonCodeAnalyzerSettings.getInstance().SHOW_METHOD_SEPARATORS && isSeparatorAllowed(element)) { - return PyLineSeparatorUtil.addLineSeparatorIfNeeded(this, element); + + // Separators are registered only on the first leaf element of a declaration + if (!DaemonCodeAnalyzerSettings.getInstance().SHOW_METHOD_SEPARATORS || element.getFirstChild() != null) { + return null; + } + PyElement parentDeclaration = PsiTreeUtil.getParentOfType(element, PyFunction.class, PyClass.class); + if (parentDeclaration == null || element != PsiTreeUtil.getDeepestFirst(parentDeclaration)) { + return null; + } + if (isSeparatorAllowed(parentDeclaration)) { + return PyLineSeparatorUtil.addLineSeparatorIfNeeded(this, parentDeclaration); } return null; } @@ -185,7 +191,16 @@ public class PyLineMarkerProvider implements LineMarkerProvider, PyLineSeparator @Override public boolean isSeparatorAllowed(@Nullable PsiElement element) { if (element == null || element.getContainingFile().getVirtualFile() instanceof BackedVirtualFile) return false; - return element instanceof PyFunction || element instanceof PyClass; + + if (element instanceof PyClass) { + return PyUtil.isTopLevel(element); + } + else if (element instanceof PyFunction) { + if (PyUtil.isTopLevel(element)) return true; + PyClass containingClass = ((PyFunction)element).getContainingClass(); + if (containingClass != null && PyUtil.isTopLevel(containingClass)) return true; + } + return false; } @Nullable diff --git a/python/testData/lineMarkers/LineMarkersOnDecoratedDeclarations.py b/python/testData/lineMarkers/LineMarkersOnDecoratedDeclarations.py new file mode 100644 index 000000000000..6e822bd29e8e --- /dev/null +++ b/python/testData/lineMarkers/LineMarkersOnDecoratedDeclarations.py @@ -0,0 +1,12 @@ +def decorator(x): + return x + + +@decorator +class MyClass: + pass + + +@decorator +def func(): + pass diff --git a/python/testData/lineMarkers/SeparatorsNotDisplayedForNestedClasses.py b/python/testData/lineMarkers/SeparatorsNotDisplayedForNestedClasses.py new file mode 100644 index 000000000000..67c58e3da047 --- /dev/null +++ b/python/testData/lineMarkers/SeparatorsNotDisplayedForNestedClasses.py @@ -0,0 +1,23 @@ +class TopLevel1: + class NestedInClass1: + pass + + class NestedInClass2: + pass + + +class TopLevel2: + def method(self): + class NestedInMethod1: + pass + + class NestedInMethod2: + pass + + +class func(): + class NestedInFunction1: + pass + + class NestedInFunction2: + pass diff --git a/python/testData/lineMarkers/SeparatorsNotDisplayedForNestedFunctions.py b/python/testData/lineMarkers/SeparatorsNotDisplayedForNestedFunctions.py new file mode 100644 index 000000000000..e67431605ac5 --- /dev/null +++ b/python/testData/lineMarkers/SeparatorsNotDisplayedForNestedFunctions.py @@ -0,0 +1,27 @@ +def top_level1(): + def nested1(): + pass + + def nested2(): + pass + + +class MyClass: + def method1(self): + def nested_in_method1(): + pass + + def nested_in_method2(): + pass + + def method2(self): + pass + + +def top_level2(): + class MyNestedClass: + def method_of_nested_class1(self): + pass + + def method_of_nested_class2(self): + pass diff --git a/python/testData/lineMarkerTest/__init__.py b/python/testData/lineMarkers/overriding/__init__.py similarity index 100% rename from python/testData/lineMarkerTest/__init__.py rename to python/testData/lineMarkers/overriding/__init__.py diff --git a/python/testData/lineMarkerTest/eggs/__init__.py b/python/testData/lineMarkers/overriding/eggs/__init__.py similarity index 100% rename from python/testData/lineMarkerTest/eggs/__init__.py rename to python/testData/lineMarkers/overriding/eggs/__init__.py diff --git a/python/testData/lineMarkerTest/eggs/spam/__init__.py b/python/testData/lineMarkers/overriding/eggs/spam/__init__.py similarity index 100% rename from python/testData/lineMarkerTest/eggs/spam/__init__.py rename to python/testData/lineMarkers/overriding/eggs/spam/__init__.py diff --git a/python/testData/lineMarkerTest/eggs/spam/eggs.py b/python/testData/lineMarkers/overriding/eggs/spam/eggs.py similarity index 100% rename from python/testData/lineMarkerTest/eggs/spam/eggs.py rename to python/testData/lineMarkers/overriding/eggs/spam/eggs.py diff --git a/python/testData/lineMarkerTest/spam.py b/python/testData/lineMarkers/overriding/spam.py similarity index 100% rename from python/testData/lineMarkerTest/spam.py rename to python/testData/lineMarkers/overriding/spam.py diff --git a/python/testSrc/com/jetbrains/python/codeInsight/PyLineMarkerProviderTest.java b/python/testSrc/com/jetbrains/python/codeInsight/PyLineMarkerProviderTest.java index 828a9a983511..342b9a16df03 100644 --- a/python/testSrc/com/jetbrains/python/codeInsight/PyLineMarkerProviderTest.java +++ b/python/testSrc/com/jetbrains/python/codeInsight/PyLineMarkerProviderTest.java @@ -15,21 +15,33 @@ */ package com.jetbrains.python.codeInsight; +import com.intellij.codeInsight.daemon.DaemonCodeAnalyzerSettings; import com.intellij.codeInsight.daemon.GutterIconNavigationHandler; import com.intellij.codeInsight.daemon.LineMarkerInfo; +import com.intellij.codeInsight.daemon.impl.DaemonCodeAnalyzerImpl; import com.intellij.lang.ASTNode; +import com.intellij.openapi.editor.Document; import com.intellij.psi.NavigatablePsiElement; import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiNamedElement; +import com.intellij.psi.SyntaxTraverser; import com.intellij.psi.tree.TokenSet; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.util.Consumer; +import com.intellij.util.containers.ContainerUtil; import com.jetbrains.python.PyTokenTypes; import com.jetbrains.python.fixtures.PyTestCase; import com.jetbrains.python.psi.PyClass; +import com.jetbrains.python.psi.PyFunction; import com.jetbrains.python.psi.PyPossibleClassMember; import org.hamcrest.Matchers; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.junit.Assert; import javax.swing.*; import java.awt.event.MouseEvent; +import java.util.List; /** @@ -41,7 +53,7 @@ public final class PyLineMarkerProviderTest extends PyTestCase { * Checks method has "up" arrow when overrides, and this arrow works */ public void testOverriding() { - myFixture.copyDirectoryToProject("lineMarkerTest", ""); + myFixture.copyDirectoryToProject(getTestName(true), ""); myFixture.configureByFile("spam.py"); final ASTNode functionNode = myFixture.getElementAtCaret().getNode(); @@ -64,4 +76,93 @@ public final class PyLineMarkerProviderTest extends PyTestCase { Assert.assertNotNull("Function overrides other function, but no parent displayed", parentClass); Assert.assertEquals("Wrong parent class name", "Eggs", parentClass.getName()); } + + // PY-4311 + public void testSeparatorsNotDisplayedForNestedFunctions() { + doSingleFileLineMarkersTest(lineMarkers -> { + assertHasNoSeparator(findElementByName("top_level1", PyFunction.class), lineMarkers); + assertHasSeparator(findElementByName("top_level2", PyFunction.class), lineMarkers); + + assertHasNoSeparator(findElementByName("nested1", PyFunction.class), lineMarkers); + assertHasNoSeparator(findElementByName("nested2", PyFunction.class), lineMarkers); + + assertHasNoSeparator(findElementByName("method1", PyFunction.class), lineMarkers); + assertHasSeparator(findElementByName("method2", PyFunction.class), lineMarkers); + + assertHasNoSeparator(findElementByName("nested_in_method1", PyFunction.class), lineMarkers); + assertHasNoSeparator(findElementByName("nested_in_method2", PyFunction.class), lineMarkers); + + assertHasNoSeparator(findElementByName("method_of_nested_class1", PyFunction.class), lineMarkers); + assertHasNoSeparator(findElementByName("method_of_nested_class2", PyFunction.class), lineMarkers); + }); + } + + // PY-4311 + public void testSeparatorsNotDisplayedForNestedClasses() { + doSingleFileLineMarkersTest(lineMarkers -> { + assertHasNoSeparator(findElementByName("TopLevel1", PyClass.class), lineMarkers); + assertHasSeparator(findElementByName("TopLevel2", PyClass.class), lineMarkers); + + assertHasNoSeparator(findElementByName("NestedInMethod1", PyClass.class), lineMarkers); + assertHasNoSeparator(findElementByName("NestedInMethod2", PyClass.class), lineMarkers); + + assertHasNoSeparator(findElementByName("NestedInFunction1", PyClass.class), lineMarkers); + assertHasNoSeparator(findElementByName("NestedInFunction2", PyClass.class), lineMarkers); + + assertHasNoSeparator(findElementByName("NestedInClass1", PyClass.class), lineMarkers); + assertHasNoSeparator(findElementByName("NestedInClass2", PyClass.class), lineMarkers); + }); + } + + public void testLineMarkersOnDecoratedDeclarations() { + doSingleFileLineMarkersTest(lineMarkers -> { + assertHasNoSeparator(findElementByName("decorator", PyFunction.class), lineMarkers); + assertHasSeparator(findElementByName("MyClass", PyClass.class), lineMarkers); + assertHasSeparator(findElementByName("func", PyFunction.class), lineMarkers); + }); + } + + private void doSingleFileLineMarkersTest(@SuppressWarnings("BoundedWildcard") Consumer>> consumer) { + myFixture.configureByFile(getTestName(false) + ".py"); + final DaemonCodeAnalyzerSettings analyzer = DaemonCodeAnalyzerSettings.getInstance(); + analyzer.SHOW_METHOD_SEPARATORS = true; + try { + myFixture.doHighlighting(); + final Document document = myFixture.getEditor().getDocument(); + final List> lineMarkers = DaemonCodeAnalyzerImpl.getLineMarkers(document, myFixture.getProject()); + consumer.consume(lineMarkers); + } + finally { + analyzer.SHOW_METHOD_SEPARATORS = false; + } + } + + + @Nullable + private T findElementByName(@NotNull String name, @NotNull Class cls) { + return SyntaxTraverser + .psiTraverser(myFixture.getFile()) + .filter(cls) + .filter(e -> name.equals(e.getName())) + .first(); + } + + private static void assertHasNoSeparator(@NotNull PsiElement element, @NotNull List> lineMarkers) { + assertFalse("Element " + element + " shouldn't have a method separator", hasSeparator(element, lineMarkers)); + } + + private static void assertHasSeparator(@NotNull PsiElement element, @NotNull List> lineMarkers) { + assertTrue("Element " + element + " should have a method separator", hasSeparator(element, lineMarkers)); + } + + private static boolean hasSeparator(@NotNull PsiElement element, @NotNull List> lineMarkers) { + final PsiElement separatorAnchor = PsiTreeUtil.getDeepestFirst(element); + final LineMarkerInfo marker = ContainerUtil.find(lineMarkers, maker -> maker.getElement() == separatorAnchor); + return marker != null && marker.separatorPlacement != null; + } + + @Override + protected String getTestDataPath() { + return super.getTestDataPath() + "/lineMarkers/"; + } } \ No newline at end of file