From 9e22611d93fb84ac56306696f4c814c640b1f855 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 30 Jun 2022 08:52:27 +0100 Subject: [PATCH] feat(rome_js_formatter): class property members as assignment like (#2797) --- .../src/js/classes/property_class_member.rs | 30 +----- .../property_signature_class_member.rs | 27 ++--- .../src/utils/assignment_like.rs | 99 ++++++++++++++++--- crates/rome_js_formatter/src/utils/object.rs | 34 +++++-- .../js/class-comment/class-property.js.snap | 9 +- .../with_comments.js.snap | 9 +- .../prettier/js/classes/property.js.snap | 13 +-- .../typescript/assignment/issue-2485.ts.snap | 9 +- 8 files changed, 146 insertions(+), 84 deletions(-) diff --git a/crates/rome_js_formatter/src/js/classes/property_class_member.rs b/crates/rome_js_formatter/src/js/classes/property_class_member.rs index 8057be6559e..ec5e707206d 100644 --- a/crates/rome_js_formatter/src/js/classes/property_class_member.rs +++ b/crates/rome_js_formatter/src/js/classes/property_class_member.rs @@ -1,38 +1,18 @@ use crate::prelude::*; -use rome_formatter::{format_args, write}; - -use crate::utils::FormatWithSemicolon; - +use crate::utils::{FormatWithSemicolon, JsAnyAssignmentLike}; +use rome_formatter::write; use rome_js_syntax::JsPropertyClassMember; -use rome_js_syntax::JsPropertyClassMemberFields; #[derive(Debug, Clone, Default)] pub struct FormatJsPropertyClassMember; impl FormatNodeRule for FormatJsPropertyClassMember { fn fmt_fields(&self, node: &JsPropertyClassMember, f: &mut JsFormatter) -> FormatResult<()> { - let JsPropertyClassMemberFields { - modifiers, - name, - property_annotation, - value, - semicolon_token, - } = node.as_fields(); - + let semicolon_token = node.semicolon_token(); + let body = format_with(|f| write!(f, [JsAnyAssignmentLike::from(node.clone())])); write!( f, - [FormatWithSemicolon::new( - &format_args!( - modifiers.format(), - space_token(), - name.format(), - property_annotation.format(), - value - .format() - .with_or_empty(|node, f| write![f, [space_token(), node]]), - ), - semicolon_token.as_ref() - )] + [FormatWithSemicolon::new(&body, semicolon_token.as_ref())] ) } } diff --git a/crates/rome_js_formatter/src/ts/classes/property_signature_class_member.rs b/crates/rome_js_formatter/src/ts/classes/property_signature_class_member.rs index de238db955f..c5c2d84584e 100644 --- a/crates/rome_js_formatter/src/ts/classes/property_signature_class_member.rs +++ b/crates/rome_js_formatter/src/ts/classes/property_signature_class_member.rs @@ -1,9 +1,7 @@ use crate::prelude::*; -use rome_formatter::{format_args, write}; - -use crate::utils::FormatWithSemicolon; - -use rome_js_syntax::{TsPropertySignatureClassMember, TsPropertySignatureClassMemberFields}; +use crate::utils::{FormatWithSemicolon, JsAnyAssignmentLike}; +use rome_formatter::write; +use rome_js_syntax::TsPropertySignatureClassMember; #[derive(Debug, Clone, Default)] pub struct FormatTsPropertySignatureClassMember; @@ -14,24 +12,11 @@ impl FormatNodeRule for FormatTsPropertySignatur node: &TsPropertySignatureClassMember, f: &mut JsFormatter, ) -> FormatResult<()> { - let TsPropertySignatureClassMemberFields { - modifiers, - name, - property_annotation, - semicolon_token, - } = node.as_fields(); - + let semicolon_token = node.semicolon_token(); + let body = format_with(|f| write!(f, [JsAnyAssignmentLike::from(node.clone())])); write!( f, - [FormatWithSemicolon::new( - &format_args!( - modifiers.format(), - space_token(), - name.format(), - property_annotation.format(), - ), - semicolon_token.as_ref() - )] + [FormatWithSemicolon::new(&body, semicolon_token.as_ref())] ) } } diff --git a/crates/rome_js_formatter/src/utils/assignment_like.rs b/crates/rome_js_formatter/src/utils/assignment_like.rs index f15136b5a19..728261e1cbc 100644 --- a/crates/rome_js_formatter/src/utils/assignment_like.rs +++ b/crates/rome_js_formatter/src/utils/assignment_like.rs @@ -3,11 +3,13 @@ use crate::utils::object::write_member_name; use crate::utils::JsAnyBinaryLikeExpression; use rome_formatter::{format_args, write, VecBuffer}; use rome_js_syntax::{ - JsAnyAssignmentPattern, JsAnyBindingPattern, JsAnyExpression, JsAnyFunctionBody, - JsAnyObjectAssignmentPatternMember, JsAnyObjectBindingPatternMember, JsAnyObjectMemberName, - JsAssignmentExpression, JsInitializerClause, JsObjectAssignmentPattern, - JsObjectAssignmentPatternProperty, JsObjectBindingPattern, JsPropertyObjectMember, - JsSyntaxKind, JsVariableDeclarator, TsAnyVariableAnnotation, TsIdentifierBinding, TsType, + JsAnyAssignmentPattern, JsAnyBindingPattern, JsAnyClassMemberName, JsAnyExpression, + JsAnyFunctionBody, JsAnyObjectAssignmentPatternMember, JsAnyObjectBindingPatternMember, + JsAnyObjectMemberName, JsAssignmentExpression, JsInitializerClause, JsLiteralMemberName, + JsObjectAssignmentPattern, JsObjectAssignmentPatternProperty, JsObjectBindingPattern, + JsPropertyClassMember, JsPropertyClassMemberFields, JsPropertyObjectMember, JsSyntaxKind, + JsVariableDeclarator, TsAnyVariableAnnotation, TsIdentifierBinding, + TsPropertySignatureClassMember, TsPropertySignatureClassMemberFields, TsType, TsTypeAliasDeclaration, }; use rome_js_syntax::{JsAnyLiteralExpression, JsSyntaxNode}; @@ -19,11 +21,19 @@ declare_node_union! { JsAssignmentExpression | JsObjectAssignmentPatternProperty | JsVariableDeclarator | - TsTypeAliasDeclaration + TsTypeAliasDeclaration | + JsPropertyClassMember | + TsPropertySignatureClassMember } declare_node_union! { - pub(crate) LeftAssignmentLike = JsAnyAssignmentPattern | JsAnyObjectMemberName | JsAnyBindingPattern | TsIdentifierBinding + pub(crate) LeftAssignmentLike = + JsAnyAssignmentPattern | + JsAnyObjectMemberName | + JsAnyBindingPattern | + TsIdentifierBinding | + JsLiteralMemberName | + JsAnyClassMemberName } declare_node_union! { @@ -297,12 +307,19 @@ impl JsAnyAssignmentLike { Ok(assignment_pattern.pattern()?.into()) } JsAnyAssignmentLike::JsVariableDeclarator(variable_declarator) => { - // SAFETY: Calling `unwrap` here is safe because we check `should_only_left` variant at the beginning of the `layout` function + // SAFETY: Calling `unwrap` here is safe because we check `has_only_left_hand_side` variant at the beginning of the `layout` function Ok(variable_declarator.initializer().unwrap().into()) } JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => { Ok(type_alias_declaration.ty()?.into()) } + JsAnyAssignmentLike::JsPropertyClassMember(n) => { + // SAFETY: Calling `unwrap` here is safe because we check `has_only_left_hand_side` variant at the beginning of the `layout` function + Ok(n.value().unwrap().into()) + } + JsAnyAssignmentLike::TsPropertySignatureClassMember(_) => { + unreachable!("TsPropertySignatureClassMember doesn't have any right side. If you're here, `has_only_left_hand_side` hasn't been called") + } } } @@ -321,6 +338,12 @@ impl JsAnyAssignmentLike { JsAnyAssignmentLike::TsTypeAliasDeclaration(type_alias_declaration) => { Ok(type_alias_declaration.binding_identifier()?.into()) } + JsAnyAssignmentLike::JsPropertyClassMember(property_class_member) => { + Ok(property_class_member.name()?.into()) + } + JsAnyAssignmentLike::TsPropertySignatureClassMember( + property_signature_class_member, + ) => Ok(property_signature_class_member.name()?.into()), } } @@ -340,7 +363,7 @@ impl JsAnyAssignmentLike { fn write_left(&self, f: &mut JsFormatter) -> FormatResult { match self { JsAnyAssignmentLike::JsPropertyObjectMember(property) => { - let width = write_member_name(&property.name()?, f)?; + let width = write_member_name(&property.name()?.into(), f)?; let text_width_for_break = (u8::from(f.context().tab_width()) + MIN_OVERLAP_FOR_BREAK) as usize; Ok(width < text_width_for_break) @@ -351,7 +374,7 @@ impl JsAnyAssignmentLike { Ok(false) } JsAnyAssignmentLike::JsObjectAssignmentPatternProperty(property) => { - let width = write_member_name(&property.member()?, f)?; + let width = write_member_name(&property.member()?.into(), f)?; let text_width_for_break = (u8::from(f.context().tab_width()) + MIN_OVERLAP_FOR_BREAK) as usize; Ok(width < text_width_for_break) @@ -373,6 +396,40 @@ impl JsAnyAssignmentLike { } Ok(false) } + JsAnyAssignmentLike::JsPropertyClassMember(property_class_member) => { + let JsPropertyClassMemberFields { + modifiers, + name, + property_annotation, + value: _, + semicolon_token: _, + } = property_class_member.as_fields(); + write!(f, [modifiers.format(), space_token()])?; + let width = write_member_name(&name?.into(), f)?; + write!(f, [property_annotation.format()])?; + let text_width_for_break = + (u8::from(f.context().tab_width()) + MIN_OVERLAP_FOR_BREAK) as usize; + Ok(width < text_width_for_break) + } + JsAnyAssignmentLike::TsPropertySignatureClassMember( + property_signature_class_member, + ) => { + let TsPropertySignatureClassMemberFields { + modifiers, + name, + property_annotation, + semicolon_token: _, + } = property_signature_class_member.as_fields(); + + write!(f, [modifiers.format(), space_token(),])?; + + let width = write_member_name(&name?.into(), f)?; + + write!(f, [property_annotation.format()])?; + let text_width_for_break = + (u8::from(f.context().tab_width()) + MIN_OVERLAP_FOR_BREAK) as usize; + Ok(width < text_width_for_break) + } } } @@ -401,6 +458,15 @@ impl JsAnyAssignmentLike { let eq_token = type_alias_declaration.eq_token()?; write!(f, [space_token(), eq_token.format()]) } + JsAnyAssignmentLike::JsPropertyClassMember(property_class_member) => { + if let Some(initializer) = property_class_member.value() { + let eq_token = initializer.eq_token()?; + write!(f, [space_token(), eq_token.format()])? + } + Ok(()) + } + // this variant doesn't have any operator + JsAnyAssignmentLike::TsPropertySignatureClassMember(_) => Ok(()), } } @@ -434,6 +500,15 @@ impl JsAnyAssignmentLike { let ty = type_alias_declaration.ty()?; write!(f, [space_token(), ty.format()]) } + JsAnyAssignmentLike::JsPropertyClassMember(property_class_member) => { + if let Some(initializer) = property_class_member.value() { + let expression = initializer.expression()?; + write!(f, [space_token(), expression.format()])?; + } + Ok(()) + } + // this variant doesn't have any right part + JsAnyAssignmentLike::TsPropertySignatureClassMember(_) => Ok(()), } } @@ -482,8 +557,10 @@ impl JsAnyAssignmentLike { fn has_only_left_hand_side(&self) -> bool { if let JsAnyAssignmentLike::JsVariableDeclarator(declarator) = self { declarator.initializer().is_none() + } else if let JsAnyAssignmentLike::JsPropertyClassMember(class_member) = self { + class_member.value().is_none() } else { - false + matches!(self, JsAnyAssignmentLike::TsPropertySignatureClassMember(_)) } } diff --git a/crates/rome_js_formatter/src/utils/object.rs b/crates/rome_js_formatter/src/utils/object.rs index 40c4970ab29..7c11912d1f7 100644 --- a/crates/rome_js_formatter/src/utils/object.rs +++ b/crates/rome_js_formatter/src/utils/object.rs @@ -2,17 +2,39 @@ use crate::prelude::*; use crate::utils::FormatLiteralStringToken; use crate::utils::StringLiteralParentKind; use rome_formatter::write; -use rome_js_syntax::JsAnyObjectMemberName; use rome_js_syntax::JsSyntaxKind::JS_STRING_LITERAL; -use rome_rowan::AstNode; +use rome_js_syntax::{JsAnyClassMemberName, JsAnyObjectMemberName}; +use rome_rowan::{declare_node_union, AstNode}; use unicode_width::UnicodeWidthStr; +declare_node_union! { + pub(crate) JsAnyMemberName = JsAnyObjectMemberName | JsAnyClassMemberName +} + +impl Format for JsAnyMemberName { + fn fmt(&self, f: &mut Formatter) -> FormatResult<()> { + match self { + JsAnyMemberName::JsAnyObjectMemberName(node) => { + write!(f, [node.format()]) + } + JsAnyMemberName::JsAnyClassMemberName(node) => { + write!(f, [node.format()]) + } + } + } +} + pub(crate) fn write_member_name( - name: &JsAnyObjectMemberName, + name: &JsAnyMemberName, f: &mut JsFormatter, ) -> FormatResult { match name { - name @ JsAnyObjectMemberName::JsLiteralMemberName(literal) => { + name @ JsAnyMemberName::JsAnyClassMemberName(JsAnyClassMemberName::JsLiteralMemberName( + literal, + )) + | name @ JsAnyMemberName::JsAnyObjectMemberName( + JsAnyObjectMemberName::JsLiteralMemberName(literal), + ) => { let value = literal.value()?; if value.kind() == JS_STRING_LITERAL { @@ -23,13 +45,13 @@ pub(crate) fn write_member_name( Ok(cleaned.width()) } else { - write!(f, [name.format()])?; + write!(f, [name])?; Ok(value.text_trimmed().width()) } } name => { - write!(f, [&name.format()])?; + write!(f, [&name])?; Ok(name.text().width()) } } diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/class-comment/class-property.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/class-comment/class-property.js.snap index 399c291d43b..240a6d1d22d 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/class-comment/class-property.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/class-comment/class-property.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 182 expression: class-property.js --- # Input @@ -18,10 +19,10 @@ class X { ```js class X { TEMPLATE = - // tab index is needed so we can focus, which is needed for keyboard events - '
' + - '
' + - "
"; + // tab index is needed so we can focus, which is needed for keyboard events + '
' + + '
' + + "
"; } ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/classes-private-fields/with_comments.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/classes-private-fields/with_comments.js.snap index 2ca519a2781..119497e50ea 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/classes-private-fields/with_comments.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/classes-private-fields/with_comments.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 182 expression: with_comments.js --- # Input @@ -18,10 +19,10 @@ class A { ```js class A { #foobar = - // comment to break - 1 + - // comment to break again - 2; + // comment to break + 1 + + // comment to break again + 2; } ``` diff --git a/crates/rome_js_formatter/tests/specs/prettier/js/classes/property.js.snap b/crates/rome_js_formatter/tests/specs/prettier/js/classes/property.js.snap index 3fe25b32f58..c884403e503 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/js/classes/property.js.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/js/classes/property.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_formatter/tests/prettier_tests.rs +assertion_line: 182 expression: property.js --- # Input @@ -29,15 +30,15 @@ class B { ```js class A { foobar = - // comment to break - 1 + - // comment to break again - 2; + // comment to break + 1 + + // comment to break again + 2; } class B { - someInstanceProperty = this.props.foofoofoofoofoofoo && this.props - .barbarbarbar; + someInstanceProperty = + this.props.foofoofoofoofoofoo && this.props.barbarbarbar; someInstanceProperty2 = { foo: this.props.foofoofoofoofoofoo && this.props.barbarbarbar, diff --git a/crates/rome_js_formatter/tests/specs/prettier/typescript/assignment/issue-2485.ts.snap b/crates/rome_js_formatter/tests/specs/prettier/typescript/assignment/issue-2485.ts.snap index 4ef55c90d37..9d93ff53b1e 100644 --- a/crates/rome_js_formatter/tests/specs/prettier/typescript/assignment/issue-2485.ts.snap +++ b/crates/rome_js_formatter/tests/specs/prettier/typescript/assignment/issue-2485.ts.snap @@ -16,15 +16,10 @@ class x { # Output ```js class x { - private readonly rawConfigFromFile$: BehaviorSubject = new BehaviorSubject( - notRead, - ); + private readonly rawConfigFromFile$: BehaviorSubject = + new BehaviorSubject(notRead); } ``` -# Lines exceeding max width of 80 characters -``` - 2: private readonly rawConfigFromFile$: BehaviorSubject = new BehaviorSubject( -```