Failure to properly handle return type metadata?

[sogaiu]

As reported by @rrudakov in this clojure-ts-mode issue, the current tree-sitter-clojure grammar might be seen to not properly handle return type metadata from a semantic perspective.

As a concrete example (slightly edited from the original), consider:

(defn to-string
  ^String
  [arg]
  (.toString arg))

^String here refers to the return type of the function. A straight-forward interpretation might consider the metadata to not be for the argument vector [arg]. (Possibly one could choose to see it that way though as argument information and return type information have to do with the input(s) and output(s) of a function and in some languages end up being expressed right next to each other.)

For reference, at clojure.org, this is currently documented at the Type Hints section of the Java Interop page:

For function return values, the type hint can be placed before the parameter vector:

(defn hinted-single ^String [])

-> #user/hinted-single

(defn hinted
  (^String [])
  (^Integer [a])
  (^java.util.List [a & args]))

-> #user/hinted

In a comment in the aforementioned issue, an alternative approach to parsing metadata was mentioned:

I think having metadata as standalone nodes would solve a lot of code navigation issues at least in Emacs.

The clojure-ts-mode issue had this comment as well:

Expressions with metadata (BTW, I really don't like how the grammar works with metadata, it's pain to work with and causes more problems than benefits)

Additional details were elaborated upon in this comment.

So the proposed alternative approach for handling metadata may have multiple motivations.

I suspect this would involve a significant reworking of the grammar. May be creating a new grammar would allow additional improvements to be incorporated such as this issue as well as an additional point made about open nodes [1].

[1] Also made in this comment.

[NoahTheDuke]

I suspect this would involve a significant reworking of the grammar.

Why would that be?

[sogaiu]

I'd like to jot down some further thoughts and the results of some investigations before attempting to answer:

I suspect this would involve a significant reworking of the grammar.

Why would that be?

---

One thing to note is that "the return type being specifiable via metadata that appears before the argument vector" appears to be a "post-read" thing in Clojure. AFAIU, the "ordinary" (other?) metadata cases are handled in LispReader.java during read or similar.

I think this line in core.clj may be where the identified return type case happens. (I'm not quite sure about this yet and might look into it further.)

Assuming this is correct for the moment, I think it's not unreasonable for the parse results from tree-sitter to associate the metadata with the argument vector because the metadata in question does actually live on the argument vector (at least initially -- I haven't checked whether it goes away or is removed).

---

On a different note, I thought about doing a "standalone metadata nodes" grammar a bit more and I think I may have tried this before [1]. I'm not sure what the results were unfortunately.

I thought it might have adverse impacts on things like the handling of #_ and what it corresponds to, but I'm not sure. Perhaps it can be made to work.

---

[1] ...though I'm not having luck turning up any code for that.

[sogaiu]

