sebastienros / esprima-dotnet

Esprima .NET (BSD license) is a .NET port of the esprima.org project. It is a standard-compliant ECMAScript parser (also popularly known as JavaScript).
BSD 3-Clause "New" or "Revised" License
425 stars 75 forks source link

Sequence expressions not formatted "correctly" #427

Open CharlieEriksen opened 9 months ago

CharlieEriksen commented 9 months ago

Version used

v3.0.2

Describe the bug

The pretty formatter handles a sequence expressions in a way that makes it hard to read. I tried looking at how this is handled, but I've gotta admit that the printing logic is a bit of voodoo for me.

To Reproduce

Consider this code:

Object.defineProperty(e, "__esModule", { value: !0 }),
  (e.STORE_NAME_KEY_VALUE = "keyValue"),
  (e.STORE_NAME_MESSAGE_LOG = "messages"),
  (e.STORE_NAME_MAIN_LOG = "log"),
  (e.STORE_NAME_INBOX_MESSAGES = "inboxMessages"),
  (e.KEY_PATH_BASE_INCREMENT = "id");

When parsed and pretty-formatted with the default options, it returns this instead:

    Object.defineProperty(e, "__esModule", { value: !0 }), e.STORE_NAME_KEY_VALUE = "keyValue", e.STORE_NAME_MESSAGE_LOG = "messages", e.STORE_NAME_MAIN_LOG = "log", e.STORE_NAME_INBOX_MESSAGES = "inboxMessages", e.KEY_PATH_BASE_INCREMENT = "id";

Note that it's stripped the parentheses, and it's now all on a single line, which is much harder to read.

Expected behavior

It should most likely retain the parentheses around each expression, and when the expression list is above a certain length, it should put each on its own line.

adams85 commented 8 months ago

It should most likely retain the parentheses around each expression

Esprima has no way to represent parenthesized expressions in the AST, so this is kinda impossible currently.

when the expression list is above a certain length, it should put each on its own line.

This can be achieved relatively easy though. But there are multiple ways to format a long sequence expr, so it's not something that Esprima should support out of the box IMO.

However, the AST to JSON conversion was designed to be customizable (via inheritance), so it shouldn't be too hard to create a custom formatter with the desired formatting for sequence exprs.

The logic of the AST to JSON conversion is not something trivial indeed, however you don't need to familiarize yourself with the whole thing to achieve what you want.

It's enough to know that the conversion logic has two main components:

So, if you decide to take a stab at it, I suggest you check out the repo and debug through this method, using a simple sequence expression as input: https://github.com/sebastienros/esprima-dotnet/blob/144ed66d802ff8686699249988ba4d4460702bc5/src/Esprima/Utils/AstToJavaScriptConverter.cs#L1484-L1492

This will help you understand what formatter methods the visitor calls and what context they pass to them. Then you can move on to subclassing KnRJavaScriptTextFormatter, override the necessary methods and add the extra logic for the custom formatting.

These pieces of code can give you further pointers:

adams85 commented 4 months ago

Just an FYI/FWIW: I released a new JS parser that is capable of preserving parentheses around expressions.

It can solve your issue partially as its feature set and public API is pretty similar to Esprima's, so migration is relatively easy. Still no formatting options for long sequence expressions though.