Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

struct snippet yields idempotency error #842

Open
Niols opened this issue Jan 21, 2025 · 1 comment
Open

struct snippet yields idempotency error #842

Niols opened this issue Jan 21, 2025 · 1 comment
Labels
language: ocaml OCaml formatting issues type: bug

Comments

@Niols
Copy link
Member

Niols commented Jan 21, 2025

As of 9b2de13,

struct
  type t = unit
end

gets formatted once as

structtype t =
  unit end

which is then rejected by the OCaml tree-sitter grammar.

Note that the former piece of code is not valid top-level OCaml. It does get accepted by the tree-sitter grammar, which probably is a feature so that we can highlight snippets? in which case it would make sense to also be able to format such snippets?

@Xophmeister> An opinion on this “formatting snippets” question?

@Niols Niols added language: ocaml OCaml formatting issues type: bug labels Jan 21, 2025
@Xophmeister
Copy link
Member

In general, I completely agree with you: de-contextualised snippets should be formattable, even if the result is not valid top-level code. The Tree-sitter grammar for all languages (AIUI) will always,1 however, assume it's valid top-level code and the root node of the tree will be something like program or source (in OCaml's case, it is compilation_unit).

As such, the formatting queries we write should, in principle, accommodate snippets.

However, having said that, in this case, while Tree-sitter parses your snippet, the parse tree is different to one that is correct top-level OCaml.

Parse tree for snippet:

struct
  type t = unit
end

Image

Parse tree for top-level version2 (highlight mine):

module Foo = struct
  type t = unit
end

Image

This is why the formatting is wrong, in this case: The snippet version gives a completely different parse tree to what's defined by the formatting queries. The type_definition node is close to what it should be, but even then that end token forces the parser to parse the RHS into something unexpected.

So, to extend my answer: the formatting queries we write should accommodate snippets, providing their parse tree doesn't change due to context. Otherwise, we'd have to potentially duplicate each set of queries for in-context and out-of-context, which would be a lot of work and may cause problematic interactions.

Footnotes

  1. I suppose it might be possible to define multiple valid root nodes and for Tree-sitter to assign the right one. Maybe; IDK 🤷

  2. According to ChatGPT. YMMV!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language: ocaml OCaml formatting issues type: bug
Projects
None yet
Development

No branches or pull requests

2 participants