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

challenge(formatter): add bracketSpacing option matching Prettier's setting #627

Merged
merged 9 commits into from
Nov 19, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
#### New features

- Add a new option [`--line-ending`](https://biomejs.dev/reference/configuration/#formatterlineending). This option allows changing the type of line endings. Contributed by @SuperchupuDev
- Added a new option called `--bracket-spacing` to the formatter. This option allows you to control whether spaces are inserted around the brackets of object literals. [#627](https://github.com/biomejs/biome/issues/627). Contributed by @faultyserver

#### Bug fixes

Expand Down
55 changes: 55 additions & 0 deletions crates/biome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ action => {};
(action = 1) => {};
"#;

const APPLY_BRACKET_SPACING_BEFORE: &str = r#"import { Foo } from "bar";
let foo = { a, b };
const { a, b } = foo;
"#;

const APPLY_BRACKET_SPACING_AFTER: &str = r#"import {Foo} from "bar";
let foo = {a, b};
const {a, b} = foo;
"#;

// Without this, Test (windows-latest) fails with: `warning: constant `DEFAULT_CONFIGURATION_BEFORE` is never used`
#[allow(dead_code)]
const DEFAULT_CONFIGURATION_BEFORE: &str = r#"function f() {
Expand Down Expand Up @@ -649,6 +659,51 @@ fn applies_custom_arrow_parentheses() {
));
}

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

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

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

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);

drop(file);
assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"applies_custom_bracket_spacing",
fs,
console,
result,
));
}

