From e60c49bd05f41a63fb82cbe69063f5ccf4236cfe Mon Sep 17 00:00:00 2001 From: Jon Date: Sun, 19 Nov 2023 15:47:27 -0800 Subject: [PATCH] challenge(formatter): add `bracketSpacing` option matching Prettier's setting (#627) --- CHANGELOG.md | 1 + crates/biome_cli/tests/commands/format.rs | 55 +++ .../main_commands_check/check_help.snap | 2 + .../snapshots/main_commands_ci/ci_help.snap | 2 + .../applies_custom_bracket_spacing.snap | 20 ++ .../main_commands_format/format_help.snap | 2 + crates/biome_formatter/src/builders.rs | 130 ++++++++ crates/biome_js_formatter/src/context.rs | 47 ++- .../src/js/module/export_named_clause.rs | 11 +- .../src/js/module/export_named_from_clause.rs | 16 +- .../src/js/module/import_assertion.rs | 6 +- .../src/js/module/import_named_clause.rs | 20 +- .../src/js/module/named_import_specifiers.rs | 6 +- .../property_signature_type_member.rs | 2 +- .../src/ts/types/mapped_type.rs | 7 +- .../src/utils/object_like.rs | 7 +- .../src/utils/object_pattern_like.rs | 8 +- crates/biome_js_formatter/tests/language.rs | 10 +- .../js/module/array/array_nested.js.snap | 1 + .../js/module/array/binding_pattern.js.snap | 1 + .../specs/js/module/array/empty_lines.js.snap | 1 + .../specs/js/module/array/spaces.js.snap | 1 + .../specs/js/module/array/spread.js.snap | 1 + .../array_trailing_comma.js.snap | 3 + .../js/module/arrow/arrow-comments.js.snap | 2 + .../module/arrow/arrow_chain_comments.js.snap | 2 + .../js/module/arrow/arrow_function.js.snap | 2 + .../js/module/arrow/arrow_nested.js.snap | 2 + .../module/arrow/arrow_test_callback.js.snap | 2 + .../tests/specs/js/module/arrow/call.js.snap | 2 + .../specs/js/module/arrow/currying.js.snap | 2 + .../specs/js/module/arrow/params.js.snap | 2 + .../assignment/array-assignment-holes.js.snap | 1 + .../js/module/assignment/assignment.js.snap | 1 + .../assignment/assignment_ignore.js.snap | 1 + .../binding/array-binding-holes.js.snap | 21 ++ .../js/module/binding/array_binding.js.snap | 26 ++ .../module/binding/identifier_binding.js.snap | 24 ++ .../js/module/binding/object_binding.js.snap | 29 ++ .../specs/js/module/binding/options.json | 7 + .../specs/js/module/bom_character.js.snap | 1 + .../specs/js/module/call_expression.js.snap | 1 + .../tests/specs/js/module/class/class.js.snap | 1 + .../js/module/class/class_comments.js.snap | 1 + .../js/module/class/private_method.js.snap | 1 + .../tests/specs/js/module/comments.js.snap | 1 + .../declarations/variable_declaration.js.snap | 1 + .../class_members_call_decorator.js.snap | 1 + .../decorators/class_members_mixed.js.snap | 1 + .../decorators/class_members_simple.js.snap | 1 + .../js/module/decorators/class_simple.js.snap | 1 + .../class_simple_call_decorator.js.snap | 1 + .../decorators/export_default_1.js.snap | 1 + .../decorators/export_default_2.js.snap | 1 + .../decorators/export_default_3.js.snap | 1 + .../decorators/export_default_4.js.snap | 1 + .../js/module/decorators/expression.js.snap | 1 + .../js/module/decorators/multiline.js.snap | 1 + .../tests/specs/js/module/each/each.js.snap | 1 + .../bracket-spacing/export_bracket_spacing.js | 20 ++ .../export_bracket_spacing.js.snap | 132 ++++++++ .../export/bracket-spacing/options.json | 7 + .../js/module/export/class_clause.js.snap | 1 + .../module/export/expression_clause.js.snap | 1 + .../js/module/export/from_clause.js.snap | 1 + .../js/module/export/function_clause.js.snap | 1 + .../js/module/export/named_clause.js.snap | 1 + .../module/export/named_from_clause.js.snap | 1 + .../export_trailing_comma.js.snap | 3 + .../export/variable_declaration.js.snap | 1 + .../expression/binary_expression.js.snap | 1 + .../binary_range_expression.js.snap | 1 + .../expression/binaryish_expression.js.snap | 1 + .../computed-member-expression.js.snap | 1 + .../expression/conditional_expression.js.snap | 1 + .../import_meta_expression.js.snap | 2 + .../expression/literal_expression.js.snap | 1 + .../expression/logical_expression.js.snap | 1 + .../member-chain/complex_arguments.js.snap | 1 + .../expression/member-chain/computed.js.snap | 1 + .../member-chain/inline-merge.js.snap | 1 + .../member-chain/static_member_regex.js.snap | 3 +- .../module/expression/new_expression.js.snap | 1 + .../expression/post_update_expression.js.snap | 1 + .../expression/pre_update_expression.js.snap | 1 + .../expression/sequence_expression.js.snap | 1 + .../static_member_expression.js.snap | 1 + .../module/expression/this_expression.js.snap | 1 + .../expression/unary_expression.js.snap | 1 + ...unary_expression_verbatim_argument.js.snap | 1 + .../specs/js/module/function/function.js.snap | 1 + .../js/module/function/function_args.js.snap | 1 + .../module/function/function_comments.js.snap | 1 + .../tests/specs/js/module/ident.js.snap | 1 + .../js/module/import/bare_import.js.snap | 1 + .../bracket-spacing/import_bracket_spacing.js | 24 ++ .../import_bracket_spacing.js.snap | 123 +++++++ .../import/bracket-spacing/options.json | 7 + .../js/module/import/default_import.js.snap | 1 + .../js/module/import/import_call.js.snap | 1 + .../module/import/import_specifiers.js.snap | 1 + .../js/module/import/namespace_import.js.snap | 1 + .../import_trailing_comma.js.snap | 3 + .../js/module/indent-width/example-1.js.snap | 3 + .../js/module/indent-width/example-2.js.snap | 3 + .../interpreter-with-trailing-spaces.js.snap | 1 + .../tests/specs/js/module/interpreter.js.snap | 1 + .../interpreter_with_empty_line.js.snap | 1 + .../js/module/invalid/block_stmt_err.js.snap | 1 + .../js/module/invalid/if_stmt_err.js.snap | 1 + .../js/module/line-ending/line_ending.js.snap | 5 +- .../tests/specs/js/module/newlines.js.snap | 1 + .../specs/js/module/no-semi/class.js.snap | 2 + .../specs/js/module/no-semi/issue2006.js.snap | 2 + .../specs/js/module/no-semi/no-semi.js.snap | 2 + .../js/module/no-semi/private-field.js.snap | 2 + .../js/module/no-semi/semicolons-asi.js.snap | 2 + .../module/no-semi/semicolons_range.js.snap | 2 + .../specs/js/module/number/number.js.snap | 1 + .../module/number/number_with_space.js.snap | 1 + .../bracket-spacing/object_bracket_spacing.js | 22 ++ .../object_bracket_spacing.js.snap | 113 +++++++ .../object/bracket-spacing/options.json | 7 + .../js/module/object/computed_member.js.snap | 1 + .../js/module/object/getter_setter.js.snap | 1 + .../js/module/object/numeric-property.js.snap | 1 + .../specs/js/module/object/object.js.snap | 1 + .../js/module/object/object_comments.js.snap | 1 + .../module/object/octal_literals_key.js.snap | 1 + .../js/module/object/property_key.js.snap | 1 + .../object/property_object_member.js.snap | 1 + .../object_trailing_comma.js.snap | 3 + .../js/module/parentheses/parentheses.js.snap | 1 + .../range_parentheses_binary.js.snap | 1 + .../fill-array-comments.js.snap | 1 + .../range_parenthesis_after_semicol.js.snap | 1 + .../range_parenthesis_after_semicol_1.js.snap | 1 + .../tests/specs/js/module/script.js.snap | 1 + .../module/statement/block_statement.js.snap | 1 + .../js/module/statement/do_while.js.snap | 1 + .../js/module/statement/empty_blocks.js.snap | 1 + .../specs/js/module/statement/for_in.js.snap | 1 + .../js/module/statement/for_loop.js.snap | 1 + .../specs/js/module/statement/for_of.js.snap | 1 + .../js/module/statement/if_chain.js.snap | 1 + .../specs/js/module/statement/if_else.js.snap | 1 + .../specs/js/module/statement/return.js.snap | 1 + .../return_verbatim_argument.js.snap | 1 + .../js/module/statement/statement.js.snap | 1 + .../specs/js/module/statement/switch.js.snap | 1 + .../specs/js/module/statement/throw.js.snap | 1 + .../statement/try_catch_finally.js.snap | 1 + .../js/module/statement/while_loop.js.snap | 1 + .../specs/js/module/string/directives.js.snap | 3 + .../module/string/parentheses_token.js.snap | 3 + .../module/string/properties_quotes.js.snap | 3 + .../specs/js/module/string/string.js.snap | 3 + .../tests/specs/js/module/suppression.js.snap | 1 + .../specs/js/module/template/template.js.snap | 1 + .../tests/specs/js/module/with.js.snap | 1 + .../tests/specs/js/script/script.js.snap | 1 + .../specs/js/script/script_with_bom.js.snap | 1 + .../tests/specs/js/script/with.js.snap | 1 + .../tests/specs/jsx/arrow_function.jsx.snap | 1 + .../tests/specs/jsx/attributes.jsx.snap | 1 + .../tests/specs/jsx/comments.jsx.snap | 1 + .../tests/specs/jsx/conditional.jsx.snap | 1 + .../tests/specs/jsx/element.jsx.snap | 1 + .../tests/specs/jsx/fragment.jsx.snap | 1 + .../tests/specs/jsx/new-lines.jsx.snap | 1 + .../specs/jsx/parentheses_range.jsx.snap | 1 + .../jsx/quote_style/quote_style.jsx.snap | 5 + .../tests/specs/jsx/self_closing.jsx.snap | 1 + .../tests/specs/jsx/smoke.jsx.snap | 1 + .../specs/ts/arrow/arrow_parentheses.ts.snap | 2 + .../tests/specs/ts/arrow_chain.ts.snap | 1 + .../specs/ts/assignment/as_assignment.ts.snap | 1 + .../specs/ts/assignment/assignment.ts.snap | 1 + .../ts/assignment/assignment_comments.ts.snap | 1 + .../property_assignment_comments.ts.snap | 1 + .../type_assertion_assignment.ts.snap | 1 + .../ts/binding/definite_variable.ts.snap | 1 + .../tests/specs/ts/call_expression.ts.snap | 1 + .../tests/specs/ts/class/accessor.ts.snap | 1 + .../specs/ts/class/assigment_layout.ts.snap | 1 + .../ts/class/constructor_parameter.ts.snap | 1 + .../specs/ts/class/implements_clause.ts.snap | 1 + .../class/readonly_ambient_property.ts.snap | 1 + .../class_trailing_comma.ts.snap | 3 + .../tests/specs/ts/declaration/class.ts.snap | 1 + .../ts/declaration/declare_function.ts.snap | 1 + .../ts/declaration/global_declaration.ts.snap | 1 + .../specs/ts/declaration/interface.ts.snap | 1 + .../declaration/variable_declaration.ts.snap | 1 + .../tests/specs/ts/declare.ts.snap | 1 + .../tests/specs/ts/decoartors.ts.snap | 1 + .../specs/ts/decorators/class_members.ts.snap | 1 + .../specs/ts/enum/enum_trailing_comma.ts.snap | 3 + .../specs/ts/expression/as_expression.ts.snap | 1 + .../expression_bracket_spacing.ts | 68 ++++ .../expression_bracket_spacing.ts.snap | 314 ++++++++++++++++++ .../expression/bracket-spacing/options.json | 7 + .../ts/expression/non_null_expression.ts.snap | 1 + .../type_assertion_expression.ts.snap | 1 + .../ts/expression/type_expression.ts.snap | 1 + .../specs/ts/expression/type_member.ts.snap | 1 + .../parameters/function_parameters.ts.snap | 3 + .../function_trailing_comma.ts.snap | 3 + .../specs/ts/module/export_clause.ts.snap | 1 + .../module/external_module_reference.ts.snap | 1 + .../ts/module/module_declaration.ts.snap | 1 + .../ts/module/qualified_module_name.ts.snap | 1 + .../tests/specs/ts/no-semi/class.ts.snap | 2 + .../tests/specs/ts/no-semi/non-null.ts.snap | 2 + .../tests/specs/ts/no-semi/statements.ts.snap | 2 + .../tests/specs/ts/no-semi/types.ts.snap | 2 + .../ts/object/object_trailing_comma.ts.snap | 3 + .../specs/ts/parameters/parameters.ts.snap | 1 + .../tests/specs/ts/parenthesis.ts.snap | 1 + .../specs/ts/statement/empty_block.ts.snap | 1 + .../specs/ts/statement/enum_statement.ts.snap | 1 + .../specs/ts/string/parameter_quotes.ts.snap | 3 + .../tests/specs/ts/suppressions.ts.snap | 1 + .../tests/specs/ts/type/conditional.ts.snap | 1 + .../tests/specs/ts/type/import_type.ts.snap | 1 + .../specs/ts/type/intersection_type.ts.snap | 1 + .../tests/specs/ts/type/mapped_type.ts.snap | 1 + .../specs/ts/type/qualified_name.ts.snap | 1 + .../tests/specs/ts/type/template_type.ts.snap | 1 + .../type_trailing_comma.ts.snap | 3 + .../tests/specs/ts/type/union_type.ts.snap | 1 + .../tests/specs/tsx/smoke.tsx.snap | 1 + .../src/configuration/javascript/formatter.rs | 7 + .../parse/json/javascript/formatter.rs | 5 + .../src/file_handlers/javascript.rs | 11 +- crates/biome_service/src/settings.rs | 4 + editors/vscode/configuration_schema.json | 4 + .../@biomejs/backend-jsonrpc/src/workspace.ts | 4 + .../@biomejs/biome/configuration_schema.json | 4 + .../src/content/docs/internals/changelog.mdx | 1 + website/src/playground/PlaygroundLoader.tsx | 3 + website/src/playground/tabs/SettingsTab.tsx | 21 ++ website/src/playground/types.ts | 2 + website/src/playground/workers/biomeWorker.ts | 2 + .../src/playground/workers/prettierWorker.ts | 4 + 245 files changed, 1609 insertions(+), 27 deletions(-) create mode 100644 crates/biome_cli/tests/snapshots/main_commands_format/applies_custom_bracket_spacing.snap create mode 100644 crates/biome_js_formatter/tests/specs/js/module/binding/options.json create mode 100644 crates/biome_js_formatter/tests/specs/js/module/export/bracket-spacing/export_bracket_spacing.js create mode 100644 crates/biome_js_formatter/tests/specs/js/module/export/bracket-spacing/export_bracket_spacing.js.snap create mode 100644 crates/biome_js_formatter/tests/specs/js/module/export/bracket-spacing/options.json create mode 100644 crates/biome_js_formatter/tests/specs/js/module/import/bracket-spacing/import_bracket_spacing.js create mode 100644 crates/biome_js_formatter/tests/specs/js/module/import/bracket-spacing/import_bracket_spacing.js.snap create mode 100644 crates/biome_js_formatter/tests/specs/js/module/import/bracket-spacing/options.json create mode 100644 crates/biome_js_formatter/tests/specs/js/module/object/bracket-spacing/object_bracket_spacing.js create mode 100644 crates/biome_js_formatter/tests/specs/js/module/object/bracket-spacing/object_bracket_spacing.js.snap create mode 100644 crates/biome_js_formatter/tests/specs/js/module/object/bracket-spacing/options.json create mode 100644 crates/biome_js_formatter/tests/specs/ts/expression/bracket-spacing/expression_bracket_spacing.ts create mode 100644 crates/biome_js_formatter/tests/specs/ts/expression/bracket-spacing/expression_bracket_spacing.ts.snap create mode 100644 crates/biome_js_formatter/tests/specs/ts/expression/bracket-spacing/options.json diff --git a/CHANGELOG.md b/CHANGELOG.md index f64c3d32f79e..fb85d096d6c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/crates/biome_cli/tests/commands/format.rs b/crates/biome_cli/tests/commands/format.rs index 67afa485b560..0566043e9be7 100644 --- a/crates/biome_cli/tests/commands/format.rs +++ b/crates/biome_cli/tests/commands/format.rs @@ -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() { @@ -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(); diff --git a/crates/biome_cli/tests/snapshots/main_commands_check/check_help.snap b/crates/biome_cli/tests/snapshots/main_commands_check/check_help.snap index 62196411ca6c..5fbc08a73388 100644 --- a/crates/biome_cli/tests/snapshots/main_commands_check/check_help.snap +++ b/crates/biome_cli/tests/snapshots/main_commands_check/check_help.snap @@ -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= Whether to add non-necessary parentheses to arrow functions. Defaults to "always". + --bracket-spacing= Whether to insert spaces around brackets in object literals. + Defaults to true. --javascript-formatter-enabled= Control the formatter for JavaScript (and its super languages) files. --javascript-formatter-indent-style= The indent style applied to JavaScript (and diff --git a/crates/biome_cli/tests/snapshots/main_commands_ci/ci_help.snap b/crates/biome_cli/tests/snapshots/main_commands_ci/ci_help.snap index c809f32b6cf5..c4a147e5660e 100644 --- a/crates/biome_cli/tests/snapshots/main_commands_ci/ci_help.snap +++ b/crates/biome_cli/tests/snapshots/main_commands_ci/ci_help.snap @@ -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= Whether to add non-necessary parentheses to arrow functions. Defaults to "always". + --bracket-spacing= Whether to insert spaces around brackets in object literals. + Defaults to true. --javascript-formatter-enabled= Control the formatter for JavaScript (and its super languages) files. --javascript-formatter-indent-style= The indent style applied to JavaScript (and diff --git a/crates/biome_cli/tests/snapshots/main_commands_format/applies_custom_bracket_spacing.snap b/crates/biome_cli/tests/snapshots/main_commands_format/applies_custom_bracket_spacing.snap new file mode 100644 index 000000000000..785ac2dd7037 --- /dev/null +++ b/crates/biome_cli/tests/snapshots/main_commands_format/applies_custom_bracket_spacing.snap @@ -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