the metadata in question does actually live on the argument vector (at least initially -- I haven't checked whether it goes away or is removed).

I think the following is consistent with the supposition above about the "return type metadata" living on the argument vector (some lines added to improve readability):

$ clj
Clojure 1.12.0

user=> (def res (read-string "(defn to-string ^String [arg] (.toString arg))"))
#'user/res

user=> (prn res)
(defn to-string [arg] (.toString arg))
nil

user=> (binding [*print-meta* true] (prn res))
(defn to-string ^String [arg] (.toString arg))
nil

user=> (binding [*print-meta* true] (prn (nth res 2)))
^String [arg]
nil

So although it might seem a bit weird, I'm inclined to say that there is a kind of consistency and correctness to the way metadata is handled currently even in the return type case.

[sogaiu]

To return to this question:

I suspect this would involve a significant reworking of the grammar.

Why would that be?

Looking over grammar.js, there are more than 10 node types that currently "include" metadata. They are:

• sym_lit
• list_lit
• map_lit
• vec_lit
• set_lit
• read_cond_lit
• splicing_read_cond_lit
• ns_map_lit
• var_quoting_lit
• evaling_lit
• tagged_or_ctor_lit
• derefing_lit
• quoting_lit
• syn_quoting_lit
• unquote_splicing_lit
• unquoting_lit

At minimum I think one approach might include the following changes:

• The node types mentioned above might all change to not contain metadata
• A replacement metadata node would be allowed to appear pretty much anywhere (like _ws, comment, and dis_expr perhaps)

Whether one considers this a significant reworking or not, it doesn't seem like a compatible change nor is it a change to a small number of node types.

---

Assuming a second grammar / parser that implements the sort of idea described above is feasible...IIUC a number of editors support the use of more than one parser per buffer (at least to handle "injections", for example clojure-ts-mode seems to do this for docstrings and regular expressions) so it might be possible to use two parsers alongside each other for non-injection activities.

One idea is for a second (newer) parser to handle certain tasks that are awkward for the first parser and gradually migrate existing functionality over to the newer parser's functionality if that seems worth it. Alternatively, one might just decide to use both.

I think at least in Emacs it might be a practical path.

As mentioned elsewhere [1], if one were going to make a new grammar / parser though, it might be good to collect other "failings" or "wishes" related to the current parser.

---

[1] At least in this comment (string_contents node) and in this one (related to the open field).

[rrudakov]

I thought it might have adverse impacts on things like the handling of #_ and what it corresponds to, but I'm not sure. Perhaps it can be made to work.

Currently in the dis_expr kind of absorbs the following node including metadata:

(defn foo
  #_^{:bla "meta"}
  [arg]
  body)

(list_lit open: (
  value: (sym_lit name: (sym_name))
  value: (sym_lit name: (sym_name))
  (dis_expr marker: #_
   value: 
    (vec_lit
     meta: 
      (meta_lit marker: ^
       value: 
        (map_lit open: {
         value: (kwd_lit marker: : name: (kwd_name))
         value: (str_lit) close: }))
     open: [
     value: (sym_lit name: (sym_name))
     close: ]))
  value: (sym_lit name: (sym_name))
  close: ))

And if there is more than one dis_expr, they are nested, which is interesting, but I guess valid:

(if ^boolean (= 2 2)
  #_#_true
  false)

(list_lit open: (
  value: (sym_lit name: (sym_name))
  value: 
   (list_lit
    meta: 
     (meta_lit marker: ^
      value: (sym_lit name: (sym_name)))
    open: (
    value: (sym_lit name: (sym_name))
    value: (num_lit) value: (num_lit) close: ))
  (dis_expr marker: #_
   (dis_expr marker: #_ value: (bool_lit))
   value: (bool_lit))
  close: ))

If meta_lit was a standalone node, the dis_expr node would have to skip zero or more meta_lit nodes and wrap the first non-meta node, is it possible to implement in the grammar?

---

I think the following is consistent with the supposition above about the "return type metadata" living on the argument vector

Looks like you're right, I also tried to following in REPL:

user=> (defn to-string #_ ^String [arg] (.toString arg))
Syntax error macroexpanding clojure.core/defn at (REPL:1:1).
.toString - failed: vector? at: [:fn-tail :arity-n :bodies :params] spec: :clojure.core.specs.alpha/param-list
(.toString arg) - failed: vector? at: [:fn-tail :arity-1 :params] spec: :clojure.core.specs.alpha/param-list

[sogaiu]

More later, but regarding this:

And if there is more than one dis_expr, they are nested, which is interesting, but I guess valid

To give some background for this, some people make use of "stacked" #_ as part of the ability to conveniently disable bits of code like in the following cases:

(let [x 1
      #_ #_ y 2]
  x)

{:a 1
 :b 2
 #_ #_ :c 3}

It's easier to put the multiple #_ instances together (and remove them) than to try to interleave them.

To spell things out a bit, in the latter example, interleaving would look like:

{:a 1
 :b 2
 #_ :c #_ 3}

which achieves the same end but is more awkward to put into place as well as remove.

AFAIU, the current grammar "understands" this "stacked" usage...at least the intent was for it do so (^^;

[sogaiu]

If meta_lit was a standalone node, the dis_expr node would have to skip zero or more meta_lit nodes and wrap the first non-meta node, is it possible to implement in the grammar?

I'm not sure -- with tree-sitter, it is oftern trial and error.

What I wrote above:

A replacement metadata node would be allowed to appear pretty much anywhere (like _ws, comment, and dis_expr perhaps)

may be close to what you described. If that worked, it might yield a similar result to the idea of dis_expr skipping "zero or more meta_lit nodes".

dis_expr currently looks like:

dis_expr: $ =>
seq(field('marker', "#_"),
    repeat($._gap),
    field('value', $._form)),

It already "skips" zero or more _gap nodes which are effectively _ws, comment, and/or dis_expr nodes:

_gap: $ =>
choice($._ws,
       $.comment,
       $.dis_expr),

In case unknown / unfamiliar, in tree-sitter-land, it's often the case that one prioritizes parsing correct code over trying to forbid incorrect code so the idea of allowing metadata nodes to be pretty much anywhere isn't as strange as it might seem.

---

As to whether this alternative approach can be made to work (and work well), I think someone would have to try. FWIW, the kinds of issues I have encountered before include:

• Unable to get the grammar to be accepted by the tree-sitter tooling (the equivalent of "won't compile")
• Error rates are too high [1] (the equivalent of too many test failures)
• Parsing is too slow

---

[1] I found this with at least one of the original grammars -- which is one of the motivations for creating yet another grammar.

[sogaiu]

I also tried to following in REPL

Thanks for the confirmation. Good to see another method of verifying.

[sogaiu]

I tried making some of the discussed changes and pushed de52a1f6 to a new branch for experimenting with standalone metadata nodes.

[rrudakov]

I tried making some of the discussed changes and pushed de52a1f6 to a new branch for experimenting with standalone metadata nodes.

Thank you! I'll check it out.

[rrudakov]

I tried making some of the discussed changes and pushed de52a1f6 to a new branch for experimenting with standalone metadata nodes.

At the first glance, besides few minor issues, it looks great! Navigation works as expected. I'll try to do some more experiments with clojure-ts-mode in a separate branch and I'll post here if I find anything broken.

Thank you @sogaiu !

[sogaiu]

Good to hear it seems to be more or less working :)

Curious about the "few minor issues", any hints about those?

[sogaiu]

BTW, I managed to find an older attempt at simplifying the grammar in various ways -- it was in a separate repository.

I've cleaned it up a bit and made a few changes.

Notable differences include:

• #_ and metadata (actually ^ and #^) can appear anywhere whitespace and comments can
• As it predates the support for picking apart namespaced symbols and keywords, symbols and keywords are "atomic"
• No fields
• set_lit and anon_fn_lit start with #{ and #( respectively (i.e. single anonymous nodes consisting of two characters each instead of two nodes where one is associated with # and the other with an opening delimiter)
• It has a different name -- within emacs lisp files it should be referred to via 'clojure-simple instead of 'clojure. This was done to allow co-existence of the grammars.

[rrudakov]

Good to hear it seems to be more or less working :)

Curious about the "few minor issues", any hints about those?

Sorry, it probably was confusing. There are no issues with the grammar itself. I meant that clojure-ts-mode didn't work with the modified grammar out of the box. I had to modify a few queries here and there and cleanup some things related to skipping metadata.

[rrudakov]

BTW, I managed to find an older attempt at simplifying the grammar in various ways -- it was in a separate repository.

I've cleaned it up a bit and made a few changes.

Notable differences include:

• #_ and metadata (actually ^ and #^) can appear anywhere whitespace and comments can
• As it predates the support for picking apart namespaced symbols and keywords, symbols and keywords are "atomic"
• No fields
• set_lit and anon_fn_lit start with #{ and #( respectively (i.e. single anonymous nodes consisting of two characters each instead of two nodes where one is associated with # and the other with an opening delimiter)
• It has a different name -- within emacs lisp files it should be referred to via 'clojure-simple instead of 'clojure. This was done to allow co-existence of the grammars.

Not sure it would be a good idea to use 2 different Clojure grammars for the same file. Also it might be a Emacs specific problem and there might be a solution (see the discussion https://debbugs.gnu.org/cgi/bugreport.cgi?bug=78458). I'll try to add sexp-default "thing" and see if it makes things better.