Skip to content

Commit

Permalink
refactor(formatter): no trailing commas in JSON files (#4803)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Jan 1, 2025
1 parent f342128 commit f86999d
Show file tree
Hide file tree
Showing 29 changed files with 395 additions and 134 deletions.
5 changes: 5 additions & 0 deletions .changeset/no_more_trailing_commas_in_json_files.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
cli: major
---

# The Biome formatter doesn't add a trailing command in `.json` files, even when `json.formatter.trailingCommas` is set to `true`
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions crates/biome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2326,7 +2326,7 @@ fn format_json_trailing_commas_all() {

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_file_contents(&fs, Utf8Path::new(file_path), "{\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n}\n");
assert_file_contents(&fs, Utf8Path::new(file_path), "{\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\"\n}\n");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -2369,7 +2369,7 @@ fn format_json_trailing_commas_overrides_all() {

assert!(result.is_ok(), "run_cli returned {result:?}");

assert_file_contents(&fs, Utf8Path::new(file_path), "{\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n}\n");
assert_file_contents(&fs, Utf8Path::new(file_path), "{\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\",\n\t\"loreum_ipsum_lorem_ipsum\": \"bar\"\n}\n");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
snapshot_kind: text
---
## `biome.json`

Expand All @@ -21,7 +22,7 @@ expression: content
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar"
}

```
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
snapshot_kind: text
---
## `biome.json`

Expand Down Expand Up @@ -29,7 +30,7 @@ expression: content
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar",
"loreum_ipsum_lorem_ipsum": "bar"
}

```
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
snapshot_kind: text
---
## `files/.babelrc`

Expand Down Expand Up @@ -45,22 +46,6 @@ format ━━━━━━━━━━━━━━━━━━━━━━━━

# Emitted Messages

```block
files/.babelrc format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Formatter would have printed the following content:
1 │ -
2 │ - /*test*/·[
3 │ -
4 │ - /*·some·other·comment*/1,·2,·3]
5 │ - →
1 │ + /*test*/·[/*·some·other·comment*/·1,·2,·3]
2 │ +
```

```block
files/.eslintrc.json format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Expand All @@ -78,22 +63,6 @@ files/.eslintrc.json format ━━━━━━━━━━━━━━━━━
```

```block
files/.jshintrc format ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
× Formatter would have printed the following content:
1 │ -
2 │ - /*test*/·[
3 │ -
4 │ - /*·some·other·comment*/1,·2,·3]
5 │ - →
1 │ + /*test*/·[/*·some·other·comment*/·1,·2,·3]
2 │ +
```

```block
Checked 3 files in <TIME>. No fixes applied.
Found 3 errors.
Checked 1 file in <TIME>. No fixes applied.
Found 1 error.
```
3 changes: 3 additions & 0 deletions crates/biome_formatter_test/src/spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ impl<'a> SpecTestFile<'a> {
root_path: &'a Utf8Path,
settings: Option<UpdateSettingsParams>,
) -> Option<SpecTestFile<'a>> {
if input_file.ends_with("options.json") {
return None;
}
let mut console = EnvConsole::default();
let app = App::with_console(&mut console);
let file_path = &input_file;
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_formatter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl CstFormatContext for JsFormatContext {
}
}

#[derive(Debug, Clone)]
#[derive(Debug, Default, Clone)]
pub struct JsFormatOptions {
/// The indent style.
indent_style: IndentStyle,
Expand Down
1 change: 1 addition & 0 deletions crates/biome_json_formatter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ biome_fs = { path = "../biome_fs" }
biome_json_parser = { path = "../biome_json_parser" }
biome_parser = { path = "../biome_parser" }
biome_service = { path = "../biome_service" }
biome_test_utils = { path = "../biome_test_utils" }
countme = { workspace = true, features = ["enable"] }
serde_json = { workspace = true }
tests_macros = { path = "../tests_macros" }
Expand Down
16 changes: 12 additions & 4 deletions crates/biome_json_formatter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use biome_formatter::{
CstFormatContext, FormatContext, FormatOptions, IndentStyle, LineEnding, LineWidth,
TransformSourceMap,
};
use biome_json_syntax::JsonLanguage;
use biome_json_syntax::{JsonFileSource, JsonLanguage};
use std::default::Default;
use std::fmt;
use std::rc::Rc;
Expand Down Expand Up @@ -67,6 +67,9 @@ pub struct JsonFormatOptions {
attribute_position: AttributePosition,
/// Print trailing commas wherever possible in multi-line comma-separated syntactic structures. Defaults to "none".
trailing_commas: TrailingCommas,

/// The kind of file
file_source: JsonFileSource,
}

#[derive(Clone, Copy, Debug, Default, Eq, Hash, Deserializable, Merge, PartialEq)]
Expand All @@ -78,9 +81,9 @@ pub struct JsonFormatOptions {
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
pub enum TrailingCommas {
#[default]
/// The formatter will remove the trailing commas
/// The formatter will remove the trailing commas.
None,
/// The trailing commas are allowed and advised
/// The trailing commas are allowed and advised only in JSONC files. Trailing commas are removed from JSON files.
All,
}

Expand All @@ -106,8 +109,9 @@ impl fmt::Display for TrailingCommas {
}

impl JsonFormatOptions {
pub fn new() -> Self {
pub fn new(file_source: JsonFileSource) -> Self {
Self {
file_source,
..Default::default()
}
}
Expand Down Expand Up @@ -163,6 +167,10 @@ impl JsonFormatOptions {
TrailingCommas::All => TrailingSeparator::Allowed,
}
}

pub(crate) fn file_source(&self) -> &JsonFileSource {
&self.file_source
}
}

impl FormatOptions for JsonFormatOptions {
Expand Down
17 changes: 14 additions & 3 deletions crates/biome_json_formatter/src/json/lists/array_element_list.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use crate::prelude::*;
use crate::separated::FormatAstSeparatedListExtension;
use biome_formatter::separated::TrailingSeparator;
use biome_formatter::write;
use biome_json_syntax::{AnyJsonValue, JsonArrayElementList};
use biome_json_syntax::{AnyJsonValue, JsonArrayElementList, JsonFileVariant};
use biome_rowan::{AstNode, AstSeparatedList};

#[derive(Debug, Clone, Default)]
Expand All @@ -18,7 +19,12 @@ impl FormatRule<JsonArrayElementList> for FormatJsonArrayElementList {

match layout {
ArrayLayout::Fill => {
let trailing_separator = f.options().to_trailing_separator();
let file_source = f.options().file_source();
let trailing_separator = if file_source.variant() == JsonFileVariant::Standard {
TrailingSeparator::Omit
} else {
f.options().to_trailing_separator()
};
let mut filler = f.fill();

for (element, formatted) in node
Expand All @@ -41,7 +47,12 @@ impl FormatRule<JsonArrayElementList> for FormatJsonArrayElementList {
}

ArrayLayout::OnePerLine => {
let trailing_separator = f.options().to_trailing_separator();
let file_source = f.options().file_source();
let trailing_separator = if file_source.variant() == JsonFileVariant::Standard {
TrailingSeparator::Omit
} else {
f.options().to_trailing_separator()
};
let mut join = f.join_nodes_with_soft_line();

for (element, formatted) in node
Expand Down
10 changes: 8 additions & 2 deletions crates/biome_json_formatter/src/json/lists/member_list.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::prelude::*;
use crate::separated::FormatAstSeparatedListExtension;
use biome_json_syntax::JsonMemberList;
use biome_formatter::separated::TrailingSeparator;
use biome_json_syntax::{JsonFileVariant, JsonMemberList};
use biome_rowan::{AstNode, AstSeparatedList};

#[derive(Debug, Clone, Default)]
Expand All @@ -9,7 +10,12 @@ pub(crate) struct FormatJsonMemberList;
impl FormatRule<JsonMemberList> for FormatJsonMemberList {
type Context = JsonFormatContext;
fn fmt(&self, node: &JsonMemberList, f: &mut JsonFormatter) -> FormatResult<()> {
let trailing_separator = f.options().to_trailing_separator();
let file_source = f.options().file_source();
let trailing_separator = if file_source.variant() == JsonFileVariant::Standard {
TrailingSeparator::Omit
} else {
f.options().to_trailing_separator()
};
let mut join = f.join_nodes_with_soft_line();

for (element, formatted) in node
Expand Down
9 changes: 7 additions & 2 deletions crates/biome_json_formatter/tests/spec_test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use biome_formatter_test::spec::{SpecSnapshot, SpecTestFile};
use biome_json_formatter::{context::JsonFormatOptions, JsonFormatLanguage};
use biome_json_formatter::JsonFormatLanguage;
use biome_json_syntax::JsonLanguage;
use biome_test_utils::create_formatting_options;
use camino::Utf8Path;

mod language {
Expand Down Expand Up @@ -29,8 +31,11 @@ pub fn run(spec_input_file: &str, _expected_file: &str, test_directory: &str, _f
let Some(test_file) = SpecTestFile::try_from_file(spec_input_file, root_path, None) else {
return;
};
let mut diagnostics = vec![];

let options =
create_formatting_options::<JsonLanguage>(test_file.input_file(), &mut diagnostics);

let options = JsonFormatOptions::default();
let language = language::JsonTestFormatLanguage::default();

let snapshot = SpecSnapshot::new(
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_json_formatter/tests/spec_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ mod spec_test;
mod formatter {

mod json_module {
tests_macros::gen_tests! {"tests/specs/json/**/*.json", crate::spec_test::run, ""}
tests_macros::gen_tests! {"tests/specs/json/**/*.{json,jsonc}", crate::spec_test::run, ""}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"list": [],
"list1": [],
"list2": [],
"list3": [],
"list4": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
---
source: crates/biome_formatter_test/src/snapshot_builder.rs
info: json/trailing_comma_never/classic.json
snapshot_kind: text
---
# Input

```json
{
"list": [],
"list1": [],
"list2": [],
"list3": [],
"list4": []
}
```


=============================

# Outputs

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Trailing commas: None
-----

```json
{
"list": [],
"list1": [],
"list2": [],
"list3": [],
"list4": []
}
```

## Output 1

-----
Indent style: Tab
Indent width: 2
Line ending: LF
Line width: 80
Trailing commas: All
-----

```json
{
"list": [],
"list1": [],
"list2": [],
"list3": [],
"list4": []
}
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"list": [],
"list1": [],
"list2": [],
"list3": [],
"list4": [],
}
Loading

0 comments on commit f86999d

Please sign in to comment.