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

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

python/testData/lineMarkers/LineMarkersOnDecoratedDeclarations.py

@@ -0,0 +1,12 @@
+def decorator(x):
+    return x
+
+
+@decorator
+class MyClass:
+    pass
+
+
+@decorator
+def func():
+    pass

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

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

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<List<LineMarkerInfo<?>>> 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<LineMarkerInfo<?>> lineMarkers = DaemonCodeAnalyzerImpl.getLineMarkers(document, myFixture.getProject());
+      consumer.consume(lineMarkers);
+    }
+    finally {
+      analyzer.SHOW_METHOD_SEPARATORS = false;
+    }
+  }
+
+
+  @Nullable
+  private <T extends PsiNamedElement> T findElementByName(@NotNull String name, @NotNull Class<? extends T> cls) {
+    return SyntaxTraverser
+      .psiTraverser(myFixture.getFile())
+      .filter(cls)
+      .filter(e -> name.equals(e.getName()))
+      .first();
+  }
+
+  private static void assertHasNoSeparator(@NotNull PsiElement element, @NotNull List<LineMarkerInfo<?>> lineMarkers) {
+    assertFalse("Element " + element + " shouldn't have a method separator", hasSeparator(element, lineMarkers));
+  }
+
+  private static void assertHasSeparator(@NotNull PsiElement element, @NotNull List<LineMarkerInfo<?>> lineMarkers) {
+    assertTrue("Element " + element + " should have a method separator", hasSeparator(element, lineMarkers));
+  }
+
+  private static boolean hasSeparator(@NotNull PsiElement element, @NotNull List<LineMarkerInfo<?>> 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/";
+  }
 }