diff --git a/crates/rome_js_analyze/src/semantic_analyzers/style/no_shouty_constants.rs b/crates/rome_js_analyze/src/semantic_analyzers/style/no_shouty_constants.rs index f04812ddbe1..e0d72eb64c2 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/style/no_shouty_constants.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/style/no_shouty_constants.rs @@ -2,13 +2,15 @@ use crate::{semantic_services::Semantic, utils::batch::JsBatchMutation, JsRuleAc use rome_analyze::{context::RuleContext, declare_rule, ActionCategory, Rule, RuleDiagnostic}; use rome_console::markup; use rome_diagnostics::Applicability; +use rome_js_factory::make::{js_literal_member_name, js_property_object_member}; use rome_js_semantic::{Reference, ReferencesExtensions}; use rome_js_syntax::{ - JsAnyExpression, JsAnyLiteralExpression, JsIdentifierBinding, JsIdentifierExpression, - JsStringLiteralExpression, JsVariableDeclaration, JsVariableDeclarator, + JsAnyExpression, JsAnyLiteralExpression, JsAnyObjectMemberName, JsIdentifierBinding, + JsIdentifierExpression, JsReferenceIdentifier, JsShorthandPropertyObjectMember, + JsStringLiteralExpression, JsSyntaxKind, JsVariableDeclaration, JsVariableDeclarator, JsVariableDeclaratorList, }; -use rome_rowan::{AstNode, BatchMutationExt, SyntaxNodeCast}; +use rome_rowan::{AstNode, BatchMutationExt, SyntaxNodeCast, SyntaxToken}; declare_rule! { /// Disallow the use of constants which its value is the upper-case version of its name. @@ -78,7 +80,7 @@ fn is_id_and_string_literal_inner_text_equal( pub struct State { literal: JsStringLiteralExpression, - references: Vec, + reference: Reference, } impl Rule for NoShoutyConstants { @@ -101,9 +103,13 @@ impl Rule for NoShoutyConstants { return None; } + if binding.all_references(model).count() > 1 { + return None; + } + return Some(State { literal, - references: binding.all_references(model).collect(), + reference: binding.all_references(model).next()?, }); } } @@ -122,10 +128,8 @@ impl Rule for NoShoutyConstants { }, ); - for reference in state.references.iter() { - let node = reference.syntax(); - diag = diag.detail(node.text_trimmed_range(), "Used here.") - } + let node = state.reference.syntax(); + diag = diag.detail(node.text_trimmed_range(), "Used here."); let diag = diag.note( markup! {"You should avoid declaring constants with a string that's the same @@ -144,16 +148,42 @@ impl Rule for NoShoutyConstants { batch.remove_js_variable_declarator(ctx.query()); - for reference in state.references.iter() { - let node = reference - .syntax() - .parent()? - .cast::()?; - + if let Some(node) = state + .reference + .syntax() + .parent()? + .cast::() + { batch.replace_node( JsAnyExpression::JsIdentifierExpression(node), - JsAnyExpression::JsAnyLiteralExpression(literal.clone()), + JsAnyExpression::JsAnyLiteralExpression(literal), + ); + } else if let Some(node) = state + .reference + .syntax() + .parent()? + .cast::() + { + // for replacing JsShorthandPropertyObjectMember + let new_element = js_property_object_member( + JsAnyObjectMemberName::JsLiteralMemberName(js_literal_member_name( + SyntaxToken::new_detached( + JsSyntaxKind::JS_LITERAL_MEMBER_NAME, + JsReferenceIdentifier::cast_ref(state.reference.syntax())? + .value_token() + .ok()? + .text(), + [], + [], + ), + )), + SyntaxToken::new_detached(JsSyntaxKind::COLON, ":", [], []), + JsAnyExpression::JsAnyLiteralExpression(literal), ); + + batch.replace_element(node.into_syntax().into(), new_element.into_syntax().into()); + } else { + return None; } Some(JsRuleAction { diff --git a/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js b/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js index 2a8d045f7fe..983754058f8 100644 --- a/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js +++ b/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js @@ -6,6 +6,24 @@ const FOO2 = "FOO2", a = "FOO3", FOO4 = "FOO4"; console.log(FOO, FOO4); let foo = "foo"; +const B = "B"; +export default B; +export const A = "A"; -const A = "A"; -export default A; \ No newline at end of file +const BAR = "BAR"; + +export const bar = { + foo: BAR, + bar: BAR, +}; + +const C = "C"; + +export const d = { + foo: C +}; + +const D ="D"; +export const e = { + D +}; \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap b/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap index 08adf2a1f52..2d72e58a160 100644 --- a/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap +++ b/crates/rome_js_analyze/tests/specs/style/noShoutyConstants.js.snap @@ -1,5 +1,6 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 73 expression: noShoutyConstants.js --- # Input @@ -12,60 +13,30 @@ const FOO2 = "FOO2", a = "FOO3", FOO4 = "FOO4"; console.log(FOO, FOO4); let foo = "foo"; +const B = "B"; +export default B; +export const A = "A"; -const A = "A"; -export default A; -``` +const BAR = "BAR"; -# Diagnostics -``` -noShoutyConstants.js:1:7 lint/style/noShoutyConstants FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +export const bar = { + foo: BAR, + bar: BAR, +}; - ! Redundant constant declaration. - - > 1 │ const FOO = "FOO"; - │ ^^^^^^^^^^^ - 2 │ console.log(FOO, FOO2); - 3 │ - - i Used here. - - 1 │ const FOO = "FOO"; - > 2 │ console.log(FOO, FOO2); - │ ^^^ - 3 │ - 4 │ const FOO2 = "FOO2", a = "FOO3", FOO4 = "FOO4"; - - i Used here. - - 4 │ const FOO2 = "FOO2", a = "FOO3", FOO4 = "FOO4"; - 5 │ - > 6 │ console.log(FOO, FOO4); - │ ^^^ - 7 │ - 8 │ let foo = "foo"; - - i You should avoid declaring constants with a string that's the same - value as the variable name. It introduces a level of unnecessary - indirection when it's only two additional characters to inline. - - i Suggested fix: Use the constant value directly - - 1 │ - const·FOO·=·"FOO"; - 2 │ - console.log(FOO,·FOO2); - 1 │ + - 2 │ + console.log("FOO",·FOO2); - 3 3 │ - 4 4 │ const FOO2 = "FOO2", a = "FOO3", FOO4 = "FOO4"; - 5 5 │ - 6 │ - console.log(FOO,·FOO4); - 6 │ + console.log("FOO",·FOO4); - 7 7 │ - 8 8 │ let foo = "foo"; - +const C = "C"; + +export const d = { + foo: C +}; +const D ="D"; +export const e = { + D +}; ``` +# Diagnostics ``` noShoutyConstants.js:4:7 lint/style/noShoutyConstants FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -144,4 +115,82 @@ noShoutyConstants.js:4:34 lint/style/noShoutyConstants FIXABLE ━━━━━ ``` +``` +noShoutyConstants.js:20:7 lint/style/noShoutyConstants FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Redundant constant declaration. + + 18 │ }; + 19 │ + > 20 │ const C = "C"; + │ ^^^^^^^ + 21 │ + 22 │ export const d = { + + i Used here. + + 22 │ export const d = { + > 23 │ foo: C + │ ^ + 24 │ }; + 25 │ + + i You should avoid declaring constants with a string that's the same + value as the variable name. It introduces a level of unnecessary + indirection when it's only two additional characters to inline. + + i Suggested fix: Use the constant value directly + + 18 18 │ }; + 19 19 │ + 20 │ - const·C·=·"C"; + 21 │ - + 22 20 │ export const d = { + 23 │ - → foo:·C + 21 │ + → foo:·"C" + 24 22 │ }; + 25 23 │ + + +``` + +``` +noShoutyConstants.js:26:7 lint/style/noShoutyConstants FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Redundant constant declaration. + + 24 │ }; + 25 │ + > 26 │ const D ="D"; + │ ^^^^^^ + 27 │ export const e = { + 28 │ D + + i Used here. + + 26 │ const D ="D"; + 27 │ export const e = { + > 28 │ D + │ ^ + 29 │ }; + + i You should avoid declaring constants with a string that's the same + value as the variable name. It introduces a level of unnecessary + indirection when it's only two additional characters to inline. + + i Suggested fix: Use the constant value directly + + 23 23 │ foo: C + 24 24 │ }; + 25 │ - + 26 │ - const·D·="D"; + 27 25 │ export const e = { + 28 │ - → D + 26 │ + → + 27 │ + → D:"D" + 29 28 │ }; + + +``` +