biomejs / biome

A toolchain for web projects, aimed to provide functionalities to maintain them. Biome offers formatter and linter, usable via CLI and LSP.
https://biomejs.dev
Apache License 2.0
15.43k stars 480 forks source link

📝 The formatter cannot remove trailing commas in JSON files, unlike Prettier #926

Closed nstepien closed 8 months ago

nstepien commented 11 months ago

Environment information

CLI:
  Version:                      1.4.0
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           windows

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         unset
  JS_RUNTIME_VERSION:           "v21.2.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "npm/10.2.4"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           false
  Linter disabled:              false
  Organize imports disabled:    true
  VCS disabled:                 true

Workspace:
  Open Documents:               0

What happened?

Consider the following JSON files:

{
  "a": "b",
}
[1, 2, 3, ]

Biome CLI/VSCode extension fail to format away the trailing commas.

Expected result

Biome's formatter should remove the trailing commas in JSON files. Prettier handles this fine, so it'd improve parity, and it's very convenient when copy/pasting JSON.

Code of Conduct

ematipico commented 11 months ago

I don't think this should be considered a bug, because trailing commas aren't valid in JSON unless you explicitly tell Biome to do so: https://biomejs.dev/reference/configuration#jsonparserallowtrailingcommas

Maybe if the option is enabled? Is this the case?

nstepien commented 11 months ago

I don't think this should be considered a bug

IMO it is a bug in three ways:

Though I don't mind if you prefer to think of it as a feature.

Maybe if the option is enabled? Is this the case?

Enabling the option doesn't help.

BjoernRave commented 11 months ago

Because trailing commas are not valid in JSON, it would be nice if Biome would remove the trailing comma on format, if it exists, just like prettier does

faultyserver commented 11 months ago

I was just looking at this yesterday, and I think this is sort of similar to the case of multiple access modifiers on a typescript method, like:

class Foo {
  private private public protected bar() {};
}

That's technically invalid syntax, and Biome appropriately fails to parse it. But syntactically there's nothing actually wrong with it. It's still possible to perfectly deduce an AST and operate on it. Prettier in fact explicitly allows this in their parsing configuration (though the way it gets resolved there is that only the first modifier gets stored and the rest are removed. Biome actually stores all of them and is even more recoverable as a result).

I mention that just as another example of a case where a parsing failure prevents formatting and compatibility with Prettier. During the challenge, I'd mentioned this as well, and one of the possible ideas was to treat these kinds of errors as Syntax Correctness analyzer rules, just like a few other things already are, like disallowing super() in a class that doesn't extend anything, or no duplicate private members (like #foo).

I think both the typescript example and this case of trailing commas in JSON might fit that same kind of bill, where we know that the code isn't strictly valid, but we can also perfectly deduce the AST and operate on it safely, whether that's formatting away a comma or sorting access modifiers.

I think it might also be useful to somehow be able to indicate in the ParseResult itself whether or not the parse was "strictly" valid, but I'm not sure of a good way to do that.

Because trailing commas are not valid in JSON, it would be nice if Biome would remove the trailing comma on format, if it exists, just like prettier does

I agree with this, that we can use the formatter to basically "correct" the JSON and make it more valid as a result.

FWIW, though, Prettier doesn't actually treat JSON as JSON, and instead just parses as a JS expression with Babel and throws errors in specific invalid cases. In fact, Prettier will be completely fine if you parse [1,,,3,,,1,2,34,] as JSON, even though it's very invalid lol, and importantly it won't correct that, but will format it in a way that remains incorrect: Prettier Playground Example

Conaclos commented 11 months ago

I pretty like your idea @faultyserver. If @ematipico also agree we could open tasks to handle these cases as you suggested. I don't see any drawback to do that.

ematipico commented 11 months ago

Relying on the formatter to change the semantics of a program is risky, and it can create a pretext for similar things in the future. A formatter shouldn't change the semantics of a program, but I can see that we aren't talking about strong semantics.

I can agree to have this feature, on these conditions:

How does that sound?

nstepien commented 11 months ago

Enabling json.parser.allowTrailingCommas is fine by me. I don't think this should require an option though 🤷‍♂️

espenja commented 11 months ago

Hey. I'm new to trying out Biome, and just wanted to add my two cents as a regular user on how this feels.

I've converted my entire project now from eslint + Prettier to Biome as a test to see how it feels and works. Overall it feels pretty nice, but this issue is one of my major annoyances, having converted.

Removing a line of JSON and having Prettier auto-fix my file on save is completely effortless and requires no thought on my part. With Biome I will probably remove a line in an object, move on (because the red squiggly under 1 character is already very hard to see), and only later notice the error when my build refuses to run or something breaks. And if I do notice it, I will be annoyed because this feels like something my formatting tool should fix (especially since this is automatically fixed with JS/TS formatting with the "trailingComma": "none" option).

After having used it for so long, I had completely forgotten about the level of comfort Prettier actually provides, but these kind of things now are making me go nuts.

nstepien commented 10 months ago

I'd like to add:

I think those are fair expectations.

faultyserver commented 10 months ago

When running biome format . --write or biome check . --apply, biome should automatically remove the trailing , in JSON files.

I think this is reasonable. When running with these flags, we can enable the parser options to allow trailing commas, which the formatter and linter will automatically remove as part of their normal operation.

Conaclos commented 10 months ago

I like this idea.

SleepDevil commented 8 months ago

Any update?I still cannot remove trailing comma in json file by format file like prettier.I think it will be helpful if this feature can be added.

ematipico commented 8 months ago

Any update?I still cannot remove trailing comma in json file by format file like prettier.I think it will be helpful if this feature can be added.

We've added a new option to control the trailing comma. It will be available soon on npm

MarArMar commented 4 months ago

@ematipico Could you share the option we have to set to get it to work ?

Sec-ant commented 4 months ago

@ematipico Could you share the option we have to set to get it to work ?

@MarArMar

{
  "json": {
    "parser": {
      "allowTrailingCommas": true,
    },
    "formatter": {
      "trailingCommas": "none"
    }
  }
}
MarArMar commented 4 months ago

Perfect it worked thanks