#[test]
fn trailing_comma_parse_errors() {
let mut console = BufferConsole::default();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ The configuration that is contained inside the file `biome.json`
only in for statements where it is necessary because of ASI.
--arrow-parentheses=<always|as-needed> Whether to add non-necessary parentheses to arrow functions.
Defaults to "always".
--bracket-spacing=<true|false> Whether to insert spaces around brackets in object literals.
Defaults to true.
--javascript-formatter-enabled=<true|false> Control the formatter for JavaScript (and its super
languages) files.
--javascript-formatter-indent-style=<tab|space> The indent style applied to JavaScript (and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ The configuration that is contained inside the file `biome.json`
only in for statements where it is necessary because of ASI.
--arrow-parentheses=<always|as-needed> Whether to add non-necessary parentheses to arrow functions.
Defaults to "always".
--bracket-spacing=<true|false> Whether to insert spaces around brackets in object literals.
Defaults to true.
--javascript-formatter-enabled=<true|false> Control the formatter for JavaScript (and its super
languages) files.
--javascript-formatter-indent-style=<tab|space> The indent style applied to JavaScript (and
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
source: crates/biome_cli/tests/snap_test.rs
expression: content
---
## `file.js`

```js
import {Foo} from "bar";
let foo = {a, b};
const {a, b} = foo;

```

# Emitted Messages

```block
Formatted 1 file(s) in <TIME>
```


Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ Formatting options specific to the JavaScript files
only in for statements where it is necessary because of ASI.
--arrow-parentheses=<always|as-needed> Whether to add non-necessary parentheses to arrow functions.
Defaults to "always".
--bracket-spacing=<true|false> Whether to insert spaces around brackets in object literals.
Defaults to true.
--javascript-formatter-enabled=<true|false> Control the formatter for JavaScript (and its super
languages) files.
--javascript-formatter-indent-style=<tab|space> The indent style applied to JavaScript (and
Expand Down
130 changes: 130 additions & 0 deletions crates/biome_formatter/src/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,32 @@ pub const fn space() -> Space {
Space
}

/// Optionally inserts a single space if the given condition is true.
///
/// # Examples
///
/// ```
/// use biome_formatter::format;
/// use biome_formatter::prelude::*;
///
/// # fn main() -> FormatResult<()> {
/// let elements = format!(SimpleFormatContext::default(), [text("a"), maybe_space(true), text("b")])?;
/// let nospace = format!(SimpleFormatContext::default(), [text("a"), maybe_space(false), text("b")])?;
///
/// assert_eq!("a b", elements.print()?.as_code());
/// assert_eq!("ab", nospace.print()?.as_code());
/// # Ok(())
/// # }
/// ```
#[inline]
pub fn maybe_space(should_insert: bool) -> Option<Space> {
if should_insert {
Some(Space)
} else {
None
}
}

#[derive(Debug, Copy, Clone, Eq, PartialEq)]
pub struct Space;

Expand Down Expand Up @@ -1047,6 +1073,110 @@ pub fn soft_block_indent<Context>(content: &impl Format<Context>) -> BlockIndent
}
}

/// Conditionally adds spaces around the content if its enclosing group fits
/// within a single line and the caller suggests that the space should be added.
/// Otherwise indents the content and separates it by line breaks.
///
/// # Examples
///
/// Adds line breaks and indents the content if the enclosing group doesn't fit on the line.
///
/// ```
/// use biome_formatter::{format, format_args, LineWidth, SimpleFormatOptions};
/// use biome_formatter::prelude::*;
///
/// # fn main() -> FormatResult<()> {
/// let context = SimpleFormatContext::new(SimpleFormatOptions {
/// line_width: LineWidth::try_from(10).unwrap(),
/// ..SimpleFormatOptions::default()
/// });
///
/// let elements = format!(context, [
/// group(&format_args![
/// text("{"),
/// soft_block_indent_with_maybe_space(&format_args![
/// text("aPropertyThatExceeds"),
/// text(":"),
/// space(),
/// text("'line width'"),
/// ], false),
/// text("}")
/// ])
/// ])?;
///
/// assert_eq!(
/// "{\n\taPropertyThatExceeds: 'line width'\n}",
/// elements.print()?.as_code()
/// );
/// # Ok(())
/// # }
/// ```
///
/// Adds spaces around the content if the caller requests it and the group fits on the line.
///
/// ```
/// use biome_formatter::{format, format_args};
/// use biome_formatter::prelude::*;
///
/// # fn main() -> FormatResult<()> {
/// let elements = format!(SimpleFormatContext::default(), [
/// group(&format_args![
/// text("{"),
/// soft_block_indent_with_maybe_space(&format_args![
/// text("a"),
/// text(":"),
/// space(),
/// text("5"),
/// ], true),
/// text("}")
/// ])
/// ])?;
///
/// assert_eq!(
/// "{ a: 5 }",
/// elements.print()?.as_code()
/// );
/// # Ok(())
/// # }
/// ```
///
/// Does not add spaces around the content if the caller denies it and the group fits on the line.
/// ```
/// use biome_formatter::{format, format_args};
/// use biome_formatter::prelude::*;
///
/// # fn main() -> FormatResult<()> {
/// let elements = format!(SimpleFormatContext::default(), [
/// group(&format_args![
/// text("{"),
/// soft_block_indent_with_maybe_space(&format_args![
/// text("a"),
/// text(":"),
/// space(),
/// text("5"),
/// ], false),
/// text("}")
/// ])
/// ])?;
///
/// assert_eq!(
/// "{a: 5}",
/// elements.print()?.as_code()
/// );
/// # Ok(())
/// # }
/// ```
pub fn soft_block_indent_with_maybe_space<Context>(
content: &impl Format<Context>,
should_add_space: bool,
) -> BlockIndent<Context> {
if should_add_space {
soft_space_or_block_indent(content)
} else {
soft_block_indent(content)
}
}

/// If the enclosing `Group` doesn't fit on a single line, inserts a line break and indent.
/// Otherwise, just inserts a space.
///
Expand Down
47 changes: 46 additions & 1 deletion crates/biome_js_formatter/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ pub struct JsFormatOptions {
/// Whether to add non-necessary parentheses to arrow functions. Defaults to "always".
arrow_parentheses: ArrowParentheses,

/// Whether to insert spaces around brackets in object literals. Defaults to true.
bracket_spacing: BracketSpacing,

/// Information related to the current file
source_type: JsFileSource,
}
Expand All @@ -180,6 +183,7 @@ impl JsFormatOptions {
trailing_comma: TrailingComma::default(),
semicolons: Semicolons::default(),
arrow_parentheses: ArrowParentheses::default(),
bracket_spacing: BracketSpacing::default(),
}
}

Expand All @@ -188,6 +192,11 @@ impl JsFormatOptions {
self
}

pub fn with_bracket_spacing(mut self, bracket_spacing: BracketSpacing) -> Self {
self.bracket_spacing = bracket_spacing;
self
}

pub fn with_indent_style(mut self, indent_style: IndentStyle) -> Self {
self.indent_style = indent_style;
self
Expand Down Expand Up @@ -237,6 +246,10 @@ impl JsFormatOptions {
self.arrow_parentheses = arrow_parentheses;
}

pub fn set_bracket_spacing(&mut self, bracket_spacing: BracketSpacing) {
self.bracket_spacing = bracket_spacing;
}

pub fn set_indent_style(&mut self, indent_style: IndentStyle) {
self.indent_style = indent_style;
}
Expand Down Expand Up @@ -277,6 +290,10 @@ impl JsFormatOptions {
self.arrow_parentheses
}

pub fn bracket_spacing(&self) -> BracketSpacing {
self.bracket_spacing
}

pub fn quote_style(&self) -> QuoteStyle {
self.quote_style
}
Expand Down Expand Up @@ -339,7 +356,8 @@ impl fmt::Display for JsFormatOptions {
writeln!(f, "Quote properties: {}", self.quote_properties)?;
writeln!(f, "Trailing comma: {}", self.trailing_comma)?;
writeln!(f, "Semicolons: {}", self.semicolons)?;
writeln!(f, "Arrow parentheses: {}", self.arrow_parentheses)
writeln!(f, "Arrow parentheses: {}", self.arrow_parentheses)?;
writeln!(f, "Bracket spacing: {}", self.bracket_spacing.value())
}
}

Expand Down Expand Up @@ -639,3 +657,30 @@ impl Deserializable for ArrowParentheses {
}
}
}

#[derive(Debug, Eq, PartialEq, Clone, Copy, Hash)]
#[cfg_attr(
feature = "serde",
derive(serde::Serialize, serde::Deserialize, schemars::JsonSchema),
serde(rename_all = "camelCase")
)]
pub struct BracketSpacing(bool);

impl BracketSpacing {
/// Return the boolean value for this [BracketSpacing]
pub fn value(&self) -> bool {
self.0
}
}

impl Default for BracketSpacing {
fn default() -> Self {
Self(true)
}
}

impl From<bool> for BracketSpacing {
fn from(value: bool) -> Self {
Self(value)
}
}
11 changes: 6 additions & 5 deletions crates/biome_js_formatter/src/js/module/export_named_clause.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::prelude::*;
use biome_formatter::{format_args, write};
use biome_formatter::write;

use crate::utils::FormatStatementSemicolon;

Expand Down Expand Up @@ -31,12 +31,13 @@ impl FormatNodeRule<JsExportNamedClause> for FormatJsExportNamedClause {
[format_dangling_comments(node.syntax()).with_block_indent()]
)?;
} else {
let should_insert_space_around_brackets = f.options().bracket_spacing().value();
write!(
f,
[group(&format_args![
soft_line_indent_or_space(&specifiers.format()),
soft_line_break_or_space(),
])]
[group(&soft_block_indent_with_maybe_space(
&specifiers.format(),
should_insert_space_around_brackets
),)]
)?;
}

Expand Down