bug#74963: Ambiguous treesit named and anonymous nodes in ruby-ts-mode

[Yuan Fu <casouri@gmail.com>]

> On Dec 18, 2024, at 11:18 PM, Juri Linkov <juri@linkov.net> wrote:
> 
> [This is a separate bug report from bug#73404]
> 
>>> While testing treesit-forward-sexp-list, I discovered that
>>> thing-navigation functions are not restricted to named nodes.
>>> 
>>> I wonder if there a reason to find anonymous nodes as things?
>> 
>> We should rather ask is there any reason to not find anonymous nodes
>> as things? Even ruby-ts-mode defines a bunch of anonymous nodes as
>> sexp, no? In any case, excluding anonymous nodes from things doesn’t
>> sound right.
> 
> Indeed, there are many anonymous nodes used in ruby-ts-mode.
> 
>>> The problem was found with the node "unless" in Ruby:
>>> 
>>> unless cond
>>>   a += 1
>>> else
>>>   b -= 1
>>> end
>>> 
>>> Here the named node 'unless' has exactly the same name
>>> as the anonymous node with the text "unless":
>>> 
>>> (unless "unless" condition: (identifier)
>> 
>> I feel like Ruby’s grammar should call the named node something else,
>> like unless_statement.
> 
> Agreed, the problem is that nodes defined in Ruby’s grammar
> are too ambiguous.  There are more such nodes with the same name
> for named and anonymous: "if", "while", "until", etc.
> 
>>> Finding anonymous nodes breaks forward-sexp when point is on "unless":
>>> 
>>> un-!-less cond
>>>   a += 1
>>> else
>>>   b -= 1
>>> end
>>> 
>>> because (treesit-thing-at (point) 'sexp t) finds
>>> 
>>> #<treesit-node "unless" in 156-162>
>>> 
>>> instead of
>>> 
>>> #<treesit-node unless in 156-203>
>>> 
>>> Also this breaks backward-sexp and backward-up-list
>>> because treesit--thing-sibling finds
>>> the anonymous node "unless" as a previous sibling
>>> instead of the named node 'unless' as a parent.
>>> 
>>> Would the right solution be to check if the found thing
>>> is a named node?  With something like:
>>> 
>>> diff --git a/lisp/treesit.el b/lisp/treesit.el
>>> index 18200acf53f..9ad879ee40c 100644
>>> --- a/lisp/treesit.el
>>> +++ b/lisp/treesit.el
>>> @@ -2711,6 +2774,7 @@ treesit--thing-sibling
>>>                     (lambda (n) (>= (treesit-node-start n) pos))))
>>>         (iter-pred (lambda (node)
>>>                      (and (treesit-node-match-p node thing t)
>>> +                           (treesit-node-check node 'named)
>>>                           (funcall pos-pred node))))
>>>         (sibling nil))
>>>    (when cursor
>>> @@ -2760,6 +2824,7 @@ treesit-thing-at
>>>  (let* ((cursor (treesit-node-at pos))
>>>         (iter-pred (lambda (node)
>>>                      (and (treesit-node-match-p node thing t)
>>> +                           (treesit-node-check node 'named)
>>>                           (if strict
>>>                               (< (treesit-node-start node) pos)
>>>                             (<= (treesit-node-start node) pos))
>> 
>> A better solution IMO is to add some way to distinguish between named and
>> anonymous nodes. I can think of two ways, either add “and” and
>> “named/anonymous” predicate, so (and named “unless”) only matches the named
>> “unless” node; or we add a special syntax such that “(unless)” only matches
>> named nodes, and “\”unless\”” only matches anonymous nodes.
> 
> Either predicate or a special syntax is welcome.
> 
> This would be more handy than writing a lambda with implicit calls
> of treesit-node-check.

I’ll go with the (and named “unless”) route because after thinking about it more, “(unless)” will be hard to work with because the string predicate is actually a regexp.

Yuan