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

refactor(formatter): no trailing commas in JSON files #4803

Merged
merged 2 commits into from
Jan 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading