From b7390da1c155f0c66007af1171b691630cd3bdc8 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Thu, 11 Jan 2024 11:51:14 +0100 Subject: [PATCH 01/14] fix(js_formatter): placements of line comments between function parentheses and body (#1485) --- CHANGELOG.md | 4 + crates/biome_js_formatter/src/comments.rs | 42 +++-- .../js/module/function/function_comments.js | 26 ++- .../module/function/function_comments.js.snap | 43 ++++- .../between-parentheses-and-function-body.js | 32 ++-- ...ween-parentheses-and-function-body.js.snap | 152 ------------------ .../src/content/docs/internals/changelog.mdx | 4 + 7 files changed, 105 insertions(+), 198 deletions(-) delete mode 100644 crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fcb2e6419f1..7c9851fe2b8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Formatter +#### Bug fixes + +- Fix [#1172](https://github.com/biomejs/biome/issues/1172). Fix placement of line comment after function expression parentheses, they are now attached to first statement in body. Contributed by @kalleep + ### JavaScript APIs ### Linter diff --git a/crates/biome_js_formatter/src/comments.rs b/crates/biome_js_formatter/src/comments.rs index 10dc33511702..1f4803259379 100644 --- a/crates/biome_js_formatter/src/comments.rs +++ b/crates/biome_js_formatter/src/comments.rs @@ -100,7 +100,7 @@ impl CommentStyle for JsCommentStyle { ) -> CommentPlacement { match comment.text_position() { CommentTextPosition::EndOfLine => handle_typecast_comment(comment) - .or_else(handle_function_declaration_comment) + .or_else(handle_function_comment) .or_else(handle_conditional_comment) .or_else(handle_if_statement_comment) .or_else(handle_while_comment) @@ -119,7 +119,7 @@ impl CommentStyle for JsCommentStyle { .or_else(handle_after_arrow_fat_arrow_comment) .or_else(handle_import_export_specifier_comment), CommentTextPosition::OwnLine => handle_member_expression_comment(comment) - .or_else(handle_function_declaration_comment) + .or_else(handle_function_comment) .or_else(handle_if_statement_comment) .or_else(handle_while_comment) .or_else(handle_try_comment) @@ -641,38 +641,32 @@ fn handle_member_expression_comment( } } -fn handle_function_declaration_comment( - comment: DecoratedComment, -) -> CommentPlacement { - let is_function_declaration = matches!( +fn handle_function_comment(comment: DecoratedComment) -> CommentPlacement { + if !matches!( comment.enclosing_node().kind(), JsSyntaxKind::JS_FUNCTION_DECLARATION | JsSyntaxKind::JS_FUNCTION_EXPORT_DEFAULT_DECLARATION - ); + | JsSyntaxKind::JS_FUNCTION_EXPRESSION + ) || !comment.kind().is_line() + { + return CommentPlacement::Default(comment); + }; - let following = match comment.following_node() { - Some(following) if is_function_declaration => following, - _ => return CommentPlacement::Default(comment), + let Some(body) = comment.following_node().and_then(JsFunctionBody::cast_ref) else { + return CommentPlacement::Default(comment); }; - // Make comments between the `)` token and the function body leading comments - // of the first non empty statement or dangling comments of the body. + // Make line comments between the `)` token and the function body leading comments + // of the first statement or dangling comments of the body. // ```javascript - // function test() /* comment */ { + // function test() // comment + // { // console.log("Hy"); // } // ``` - if let Some(body) = JsFunctionBody::cast_ref(following) { - match body - .statements() - .iter() - .find(|statement| !matches!(statement, AnyJsStatement::JsEmptyStatement(_))) - { - Some(first) => CommentPlacement::leading(first.into_syntax(), comment), - None => CommentPlacement::dangling(body.into_syntax(), comment), - } - } else { - CommentPlacement::Default(comment) + match body.statements().first() { + Some(first) => CommentPlacement::leading(first.into_syntax(), comment), + None => CommentPlacement::dangling(body.into_syntax(), comment), } } diff --git a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js index 996c1ab6901d..a39c7ca35c5c 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js +++ b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js @@ -11,7 +11,29 @@ function b() // leading comment } - function c( //some comment foo, bar, -) {} \ No newline at end of file +) {} + + +(function d() +// a +{ + return 42 +}); + +function e() +// a +{ + ; +}; + +function f() +// a +{ + a; +}; + +function h() /* a */ { + a; +}; diff --git a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap index 196967aa18ae..724e07fe5fb6 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap +++ b/crates/biome_js_formatter/tests/specs/js/module/function/function_comments.js.snap @@ -19,10 +19,33 @@ function b() // leading comment } - function c( //some comment foo, bar, ) {} + + +(function d() +// a +{ + return 42 +}); + +function e() +// a +{ + ; +}; + +function f() +// a +{ + a; +}; + +function h() /* a */ { + a; +}; + ``` @@ -65,6 +88,24 @@ function c( foo, bar, ) {} + +(function d() { + // a + return 42; +}); + +function e() { + // a +} + +function f() { + // a + a; +} + +function h() /* a */ { + a; +} ``` diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js b/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js index b5c47f27f542..c3553778f752 100644 --- a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js +++ b/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js @@ -4,22 +4,18 @@ function function_declaration() return 42 } -// FIXME -// TODO: reformat issue -// (function named() -// // this is a function -// { -// return 42 -// })(); +(function named() +// this is a function +{ + return 42 +})(); -// FIXME -// TODO: reformat issue -// (function () -// // this is a function -// { -// return 42 -// })(); +(function () +// this is a function +{ + return 42 +})(); /* anonymous declaration */ export default function () @@ -28,14 +24,12 @@ export default function () return 42 } -// FIXME -// TODO: reformat issue a = { foo() // this is a function {}, -// bar: function() -// // this is a function -// {}, + bar: function() + // this is a function + {}, } diff --git a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap b/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap deleted file mode 100644 index a09cbbd0c72a..000000000000 --- a/crates/biome_js_formatter/tests/specs/prettier/js/comments/function/between-parentheses-and-function-body.js.snap +++ /dev/null @@ -1,152 +0,0 @@ ---- -source: crates/biome_formatter_test/src/snapshot_builder.rs -info: js/comments/function/between-parentheses-and-function-body.js ---- - -# Input - -```js -function function_declaration() -// this is a function -{ - return 42 -} - -// FIXME -// TODO: reformat issue -// (function named() -// // this is a function -// { -// return 42 -// })(); - - -// FIXME -// TODO: reformat issue -// (function () -// // this is a function -// { -// return 42 -// })(); - -/* anonymous declaration */ -export default function () -// this is a function -{ - return 42 -} - -// FIXME -// TODO: reformat issue -a = { - foo() - // this is a function - {}, - -// bar: function() -// // this is a function -// {}, -} - -``` - - -# Prettier differences - -```diff ---- Prettier -+++ Biome -@@ -3,15 +3,21 @@ - return 42; - } - --(function named() { -- // this is a function -- return 42; --})(); -+// FIXME -+// TODO: reformat issue -+// (function named() -+// // this is a function -+// { -+// return 42 -+// })(); - --(function () { -- // this is a function -- return 42; --})(); -+// FIXME -+// TODO: reformat issue -+// (function () -+// // this is a function -+// { -+// return 42 -+// })(); - - /* anonymous declaration */ - export default function () { -@@ -19,12 +25,14 @@ - return 42; - } - -+// FIXME -+// TODO: reformat issue - a = { - foo() { - // this is a function - }, - -- bar: function () { -- // this is a function -- }, -+ // bar: function() -+ // // this is a function -+ // {}, - }; -``` - -# Output - -```js -function function_declaration() { - // this is a function - return 42; -} - -// FIXME -// TODO: reformat issue -// (function named() -// // this is a function -// { -// return 42 -// })(); - -// FIXME -// TODO: reformat issue -// (function () -// // this is a function -// { -// return 42 -// })(); - -/* anonymous declaration */ -export default function () { - // this is a function - return 42; -} - -// FIXME -// TODO: reformat issue -a = { - foo() { - // this is a function - }, - - // bar: function() - // // this is a function - // {}, -}; -``` - - diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 4a8b9e1e6f3e..1296923288a9 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -29,6 +29,10 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom ### Formatter +#### Bug fixes + +- Fix [#1172](https://github.com/biomejs/biome/issues/1172). Fix placement of line comment after function expression parentheses, they are now attached to first statement in body. Contributed by @kalleep + ### JavaScript APIs ### Linter From 7042ef092f9ad5a3243ee67b5001c842727d46fc Mon Sep 17 00:00:00 2001 From: Arend van Beelen jr Date: Thu, 11 Jan 2024 11:55:27 +0100 Subject: [PATCH 02/14] fix(lint/useHookAtTopLevel): fix nested components and hooks (#1517) --- CHANGELOG.md | 6 ++- crates/biome_js_analyze/src/react/hooks.rs | 23 ++++++-- .../correctness/use_hook_at_top_level.rs | 54 ++++++++++++++----- .../correctness/useHookAtTopLevel/invalid.js | 11 ++++ .../useHookAtTopLevel/invalid.js.snap | 30 +++++++++++ .../correctness/useHookAtTopLevel/valid.js | 24 +++++++++ .../useHookAtTopLevel/valid.js.snap | 24 +++++++++ .../src/semantic_model/model.rs | 19 ++----- crates/biome_js_syntax/src/union_ext.rs | 25 +++++++-- .../src/content/docs/internals/changelog.mdx | 6 ++- 10 files changed, 186 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c9851fe2b8b..e26e8af2cc3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -66,9 +66,9 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [#1335](https://github.com/biomejs/biome/issues/1335). [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) now ignores code action on component props when the fragment is empty. Contributed by @vasucp1207 -- [useConsistentArrayType](https://biomejs.dev/rules/use-consistent-array-type) was accidentally placed in the `style` rule group instead of the `nursery` group. It is now correctly placed under `nursery`. +- [useConsistentArrayType](https://biomejs.dev/linter/rules/use-consistent-array-type) was accidentally placed in the `style` rule group instead of the `nursery` group. It is now correctly placed under `nursery`. -- Fix [#1483](https://github.com/biomejs/biome/issues/1483). [useConsistentArrayType](https://biomejs.dev/rules/use-consistent-array-type) now correctly handles its option. Contributed by @Conaclos +- Fix [#1483](https://github.com/biomejs/biome/issues/1483). [useConsistentArrayType](https://biomejs.dev/linter/rules/use-consistent-array-type) now correctly handles its option. Contributed by @Conaclos - Fix [#1502](https://github.com/biomejs/biome/issues/1502). [useArrowFunction](https://biomejs.dev/rules/) now correctly handle functions that return a (comma) sequence expression. Contributed by @Conaclos @@ -86,6 +86,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom + f(() => (0, 1), "") ``` +- Fix [#1473](https://github.com/biomejs/biome/issues/1473): [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/) now correctly handles React components and hooks that are nested inside other functions. Contributed by @arendjr + ## 1.5.0 (2024-01-08) diff --git a/crates/biome_js_analyze/src/react/hooks.rs b/crates/biome_js_analyze/src/react/hooks.rs index 2c98e8bf7d34..91f83a425817 100644 --- a/crates/biome_js_analyze/src/react/hooks.rs +++ b/crates/biome_js_analyze/src/react/hooks.rs @@ -108,18 +108,35 @@ fn get_untrimmed_callee_name(call: &JsCallExpression) -> Option bool { + name.chars().next().is_some_and(char::is_uppercase) +} + +/// Checks whether the given function name belongs to a React hook, based on the /// official convention for React hook naming: Hook names must start with `use` /// followed by a capital letter. /// /// Source: https://react.dev/learn/reusing-logic-with-custom-hooks#hook-names-always-start-with-use +pub(crate) fn is_react_hook(name: &str) -> bool { + name.starts_with("use") && name.chars().nth(3).is_some_and(char::is_uppercase) +} + +/// Checks whether the given call expression calls a React hook, based on the +/// official convention for React hook naming: Hook names must start with `use` +/// followed by a capital letter. +/// +/// See [is_react_hook()]. pub(crate) fn is_react_hook_call(call: &JsCallExpression) -> bool { let Some(name) = get_untrimmed_callee_name(call) else { return false; }; - let name = name.text_trimmed(); - name.starts_with("use") && name.chars().nth(3).is_some_and(char::is_uppercase) + is_react_hook(name.text_trimmed()) } const HOOKS_WITH_DEPS_API: [&str; 6] = [ diff --git a/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_hook_at_top_level.rs b/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_hook_at_top_level.rs index b7bce896fdb9..12d1623baa0e 100644 --- a/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_hook_at_top_level.rs +++ b/crates/biome_js_analyze/src/semantic_analyzers/correctness/use_hook_at_top_level.rs @@ -1,4 +1,4 @@ -use crate::react::hooks::is_react_hook_call; +use crate::react::hooks::{is_react_component, is_react_hook, is_react_hook_call}; use crate::semantic_services::{SemanticModelBuilderVisitor, SemanticServices}; use biome_analyze::{ context::RuleContext, declare_rule, AddVisitor, FromServices, MissingServicesDiagnostic, Phase, @@ -12,10 +12,11 @@ use biome_deserialize::{ }; use biome_js_semantic::{CallsExtensions, SemanticModel}; use biome_js_syntax::{ - AnyFunctionLike, AnyJsExpression, AnyJsFunction, JsAssignmentWithDefault, - JsBindingPatternWithDefault, JsCallExpression, JsConditionalExpression, JsIfStatement, - JsLanguage, JsLogicalExpression, JsMethodObjectMember, JsObjectBindingPatternShorthandProperty, - JsReturnStatement, JsSyntaxKind, JsTryFinallyStatement, TextRange, + AnyFunctionLike, AnyJsBinding, AnyJsExpression, AnyJsFunction, AnyJsObjectMemberName, + JsAssignmentWithDefault, JsBindingPatternWithDefault, JsCallExpression, + JsConditionalExpression, JsIfStatement, JsLanguage, JsLogicalExpression, JsMethodObjectMember, + JsObjectBindingPatternShorthandProperty, JsReturnStatement, JsSyntaxKind, + JsTryFinallyStatement, TextRange, }; use biome_rowan::{declare_node_union, AstNode, Language, SyntaxNode, WalkEvent}; use rustc_hash::FxHashMap; @@ -71,6 +72,29 @@ declare_node_union! { pub AnyJsFunctionOrMethod = AnyJsFunction | JsMethodObjectMember } +impl AnyJsFunctionOrMethod { + fn is_react_component_or_hook(&self) -> bool { + if let Some(name) = self.name() { + if is_react_component(&name) || is_react_hook(&name) { + return true; + } + } + + false + } + + fn name(&self) -> Option { + match self { + AnyJsFunctionOrMethod::AnyJsFunction(function) => { + function.binding().as_ref().map(AnyJsBinding::text) + } + AnyJsFunctionOrMethod::JsMethodObjectMember(method) => { + method.name().ok().as_ref().map(AnyJsObjectMemberName::text) + } + } + } +} + pub enum Suggestion { None { hook_name_range: TextRange, @@ -189,14 +213,19 @@ fn is_conditional_expression( ) } -fn is_nested_function(function: &AnyJsFunctionOrMethod) -> bool { +fn is_nested_function_inside_component_or_hook(function: &AnyJsFunctionOrMethod) -> bool { + if function.is_react_component_or_hook() { + return false; + } + let Some(parent) = function.syntax().parent() else { return false; }; - parent - .ancestors() - .any(|node| AnyJsFunctionOrMethod::can_cast(node.kind())) + parent.ancestors().any(|node| { + AnyJsFunctionOrMethod::cast(node) + .is_some_and(|enclosing_function| enclosing_function.is_react_component_or_hook()) + }) } /// Model for tracking which function calls are preceeded by an early return. @@ -400,9 +429,10 @@ impl Rule for UseHookAtTopLevel { path.push(range); if let Some(enclosing_function) = enclosing_function_if_call_is_at_top_level(&call) { - if is_nested_function(&enclosing_function) { - // We cannot allow nested functions, since it would break - // the requirement for hooks to be called from the top-level. + if is_nested_function_inside_component_or_hook(&enclosing_function) { + // We cannot allow nested functions inside hooks and + // components, since it would break the requirement for + // hooks to be called from the top-level. return Some(Suggestion::None { hook_name_range: get_hook_name_range()?, path, diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js index 826cc3937eb4..da348b4b2653 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js @@ -155,3 +155,14 @@ function useHookInsideArrayAssignmentInitializer(props) { function useHookInsideArrayBindingInitializer(props) { const [item = useDefaultItem()] = props.array; } + +test('b', () => { + const TestComponent = () => { + useState(); + const handler = () => { + useHook(); + }; + }; + + render(); +}); diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js.snap b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js.snap index d98b028f4239..84d8503d0eaf 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js.snap +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.js.snap @@ -162,6 +162,17 @@ function useHookInsideArrayBindingInitializer(props) { const [item = useDefaultItem()] = props.array; } +test('b', () => { + const TestComponent = () => { + useState(); + const handler = () => { + useHook(); + }; + }; + + render(); +}); + ``` # Diagnostics @@ -770,4 +781,23 @@ invalid.js:156:19 lint/correctness/useHookAtTopLevel ━━━━━━━━━ ``` +``` +invalid.js:163:13 lint/correctness/useHookAtTopLevel ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This hook is being called from a nested function, but all hooks must be called unconditionally from the top-level component. + + 161 │ useState(); + 162 │ const handler = () => { + > 163 │ useHook(); + │ ^^^^^^^ + 164 │ }; + 165 │ }; + + i For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order. + + i See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level + + +``` + diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js index 18feff759e95..84f7de033efa 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js @@ -101,3 +101,27 @@ function Component8() { useEffect(); }; + +test('a', () => { + function TestComponent() { + useState(); + useHook(); + } + + render(); +}); + +test('b', () => { + const TestComponent = () => { + useState(); + useHook(); + }; + + render(); +}); + +test('c', () => { + const { result } = renderHook(() => useHook()); + + expect(result.current).toBeDefined(); +}); diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js.snap b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js.snap index 23d34964e5dc..86e58627acd8 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js.snap +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.js.snap @@ -108,6 +108,30 @@ function Component8() { useEffect(); }; +test('a', () => { + function TestComponent() { + useState(); + useHook(); + } + + render(); +}); + +test('b', () => { + const TestComponent = () => { + useState(); + useHook(); + }; + + render(); +}); + +test('c', () => { + const { result } = renderHook(() => useHook()); + + expect(result.current).toBeDefined(); +}); + ``` diff --git a/crates/biome_js_semantic/src/semantic_model/model.rs b/crates/biome_js_semantic/src/semantic_model/model.rs index 2bc197e04ac5..6303c45bf5ee 100644 --- a/crates/biome_js_semantic/src/semantic_model/model.rs +++ b/crates/biome_js_semantic/src/semantic_model/model.rs @@ -1,5 +1,5 @@ use super::*; -use biome_js_syntax::{AnyJsFunction, AnyJsRoot, JsInitializerClause, JsVariableDeclarator}; +use biome_js_syntax::{AnyJsFunction, AnyJsRoot}; #[derive(Copy, Clone, Debug)] pub(crate) struct BindingIndex(usize); @@ -363,20 +363,11 @@ impl SemanticModel { /// assert_eq!(2, all_calls_to_f.unwrap().count()); /// ``` pub fn all_calls(&self, function: &AnyJsFunction) -> Option { - let identifier = match function { - AnyJsFunction::JsFunctionDeclaration(declaration) => declaration.id().ok()?, - AnyJsFunction::JsFunctionExportDefaultDeclaration(declaration) => declaration.id()?, - AnyJsFunction::JsArrowFunctionExpression(_) - | AnyJsFunction::JsFunctionExpression(_) => { - let parent = function - .parent::()? - .parent::()?; - parent.id().ok()?.as_any_js_binding()?.clone() - } - }; - Some(AllCallsIter { - references: identifier.as_js_identifier_binding()?.all_reads(self), + references: function + .binding()? + .as_js_identifier_binding()? + .all_reads(self), }) } } diff --git a/crates/biome_js_syntax/src/union_ext.rs b/crates/biome_js_syntax/src/union_ext.rs index 3b644b67994c..61502601c593 100644 --- a/crates/biome_js_syntax/src/union_ext.rs +++ b/crates/biome_js_syntax/src/union_ext.rs @@ -1,10 +1,11 @@ use crate::{ AnyJsArrowFunctionParameters, AnyJsBinding, AnyJsClass, AnyJsClassMember, AnyJsClassMemberName, AnyJsFunction, AnyJsFunctionBody, AnyTsPropertyAnnotation, AnyTsVariableAnnotation, - JsClassMemberList, JsDecoratorList, JsExtendsClause, JsSyntaxToken, TsImplementsClause, - TsReturnTypeAnnotation, TsTypeAnnotation, TsTypeParameters, + JsClassMemberList, JsDecoratorList, JsExtendsClause, JsInitializerClause, JsSyntaxToken, + JsVariableDeclarator, TsImplementsClause, TsReturnTypeAnnotation, TsTypeAnnotation, + TsTypeParameters, }; -use biome_rowan::{AstSeparatedList, SyntaxResult}; +use biome_rowan::{AstNode, AstSeparatedList, SyntaxResult}; impl AnyJsClass { pub fn decorators(&self) -> JsDecoratorList { @@ -149,6 +150,24 @@ impl AnyJsFunction { } } + /// Returns the binding by which the function can be accessed. + /// + /// This may be a binding for the function's identifier, or a binding for + /// the variable to which the function is assigned. + pub fn binding(&self) -> Option { + match self { + AnyJsFunction::JsFunctionDeclaration(declaration) => declaration.id().ok(), + AnyJsFunction::JsFunctionExportDefaultDeclaration(declaration) => declaration.id(), + AnyJsFunction::JsArrowFunctionExpression(_) + | AnyJsFunction::JsFunctionExpression(_) => { + let parent = self + .parent::()? + .parent::()?; + parent.id().ok()?.as_any_js_binding().cloned() + } + } + } + pub fn is_async(&self) -> bool { self.async_token().is_some() } diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 1296923288a9..defa499410af 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -72,9 +72,9 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [#1335](https://github.com/biomejs/biome/issues/1335). [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) now ignores code action on component props when the fragment is empty. Contributed by @vasucp1207 -- [useConsistentArrayType](https://biomejs.dev/rules/use-consistent-array-type) was accidentally placed in the `style` rule group instead of the `nursery` group. It is now correctly placed under `nursery`. +- [useConsistentArrayType](https://biomejs.dev/linter/rules/use-consistent-array-type) was accidentally placed in the `style` rule group instead of the `nursery` group. It is now correctly placed under `nursery`. -- Fix [#1483](https://github.com/biomejs/biome/issues/1483). [useConsistentArrayType](https://biomejs.dev/rules/use-consistent-array-type) now correctly handles its option. Contributed by @Conaclos +- Fix [#1483](https://github.com/biomejs/biome/issues/1483). [useConsistentArrayType](https://biomejs.dev/linter/rules/use-consistent-array-type) now correctly handles its option. Contributed by @Conaclos - Fix [#1502](https://github.com/biomejs/biome/issues/1502). [useArrowFunction](https://biomejs.dev/rules/) now correctly handle functions that return a (comma) sequence expression. Contributed by @Conaclos @@ -92,6 +92,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom + f(() => (0, 1), "") ``` +- Fix [#1473](https://github.com/biomejs/biome/issues/1473): [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/) now correctly handles React components and hooks that are nested inside other functions. Contributed by @arendjr + ## 1.5.0 (2024-01-08) From 73bced135ed4f5158cf8a5635a09e0a88f798d9d Mon Sep 17 00:00:00 2001 From: Zheyu Zhang Date: Thu, 11 Jan 2024 19:04:23 +0800 Subject: [PATCH 03/14] fix(js_formatter): ensure comments before `async` keyword are placed before it (#1500) Co-authored-by: Victorien Elvinger --- CHANGELOG.md | 2 ++ crates/biome_js_formatter/src/comments.rs | 29 ++++++++++++++++++- .../class_members_call_decorator.js.snap | 3 +- .../decorators/class_members_mixed.js.snap | 3 +- .../decorators/class_members_simple.js.snap | 3 +- .../decorators/decorators-comments.ts.snap | 28 ++++-------------- .../src/content/docs/internals/changelog.mdx | 2 ++ 7 files changed, 43 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e26e8af2cc3c..58213187d0c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom #### Bug fixes +- Fix [#1406](https://github.com/biomejs/biome/issues/1406). Ensure comments before the `async` keyword are placed before it. Contributed by @ah-yu + - Fix [#1172](https://github.com/biomejs/biome/issues/1172). Fix placement of line comment after function expression parentheses, they are now attached to first statement in body. Contributed by @kalleep ### JavaScript APIs diff --git a/crates/biome_js_formatter/src/comments.rs b/crates/biome_js_formatter/src/comments.rs index 1f4803259379..4c4dbde0d427 100644 --- a/crates/biome_js_formatter/src/comments.rs +++ b/crates/biome_js_formatter/src/comments.rs @@ -133,7 +133,8 @@ impl CommentStyle for JsCommentStyle { .or_else(handle_mapped_type_comment) .or_else(handle_continue_break_comment) .or_else(handle_union_type_comment) - .or_else(handle_import_export_specifier_comment), + .or_else(handle_import_export_specifier_comment) + .or_else(handle_class_method_comment), CommentTextPosition::SameLine => handle_if_statement_comment(comment) .or_else(handle_while_comment) .or_else(handle_for_comment) @@ -1255,6 +1256,32 @@ fn handle_import_export_specifier_comment( } } +/// Ensure that comments before the `async`` keyword are placed just before it. +/// ```javascript +/// class Foo { +/// @decorator() +/// // comment +/// async method() {} +/// } +fn handle_class_method_comment( + comment: DecoratedComment, +) -> CommentPlacement { + let enclosing_node = comment.enclosing_node(); + match enclosing_node.kind() { + JsSyntaxKind::JS_METHOD_CLASS_MEMBER => { + if let Some(following_token) = comment.following_token() { + if following_token.kind() == JsSyntaxKind::ASYNC_KW { + if let Some(preceding) = comment.preceding_node() { + return CommentPlacement::trailing(preceding.clone(), comment); + } + } + } + CommentPlacement::Default(comment) + } + _ => CommentPlacement::Default(comment), + } +} + fn place_leading_statement_comment( statement: AnyJsStatement, comment: DecoratedComment, diff --git a/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_call_decorator.js.snap b/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_call_decorator.js.snap index 54765c91852c..2835f912cad5 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_call_decorator.js.snap +++ b/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_call_decorator.js.snap @@ -211,7 +211,8 @@ class Foo { @decorator.method(value) /*before*/ /*after*/ method() {} @decorator.method(value) /*before*/ - async /*after*/ method() {} + /*after*/ + async method() {} @decorator.method(value) /*before*/ */*after*/ method() {} @decorator.method(value) /*before*/ diff --git a/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_mixed.js.snap b/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_mixed.js.snap index 284f0db6b6d5..dc650d8ceef0 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_mixed.js.snap +++ b/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_mixed.js.snap @@ -209,7 +209,8 @@ class Foo { @decorator /*before*/ /*after*/ method() {} @decorator.method(value) /*before*/ - async /*after*/ method() {} + /*after*/ + async method() {} @decorator /*before*/ */*after*/ method() {} @decorator.method(value) /*before*/ diff --git a/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_simple.js.snap b/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_simple.js.snap index f4521c377a9e..303665e1278e 100644 --- a/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_simple.js.snap +++ b/crates/biome_js_formatter/tests/specs/js/module/decorators/class_members_simple.js.snap @@ -199,7 +199,8 @@ class Foo { @dec /*before*/ /*after*/ method() {} @dec /*before*/ - async /*after*/ method() {} + /*after*/ + async method() {} @dec /*before*/ */*after*/ method() {} @dec /*before*/ diff --git a/crates/biome_js_formatter/tests/specs/prettier/typescript/decorators/decorators-comments.ts.snap b/crates/biome_js_formatter/tests/specs/prettier/typescript/decorators/decorators-comments.ts.snap index 0ea5f71bba68..2bc85fd6d26d 100644 --- a/crates/biome_js_formatter/tests/specs/prettier/typescript/decorators/decorators-comments.ts.snap +++ b/crates/biome_js_formatter/tests/specs/prettier/typescript/decorators/decorators-comments.ts.snap @@ -51,17 +51,7 @@ class Something2 { ```diff --- Prettier +++ Biome -@@ -1,7 +1,7 @@ - class Foo1 { - @foo -- // comment -- async method() {} -+ async // comment -+ method() {} - } - - class Foo2 { -@@ -12,14 +12,14 @@ +@@ -12,8 +12,8 @@ class Foo3 { @foo @@ -72,14 +62,6 @@ class Something2 { } class Foo4 { - @foo -- // comment -- async *method() {} -+ async *// comment -+ method() {} - } - - class Something { @@ -30,6 +30,6 @@ class Something2 { @@ -96,8 +78,8 @@ class Something2 { ```ts class Foo1 { @foo - async // comment - method() {} + // comment + async method() {} } class Foo2 { @@ -114,8 +96,8 @@ class Foo3 { class Foo4 { @foo - async *// comment - method() {} + // comment + async *method() {} } class Something { diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index defa499410af..ee932d97de66 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -31,6 +31,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom #### Bug fixes +- Fix [#1406](https://github.com/biomejs/biome/issues/1406). Ensure comments before the `async` keyword are placed before it. Contributed by @ah-yu + - Fix [#1172](https://github.com/biomejs/biome/issues/1172). Fix placement of line comment after function expression parentheses, they are now attached to first statement in body. Contributed by @kalleep ### JavaScript APIs From 57f454976a02d34581291e6f06a10caa34953745 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 11 Jan 2024 11:37:46 +0000 Subject: [PATCH 04/14] refactor(cli): some cleanup (#1526) --- crates/biome_cli/src/cli_options.rs | 12 ++++++++ crates/biome_cli/src/commands/check.rs | 29 ++++------------- crates/biome_cli/src/commands/ci.rs | 13 +++----- crates/biome_cli/src/commands/format.rs | 27 +++------------- crates/biome_cli/src/commands/lint.rs | 32 ++++--------------- crates/biome_cli/src/commands/mod.rs | 26 ++++++++++++++++ crates/biome_cli/src/execute/mod.rs | 41 ++++++++++++++++++++----- crates/biome_wasm/Cargo.toml | 22 ++++++------- 8 files changed, 104 insertions(+), 98 deletions(-) diff --git a/crates/biome_cli/src/cli_options.rs b/crates/biome_cli/src/cli_options.rs index 404d057ac671..a736adaab9f7 100644 --- a/crates/biome_cli/src/cli_options.rs +++ b/crates/biome_cli/src/cli_options.rs @@ -1,7 +1,9 @@ use crate::logging::LoggingKind; use crate::LoggingLevel; use biome_diagnostics::Severity; +use biome_service::ConfigurationBasePath; use bpaf::Bpaf; +use std::path::PathBuf; use std::str::FromStr; /// Global options applied to all commands @@ -78,6 +80,16 @@ pub struct CliOptions { pub diagnostic_level: Severity, } +impl CliOptions { + /// Computes the [ConfigurationBasePath] based on the options passed by the user + pub(crate) fn as_configuration_base_path(&self) -> ConfigurationBasePath { + match self.config_path.as_ref() { + None => ConfigurationBasePath::default(), + Some(path) => ConfigurationBasePath::FromUser(PathBuf::from(path)), + } + } +} + #[derive(Debug, Clone)] pub enum ColorsArg { Off, diff --git a/crates/biome_cli/src/commands/check.rs b/crates/biome_cli/src/commands/check.rs index 49c9ef90d0b3..ac99ef336b6b 100644 --- a/crates/biome_cli/src/commands/check.rs +++ b/crates/biome_cli/src/commands/check.rs @@ -1,6 +1,6 @@ use crate::changed::get_changed_files; use crate::cli_options::CliOptions; -use crate::commands::validate_configuration_diagnostics; +use crate::commands::{get_stdin, validate_configuration_diagnostics}; use crate::{ execute_mode, setup_cli_subscriber, CliDiagnostic, CliSession, Execution, TraversalMode, }; @@ -9,9 +9,8 @@ use biome_service::configuration::{ load_configuration, FormatterConfiguration, LinterConfiguration, LoadedConfiguration, }; use biome_service::workspace::{FixFileMode, UpdateSettingsParams}; -use biome_service::{Configuration, ConfigurationBasePath, MergeWith}; +use biome_service::{Configuration, MergeWith}; use std::ffi::OsString; -use std::path::PathBuf; pub(crate) struct CheckCommandPayload { pub(crate) apply: bool, @@ -29,7 +28,7 @@ pub(crate) struct CheckCommandPayload { /// Handler for the "check" command of the Biome CLI pub(crate) fn check( - mut session: CliSession, + session: CliSession, payload: CheckCommandPayload, ) -> Result<(), CliDiagnostic> { let CheckCommandPayload { @@ -60,12 +59,8 @@ pub(crate) fn check( Some(FixFileMode::SafeAndUnsafeFixes) }; - let base_path = match cli_options.config_path.as_ref() { - None => ConfigurationBasePath::default(), - Some(path) => ConfigurationBasePath::FromUser(PathBuf::from(path)), - }; - - let loaded_configuration = load_configuration(&session.app.fs, base_path)?; + let loaded_configuration = + load_configuration(&session.app.fs, cli_options.as_configuration_base_path())?; validate_configuration_diagnostics( &loaded_configuration, session.app.console, @@ -109,19 +104,7 @@ pub(crate) fn check( let (vcs_base_path, gitignore_matches) = fs_configuration.retrieve_gitignore_matches(&session.app.fs, vcs_base_path.as_deref())?; - let stdin = if let Some(stdin_file_path) = stdin_file_path { - let console = &mut session.app.console; - let input_code = console.read(); - if let Some(input_code) = input_code { - let path = PathBuf::from(stdin_file_path); - Some((path, input_code)) - } else { - // we provided the argument without a piped stdin, we bail - return Err(CliDiagnostic::missing_argument("stdin", "check")); - } - } else { - None - }; + let stdin = get_stdin(stdin_file_path, &mut *session.app.console, "check")?; if since.is_some() && !changed { return Err(CliDiagnostic::incompatible_arguments("since", "changed")); diff --git a/crates/biome_cli/src/commands/ci.rs b/crates/biome_cli/src/commands/ci.rs index 3d8100e05d8d..37b9bc440163 100644 --- a/crates/biome_cli/src/commands/ci.rs +++ b/crates/biome_cli/src/commands/ci.rs @@ -7,9 +7,8 @@ use biome_service::configuration::{ load_configuration, FormatterConfiguration, LinterConfiguration, LoadedConfiguration, }; use biome_service::workspace::UpdateSettingsParams; -use biome_service::{Configuration, ConfigurationBasePath, MergeWith}; +use biome_service::{Configuration, MergeWith}; use std::ffi::OsString; -use std::path::PathBuf; pub(crate) struct CiCommandPayload { pub(crate) formatter_enabled: Option, @@ -29,12 +28,10 @@ pub(crate) fn ci(session: CliSession, mut payload: CiCommandPayload) -> Result<( payload.cli_options.log_kind.clone(), ); - let base_path = match payload.cli_options.config_path.as_ref() { - None => ConfigurationBasePath::default(), - Some(path) => ConfigurationBasePath::FromUser(PathBuf::from(path)), - }; - - let loaded_configuration = load_configuration(&session.app.fs, base_path)?; + let loaded_configuration = load_configuration( + &session.app.fs, + payload.cli_options.as_configuration_base_path(), + )?; validate_configuration_diagnostics( &loaded_configuration, diff --git a/crates/biome_cli/src/commands/format.rs b/crates/biome_cli/src/commands/format.rs index 0d79ffa63b4c..06a20d27e5ba 100644 --- a/crates/biome_cli/src/commands/format.rs +++ b/crates/biome_cli/src/commands/format.rs @@ -1,6 +1,6 @@ use crate::changed::get_changed_files; use crate::cli_options::CliOptions; -use crate::commands::validate_configuration_diagnostics; +use crate::commands::{get_stdin, validate_configuration_diagnostics}; use crate::diagnostics::DeprecatedArgument; use crate::execute::ReportMode; use crate::{ @@ -15,9 +15,8 @@ use biome_service::configuration::{ load_configuration, FilesConfiguration, FormatterConfiguration, LoadedConfiguration, }; use biome_service::workspace::UpdateSettingsParams; -use biome_service::{ConfigurationBasePath, JavascriptFormatter, MergeWith}; +use biome_service::{JavascriptFormatter, MergeWith}; use std::ffi::OsString; -use std::path::PathBuf; pub(crate) struct FormatCommandPayload { pub(crate) javascript_formatter: Option, @@ -55,12 +54,8 @@ pub(crate) fn format( } = payload; setup_cli_subscriber(cli_options.log_level.clone(), cli_options.log_kind.clone()); - let base_path = match cli_options.config_path.as_ref() { - None => ConfigurationBasePath::default(), - Some(path) => ConfigurationBasePath::FromUser(PathBuf::from(path)), - }; - - let loaded_configuration = load_configuration(&session.app.fs, base_path)?; + let loaded_configuration = + load_configuration(&session.app.fs, cli_options.as_configuration_base_path())?; validate_configuration_diagnostics( &loaded_configuration, session.app.console, @@ -154,19 +149,7 @@ pub(crate) fn format( gitignore_matches, })?; - let stdin = if let Some(stdin_file_path) = stdin_file_path { - let console = &mut session.app.console; - let input_code = console.read(); - if let Some(input_code) = input_code { - let path = PathBuf::from(stdin_file_path); - Some((path, input_code)) - } else { - // we provided the argument without a piped stdin, we bail - return Err(CliDiagnostic::missing_argument("stdin", "format")); - } - } else { - None - }; + let stdin = get_stdin(stdin_file_path, &mut *session.app.console, "format")?; let execution = if cli_options.json { Execution::with_report( diff --git a/crates/biome_cli/src/commands/lint.rs b/crates/biome_cli/src/commands/lint.rs index 2a9a1b8b60ca..2bc7124f715b 100644 --- a/crates/biome_cli/src/commands/lint.rs +++ b/crates/biome_cli/src/commands/lint.rs @@ -1,6 +1,6 @@ use crate::changed::get_changed_files; use crate::cli_options::CliOptions; -use crate::commands::validate_configuration_diagnostics; +use crate::commands::{get_stdin, validate_configuration_diagnostics}; use crate::{ execute_mode, setup_cli_subscriber, CliDiagnostic, CliSession, Execution, TraversalMode, }; @@ -9,9 +9,8 @@ use biome_service::configuration::{ load_configuration, FilesConfiguration, LinterConfiguration, LoadedConfiguration, }; use biome_service::workspace::{FixFileMode, UpdateSettingsParams}; -use biome_service::{ConfigurationBasePath, MergeWith}; +use biome_service::MergeWith; use std::ffi::OsString; -use std::path::PathBuf; pub(crate) struct LintCommandPayload { pub(crate) apply: bool, @@ -27,10 +26,7 @@ pub(crate) struct LintCommandPayload { } /// Handler for the "lint" command of the Biome CLI -pub(crate) fn lint( - mut session: CliSession, - payload: LintCommandPayload, -) -> Result<(), CliDiagnostic> { +pub(crate) fn lint(session: CliSession, payload: LintCommandPayload) -> Result<(), CliDiagnostic> { let LintCommandPayload { apply, apply_unsafe, @@ -58,12 +54,8 @@ pub(crate) fn lint( Some(FixFileMode::SafeAndUnsafeFixes) }; - let base_path = match cli_options.config_path.as_ref() { - None => ConfigurationBasePath::default(), - Some(path) => ConfigurationBasePath::FromUser(PathBuf::from(path)), - }; - - let loaded_configuration = load_configuration(&session.app.fs, base_path)?; + let loaded_configuration = + load_configuration(&session.app.fs, cli_options.as_configuration_base_path())?; validate_configuration_diagnostics( &loaded_configuration, session.app.console, @@ -92,19 +84,7 @@ pub(crate) fn lint( paths = get_changed_files(&session.app.fs, &fs_configuration, since)?; } - let stdin = if let Some(stdin_file_path) = stdin_file_path { - let console = &mut session.app.console; - let input_code = console.read(); - if let Some(input_code) = input_code { - let path = PathBuf::from(stdin_file_path); - Some((path, input_code)) - } else { - // we provided the argument without a piped stdin, we bail - return Err(CliDiagnostic::missing_argument("stdin", "lint")); - } - } else { - None - }; + let stdin = get_stdin(stdin_file_path, &mut *session.app.console, "lint")?; session .app diff --git a/crates/biome_cli/src/commands/mod.rs b/crates/biome_cli/src/commands/mod.rs index e5613987ed53..bb5b66b98cb3 100644 --- a/crates/biome_cli/src/commands/mod.rs +++ b/crates/biome_cli/src/commands/mod.rs @@ -1,5 +1,6 @@ use crate::cli_options::{cli_options, CliOptions, ColorsArg}; use crate::diagnostics::DeprecatedConfigurationFile; +use crate::execute::Stdin; use crate::logging::LoggingKind; use crate::{CliDiagnostic, LoggingLevel, VERSION}; use biome_console::{markup, Console, ConsoleExt}; @@ -426,3 +427,28 @@ pub(crate) fn validate_configuration_diagnostics( Ok(()) } + +/// Computes [Stdin] if the CLI has the necessary information. +/// +/// ## Errors +/// - If the user didn't provide anything via `stdin` but the option `--stdin-file-path` is passed. +pub(crate) fn get_stdin( + stdin_file_path: Option, + console: &mut dyn Console, + command_name: &str, +) -> Result, CliDiagnostic> { + let stdin = if let Some(stdin_file_path) = stdin_file_path { + let input_code = console.read(); + if let Some(input_code) = input_code { + let path = PathBuf::from(stdin_file_path); + Some((path, input_code).into()) + } else { + // we provided the argument without a piped stdin, we bail + return Err(CliDiagnostic::missing_argument("stdin", command_name)); + } + } else { + None + }; + + Ok(stdin) +} diff --git a/crates/biome_cli/src/execute/mod.rs b/crates/biome_cli/src/execute/mod.rs index d7cf94936cfa..c3d26fcd5e5a 100644 --- a/crates/biome_cli/src/execute/mod.rs +++ b/crates/biome_cli/src/execute/mod.rs @@ -12,7 +12,7 @@ use biome_fs::RomePath; use biome_service::workspace::{FeatureName, FixFileMode}; use std::ffi::OsString; use std::fmt::{Display, Formatter}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// Useful information during the traversal of files and virtual content pub(crate) struct Execution { @@ -40,6 +40,31 @@ pub(crate) enum ExecutionEnvironment { GitHub, } +/// A type that holds the information to execute the CLI via `stdin +#[derive(Debug)] +pub(crate) struct Stdin( + /// The virtual path to the file + PathBuf, + /// The content of the file + String, +); + +impl Stdin { + fn as_path(&self) -> &Path { + self.0.as_path() + } + + fn as_content(&self) -> &str { + self.1.as_str() + } +} + +impl From<(PathBuf, String)> for Stdin { + fn from((path, content): (PathBuf, String)) -> Self { + Self(path, content) + } +} + #[derive(Debug)] pub(crate) enum TraversalMode { /// This mode is enabled when running the command `biome check` @@ -52,7 +77,7 @@ pub(crate) enum TraversalMode { /// An optional tuple. /// 1. The virtual path to the file /// 2. The content of the file - stdin: Option<(PathBuf, String)>, + stdin: Option, }, /// This mode is enabled when running the command `biome lint` Lint { @@ -64,7 +89,7 @@ pub(crate) enum TraversalMode { /// An optional tuple. /// 1. The virtual path to the file /// 2. The content of the file - stdin: Option<(PathBuf, String)>, + stdin: Option, }, /// This mode is enabled when running the command `biome ci` CI { @@ -80,7 +105,7 @@ pub(crate) enum TraversalMode { /// An optional tuple. /// 1. The virtual path to the file /// 2. The content of the file - stdin: Option<(PathBuf, String)>, + stdin: Option, }, /// This mode is enabled when running the command `biome migrate` Migrate { @@ -239,7 +264,7 @@ impl Execution { } } - pub(crate) fn as_stdin_file(&self) -> Option<&(PathBuf, String)> { + pub(crate) fn as_stdin_file(&self) -> Option<&Stdin> { match &self.traversal_mode { TraversalMode::Format { stdin, .. } | TraversalMode::Lint { stdin, .. } @@ -260,13 +285,13 @@ pub(crate) fn execute_mode( mode.max_diagnostics = cli_options.max_diagnostics; // don't do any traversal if there's some content coming from stdin - if let Some((path, content)) = mode.as_stdin_file() { - let rome_path = RomePath::new(path); + if let Some(stdin) = mode.as_stdin_file() { + let rome_path = RomePath::new(stdin.as_path()); std_in::run( session, &mode, rome_path, - content.as_str(), + stdin.as_content(), cli_options.verbose, ) } else if let TraversalMode::Migrate { diff --git a/crates/biome_wasm/Cargo.toml b/crates/biome_wasm/Cargo.toml index c0585c65041e..726f44694bb6 100644 --- a/crates/biome_wasm/Cargo.toml +++ b/crates/biome_wasm/Cargo.toml @@ -1,15 +1,15 @@ [package] -authors.workspace = true -categories.workspace = true -description = "WebAssembly bindings to the Biome workspace API" -edition.workspace = true -homepage.workspace = true -keywords.workspace = true -license.workspace = true -name = "biome_wasm" -publish = false -repository.workspace = true -version = "0.0.0" +authors = ["Biome Developers and Contributors"] +categories = ["development-tools", "web-programming"] +description = "WebAssembly bindings to the Biome workspace API" +edition = "2021" +homepage = "https://biomejs.dev/" +keywords = ["parser", "linter", "formatter"] +license = "MIT OR Apache-2.0" +name = "biome_wasm" +publish = false +repository = "https://github.com/biomejs/biome" +version = "0.0.0" [lib] From 2c77adefcd4d62452f250077d66655923e7b45c1 Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Thu, 11 Jan 2024 17:51:28 +0100 Subject: [PATCH 05/14] fix(lint/useArrowFunction): enclose with parens when needed (#1527) --- CHANGELOG.md | 27 ++ .../complexity/use_arrow_function.rs | 60 +++- .../complexity/useArrowFunction/invalid.ts | 18 +- .../useArrowFunction/invalid.ts.snap | 290 ++++++++++++++---- .../complexity/useArrowFunction/valid.ts | 2 + .../complexity/useArrowFunction/valid.ts.snap | 2 + .../src/content/docs/internals/changelog.mdx | 27 ++ 7 files changed, 354 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58213187d0c5..fe0a18849a4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,8 +39,35 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Add an unsafe code fix for [noConsoleLog](https://biomejs.dev/linter/rules/no-console-log/). Contributed by @vasucp1207 +- [useArrowFunction](https://biomejs.dev/rules/) no longer reports function in `extends` clauses or in a `new` expression. COntributed by @Conaclos + + This cases requires the presence of a prototype. + #### Bug fixes +- The fix of [useArrowFunction](https://biomejs.dev/rules/) now adds parentheses around the arrow function in more cases where it is needed ([#1524](https://github.com/biomejs/biome/issues/1524)). + + A function expression doesn't need parentheses in most expressions where it can appear. + This is not the case with the arrow function. + We previously added parentheses when the function appears in a call or member expression. + We now add parentheses in binary-like expressions and other cases where they are needed, hopefully covering all cases. + + Previously: + + ```diff + - f = f ?? function() {}; + + f = f ?? () => {}; + ``` + + Now: + + ```diff + - f = f ?? function() {}; + + f = f ?? (() => {}); + ``` + + Contributed by @Conaclos + ### Parser diff --git a/crates/biome_js_analyze/src/analyzers/complexity/use_arrow_function.rs b/crates/biome_js_analyze/src/analyzers/complexity/use_arrow_function.rs index cb8714efe8ab..4dd4ab493a6d 100644 --- a/crates/biome_js_analyze/src/analyzers/complexity/use_arrow_function.rs +++ b/crates/biome_js_analyze/src/analyzers/complexity/use_arrow_function.rs @@ -15,7 +15,7 @@ use biome_js_syntax::{ }; use biome_rowan::{ declare_node_union, AstNode, AstNodeList, AstSeparatedList, BatchMutationExt, Language, - SyntaxNode, SyntaxNodeOptionExt, TextRange, TriviaPieceKind, WalkEvent, + SyntaxNode, TextRange, TriviaPieceKind, WalkEvent, }; declare_rule! { @@ -104,6 +104,21 @@ impl Rule for UseArrowFunction { // Ignore functions that explicitly declare a `this` type. return None; } + let requires_prototype = function_expression + .syntax() + .ancestors() + .skip(1) + .find(|ancestor| ancestor.kind() != JsSyntaxKind::JS_PARENTHESIZED_EXPRESSION) + .is_some_and(|ancestor| { + matches!( + ancestor.kind(), + JsSyntaxKind::JS_NEW_EXPRESSION | JsSyntaxKind::JS_EXTENDS_CLAUSE + ) + }); + if requires_prototype { + // Ignore cases where a prototype is required + return None; + } Some(()) } @@ -144,7 +159,7 @@ impl Rule for UseArrowFunction { } let arrow_function = arrow_function_builder.build(); let arrow_function = if needs_parentheses(function_expression) { - AnyJsExpression::from(make::parenthesized(arrow_function)) + AnyJsExpression::from(make::parenthesized(arrow_function.trim_trailing_trivia()?)) } else { AnyJsExpression::from(arrow_function) }; @@ -165,20 +180,41 @@ impl Rule for UseArrowFunction { /// Returns `true` if `function_expr` needs parenthesis when turned into an arrow function. fn needs_parentheses(function_expression: &JsFunctionExpression) -> bool { - function_expression - .syntax() - .parent() - .kind() - .is_some_and(|parent_kind| { - matches!( - parent_kind, - JsSyntaxKind::JS_CALL_EXPRESSION + function_expression.syntax().parent().is_some_and(|parent| { + // Copied from the implementation of `NeedsParentheses` for `JsArrowFunctionExpression` + // in the `biome_js_formatter` crate. + // TODO: Should `NeedsParentheses` be moved in `biome_js_syntax`? + matches!( + parent.kind(), + JsSyntaxKind::TS_AS_EXPRESSION + | JsSyntaxKind::TS_SATISFIES_EXPRESSION + | JsSyntaxKind::JS_UNARY_EXPRESSION + | JsSyntaxKind::JS_AWAIT_EXPRESSION + | JsSyntaxKind::TS_TYPE_ASSERTION_EXPRESSION + // Conditional expression + // NOTE: parens are only needed when the arrow function appears in the test. + // To simplify we always add parens. + | JsSyntaxKind::JS_CONDITIONAL_EXPRESSION + // Lower expression + | JsSyntaxKind::JS_EXTENDS_CLAUSE + | JsSyntaxKind::TS_NON_NULL_ASSERTION_EXPRESSION + // Callee + | JsSyntaxKind::JS_CALL_EXPRESSION + | JsSyntaxKind::JS_NEW_EXPRESSION + // Member-like | JsSyntaxKind::JS_STATIC_MEMBER_ASSIGNMENT | JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION | JsSyntaxKind::JS_COMPUTED_MEMBER_ASSIGNMENT | JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION - ) - }) + // Template tag + | JsSyntaxKind::JS_TEMPLATE_EXPRESSION + // Binary-like + | JsSyntaxKind::JS_LOGICAL_EXPRESSION + | JsSyntaxKind::JS_BINARY_EXPRESSION + | JsSyntaxKind::JS_INSTANCEOF_EXPRESSION + | JsSyntaxKind::JS_IN_EXPRESSION + ) + }) } declare_node_union! { diff --git a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts index d33199f5d8c8..b27b3ac786f7 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts +++ b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts @@ -36,12 +36,18 @@ function f7() { }; } -const f8 = function(a) {}.bind(null, 0); - -const f9 = function(a) {}["bind"](null, 0); - -const called = function () {}(); - const f10 = function(x) { return 0, 1; } + +const as = function () {} as () => void; +const satisfies = function () {} satisfies () => void; +const unary = +function () {}; +const conditionalTest = function () {} ? true : false; +class ExtendsClause extends function() {} {}; +const non_null_assertion = function () {}!; +const call = function () {}(); +const staticMember = function(a) {}.bind(null, 0); +const computedMember = function(a) {}["bind"](null, 0); +const logical = false || function () {}; +const binary = false + function () {}; \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts.snap b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts.snap index ec6905353128..1abb1f1c9b86 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/invalid.ts.snap @@ -42,16 +42,21 @@ function f7() { }; } -const f8 = function(a) {}.bind(null, 0); - -const f9 = function(a) {}["bind"](null, 0); - -const called = function () {}(); - const f10 = function(x) { return 0, 1; } +const as = function () {} as () => void; +const satisfies = function () {} satisfies () => void; +const unary = +function () {}; +const conditionalTest = function () {} ? true : false; +class ExtendsClause extends function() {} {}; +const non_null_assertion = function () {}!; +const call = function () {}(); +const staticMember = function(a) {}.bind(null, 0); +const computedMember = function(a) {}["bind"](null, 0); +const logical = false || function () {}; +const binary = false + function () {}; ``` # Diagnostics @@ -268,16 +273,19 @@ invalid.ts:30:12 lint/complexity/useArrowFunction FIXABLE ━━━━━━ ``` ``` -invalid.ts:39:12 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.ts:39:13 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This function expression can be turned into an arrow function. 37 │ } 38 │ - > 39 │ const f8 = function(a) {}.bind(null, 0); - │ ^^^^^^^^^^^^^^ - 40 │ - 41 │ const f9 = function(a) {}["bind"](null, 0); + > 39 │ const f10 = function(x) { + │ ^^^^^^^^^^^^^ + > 40 │ return 0, 1; + > 41 │ } + │ ^ + 42 │ + 43 │ const as = function () {} as () => void; i Function expressions that don't use this can be turned into arrow functions. @@ -285,91 +293,265 @@ invalid.ts:39:12 lint/complexity/useArrowFunction FIXABLE ━━━━━━ 37 37 │ } 38 38 │ - 39 │ - const·f8·=·function(a)·{}.bind(null,·0); - 39 │ + const·f8·=·((a)·=>·{}).bind(null,·0); - 40 40 │ - 41 41 │ const f9 = function(a) {}["bind"](null, 0); + 39 │ - const·f10·=·function(x)·{ + 40 │ - ····return·0,·1; + 41 │ - } + 39 │ + const·f10·=·(x)·=>·(0,·1) + 42 40 │ + 43 41 │ const as = function () {} as () => void; ``` ``` -invalid.ts:41:12 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.ts:43:12 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This function expression can be turned into an arrow function. - 39 │ const f8 = function(a) {}.bind(null, 0); - 40 │ - > 41 │ const f9 = function(a) {}["bind"](null, 0); - │ ^^^^^^^^^^^^^^ + 41 │ } 42 │ - 43 │ const called = function () {}(); + > 43 │ const as = function () {} as () => void; + │ ^^^^^^^^^^^^^^ + 44 │ const satisfies = function () {} satisfies () => void; + 45 │ const unary = +function () {}; i Function expressions that don't use this can be turned into arrow functions. i Safe fix: Use an arrow function instead. - 39 39 │ const f8 = function(a) {}.bind(null, 0); - 40 40 │ - 41 │ - const·f9·=·function(a)·{}["bind"](null,·0); - 41 │ + const·f9·=·((a)·=>·{})["bind"](null,·0); + 41 41 │ } 42 42 │ - 43 43 │ const called = function () {}(); + 43 │ - const·as·=·function·()·{}·as·()·=>·void; + 43 │ + const·as·=·(()·=>·{})·as·()·=>·void; + 44 44 │ const satisfies = function () {} satisfies () => void; + 45 45 │ const unary = +function () {}; ``` ``` -invalid.ts:43:16 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.ts:44:19 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This function expression can be turned into an arrow function. - 41 │ const f9 = function(a) {}["bind"](null, 0); - 42 │ - > 43 │ const called = function () {}(); - │ ^^^^^^^^^^^^^^ - 44 │ - 45 │ const f10 = function(x) { + 43 │ const as = function () {} as () => void; + > 44 │ const satisfies = function () {} satisfies () => void; + │ ^^^^^^^^^^^^^^ + 45 │ const unary = +function () {}; + 46 │ const conditionalTest = function () {} ? true : false; i Function expressions that don't use this can be turned into arrow functions. i Safe fix: Use an arrow function instead. - 41 41 │ const f9 = function(a) {}["bind"](null, 0); 42 42 │ - 43 │ - const·called·=·function·()·{}(); - 43 │ + const·called·=·(()·=>·{})(); - 44 44 │ - 45 45 │ const f10 = function(x) { + 43 43 │ const as = function () {} as () => void; + 44 │ - const·satisfies·=·function·()·{}·satisfies·()·=>·void; + 44 │ + const·satisfies·=·(()·=>·{})·satisfies·()·=>·void; + 45 45 │ const unary = +function () {}; + 46 46 │ const conditionalTest = function () {} ? true : false; ``` ``` -invalid.ts:45:13 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.ts:45:16 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This function expression can be turned into an arrow function. - 43 │ const called = function () {}(); - 44 │ - > 45 │ const f10 = function(x) { - │ ^^^^^^^^^^^^^ - > 46 │ return 0, 1; - > 47 │ } - │ ^ - 48 │ + 43 │ const as = function () {} as () => void; + 44 │ const satisfies = function () {} satisfies () => void; + > 45 │ const unary = +function () {}; + │ ^^^^^^^^^^^^^^ + 46 │ const conditionalTest = function () {} ? true : false; + 47 │ class ExtendsClause extends function() {} {}; + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 43 43 │ const as = function () {} as () => void; + 44 44 │ const satisfies = function () {} satisfies () => void; + 45 │ - const·unary·=·+function·()·{}; + 45 │ + const·unary·=·+(()·=>·{}); + 46 46 │ const conditionalTest = function () {} ? true : false; + 47 47 │ class ExtendsClause extends function() {} {}; + + +``` + +``` +invalid.ts:46:25 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 44 │ const satisfies = function () {} satisfies () => void; + 45 │ const unary = +function () {}; + > 46 │ const conditionalTest = function () {} ? true : false; + │ ^^^^^^^^^^^^^^ + 47 │ class ExtendsClause extends function() {} {}; + 48 │ const non_null_assertion = function () {}!; + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 44 44 │ const satisfies = function () {} satisfies () => void; + 45 45 │ const unary = +function () {}; + 46 │ - const·conditionalTest·=·function·()·{}·?·true·:·false; + 46 │ + const·conditionalTest·=·(()·=>·{})·?·true·:·false; + 47 47 │ class ExtendsClause extends function() {} {}; + 48 48 │ const non_null_assertion = function () {}!; + + +``` + +``` +invalid.ts:48:28 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 46 │ const conditionalTest = function () {} ? true : false; + 47 │ class ExtendsClause extends function() {} {}; + > 48 │ const non_null_assertion = function () {}!; + │ ^^^^^^^^^^^^^^ + 49 │ const call = function () {}(); + 50 │ const staticMember = function(a) {}.bind(null, 0); + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 46 46 │ const conditionalTest = function () {} ? true : false; + 47 47 │ class ExtendsClause extends function() {} {}; + 48 │ - const·non_null_assertion·=·function·()·{}!; + 48 │ + const·non_null_assertion·=·(()·=>·{})!; + 49 49 │ const call = function () {}(); + 50 50 │ const staticMember = function(a) {}.bind(null, 0); + + +``` + +``` +invalid.ts:49:14 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 47 │ class ExtendsClause extends function() {} {}; + 48 │ const non_null_assertion = function () {}!; + > 49 │ const call = function () {}(); + │ ^^^^^^^^^^^^^^ + 50 │ const staticMember = function(a) {}.bind(null, 0); + 51 │ const computedMember = function(a) {}["bind"](null, 0); + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 47 47 │ class ExtendsClause extends function() {} {}; + 48 48 │ const non_null_assertion = function () {}!; + 49 │ - const·call·=·function·()·{}(); + 49 │ + const·call·=·(()·=>·{})(); + 50 50 │ const staticMember = function(a) {}.bind(null, 0); + 51 51 │ const computedMember = function(a) {}["bind"](null, 0); + + +``` + +``` +invalid.ts:50:22 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 48 │ const non_null_assertion = function () {}!; + 49 │ const call = function () {}(); + > 50 │ const staticMember = function(a) {}.bind(null, 0); + │ ^^^^^^^^^^^^^^ + 51 │ const computedMember = function(a) {}["bind"](null, 0); + 52 │ const logical = false || function () {}; + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 48 48 │ const non_null_assertion = function () {}!; + 49 49 │ const call = function () {}(); + 50 │ - const·staticMember·=·function(a)·{}.bind(null,·0); + 50 │ + const·staticMember·=·((a)·=>·{}).bind(null,·0); + 51 51 │ const computedMember = function(a) {}["bind"](null, 0); + 52 52 │ const logical = false || function () {}; + + +``` + +``` +invalid.ts:51:24 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 49 │ const call = function () {}(); + 50 │ const staticMember = function(a) {}.bind(null, 0); + > 51 │ const computedMember = function(a) {}["bind"](null, 0); + │ ^^^^^^^^^^^^^^ + 52 │ const logical = false || function () {}; + 53 │ const binary = false + function () {}; + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 49 49 │ const call = function () {}(); + 50 50 │ const staticMember = function(a) {}.bind(null, 0); + 51 │ - const·computedMember·=·function(a)·{}["bind"](null,·0); + 51 │ + const·computedMember·=·((a)·=>·{})["bind"](null,·0); + 52 52 │ const logical = false || function () {}; + 53 53 │ const binary = false + function () {}; + + +``` + +``` +invalid.ts:52:26 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 50 │ const staticMember = function(a) {}.bind(null, 0); + 51 │ const computedMember = function(a) {}["bind"](null, 0); + > 52 │ const logical = false || function () {}; + │ ^^^^^^^^^^^^^^ + 53 │ const binary = false + function () {}; + + i Function expressions that don't use this can be turned into arrow functions. + + i Safe fix: Use an arrow function instead. + + 50 50 │ const staticMember = function(a) {}.bind(null, 0); + 51 51 │ const computedMember = function(a) {}["bind"](null, 0); + 52 │ - const·logical·=·false·||·function·()·{}; + 52 │ + const·logical·=·false·||·(()·=>·{}); + 53 53 │ const binary = false + function () {}; + + +``` + +``` +invalid.ts:53:24 lint/complexity/useArrowFunction FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This function expression can be turned into an arrow function. + + 51 │ const computedMember = function(a) {}["bind"](null, 0); + 52 │ const logical = false || function () {}; + > 53 │ const binary = false + function () {}; + │ ^^^^^^^^^^^^^^ i Function expressions that don't use this can be turned into arrow functions. i Safe fix: Use an arrow function instead. - 43 43 │ const called = function () {}(); - 44 44 │ - 45 │ - const·f10·=·function(x)·{ - 46 │ - ····return·0,·1; - 47 │ - } - 45 │ + const·f10·=·(x)·=>·(0,·1) - 48 46 │ + 51 51 │ const computedMember = function(a) {}["bind"](null, 0); + 52 52 │ const logical = false || function () {}; + 53 │ - const·binary·=·false·+·function·()·{}; + 53 │ + const·binary·=·false·+·(()·=>·{}); ``` diff --git a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts index 05221d90eded..3065e1d6f702 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts +++ b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts @@ -84,3 +84,5 @@ export default function() { const usingNewTarget = function () { return new.target; } + +class ExtendsClause extends (function() {}) {} diff --git a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts.snap b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts.snap index f000155f363c..d6aa6b5a7f58 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/useArrowFunction/valid.ts.snap @@ -91,6 +91,8 @@ const usingNewTarget = function () { return new.target; } +class ExtendsClause extends (function() {}) {} + ``` diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index ee932d97de66..0a145fe4c13a 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -45,8 +45,35 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Add an unsafe code fix for [noConsoleLog](https://biomejs.dev/linter/rules/no-console-log/). Contributed by @vasucp1207 +- [useArrowFunction](https://biomejs.dev/rules/) no longer reports function in `extends` clauses or in a `new` expression. COntributed by @Conaclos + + This cases requires the presence of a prototype. + #### Bug fixes +- The fix of [useArrowFunction](https://biomejs.dev/rules/) now adds parentheses around the arrow function in more cases where it is needed ([#1524](https://github.com/biomejs/biome/issues/1524)). + + A function expression doesn't need parentheses in most expressions where it can appear. + This is not the case with the arrow function. + We previously added parentheses when the function appears in a call or member expression. + We now add parentheses in binary-like expressions and other cases where they are needed, hopefully covering all cases. + + Previously: + + ```diff + - f = f ?? function() {}; + + f = f ?? () => {}; + ``` + + Now: + + ```diff + - f = f ?? function() {}; + + f = f ?? (() => {}); + ``` + + Contributed by @Conaclos + ### Parser From e65764b4f59ee78bd425e2ce3adc0283fd2bcf25 Mon Sep 17 00:00:00 2001 From: Vasu Singh Date: Thu, 11 Jan 2024 23:24:21 +0530 Subject: [PATCH 06/14] docs(website): fix code snippets that take more space (#1529) --- website/src/styles/_overrides.scss | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/website/src/styles/_overrides.scss b/website/src/styles/_overrides.scss index adc64cbb4aaf..cc0ebdd35d82 100644 --- a/website/src/styles/_overrides.scss +++ b/website/src/styles/_overrides.scss @@ -132,7 +132,7 @@ padding: 1.2rem; } -.expressive-code:nth-child(-n+2)>figure>pre>code { +.linter>div>.expressive-code:nth-child(-n+2)>figure>pre>code { height: 400px !important; overflow: scroll; font-size: 0.7rem; @@ -146,7 +146,7 @@ display: none; } -.expressive-code:nth-child(-n+2)>figure>pre>code { +.linter>div>.expressive-code:nth-child(-n+2)>figure>pre>code { height: 400px !important; overflow: scroll; font-size: 0.7rem; @@ -161,7 +161,7 @@ } @media only screen and (max-width: 960px) { - .expressive-code:nth-child(-n+2)>figure>pre>code { + .linter>div>.expressive-code:nth-child(-n+2)>figure>pre>code { height: 250px !important; overflow: scroll; font-size: 0.7rem; @@ -170,4 +170,4 @@ .content-panel { overflow: hidden; -} +} \ No newline at end of file From 2cd6eafa0da17bc24a0d05cb59135184db89a6a6 Mon Sep 17 00:00:00 2001 From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com> Date: Thu, 11 Jan 2024 23:16:16 +0200 Subject: [PATCH 07/14] feat(css-parser): CSS Parser: parse starting-style at rule #1478 (#1518) --- .../src/generated/node_factory.rs | 12 + .../src/generated/syntax_factory.rs | 26 ++ .../src/css/any/at_rule.rs | 1 + crates/biome_css_formatter/src/css/any/mod.rs | 1 + .../src/css/any/starting_style_block.rs | 16 ++ .../src/css/statements/mod.rs | 1 + .../css/statements/starting_style_at_rule.rs | 10 + crates/biome_css_formatter/src/generated.rs | 67 +++++ crates/biome_css_parser/src/lexer/mod.rs | 1 + crates/biome_css_parser/src/state.rs | 10 + .../src/syntax/at_rule/keyframes.rs | 11 +- .../src/syntax/at_rule/mod.rs | 6 + .../src/syntax/at_rule/page.rs | 9 +- .../src/syntax/at_rule/starting_style.rs | 65 +++++ crates/biome_css_parser/src/syntax/blocks.rs | 65 +++-- .../ok/at_rule/at_rule_starting_style.css | 15 + .../at_rule/at_rule_starting_style.css.snap | 266 ++++++++++++++++++ crates/biome_css_parser/tests/spec_test.rs | 13 +- crates/biome_css_syntax/src/generated/kind.rs | 6 +- .../biome_css_syntax/src/generated/macros.rs | 4 + .../biome_css_syntax/src/generated/nodes.rs | 221 +++++++++++++++ .../src/generated/nodes_mut.rs | 14 + xtask/codegen/css.ungram | 22 +- xtask/codegen/src/css_kinds_src.rs | 2 + 24 files changed, 826 insertions(+), 38 deletions(-) create mode 100644 crates/biome_css_formatter/src/css/any/starting_style_block.rs create mode 100644 crates/biome_css_formatter/src/css/statements/starting_style_at_rule.rs create mode 100644 crates/biome_css_parser/src/syntax/at_rule/starting_style.rs create mode 100644 crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css create mode 100644 crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css.snap diff --git a/crates/biome_css_factory/src/generated/node_factory.rs b/crates/biome_css_factory/src/generated/node_factory.rs index 1a3cf39fd5bf..1debcfed1990 100644 --- a/crates/biome_css_factory/src/generated/node_factory.rs +++ b/crates/biome_css_factory/src/generated/node_factory.rs @@ -1746,6 +1746,18 @@ pub fn css_simple_function( ], )) } +pub fn css_starting_style_at_rule( + starting_style_token: SyntaxToken, + block: AnyCssStartingStyleBlock, +) -> CssStartingStyleAtRule { + CssStartingStyleAtRule::unwrap_cast(SyntaxNode::new_detached( + CssSyntaxKind::CSS_STARTING_STYLE_AT_RULE, + [ + Some(SyntaxElement::Token(starting_style_token)), + Some(SyntaxElement::Node(block.into_syntax())), + ], + )) +} pub fn css_string(value_token: SyntaxToken) -> CssString { CssString::unwrap_cast(SyntaxNode::new_detached( CssSyntaxKind::CSS_STRING, diff --git a/crates/biome_css_factory/src/generated/syntax_factory.rs b/crates/biome_css_factory/src/generated/syntax_factory.rs index b7bbcbe96629..d715c2aca649 100644 --- a/crates/biome_css_factory/src/generated/syntax_factory.rs +++ b/crates/biome_css_factory/src/generated/syntax_factory.rs @@ -3525,6 +3525,32 @@ impl SyntaxFactory for CssSyntaxFactory { } slots.into_node(CSS_SIMPLE_FUNCTION, children) } + CSS_STARTING_STYLE_AT_RULE => { + let mut elements = (&children).into_iter(); + let mut slots: RawNodeSlots<2usize> = RawNodeSlots::default(); + let mut current_element = elements.next(); + if let Some(element) = ¤t_element { + if element.kind() == T![starting_style] { + slots.mark_present(); + current_element = elements.next(); + } + } + slots.next_slot(); + if let Some(element) = ¤t_element { + if AnyCssStartingStyleBlock::can_cast(element.kind()) { + slots.mark_present(); + current_element = elements.next(); + } + } + slots.next_slot(); + if current_element.is_some() { + return RawSyntaxNode::new( + CSS_STARTING_STYLE_AT_RULE.to_bogus(), + children.into_iter().map(Some), + ); + } + slots.into_node(CSS_STARTING_STYLE_AT_RULE, children) + } CSS_STRING => { let mut elements = (&children).into_iter(); let mut slots: RawNodeSlots<1usize> = RawNodeSlots::default(); diff --git a/crates/biome_css_formatter/src/css/any/at_rule.rs b/crates/biome_css_formatter/src/css/any/at_rule.rs index 8bc2986486db..438703a98065 100644 --- a/crates/biome_css_formatter/src/css/any/at_rule.rs +++ b/crates/biome_css_formatter/src/css/any/at_rule.rs @@ -22,6 +22,7 @@ impl FormatRule for FormatAnyCssAtRule { AnyCssAtRule::CssScopeAtRule(node) => node.format().fmt(f), AnyCssAtRule::CssImportAtRule(node) => node.format().fmt(f), AnyCssAtRule::CssNamespaceAtRule(node) => node.format().fmt(f), + AnyCssAtRule::CssStartingStyleAtRule(node) => node.format().fmt(f), AnyCssAtRule::CssBogusAtRule(node) => node.format().fmt(f), } } diff --git a/crates/biome_css_formatter/src/css/any/mod.rs b/crates/biome_css_formatter/src/css/any/mod.rs index ceb588b3988c..c64a142f7ee5 100644 --- a/crates/biome_css_formatter/src/css/any/mod.rs +++ b/crates/biome_css_formatter/src/css/any/mod.rs @@ -56,6 +56,7 @@ pub(crate) mod rule_list_block; pub(crate) mod scope_range; pub(crate) mod selector; pub(crate) mod simple_selector; +pub(crate) mod starting_style_block; pub(crate) mod sub_selector; pub(crate) mod supports_and_combinable_condition; pub(crate) mod supports_condition; diff --git a/crates/biome_css_formatter/src/css/any/starting_style_block.rs b/crates/biome_css_formatter/src/css/any/starting_style_block.rs new file mode 100644 index 000000000000..4659a7af2544 --- /dev/null +++ b/crates/biome_css_formatter/src/css/any/starting_style_block.rs @@ -0,0 +1,16 @@ +//! This is a generated file. Don't modify it by hand! Run 'cargo codegen formatter' to re-generate the file. + +use crate::prelude::*; +use biome_css_syntax::AnyCssStartingStyleBlock; +#[derive(Debug, Clone, Default)] +pub(crate) struct FormatAnyCssStartingStyleBlock; +impl FormatRule for FormatAnyCssStartingStyleBlock { + type Context = CssFormatContext; + fn fmt(&self, node: &AnyCssStartingStyleBlock, f: &mut CssFormatter) -> FormatResult<()> { + match node { + AnyCssStartingStyleBlock::CssRuleListBlock(node) => node.format().fmt(f), + AnyCssStartingStyleBlock::CssDeclarationListBlock(node) => node.format().fmt(f), + AnyCssStartingStyleBlock::CssBogusBlock(node) => node.format().fmt(f), + } + } +} diff --git a/crates/biome_css_formatter/src/css/statements/mod.rs b/crates/biome_css_formatter/src/css/statements/mod.rs index a7dcb680ed5c..e5d9b2ff58b9 100644 --- a/crates/biome_css_formatter/src/css/statements/mod.rs +++ b/crates/biome_css_formatter/src/css/statements/mod.rs @@ -15,4 +15,5 @@ pub(crate) mod media_at_rule; pub(crate) mod namespace_at_rule; pub(crate) mod page_at_rule; pub(crate) mod scope_at_rule; +pub(crate) mod starting_style_at_rule; pub(crate) mod supports_at_rule; diff --git a/crates/biome_css_formatter/src/css/statements/starting_style_at_rule.rs b/crates/biome_css_formatter/src/css/statements/starting_style_at_rule.rs new file mode 100644 index 000000000000..d955b2afc2f2 --- /dev/null +++ b/crates/biome_css_formatter/src/css/statements/starting_style_at_rule.rs @@ -0,0 +1,10 @@ +use crate::prelude::*; +use biome_css_syntax::CssStartingStyleAtRule; +use biome_rowan::AstNode; +#[derive(Debug, Clone, Default)] +pub(crate) struct FormatCssStartingStyleAtRule; +impl FormatNodeRule for FormatCssStartingStyleAtRule { + fn fmt_fields(&self, node: &CssStartingStyleAtRule, f: &mut CssFormatter) -> FormatResult<()> { + format_verbatim_node(node.syntax()).fmt(f) + } +} diff --git a/crates/biome_css_formatter/src/generated.rs b/crates/biome_css_formatter/src/generated.rs index 0d4999f85a49..dd2bb1155ec0 100644 --- a/crates/biome_css_formatter/src/generated.rs +++ b/crates/biome_css_formatter/src/generated.rs @@ -2680,6 +2680,46 @@ impl IntoFormat for biome_css_syntax::CssNamespaceAtRule { ) } } +impl FormatRule + for crate::css::statements::starting_style_at_rule::FormatCssStartingStyleAtRule +{ + type Context = CssFormatContext; + #[inline(always)] + fn fmt( + &self, + node: &biome_css_syntax::CssStartingStyleAtRule, + f: &mut CssFormatter, + ) -> FormatResult<()> { + FormatNodeRule::::fmt(self, node, f) + } +} +impl AsFormat for biome_css_syntax::CssStartingStyleAtRule { + type Format<'a> = FormatRefWithRule< + 'a, + biome_css_syntax::CssStartingStyleAtRule, + crate::css::statements::starting_style_at_rule::FormatCssStartingStyleAtRule, + >; + fn format(&self) -> Self::Format<'_> { + #![allow(clippy::default_constructed_unit_structs)] + FormatRefWithRule::new( + self, + crate::css::statements::starting_style_at_rule::FormatCssStartingStyleAtRule::default(), + ) + } +} +impl IntoFormat for biome_css_syntax::CssStartingStyleAtRule { + type Format = FormatOwnedWithRule< + biome_css_syntax::CssStartingStyleAtRule, + crate::css::statements::starting_style_at_rule::FormatCssStartingStyleAtRule, + >; + fn into_format(self) -> Self::Format { + #![allow(clippy::default_constructed_unit_structs)] + FormatOwnedWithRule::new( + self, + crate::css::statements::starting_style_at_rule::FormatCssStartingStyleAtRule::default(), + ) + } +} impl FormatRule for crate::css::auxiliary::container_not_query::FormatCssContainerNotQuery { @@ -7621,6 +7661,33 @@ impl IntoFormat for biome_css_syntax::AnyCssNamespaceUrl { ) } } +impl AsFormat for biome_css_syntax::AnyCssStartingStyleBlock { + type Format<'a> = FormatRefWithRule< + 'a, + biome_css_syntax::AnyCssStartingStyleBlock, + crate::css::any::starting_style_block::FormatAnyCssStartingStyleBlock, + >; + fn format(&self) -> Self::Format<'_> { + #![allow(clippy::default_constructed_unit_structs)] + FormatRefWithRule::new( + self, + crate::css::any::starting_style_block::FormatAnyCssStartingStyleBlock::default(), + ) + } +} +impl IntoFormat for biome_css_syntax::AnyCssStartingStyleBlock { + type Format = FormatOwnedWithRule< + biome_css_syntax::AnyCssStartingStyleBlock, + crate::css::any::starting_style_block::FormatAnyCssStartingStyleBlock, + >; + fn into_format(self) -> Self::Format { + #![allow(clippy::default_constructed_unit_structs)] + FormatOwnedWithRule::new( + self, + crate::css::any::starting_style_block::FormatAnyCssStartingStyleBlock::default(), + ) + } +} impl AsFormat for biome_css_syntax::AnyCssUrlValue { type Format<'a> = FormatRefWithRule< 'a, diff --git a/crates/biome_css_parser/src/lexer/mod.rs b/crates/biome_css_parser/src/lexer/mod.rs index a0be79852968..d83e9e70c44c 100644 --- a/crates/biome_css_parser/src/lexer/mod.rs +++ b/crates/biome_css_parser/src/lexer/mod.rs @@ -997,6 +997,7 @@ impl<'src> CssLexer<'src> { b"scope" => SCOPE_KW, b"import" => IMPORT_KW, b"namespace" => NAMESPACE_KW, + b"starting-style" => STARTING_STYLE_KW, _ => IDENT, } } diff --git a/crates/biome_css_parser/src/state.rs b/crates/biome_css_parser/src/state.rs index 098f104d7061..d47e9120f814 100644 --- a/crates/biome_css_parser/src/state.rs +++ b/crates/biome_css_parser/src/state.rs @@ -11,12 +11,22 @@ pub(crate) struct CssParserState { /// The challenge is, that it isn't possible to tell which of the two kinds it is until the parser /// processed all of `(a, b)`. pub(crate) speculative_parsing: bool, + + /// Indicates whether the parser is currently dealing with a nesting block in the CSS document. + /// + /// This field is essential for understanding the current context of the parser. When set to `true`, + /// it indicates that the parser is inside a nested or inner element, such as rules within media queries + /// or other nested structures. Conversely, when set to `false`, it implies that the parser is at the root level, + /// handling top-level `@rules` or style declarations directly under the stylesheet. + /// This distinction is critical for correctly interpreting and parsing different sections of a CSS document. + pub(crate) is_nesting_block: bool, } impl CssParserState { pub fn new() -> Self { Self { speculative_parsing: false, + is_nesting_block: false, } } } diff --git a/crates/biome_css_parser/src/syntax/at_rule/keyframes.rs b/crates/biome_css_parser/src/syntax/at_rule/keyframes.rs index aa936cafb61e..747d21320f89 100644 --- a/crates/biome_css_parser/src/syntax/at_rule/keyframes.rs +++ b/crates/biome_css_parser/src/syntax/at_rule/keyframes.rs @@ -3,7 +3,7 @@ use crate::parser::CssParser; use crate::syntax::at_rule::parse_error::{ expected_keyframes_item, expected_keyframes_item_selector, }; -use crate::syntax::blocks::parse_declaration_list_block; +use crate::syntax::blocks::{parse_block_body, parse_declaration_list_block}; use crate::syntax::css_dimension::{is_at_percentage_dimension, parse_percentage_dimension}; use crate::syntax::parse_error::{expected_block, expected_non_css_wide_keyword_identifier}; use crate::syntax::{ @@ -74,10 +74,9 @@ fn parse_keyframes_block(p: &mut CssParser) -> ParsedSyntax { return Absent; } - let m = p.start(); - p.bump(T!['{']); - KeyframesItemList.parse_list(p); - p.expect(T!['}']); + let m = parse_block_body(p, |p| { + KeyframesItemList.parse_list(p); + }); Present(m.complete(p, CSS_KEYFRAMES_BLOCK)) } @@ -141,7 +140,7 @@ impl ParseRecovery for KeyframesItemBlockParseRecovery { // color: blue; // } // } - p.at_ts(token_set!(T!['}'])) || is_at_keyframes_item_selector(p) + p.at(T!['}']) || is_at_keyframes_item_selector(p) } } diff --git a/crates/biome_css_parser/src/syntax/at_rule/mod.rs b/crates/biome_css_parser/src/syntax/at_rule/mod.rs index 9ba7b25b862d..dcba616b66df 100644 --- a/crates/biome_css_parser/src/syntax/at_rule/mod.rs +++ b/crates/biome_css_parser/src/syntax/at_rule/mod.rs @@ -13,6 +13,7 @@ mod namespace; mod page; mod parse_error; mod scope; +mod starting_style; mod supports; use crate::parser::CssParser; @@ -35,6 +36,9 @@ use crate::syntax::at_rule::media::{is_at_media_at_rule, parse_media_at_rule}; use crate::syntax::at_rule::namespace::{is_at_namespace_at_rule, parse_namespace_at_rule}; use crate::syntax::at_rule::page::{is_at_page_at_rule, parse_page_at_rule}; use crate::syntax::at_rule::scope::{is_at_scope_at_rule, parse_scope_at_rule}; +use crate::syntax::at_rule::starting_style::{ + is_at_starting_style_at_rule, parse_starting_style_at_rule, +}; use crate::syntax::at_rule::supports::{is_at_supports_at_rule, parse_supports_at_rule}; use crate::syntax::parse_error::expected_any_at_rule; use biome_css_syntax::CssSyntaxKind::*; @@ -99,6 +103,8 @@ pub(crate) fn parse_any_at_rule(p: &mut CssParser) -> ParsedSyntax { parse_import_at_rule(p) } else if is_at_namespace_at_rule(p) { parse_namespace_at_rule(p) + } else if is_at_starting_style_at_rule(p) { + parse_starting_style_at_rule(p) } else { Absent } diff --git a/crates/biome_css_parser/src/syntax/at_rule/page.rs b/crates/biome_css_parser/src/syntax/at_rule/page.rs index 11374805672d..6a1d4b208019 100644 --- a/crates/biome_css_parser/src/syntax/at_rule/page.rs +++ b/crates/biome_css_parser/src/syntax/at_rule/page.rs @@ -4,7 +4,7 @@ use crate::syntax::at_rule::parse_error::{ expected_any_page_at_rule_item, expected_page_selector, expected_page_selector_pseudo, }; use crate::syntax::at_rule::{is_at_at_rule, parse_at_rule}; -use crate::syntax::blocks::parse_or_recover_declaration_or_rule_list_block; +use crate::syntax::blocks::{parse_block_body, parse_or_recover_declaration_or_rule_list_block}; use crate::syntax::parse_error::expected_block; use crate::syntax::{ is_at_identifier, parse_custom_identifier_with_keywords, parse_declaration_with_semicolon, @@ -176,11 +176,10 @@ pub(crate) fn parse_page_block(p: &mut CssParser) -> ParsedSyntax { if !p.at(T!['{']) { return Absent; } - let m = p.start(); - p.expect(T!['{']); - PageAtRuleItemList.parse_list(p); - p.expect(T!['}']); + let m = parse_block_body(p, |p| { + PageAtRuleItemList.parse_list(p); + }); Present(m.complete(p, CSS_PAGE_AT_RULE_BLOCK)) } diff --git a/crates/biome_css_parser/src/syntax/at_rule/starting_style.rs b/crates/biome_css_parser/src/syntax/at_rule/starting_style.rs new file mode 100644 index 000000000000..2c4a889ba661 --- /dev/null +++ b/crates/biome_css_parser/src/syntax/at_rule/starting_style.rs @@ -0,0 +1,65 @@ +use crate::parser::CssParser; +use crate::syntax::blocks::{ + parse_or_recover_declaration_list_block, parse_or_recover_rule_list_block, +}; +use biome_css_syntax::CssSyntaxKind::*; +use biome_css_syntax::T; +use biome_parser::parsed_syntax::ParsedSyntax::Present; +use biome_parser::prelude::ParsedSyntax::Absent; +use biome_parser::prelude::*; + +/// Checks if the current token in the parser is a `@starting-style` at-rule. +/// +/// This function verifies if the current token matches the `@starting-style` rule, +/// which is a custom at-rule used for specific parsing scenarios. +#[inline] +pub(crate) fn is_at_starting_style_at_rule(p: &mut CssParser) -> bool { + p.at(T![starting_style]) +} + +/// Parses a `@starting-style` at-rule in a CSS stylesheet. +/// +/// This function handles the parsing of a `@starting-style` at-rule, which is defined in the +/// CSS Transitions Level 2 specification. It starts by confirming the presence of such a rule and then +/// processes the content depending on whether the parser is currently inside a nesting block or at the root level. +/// It employs different parsing strategies for declarations or rules based on the parser state `is_nesting_block`. +/// +/// Specification: [CSS Transitions Level 2 - @starting-style](https://drafts.csswg.org/css-transitions-2/#at-ruledef-starting-style) +/// # Examples +/// Basic usage in a CSS stylesheet: +/// +/// ```css +/// // At the root level of a stylesheet +/// @starting-style { +/// /* rulesets */ +/// } +/// +/// // Inside a selector +/// selector { +/// @starting-style { +/// /* declarations */ +/// } +/// } +/// ``` +#[inline] +pub(crate) fn parse_starting_style_at_rule(p: &mut CssParser) -> ParsedSyntax { + if !is_at_starting_style_at_rule(p) { + return Absent; + } + + let m = p.start(); + + p.bump(T![starting_style]); + + let block = if p.state().is_nesting_block { + parse_or_recover_declaration_list_block(p) + } else { + parse_or_recover_rule_list_block(p) + }; + + if block.is_err() { + return Present(m.complete(p, CSS_BOGUS_AT_RULE)); + } + + Present(m.complete(p, CSS_STARTING_STYLE_AT_RULE)) +} diff --git a/crates/biome_css_parser/src/syntax/blocks.rs b/crates/biome_css_parser/src/syntax/blocks.rs index aa00755b67fe..c44af41f0a93 100644 --- a/crates/biome_css_parser/src/syntax/blocks.rs +++ b/crates/biome_css_parser/src/syntax/blocks.rs @@ -2,15 +2,16 @@ use crate::parser::CssParser; use crate::syntax::at_rule::{is_at_at_rule, parse_at_rule}; use crate::syntax::parse_error::{expected_any_declaration_or_at_rule, expected_block}; use crate::syntax::{ - parse_declaration_with_semicolon, DeclarationList, RuleList, BODY_RECOVERY_SET, + is_at_declaration, parse_declaration_with_semicolon, DeclarationList, RuleList, + BODY_RECOVERY_SET, }; use biome_css_syntax::CssSyntaxKind::*; use biome_css_syntax::{CssSyntaxKind, T}; use biome_parser::parse_lists::{ParseNodeList, ParseSeparatedList}; -use biome_parser::parse_recovery::{ParseRecoveryTokenSet, RecoveryResult}; +use biome_parser::parse_recovery::{ParseRecovery, ParseRecoveryTokenSet, RecoveryResult}; use biome_parser::parsed_syntax::ParsedSyntax; use biome_parser::parsed_syntax::ParsedSyntax::{Absent, Present}; -use biome_parser::{token_set, Parser, TokenSet}; +use biome_parser::{Marker, Parser}; #[inline] pub(crate) fn parse_or_recover_declaration_list_block(p: &mut CssParser) -> RecoveryResult { @@ -28,11 +29,9 @@ pub(crate) fn parse_declaration_list_block(p: &mut CssParser) -> ParsedSyntax { return Absent; } - let m = p.start(); - - p.bump(T!['{']); - DeclarationList.parse_list(p); - p.expect(T!['}']); + let m = parse_block_body(p, |p| { + DeclarationList.parse_list(p); + }); Present(m.complete(p, CSS_DECLARATION_LIST_BLOCK)) } @@ -53,11 +52,9 @@ pub(crate) fn parse_rule_list_block(p: &mut CssParser) -> ParsedSyntax { return Absent; } - let m = p.start(); - - p.expect(T!['{']); - RuleList::new(T!['}']).parse_list(p); - p.expect(T!['}']); + let m = parse_block_body(p, |p| { + RuleList::new(T!['}']).parse_list(p); + }); Present(m.complete(p, CSS_RULE_LIST_BLOCK)) } @@ -78,17 +75,24 @@ pub(crate) fn parse_declaration_or_rule_list_block(p: &mut CssParser) -> ParsedS return Absent; } - let m = p.start(); - - p.expect(T!['{']); - DeclarationOrAtRuleList.parse_list(p); - p.expect(T!['}']); + let m = parse_block_body(p, |p| { + DeclarationOrAtRuleList.parse_list(p); + }); Present(m.complete(p, CSS_DECLARATION_OR_AT_RULE_BLOCK)) } -const CSS_DECLARATION_OR_AT_RULE_LIST_RECOVERY_SET: TokenSet = - token_set!(T![@], T![ident]); +struct DeclarationOrAtRuleListParseRecovery; +impl ParseRecovery for DeclarationOrAtRuleListParseRecovery { + type Kind = CssSyntaxKind; + type Parser<'source> = CssParser<'source>; + const RECOVERED_KIND: Self::Kind = CSS_BOGUS; + + fn is_at_recovered(&self, p: &mut Self::Parser<'_>) -> bool { + p.at(T!['}']) || is_at_at_rule(p) || is_at_declaration(p) + } +} + struct DeclarationOrAtRuleList; impl ParseNodeList for DeclarationOrAtRuleList { type Kind = CssSyntaxKind; @@ -112,10 +116,27 @@ impl ParseNodeList for DeclarationOrAtRuleList { p: &mut Self::Parser<'_>, parsed_element: ParsedSyntax, ) -> RecoveryResult { - parsed_element.or_recover_with_token_set( + parsed_element.or_recover( p, - &ParseRecoveryTokenSet::new(CSS_BOGUS, CSS_DECLARATION_OR_AT_RULE_LIST_RECOVERY_SET), + &DeclarationOrAtRuleListParseRecovery, expected_any_declaration_or_at_rule, ) } } + +/// Parses the body of a block in CSS. +/// +/// This function handles the parsing of a block's content, delimited by curly braces `{}`. +/// It temporarily sets the parser's state to indicate it is within a nesting block and then +/// processes the content of the block using the provided callback function. +pub(crate) fn parse_block_body(p: &mut CssParser, func: impl FnOnce(&mut CssParser)) -> Marker { + let old_nesting_block = std::mem::replace(&mut p.state_mut().is_nesting_block, true); + + let m = p.start(); + p.bump(T!['{']); + func(p); + p.expect(T!['}']); + + p.state_mut().is_nesting_block = old_nesting_block; + m +} diff --git a/crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css b/crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css new file mode 100644 index 000000000000..f3448388d27f --- /dev/null +++ b/crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css @@ -0,0 +1,15 @@ +@starting-style { + h1 { + background-color: transparent; + } + + @layer foo { + @starting-style { + background-color: transparent; + } + + div { + height: 100px; + } + } +} diff --git a/crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css.snap b/crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css.snap new file mode 100644 index 000000000000..9bd15c775fad --- /dev/null +++ b/crates/biome_css_parser/tests/css_test_suite/ok/at_rule/at_rule_starting_style.css.snap @@ -0,0 +1,266 @@ +--- +source: crates/biome_css_parser/tests/spec_test.rs +expression: snapshot +--- + +## Input + +```css +@starting-style { + h1 { + background-color: transparent; + } + + @layer foo { + @starting-style { + background-color: transparent; + } + + div { + height: 100px; + } + } +} + +``` + + +## AST + +``` +CssRoot { + bom_token: missing (optional), + rules: CssRuleList [ + CssAtRule { + at_token: AT@0..1 "@" [] [], + rule: CssStartingStyleAtRule { + starting_style_token: STARTING_STYLE_KW@1..16 "starting-style" [] [Whitespace(" ")], + block: CssRuleListBlock { + l_curly_token: L_CURLY@16..17 "{" [] [], + rules: CssRuleList [ + CssRule { + prelude: CssSelectorList [ + CssCompoundSelector { + nesting_selector_token: missing (optional), + simple_selector: CssTypeSelector { + namespace: missing (optional), + ident: CssIdentifier { + value_token: IDENT@17..22 "h1" [Newline("\n"), Whitespace("\t")] [Whitespace(" ")], + }, + }, + sub_selectors: CssSubSelectorList [], + }, + ], + block: CssDeclarationListBlock { + l_curly_token: L_CURLY@22..23 "{" [] [], + declarations: CssDeclarationList [ + CssDeclaration { + property: CssGenericProperty { + name: CssIdentifier { + value_token: IDENT@23..42 "background-color" [Newline("\n"), Whitespace("\t\t")] [], + }, + colon_token: COLON@42..44 ":" [] [Whitespace(" ")], + value: CssGenericComponentValueList [ + CssIdentifier { + value_token: IDENT@44..55 "transparent" [] [], + }, + ], + }, + important: missing (optional), + }, + SEMICOLON@55..56 ";" [] [], + ], + r_curly_token: R_CURLY@56..59 "}" [Newline("\n"), Whitespace("\t")] [], + }, + }, + CssAtRule { + at_token: AT@59..63 "@" [Newline("\n"), Newline("\n"), Whitespace("\t")] [], + rule: CssLayerAtRule { + layer_token: LAYER_KW@63..69 "layer" [] [Whitespace(" ")], + layer: CssLayerDeclaration { + references: CssLayerReferenceList [ + CssLayerNameList [ + CssIdentifier { + value_token: IDENT@69..73 "foo" [] [Whitespace(" ")], + }, + ], + ], + block: CssRuleListBlock { + l_curly_token: L_CURLY@73..74 "{" [] [], + rules: CssRuleList [ + CssAtRule { + at_token: AT@74..78 "@" [Newline("\n"), Whitespace("\t\t")] [], + rule: CssStartingStyleAtRule { + starting_style_token: STARTING_STYLE_KW@78..93 "starting-style" [] [Whitespace(" ")], + block: CssDeclarationListBlock { + l_curly_token: L_CURLY@93..94 "{" [] [], + declarations: CssDeclarationList [ + CssDeclaration { + property: CssGenericProperty { + name: CssIdentifier { + value_token: IDENT@94..114 "background-color" [Newline("\n"), Whitespace("\t\t\t")] [], + }, + colon_token: COLON@114..116 ":" [] [Whitespace(" ")], + value: CssGenericComponentValueList [ + CssIdentifier { + value_token: IDENT@116..127 "transparent" [] [], + }, + ], + }, + important: missing (optional), + }, + SEMICOLON@127..128 ";" [] [], + ], + r_curly_token: R_CURLY@128..132 "}" [Newline("\n"), Whitespace("\t\t")] [], + }, + }, + }, + CssRule { + prelude: CssSelectorList [ + CssCompoundSelector { + nesting_selector_token: missing (optional), + simple_selector: CssTypeSelector { + namespace: missing (optional), + ident: CssIdentifier { + value_token: IDENT@132..140 "div" [Newline("\n"), Newline("\n"), Whitespace("\t\t")] [Whitespace(" ")], + }, + }, + sub_selectors: CssSubSelectorList [], + }, + ], + block: CssDeclarationListBlock { + l_curly_token: L_CURLY@140..141 "{" [] [], + declarations: CssDeclarationList [ + CssDeclaration { + property: CssGenericProperty { + name: CssIdentifier { + value_token: IDENT@141..151 "height" [Newline("\n"), Whitespace("\t\t\t")] [], + }, + colon_token: COLON@151..153 ":" [] [Whitespace(" ")], + value: CssGenericComponentValueList [ + CssRegularDimension { + value_token: CSS_NUMBER_LITERAL@153..156 "100" [] [], + unit_token: IDENT@156..158 "px" [] [], + }, + ], + }, + important: missing (optional), + }, + SEMICOLON@158..159 ";" [] [], + ], + r_curly_token: R_CURLY@159..163 "}" [Newline("\n"), Whitespace("\t\t")] [], + }, + }, + ], + r_curly_token: R_CURLY@163..166 "}" [Newline("\n"), Whitespace("\t")] [], + }, + }, + }, + }, + ], + r_curly_token: R_CURLY@166..168 "}" [Newline("\n")] [], + }, + }, + }, + ], + eof_token: EOF@168..169 "" [Newline("\n")] [], +} +``` + +## CST + +``` +0: CSS_ROOT@0..169 + 0: (empty) + 1: CSS_RULE_LIST@0..168 + 0: CSS_AT_RULE@0..168 + 0: AT@0..1 "@" [] [] + 1: CSS_STARTING_STYLE_AT_RULE@1..168 + 0: STARTING_STYLE_KW@1..16 "starting-style" [] [Whitespace(" ")] + 1: CSS_RULE_LIST_BLOCK@16..168 + 0: L_CURLY@16..17 "{" [] [] + 1: CSS_RULE_LIST@17..166 + 0: CSS_RULE@17..59 + 0: CSS_SELECTOR_LIST@17..22 + 0: CSS_COMPOUND_SELECTOR@17..22 + 0: (empty) + 1: CSS_TYPE_SELECTOR@17..22 + 0: (empty) + 1: CSS_IDENTIFIER@17..22 + 0: IDENT@17..22 "h1" [Newline("\n"), Whitespace("\t")] [Whitespace(" ")] + 2: CSS_SUB_SELECTOR_LIST@22..22 + 1: CSS_DECLARATION_LIST_BLOCK@22..59 + 0: L_CURLY@22..23 "{" [] [] + 1: CSS_DECLARATION_LIST@23..56 + 0: CSS_DECLARATION@23..55 + 0: CSS_GENERIC_PROPERTY@23..55 + 0: CSS_IDENTIFIER@23..42 + 0: IDENT@23..42 "background-color" [Newline("\n"), Whitespace("\t\t")] [] + 1: COLON@42..44 ":" [] [Whitespace(" ")] + 2: CSS_GENERIC_COMPONENT_VALUE_LIST@44..55 + 0: CSS_IDENTIFIER@44..55 + 0: IDENT@44..55 "transparent" [] [] + 1: (empty) + 1: SEMICOLON@55..56 ";" [] [] + 2: R_CURLY@56..59 "}" [Newline("\n"), Whitespace("\t")] [] + 1: CSS_AT_RULE@59..166 + 0: AT@59..63 "@" [Newline("\n"), Newline("\n"), Whitespace("\t")] [] + 1: CSS_LAYER_AT_RULE@63..166 + 0: LAYER_KW@63..69 "layer" [] [Whitespace(" ")] + 1: CSS_LAYER_DECLARATION@69..166 + 0: CSS_LAYER_REFERENCE_LIST@69..73 + 0: CSS_LAYER_NAME_LIST@69..73 + 0: CSS_IDENTIFIER@69..73 + 0: IDENT@69..73 "foo" [] [Whitespace(" ")] + 1: CSS_RULE_LIST_BLOCK@73..166 + 0: L_CURLY@73..74 "{" [] [] + 1: CSS_RULE_LIST@74..163 + 0: CSS_AT_RULE@74..132 + 0: AT@74..78 "@" [Newline("\n"), Whitespace("\t\t")] [] + 1: CSS_STARTING_STYLE_AT_RULE@78..132 + 0: STARTING_STYLE_KW@78..93 "starting-style" [] [Whitespace(" ")] + 1: CSS_DECLARATION_LIST_BLOCK@93..132 + 0: L_CURLY@93..94 "{" [] [] + 1: CSS_DECLARATION_LIST@94..128 + 0: CSS_DECLARATION@94..127 + 0: CSS_GENERIC_PROPERTY@94..127 + 0: CSS_IDENTIFIER@94..114 + 0: IDENT@94..114 "background-color" [Newline("\n"), Whitespace("\t\t\t")] [] + 1: COLON@114..116 ":" [] [Whitespace(" ")] + 2: CSS_GENERIC_COMPONENT_VALUE_LIST@116..127 + 0: CSS_IDENTIFIER@116..127 + 0: IDENT@116..127 "transparent" [] [] + 1: (empty) + 1: SEMICOLON@127..128 ";" [] [] + 2: R_CURLY@128..132 "}" [Newline("\n"), Whitespace("\t\t")] [] + 1: CSS_RULE@132..163 + 0: CSS_SELECTOR_LIST@132..140 + 0: CSS_COMPOUND_SELECTOR@132..140 + 0: (empty) + 1: CSS_TYPE_SELECTOR@132..140 + 0: (empty) + 1: CSS_IDENTIFIER@132..140 + 0: IDENT@132..140 "div" [Newline("\n"), Newline("\n"), Whitespace("\t\t")] [Whitespace(" ")] + 2: CSS_SUB_SELECTOR_LIST@140..140 + 1: CSS_DECLARATION_LIST_BLOCK@140..163 + 0: L_CURLY@140..141 "{" [] [] + 1: CSS_DECLARATION_LIST@141..159 + 0: CSS_DECLARATION@141..158 + 0: CSS_GENERIC_PROPERTY@141..158 + 0: CSS_IDENTIFIER@141..151 + 0: IDENT@141..151 "height" [Newline("\n"), Whitespace("\t\t\t")] [] + 1: COLON@151..153 ":" [] [Whitespace(" ")] + 2: CSS_GENERIC_COMPONENT_VALUE_LIST@153..158 + 0: CSS_REGULAR_DIMENSION@153..158 + 0: CSS_NUMBER_LITERAL@153..156 "100" [] [] + 1: IDENT@156..158 "px" [] [] + 1: (empty) + 1: SEMICOLON@158..159 ";" [] [] + 2: R_CURLY@159..163 "}" [Newline("\n"), Whitespace("\t\t")] [] + 2: R_CURLY@163..166 "}" [Newline("\n"), Whitespace("\t")] [] + 2: R_CURLY@166..168 "}" [Newline("\n")] [] + 2: EOF@168..169 "" [Newline("\n")] [] + +``` + + diff --git a/crates/biome_css_parser/tests/spec_test.rs b/crates/biome_css_parser/tests/spec_test.rs index 779c6f0ef44d..bfdc68a1e4ca 100644 --- a/crates/biome_css_parser/tests/spec_test.rs +++ b/crates/biome_css_parser/tests/spec_test.rs @@ -135,9 +135,16 @@ pub fn run(test_case: &str, _snapshot_name: &str, test_directory: &str, outcome_ pub fn quick_test() { let code = r#" - -@namespace url(http://www.w3.org/1999/xhtml); - +@starting-style { + h1 { + background-color: transparent; + } + @layer foo { + @starting-style { + height: 100px; + } + } +} "#; let root = parse_css( diff --git a/crates/biome_css_syntax/src/generated/kind.rs b/crates/biome_css_syntax/src/generated/kind.rs index 4c9eb3e047d8..e246840a8215 100644 --- a/crates/biome_css_syntax/src/generated/kind.rs +++ b/crates/biome_css_syntax/src/generated/kind.rs @@ -212,6 +212,7 @@ pub enum CssSyntaxKind { SELECTOR_KW, IMPORT_KW, NAMESPACE_KW, + STARTING_STYLE_KW, FONT_FACE_KW, CSS_STRING_LITERAL, CSS_NUMBER_LITERAL, @@ -388,6 +389,7 @@ pub enum CssSyntaxKind { CSS_IMPORT_NAMED_LAYER, CSS_IMPORT_SUPPORTS, CSS_NAMESPACE_AT_RULE, + CSS_STARTING_STYLE_AT_RULE, CSS_BOGUS, CSS_BOGUS_BLOCK, CSS_BOGUS_KEYFRAMES_ITEM, @@ -627,6 +629,7 @@ impl CssSyntaxKind { "selector" => SELECTOR_KW, "import" => IMPORT_KW, "namespace" => NAMESPACE_KW, + "starting-style" => STARTING_STYLE_KW, "font-face" => FONT_FACE_KW, _ => return None, }; @@ -833,6 +836,7 @@ impl CssSyntaxKind { SELECTOR_KW => "selector", IMPORT_KW => "import", NAMESPACE_KW => "namespace", + STARTING_STYLE_KW => "starting-style", FONT_FACE_KW => "font-face", CSS_STRING_LITERAL => "string literal", _ => return None, @@ -842,4 +846,4 @@ impl CssSyntaxKind { } #[doc = r" Utility macro for creating a SyntaxKind through simple macro syntax"] #[macro_export] -macro_rules ! T { [;] => { $ crate :: CssSyntaxKind :: SEMICOLON } ; [,] => { $ crate :: CssSyntaxKind :: COMMA } ; ['('] => { $ crate :: CssSyntaxKind :: L_PAREN } ; [')'] => { $ crate :: CssSyntaxKind :: R_PAREN } ; ['{'] => { $ crate :: CssSyntaxKind :: L_CURLY } ; ['}'] => { $ crate :: CssSyntaxKind :: R_CURLY } ; ['['] => { $ crate :: CssSyntaxKind :: L_BRACK } ; [']'] => { $ crate :: CssSyntaxKind :: R_BRACK } ; [<] => { $ crate :: CssSyntaxKind :: L_ANGLE } ; [>] => { $ crate :: CssSyntaxKind :: R_ANGLE } ; [~] => { $ crate :: CssSyntaxKind :: TILDE } ; [#] => { $ crate :: CssSyntaxKind :: HASH } ; [&] => { $ crate :: CssSyntaxKind :: AMP } ; [|] => { $ crate :: CssSyntaxKind :: PIPE } ; [||] => { $ crate :: CssSyntaxKind :: PIPE2 } ; [+] => { $ crate :: CssSyntaxKind :: PLUS } ; [*] => { $ crate :: CssSyntaxKind :: STAR } ; [/] => { $ crate :: CssSyntaxKind :: SLASH } ; [^] => { $ crate :: CssSyntaxKind :: CARET } ; [%] => { $ crate :: CssSyntaxKind :: PERCENT } ; [.] => { $ crate :: CssSyntaxKind :: DOT } ; [:] => { $ crate :: CssSyntaxKind :: COLON } ; [::] => { $ crate :: CssSyntaxKind :: COLON2 } ; [=] => { $ crate :: CssSyntaxKind :: EQ } ; [!] => { $ crate :: CssSyntaxKind :: BANG } ; [!=] => { $ crate :: CssSyntaxKind :: NEQ } ; [-] => { $ crate :: CssSyntaxKind :: MINUS } ; [<=] => { $ crate :: CssSyntaxKind :: LTEQ } ; [>=] => { $ crate :: CssSyntaxKind :: GTEQ } ; [+=] => { $ crate :: CssSyntaxKind :: PLUSEQ } ; [|=] => { $ crate :: CssSyntaxKind :: PIPEEQ } ; [&=] => { $ crate :: CssSyntaxKind :: AMPEQ } ; [^=] => { $ crate :: CssSyntaxKind :: CARETEQ } ; [/=] => { $ crate :: CssSyntaxKind :: SLASHEQ } ; [*=] => { $ crate :: CssSyntaxKind :: STAREQ } ; [%=] => { $ crate :: CssSyntaxKind :: PERCENTEQ } ; [@] => { $ crate :: CssSyntaxKind :: AT } ; ["$="] => { $ crate :: CssSyntaxKind :: DOLLAR_EQ } ; [~=] => { $ crate :: CssSyntaxKind :: TILDE_EQ } ; [-->] => { $ crate :: CssSyntaxKind :: CDC } ; [] => { $ crate :: CssSyntaxKind :: CDC } ; [