Miscellaneous improvements to the behavior and documentation of Visitor, Transformer, Interpreter, and friends (#1543)

* tweak docs

* add missing docstrings, have decorators error when used incorrectly

* add tests for changed behavior

* remove implementation of visit_topdown

* revert changes to _visitor_args_dec

* remove associated tests

* remove more bad tests i added

also tweak new test for decorator use

* factor repeated test

* Small phrasing update

---------

Co-authored-by: Erez Shinan <erezshin+git@gmail.com>

Author: nchammas

docs/visitors.rst

@@ -4,7 +4,7 @@ Transformers & Visitors
 Transformers & Visitors provide a convenient interface to process the
 parse-trees that Lark returns.
 
-They are used by inheriting from the correct class (visitor or transformer),
+They are used by inheriting from one of the classes described here,
 and implementing methods corresponding to the rule you wish to process. Each
 method accepts the children as an argument. That can be modified using the
 ``v_args`` decorator, which allows one to inline the arguments (akin to ``*args``),
@@ -17,9 +17,8 @@ See: `visitors.py`_
 Visitor
 -------
 
-Visitors visit each node of the tree, and run the appropriate method on it according to the node's data.
-
-They work bottom-up, starting with the leaves and ending at the root of the tree.
+Visitors visit each node of the tree and run the appropriate method on it according to the node's data.
+They can traverse top-down or bottom-up, depending on the class and method.
 
 There are two classes that implement the visitor interface:
 
@@ -45,12 +44,12 @@ Example:
 Interpreter
 -----------
 
-.. autoclass:: lark.visitors.Interpreter
-
-
 Example:
     ::
 
+        from lark.visitors import Interpreter
+
+
         class IncreaseSomeOfTheNumbers(Interpreter):
             def number(self, tree):
                 tree.children[0] += 1
@@ -59,7 +58,12 @@ Example:
                 # skip this subtree. don't change any number node inside it.
                 pass
 
-            IncreaseSomeOfTheNumbers().visit(parse_tree)
+
+        IncreaseSomeOfTheNumbers().visit(parse_tree)
+
+
+.. autoclass:: lark.visitors.Interpreter
+    :members: visit, visit_children, __default__
 
 Transformer
 -----------
@@ -98,14 +102,11 @@ Example:
 
 .. autoclass:: lark.visitors.Transformer_InPlaceRecursive
 
-v_args
-------
+Useful Utilities
+----------------
 
 .. autofunction:: lark.visitors.v_args
-
-merge_transformers
-------------------
-
+.. autofunction:: lark.visitors.visit_children_decor
 .. autofunction:: lark.visitors.merge_transformers
 
 Discard

lark/visitors.py

@@ -417,6 +417,7 @@ class Interpreter(_Decoratable, ABC, Generic[_Leaf_T, _Return_T]):
     """
 
     def visit(self, tree: Tree[_Leaf_T]) -> _Return_T:
+        "Visit the tree, starting with the root and finally the leaves (top-down)."
         # There are no guarantees on the type of the value produced by calling a user func for a
         # child will produce. So only annotate the public method and use an internal method when
         # visiting child trees.
@@ -431,22 +432,45 @@ def _visit_tree(self, tree: Tree[_Leaf_T]):
             return f(tree)
 
     def visit_children(self, tree: Tree[_Leaf_T]) -> List:
-        return [self._visit_tree(child) if isinstance(child, Tree) else child
-                for child in tree.children]
+        "Visit all the children of this tree and return the results as a list."
+        return [
+            self._visit_tree(child)
+            if isinstance(child, Tree)
+            else child
+            for child in tree.children
+        ]
 
     def __getattr__(self, name):
         return self.__default__
 
     def __default__(self, tree):
+        """
+        Default function that is called if there is no attribute matching ``tree.data``.
+
+        Can be overridden. Defaults to visiting all the tree's children.
+        """
         return self.visit_children(tree)
 
 
 _InterMethod = Callable[[Type[Interpreter], _Return_T], _R]
 
 def visit_children_decor(func: _InterMethod) -> _InterMethod:
-    "See Interpreter"
+    """
+    A wrapper around Interpreter methods. It makes the wrapped node method automatically visit the
+    node's children before proceeding with the logic you have defined for that node.
+
+    Example:
+        ::
+
+            class ProcessQuery(Interpreter):
+                @visit_children_decor
+                def query(self, tree):
+                    pass
+    """
     @wraps(func)
     def inner(cls, tree):
+        if not isinstance(cls, Interpreter):
+            raise TypeError("visit_children_decor can only be applied to Interpreter methods.")
         values = cls.visit_children(tree)
         return func(cls, values)
     return inner
@@ -511,11 +535,12 @@ def _vargs_tree(f, data, children, meta):
 
 
 def v_args(inline: bool = False, meta: bool = False, tree: bool = False, wrapper: Optional[Callable] = None) -> _DECORATOR:
-    """A convenience decorator factory for modifying the behavior of user-supplied callback methods of ``Transformer`` classes.
+    """A convenience decorator factory for modifying the behavior of user-supplied callback methods
+    of ``Transformer`` or ``Interpreter`` classes.
 
-    By default, transformer callback methods accept one argument - a list of the node's children.
+    By default, the callback methods for these classes accept one argument - a list of the node's children.
 
-    ``v_args`` can modify this behavior. When used on a ``Transformer`` class definition, it applies to
+    ``v_args`` can modify this behavior. When used on the class definition, it applies to
     all the callback methods inside it.
 
     ``v_args`` can be applied to a single method, or to an entire class. When applied to both,

tests/test_trees.py

@@ -471,6 +471,20 @@ def INT(self, value):
 
         assert MyTransformer().transform(t) is None
 
+    def test_incorrect_use_of_decorators(self):
+        class TestVisitor1(Visitor):
+            @visit_children_decor
+            def a(self, tree):
+                pass
+
+        for visit_method in [
+            TestVisitor1().visit,
+            TestVisitor1().visit_topdown,
+        ]:
+            with self.assertRaises(TypeError) as e:
+                visit_method(self.tree1)
+            self.assertIn("visit_children_decor", str(e.exception))
+            self.assertIn("Interpreter", str(e.exception))
 
 if __name__ == '__main__':
     unittest.main()