Skip to content

Commit

Permalink
fix(format): correctly apply trailing comma to formatting (#1944)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Mar 1, 2024
1 parent 864f915 commit 4c92db2
Show file tree
Hide file tree
Showing 41 changed files with 785 additions and 466 deletions.
17 changes: 9 additions & 8 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

```diff
<script setup lang="ts">
- import Button from "./components/Button.vue";
- import * as vueUse from "vue-use";
+ import * as vueUse from "vue-use";
+ import Button from "./components/Button.vue";
- import Button from "./components/Button.vue";
- import * as vueUse from "vue-use";
+ import * as vueUse from "vue-use";
+ import Button from "./components/Button.vue";
</script/>

<template></template>
Expand All @@ -47,10 +47,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

```diff
<script setup lang="ts">
- import Button from "./components/Button.svelte";
- import * as svelteUse from "svelte-use";
+ import * as svelteUse from "svelte-use";
+ import Button from "./components/Button.svelte";
- import Button from "./components/Button.svelte";
- import * as svelteUse from "svelte-use";
+ import * as svelteUse from "svelte-use";
+ import Button from "./components/Button.svelte";
</script/>

<div></div>
Expand Down Expand Up @@ -296,6 +296,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

- Fix [#1218](https://github.com/biomejs/biome/issues/1218), by correctly preserving empty lines in member chains.
Contributed by @ah-yu
- Fix [#1659](https://github.com/biomejs/biome/issues/1659) and [#1662](https://github.com/biomejs/biome/issues/1662), by correctly taking into account the leading comma inside the formatter options. Contributed by @ematipico

### JavaScript APIs

Expand Down
4 changes: 4 additions & 0 deletions Cargo.lock

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

8 changes: 3 additions & 5 deletions crates/biome_cli/src/execute/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,10 @@ use biome_diagnostics::Diagnostic;
use biome_diagnostics::{category, PrintDiagnostic};
use biome_fs::{BiomePath, ConfigName, FileSystemExt, OpenOptions};
use biome_json_parser::{parse_json_with_cache, JsonParserOptions};
use biome_json_syntax::JsonRoot;
use biome_json_syntax::{JsonFileSource, JsonRoot};
use biome_migrate::{migrate_configuration, ControlFlow};
use biome_rowan::{AstNode, NodeCache};
use biome_service::workspace::{
ChangeFileParams, FixAction, FormatFileParams, Language, OpenFileParams,
};
use biome_service::workspace::{ChangeFileParams, FixAction, FormatFileParams, OpenFileParams};
use biome_service::{PartialConfiguration, VERSION};
use std::borrow::Cow;
use std::ffi::OsStr;
Expand Down Expand Up @@ -65,7 +63,7 @@ pub(crate) fn run(migrate_payload: MigratePayload) -> Result<(), CliDiagnostic>
path: biome_path.clone(),
content: configuration_content.to_string(),
version: 0,
language_hint: Language::Json,
document_file_source: JsonFileSource::json().into(),
})?;

let parsed = parse_json_with_cache(
Expand Down
5 changes: 2 additions & 3 deletions crates/biome_cli/src/execute/process_file/workspace_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use crate::execute::diagnostics::{ResultExt, ResultIoExt};
use crate::execute::process_file::SharedTraversalOptions;
use biome_diagnostics::{category, Error};
use biome_fs::{BiomePath, File, OpenOptions};
use biome_service::file_handlers::Language;
use biome_service::workspace::{FileGuard, OpenFileParams};
use biome_service::workspace::{DocumentFileSource, FileGuard, OpenFileParams};
use biome_service::{Workspace, WorkspaceError};
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -37,7 +36,7 @@ impl<'ctx, 'app> WorkspaceFile<'ctx, 'app> {
let guard = FileGuard::open(
ctx.workspace,
OpenFileParams {
language_hint: Language::from_path(&biome_path),
document_file_source: DocumentFileSource::from_path(&biome_path),
path: biome_path,
version: 0,
content: input.clone(),
Expand Down
9 changes: 5 additions & 4 deletions crates/biome_cli/src/execute/std_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ use biome_diagnostics::Diagnostic;
use biome_diagnostics::PrintDiagnostic;
use biome_fs::BiomePath;
use biome_service::workspace::{
ChangeFileParams, FeaturesBuilder, FixFileParams, FormatFileParams, Language, OpenFileParams,
OrganizeImportsParams, PullDiagnosticsParams, RuleCategories, SupportsFeatureParams,
ChangeFileParams, DocumentFileSource, FeaturesBuilder, FixFileParams, FormatFileParams,
OpenFileParams, OrganizeImportsParams, PullDiagnosticsParams, RuleCategories,
SupportsFeatureParams,
};
use biome_service::WorkspaceError;
use std::borrow::Cow;
Expand Down Expand Up @@ -48,7 +49,7 @@ pub(crate) fn run<'a>(
path: biome_path.clone(),
version: 0,
content: content.into(),
language_hint: Language::default(),
document_file_source: DocumentFileSource::default(),
})?;
let printed = workspace.format_file(FormatFileParams { path: biome_path })?;

Expand All @@ -71,7 +72,7 @@ pub(crate) fn run<'a>(
path: biome_path.clone(),
version: 0,
content: content.into(),
language_hint: Language::default(),
document_file_source: DocumentFileSource::default(),
})?;
// apply fix file of the linter
let file_features = workspace.file_features(SupportsFeatureParams {
Expand Down
69 changes: 47 additions & 22 deletions crates/biome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ fn applies_custom_jsx_quote_style() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let file_path = Path::new("file.js");
let file_path = Path::new("file.jsx");
fs.insert(file_path.into(), APPLY_JSX_QUOTE_STYLE_BEFORE.as_bytes());

let result = run_cli(
Expand Down Expand Up @@ -850,17 +850,8 @@ fn applies_custom_bracket_spacing() {

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

let mut file = fs
.open(file_path)
.expect("formatting target file was removed by the CLI");

let mut content = String::new();
file.read_to_string(&mut content)
.expect("failed to read file from memory FS");

assert_eq!(content, APPLY_BRACKET_SPACING_AFTER);
assert_file_contents(&fs, file_path, APPLY_BRACKET_SPACING_AFTER);

drop(file);
assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"applies_custom_bracket_spacing",
Expand All @@ -875,7 +866,7 @@ fn applies_custom_bracket_same_line() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let file_path = Path::new("file.js");
let file_path = Path::new("file.jsx");
fs.insert(file_path.into(), APPLY_BRACKET_SAME_LINE_BEFORE.as_bytes());

let result = run_cli(
Expand All @@ -895,17 +886,8 @@ fn applies_custom_bracket_same_line() {

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

let mut file = fs
.open(file_path)
.expect("formatting target file was removed by the CLI");
assert_file_contents(&fs, file_path, APPLY_BRACKET_SAME_LINE_AFTER);

let mut content = String::new();
file.read_to_string(&mut content)
.expect("failed to read file from memory FS");

assert_eq!(content, APPLY_BRACKET_SAME_LINE_AFTER);

drop(file);
assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"applies_custom_bracket_same_line",
Expand Down Expand Up @@ -2251,6 +2233,49 @@ fn format_json_when_allow_trailing_commas() {
));
}

#[test]
fn format_json_when_allow_trailing_commas_write() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let config_json = r#"{
"json": {
"parser": { "allowTrailingCommas": true }
}
}"#;
let biome_config = "biome.json";
let code = r#"{ "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",
}"#;
let file_path = Path::new("file.json");
fs.insert(file_path.into(), code.as_bytes());
fs.insert(biome_config.into(), config_json);

let result = run_cli(
DynRef::Borrowed(&mut fs),
&mut console,
Args::from(
[
("format"),
"--write",
file_path.as_os_str().to_str().unwrap(),
]
.as_slice(),
),
);

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

assert_file_contents(&fs, Path::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!(),
"format_json_when_allow_trailing_commas_write",
fs,
console,
result,
));
}

#[test]
fn treat_known_json_files_as_jsonc_files() {
let mut fs = MemoryFileSystem::default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `file.js`
## `file.jsx`

```js
```jsx
<Foo
className={style}
reallyLongAttributeName1={longComplexValue}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `file.js`
## `file.jsx`

```js
```jsx
<div bar='foo' baz={"foo"} />;

```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@ file.json format ━━━━━━━━━━━━━━━━━━━━━
i Formatter would have printed the following content:
3 3 │ 1,
4 4 │ ],
1 1 │ {
2 │ - ····"array":·[
3 │ - ········1,
4 │ - ····],
5 │ - }
5 │ + }
6 │ +
2 │ + → "array":·[1],
3 │ + }
4 │ +
```
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `biome.json`

```json
{
"json": {
"parser": { "allowTrailingCommas": true }
}
}
```

## `file.json`

```json
{
"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",
}

```

# Emitted Messages

```block
Formatted 1 file in <TIME>. Fixed 1 file.
```


5 changes: 5 additions & 0 deletions crates/biome_css_syntax/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ version = "0.4.0"

[dependencies]
biome_rowan = { workspace = true }
schemars = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive"] }

[features]
schema = ["schemars", "biome_rowan/serde"]

[lints]
workspace = true
10 changes: 8 additions & 2 deletions crates/biome_css_syntax/src/file_source.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use biome_rowan::FileSourceError;
use std::path::Path;

#[derive(Debug, Default, Clone, Copy)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[derive(
Debug, Clone, Default, Copy, Eq, PartialEq, Hash, serde::Serialize, serde::Deserialize,
)]
pub struct CssFileSource {
// Unused until we potentially support postcss/less/sass
#[allow(unused)]
Expand All @@ -12,7 +15,10 @@ pub struct CssFileSource {
///
/// Currently, Biome only supports plain CSS, and aims to be compatible with
/// the latest Recommendation level standards.
#[derive(Debug, Default, Clone, Copy)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[derive(
Debug, Clone, Default, Copy, Eq, PartialEq, Hash, serde::Serialize, serde::Deserialize,
)]
enum CssVariant {
#[default]
Standard,
Expand Down
2 changes: 1 addition & 1 deletion crates/biome_js_parser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ tests_macros = { workspace = true }

[features]
schemars = ["dep:schemars"]
serde = ["biome_js_syntax/serde"]
serde = ["biome_js_syntax/schema"]
tests = []

# cargo-workspaces metadata
Expand Down
7 changes: 4 additions & 3 deletions crates/biome_js_syntax/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,16 @@ version = "0.4.0"
biome_console = { workspace = true }
biome_diagnostics = { workspace = true }
biome_rowan = { workspace = true }
schemars = { version = "0.8.10", optional = true }
serde = { version = "1.0.136", features = ["derive"], optional = true }
schemars = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive"] }

[dev-dependencies]
biome_js_factory = { path = "../biome_js_factory" }
biome_js_parser = { path = "../biome_js_parser" }

[features]
serde = ["dep:serde", "schemars", "biome_rowan/serde"]
schema = ["schemars", "biome_rowan/serde"]
serde = ["biome_rowan/serde"]

[lints]
workspace = true
Loading

0 comments on commit 4c92db2

Please sign in to comment.