From a7fe396064139d4352212db326b3f6a6a1b3083f Mon Sep 17 00:00:00 2001 From: IWANABETHATGUY Date: Fri, 17 Jun 2022 19:45:52 +0800 Subject: [PATCH] feat(rome_js_analyze): useSelfClosingElements (#2707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: 🎸 basic implement selfclosing element * fix: 🐛 clippy * test: 💍 snapshto * chore: 🤖 self closing element * refactor: 💡 do some refactor * test: 💍 add some trivia snapshot * chore: 🤖 clippy * chore: 🤖 tweak the case when open element has multiline * test: 💍 snpashot * fix: 🐛 clippy * chore: 🤖 merge main resolve * test: 💍 add another test case * fix: 🐛 extra test case * fix: 🐛 don't not edit generated code * fix: 🐛 address cr --- crates/rome_js_analyze/src/analyzers.rs | 2 + .../analyzers/use_self_closing_elements.rs | 94 ++++++++++ crates/rome_js_analyze/src/registry.rs | 3 + crates/rome_js_analyze/tests/spec_tests.rs | 2 +- .../tests/specs/useSelfClosingElements.tsx | 19 ++ .../specs/useSelfClosingElements.tsx.snap | 173 ++++++++++++++++++ 6 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 crates/rome_js_analyze/src/analyzers/use_self_closing_elements.rs create mode 100644 crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx create mode 100644 crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx.snap diff --git a/crates/rome_js_analyze/src/analyzers.rs b/crates/rome_js_analyze/src/analyzers.rs index 39a0f5db5d4..46635d1517b 100644 --- a/crates/rome_js_analyze/src/analyzers.rs +++ b/crates/rome_js_analyze/src/analyzers.rs @@ -16,6 +16,8 @@ mod no_sparse_array; pub(crate) use no_sparse_array::NoSparseArray; mod no_unused_template_literal; pub(crate) use no_unused_template_literal::NoUnusedTemplateLiteral; +mod use_self_closing_elements; +pub(crate) use use_self_closing_elements::UseSelfClosingElements; mod use_single_case_statement; pub(crate) use use_single_case_statement::UseSingleCaseStatement; mod use_single_var_declarator; diff --git a/crates/rome_js_analyze/src/analyzers/use_self_closing_elements.rs b/crates/rome_js_analyze/src/analyzers/use_self_closing_elements.rs new file mode 100644 index 00000000000..7de0e2f87b9 --- /dev/null +++ b/crates/rome_js_analyze/src/analyzers/use_self_closing_elements.rs @@ -0,0 +1,94 @@ +use rome_analyze::{ActionCategory, Rule, RuleCategory, RuleDiagnostic}; +use rome_console::markup; +use rome_diagnostics::Applicability; +use rome_js_factory::make; +use rome_js_syntax::{JsAnyRoot, JsSyntaxToken, JsxAnyTag, JsxElement, JsxOpeningElementFields, T}; +use rome_rowan::{AstNode, AstNodeExt, AstNodeList, TriviaPiece}; + +use crate::JsRuleAction; + +pub(crate) enum UseSelfClosingElements {} + +impl Rule for UseSelfClosingElements { + const NAME: &'static str = "useSelfClosingElements"; + const CATEGORY: RuleCategory = RuleCategory::Lint; + + type Query = JsxElement; + type State = (); + + fn run(n: &Self::Query) -> Option { + if n.children().is_empty() { + Some(()) + } else { + None + } + } + + fn diagnostic(node: &Self::Query, _: &Self::State) -> Option { + Some(RuleDiagnostic::warning( + node.range(), + markup! { + "JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing." + }, + )) + } + + fn action(root: JsAnyRoot, node: &Self::Query, _: &Self::State) -> Option { + let open_element = node.opening_element().ok()?; + let JsxOpeningElementFields { + l_angle_token, + name, + type_arguments, + attributes, + r_angle_token, + } = open_element.as_fields(); + let mut r_angle_token = r_angle_token.ok()?; + let mut leading_trivia = vec![]; + let mut slash_token = String::new(); + + for trivia in r_angle_token.leading_trivia().pieces() { + leading_trivia.push(TriviaPiece::new(trivia.kind(), trivia.text_len())); + slash_token.push_str(trivia.text()); + } + // check if previous `open_element` have a whitespace before `>` + // this step make sure we could convert
->
+ //
->
+ let prev_token = r_angle_token.prev_token(); + let need_extra_whitespace = prev_token + .as_ref() + .map_or(true, |token| !token.trailing_trivia().text().ends_with(' ')); + + // drop the leading trivia of `r_angle_token` + r_angle_token = r_angle_token.with_leading_trivia(std::iter::empty()); + + if leading_trivia.is_empty() && need_extra_whitespace { + slash_token.push(' '); + leading_trivia.push(TriviaPiece::whitespace(1)); + } + + slash_token += "/"; + + let mut self_closing_element_builder = make::jsx_self_closing_element( + l_angle_token.ok()?, + name.ok()?, + attributes, + JsSyntaxToken::new_detached(T![/], &slash_token, leading_trivia, []), + r_angle_token, + ); + if let Some(type_arguments) = type_arguments { + self_closing_element_builder = + self_closing_element_builder.with_type_arguments(type_arguments); + } + let self_closing_element = self_closing_element_builder.build(); + let root = root.replace_node( + JsxAnyTag::JsxElement(node.clone()), + JsxAnyTag::JsxSelfClosingElement(self_closing_element), + )?; + Some(JsRuleAction { + category: ActionCategory::QuickFix, + applicability: Applicability::MaybeIncorrect, + message: markup! { "Use a SelfClosingElement instead" }.to_owned(), + root, + }) + } +} diff --git a/crates/rome_js_analyze/src/registry.rs b/crates/rome_js_analyze/src/registry.rs index e94773b655e..012255ceb95 100644 --- a/crates/rome_js_analyze/src/registry.rs +++ b/crates/rome_js_analyze/src/registry.rs @@ -29,6 +29,9 @@ pub(crate) fn build_registry(filter: &AnalysisFilter) -> RuleRegistry() { rules.push::(); } + if filter.match_rule::() { + rules.push::(); + } if filter.match_rule::() { rules.push::(); } diff --git a/crates/rome_js_analyze/tests/spec_tests.rs b/crates/rome_js_analyze/tests/spec_tests.rs index 512e19ec555..a68e6dde49e 100644 --- a/crates/rome_js_analyze/tests/spec_tests.rs +++ b/crates/rome_js_analyze/tests/spec_tests.rs @@ -12,7 +12,7 @@ use rome_diagnostics::{file::SimpleFile, termcolor::NoColor, Diagnostic}; use rome_js_parser::parse; use rome_rowan::{AstNode, Language}; -tests_macros::gen_tests! {"tests/specs/**/*.{js,jsx}", crate::run_test, "module"} +tests_macros::gen_tests! {"tests/specs/**/*.{js,jsx,tsx}", crate::run_test, "module"} fn run_test(input: &'static str, _: &str, _: &str, _: &str) { register_leak_checker(); diff --git a/crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx b/crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx new file mode 100644 index 00000000000..317ee491702 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx @@ -0,0 +1,19 @@ +// valid +
; +
child
; +; +child; +; +child; + +// invalid +
; +; +; +
; + +
/* comment */; +/* comment */
; +>; \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx.snap b/crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx.snap new file mode 100644 index 00000000000..14fa9e3af28 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/useSelfClosingElements.tsx.snap @@ -0,0 +1,173 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 98 +expression: useSelfClosingElements.tsx +--- +# Input +```js +// valid +
; +
child
; +; +child; +; +child; + +// invalid +
; +; +; +
; + +
/* comment */; +/* comment */
; +>; +``` + +# Diagnostics +``` +warning[useSelfClosingElements]: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. + ┌─ useSelfClosingElements.tsx:10:1 + │ +10 │
; + │ -------------------------- + +Suggested fix: Use a SelfClosingElement instead + | @@ -7,7 +7,7 @@ + 6 6 | child; + 7 7 | + 8 8 | // invalid + 9 | -
; + 9 | +
; +10 10 | ; +11 11 | ; +12 12 |
; + │ ----------------------- + +Suggested fix: Use a SelfClosingElement instead + | @@ -8,7 +8,7 @@ + 7 7 | + 8 8 | // invalid + 9 9 |
; +10 | - ; + 10 | + ; +11 11 | ; +12 12 |
; + │ ------------------- + +Suggested fix: Use a SelfClosingElement instead + | @@ -9,7 +9,7 @@ + 8 8 | // invalid + 9 9 |
; +10 10 | ; +11 | - ; + 11 | + ; +12 12 |
; + + +``` + +``` +warning[useSelfClosingElements]: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. + ┌─ useSelfClosingElements.tsx:13:1 + │ +13 │ ┌
; + │ └───────' + +Suggested fix: Use a SelfClosingElement instead + | @@ -12,7 +12,7 @@ +11 11 | ; +12 12 |
; + 14 | + />; +15 15 | +16 16 |
/* comment */; +17 17 | /* comment */
; + + +``` + +``` +warning[useSelfClosingElements]: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. + ┌─ useSelfClosingElements.tsx:17:1 + │ +17 │
/* comment */; + │ ------------ + +Suggested fix: Use a SelfClosingElement instead + | @@ -14,6 +14,6 @@ +13 13 | +14 14 | >
; +15 15 | +16 | -
/* comment */; + 16 | +
/* comment */; +17 17 | /* comment */
; +18 18 | >; + + +``` + +``` +warning[useSelfClosingElements]: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. + ┌─ useSelfClosingElements.tsx:18:15 + │ +18 │ /* comment */
; + │ ------------ + +Suggested fix: Use a SelfClosingElement instead + | @@ -15,5 +15,5 @@ +14 14 | >
; +15 15 | +16 16 |
/* comment */; +17 | - /* comment */
; + 17 | + /* comment */
; +18 18 | >; + + +``` + +``` +warning[useSelfClosingElements]: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. + ┌─ useSelfClosingElements.tsx:19:1 + │ +19 │ >; + │ ------------------------- + +Suggested fix: Use a SelfClosingElement instead + | @@ -16,4 +16,4 @@ +15 15 | +16 16 |
/* comment */; +17 17 | /* comment */
; +18 | - >; + 18 | + />; + + +``` + +