sogaiu / tree-sitter-clojure

Clojure(Script) grammar for tree-sitter
Creative Commons Zero v1.0 Universal
152 stars 18 forks source link

Warning emitted for rule that contains a seq with a single argument #62

Open sogaiu opened 2 months ago

sogaiu commented 2 months ago

Since commit https://github.com/tree-sitter/tree-sitter/commit/252e2a4bc09c14ef5289fad89acd2a92f02aa66a of the tree-sitter cli, the generate subcommand gives a warning for our grammar (at least at f4236d4da8aa92bc105d9c118746474c608e6af7):

Warning: rule char_lit is just a `seq` or `choice` rule with a single element. This is unnecessary.

It takes a bit of digging, but the culprit is here [1]:

const OCTAL_CHAR =
      seq("o",
          choice(seq(DIGIT, DIGIT, DIGIT),
                 seq(DIGIT, DIGIT),
                 seq(DIGIT)));

The last seq is seq(DIGIT), which does indeed have a single argument.

I don't know if this is a serious issue, but perhaps it makes sense to remove the seq there.


[1] Thanks to clason and amaanq for getting to the bottom of this. Some more background can be found here.

sogaiu commented 2 months ago

As a bit of a follow-up, I came across this text:

(char_lit) being marked as having an unnecessary seq is part of a token construct, and so the warning isn't necessary in this case.

here.

Not sure if that means the grammar is fine in its current state.

The seq in question does seem spurious to me though.

sogaiu commented 1 week ago

Apparently, the change to tree-sitter was reverted.

Might still be worth making the change at some point, but it doesn't seem like an urgent matter.