From ac87f6d129acdb7796268ae15eedba415dc9ba8d Mon Sep 17 00:00:00 2001 From: xunilrj Date: Thu, 4 Aug 2022 11:43:15 +0100 Subject: [PATCH] moving linter to support ts --- crates/rome_js_analyze/src/registry.rs | 1 + .../rome_js_analyze/src/semantic_analyzers.rs | 2 + .../src/semantic_analyzers/js.rs | 3 +- .../semantic_analyzers/no_unused_variables.rs | 5 + .../no_unused_variables.rs | 0 crates/rome_js_analyze/tests/spec_tests.rs | 3 +- .../js/noUnusedVariablesTypescript.ts.snap | 40 -------- .../noUnusedVariables.js | 0 .../noUnusedVariables.js.snap | 22 ++--- .../noUnusedVariables.ts} | 0 .../noUnusedVariables.ts.snap | 99 +++++++++++++++++++ .../src/configuration/linter/rules.rs | 87 +++++++++++++++- crates/tests_macros/src/lib.rs | 10 +- 13 files changed, 212 insertions(+), 60 deletions(-) create mode 100644 crates/rome_js_analyze/src/semantic_analyzers/no_unused_variables.rs rename crates/rome_js_analyze/src/semantic_analyzers/{js => no_unused_variables}/no_unused_variables.rs (100%) delete mode 100644 crates/rome_js_analyze/tests/specs/js/noUnusedVariablesTypescript.ts.snap rename crates/rome_js_analyze/tests/specs/{js => no_unused_variables}/noUnusedVariables.js (100%) rename crates/rome_js_analyze/tests/specs/{js => no_unused_variables}/noUnusedVariables.js.snap (86%) rename crates/rome_js_analyze/tests/specs/{js/noUnusedVariablesTypescript.ts => no_unused_variables/noUnusedVariables.ts} (100%) create mode 100644 crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.ts.snap diff --git a/crates/rome_js_analyze/src/registry.rs b/crates/rome_js_analyze/src/registry.rs index a83a49a6e0c7..469feb16be1e 100644 --- a/crates/rome_js_analyze/src/registry.rs +++ b/crates/rome_js_analyze/src/registry.rs @@ -9,6 +9,7 @@ pub(crate) fn build_registry(filter: &AnalysisFilter) -> RuleRegistry(filter); registry.push_group::(filter); registry.push_group::(filter); + registry.push_group::(filter); registry.push_group::(filter); registry } diff --git a/crates/rome_js_analyze/src/semantic_analyzers.rs b/crates/rome_js_analyze/src/semantic_analyzers.rs index 32a6ba28111d..efa728345af9 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers.rs @@ -2,3 +2,5 @@ mod js; pub(super) use self::js::Js; +mod no_unused_variables; +pub(super) use self::no_unused_variables::NoUnusedVariables; diff --git a/crates/rome_js_analyze/src/semantic_analyzers/js.rs b/crates/rome_js_analyze/src/semantic_analyzers/js.rs index 84fd3e19d165..0b63f6c00575 100644 --- a/crates/rome_js_analyze/src/semantic_analyzers/js.rs +++ b/crates/rome_js_analyze/src/semantic_analyzers/js.rs @@ -7,6 +7,5 @@ mod no_function_assign; mod no_import_assign; mod no_label_var; mod no_shouty_constants; -mod no_unused_variables; mod use_camel_case; -declare_group! { pub (crate) Js { name : "js" , rules : [self :: no_arguments :: NoArguments , self :: no_catch_assign :: NoCatchAssign , self :: no_function_assign :: NoFunctionAssign , self :: no_import_assign :: NoImportAssign , self :: no_label_var :: NoLabelVar , self :: no_shouty_constants :: NoShoutyConstants , self :: no_unused_variables :: NoUnusedVariables , self :: use_camel_case :: UseCamelCase ,] } } +declare_group! { pub (crate) Js { name : "js" , rules : [self :: no_arguments :: NoArguments , self :: no_catch_assign :: NoCatchAssign , self :: no_function_assign :: NoFunctionAssign , self :: no_import_assign :: NoImportAssign , self :: no_label_var :: NoLabelVar , self :: no_shouty_constants :: NoShoutyConstants , self :: use_camel_case :: UseCamelCase ,] } } diff --git a/crates/rome_js_analyze/src/semantic_analyzers/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/no_unused_variables.rs new file mode 100644 index 000000000000..d9162d091091 --- /dev/null +++ b/crates/rome_js_analyze/src/semantic_analyzers/no_unused_variables.rs @@ -0,0 +1,5 @@ +//! Generated file, do not edit by hand, see `xtask/codegen` + +use rome_analyze::declare_group; +mod no_unused_variables; +declare_group! { pub (crate) NoUnusedVariables { name : "no_unused_variables" , rules : [self :: no_unused_variables :: NoUnusedVariables ,] } } diff --git a/crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs b/crates/rome_js_analyze/src/semantic_analyzers/no_unused_variables/no_unused_variables.rs similarity index 100% rename from crates/rome_js_analyze/src/semantic_analyzers/js/no_unused_variables.rs rename to crates/rome_js_analyze/src/semantic_analyzers/no_unused_variables/no_unused_variables.rs diff --git a/crates/rome_js_analyze/tests/spec_tests.rs b/crates/rome_js_analyze/tests/spec_tests.rs index b4e838a6a0e0..0eea40dafc77 100644 --- a/crates/rome_js_analyze/tests/spec_tests.rs +++ b/crates/rome_js_analyze/tests/spec_tests.rs @@ -31,7 +31,8 @@ fn run_test(input: &'static str, _: &str, _: &str, _: &str) { let parsed = parse(&input_code, 0, source_type); let root = parsed.tree(); - let (group, rule) = parse_test_path(input_file); + let (group, rule) = dbg!(parse_test_path(input_file)); + let rule_filter = RuleFilter::Rule(group, rule); let filter = AnalysisFilter { enabled_rules: Some(slice::from_ref(&rule_filter)), diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariablesTypescript.ts.snap b/crates/rome_js_analyze/tests/specs/js/noUnusedVariablesTypescript.ts.snap deleted file mode 100644 index 93edb9e3ed68..000000000000 --- a/crates/rome_js_analyze/tests/specs/js/noUnusedVariablesTypescript.ts.snap +++ /dev/null @@ -1,40 +0,0 @@ ---- -source: crates/rome_js_analyze/tests/spec_tests.rs -expression: noUnusedVariablesTypescript.ts ---- -# Input -```js -// Invalid - -class D { - constructor(a: number) {} - f(a: number) {} - set a(a: number) {} -} -console.log(new D()); - -// Valid - -interface A { - f(a: number); - set a(a: number); - [key: string]: string; -} - -abstract class B { - constructor(a: number); - abstract f(a: number); - g(a: number); - abstract set a(a: number); -} -console.log(new B()); - -class C { - constructor(a: number); - f(a: number); -} -console.log(new C()); - -``` - - diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js b/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.js similarity index 100% rename from crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js rename to crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.js diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js.snap b/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.js.snap similarity index 86% rename from crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js.snap rename to crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.js.snap index eb15ebf3f24d..135236f49be2 100644 --- a/crates/rome_js_analyze/tests/specs/js/noUnusedVariables.js.snap +++ b/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.js.snap @@ -48,7 +48,7 @@ try { # Diagnostics ``` -warning[js/noUnusedVariables]: This variable is unused. +warning[no_unused_variables/noUnusedVariables]: This variable is unused. ┌─ noUnusedVariables.js:1:7 │ 1 │ const a = 1; @@ -68,7 +68,7 @@ Suggested fix: Remove this variable. ``` ``` -warning[js/noUnusedVariables]: This variable is unused. +warning[no_unused_variables/noUnusedVariables]: This variable is unused. ┌─ noUnusedVariables.js:2:7 │ 2 │ const b = 2, @@ -89,7 +89,7 @@ Suggested fix: Remove this variable. ``` ``` -warning[js/noUnusedVariables]: This function is unused. +warning[no_unused_variables/noUnusedVariables]: This function is unused. ┌─ noUnusedVariables.js:6:10 │ 6 │ function f1() {} @@ -112,7 +112,7 @@ Suggested fix: Remove this function. ``` ``` -warning[js/noUnusedVariables]: This function is unused. +warning[no_unused_variables/noUnusedVariables]: This function is unused. ┌─ noUnusedVariables.js:8:10 │ 8 │ function f2() { @@ -137,7 +137,7 @@ Suggested fix: Remove this function. ``` ``` -warning[js/noUnusedVariables]: This function is unused. +warning[no_unused_variables/noUnusedVariables]: This function is unused. ┌─ noUnusedVariables.js:12:10 │ 12 │ function f3() { @@ -165,7 +165,7 @@ Suggested fix: Remove this function. ``` ``` -warning[js/noUnusedVariables]: This parameter is unused. +warning[no_unused_variables/noUnusedVariables]: This parameter is unused. ┌─ noUnusedVariables.js:19:14 │ 19 │ function f41(a) {} @@ -188,7 +188,7 @@ Suggested fix: Remove this parameter. ``` ``` -warning[js/noUnusedVariables]: This parameter is unused. +warning[no_unused_variables/noUnusedVariables]: This parameter is unused. ┌─ noUnusedVariables.js:22:17 │ 22 │ function f42(a, b) { @@ -211,7 +211,7 @@ Suggested fix: Remove this parameter. ``` ``` -warning[js/noUnusedVariables]: This parameter is unused. +warning[no_unused_variables/noUnusedVariables]: This parameter is unused. ┌─ noUnusedVariables.js:27:17 │ 27 │ function f43(a, b) { @@ -234,7 +234,7 @@ Suggested fix: Remove this parameter. ``` ``` -warning[js/noUnusedVariables]: This variable is unused. +warning[no_unused_variables/noUnusedVariables]: This variable is unused. ┌─ noUnusedVariables.js:32:7 │ 32 │ const f5 = () => {}; @@ -257,7 +257,7 @@ Suggested fix: Remove this variable. ``` ``` -warning[js/noUnusedVariables]: This variable is unused. +warning[no_unused_variables/noUnusedVariables]: This variable is unused. ┌─ noUnusedVariables.js:34:7 │ 34 │ const f6 = () => { @@ -281,7 +281,7 @@ Suggested fix: Remove this variable. ``` ``` -warning[js/noUnusedVariables]: This variable is unused. +warning[no_unused_variables/noUnusedVariables]: This variable is unused. ┌─ noUnusedVariables.js:39:10 │ 39 │ } catch (e) {} diff --git a/crates/rome_js_analyze/tests/specs/js/noUnusedVariablesTypescript.ts b/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.ts similarity index 100% rename from crates/rome_js_analyze/tests/specs/js/noUnusedVariablesTypescript.ts rename to crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.ts diff --git a/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.ts.snap b/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.ts.snap new file mode 100644 index 000000000000..1d18ba98ee79 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/no_unused_variables/noUnusedVariables.ts.snap @@ -0,0 +1,99 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: noUnusedVariables.ts +--- +# Input +```js +// Invalid + +class D { + constructor(a: number) {} + f(a: number) {} + set a(a: number) {} +} +console.log(new D()); + +// Valid + +interface A { + f(a: number); + set a(a: number); + [key: string]: string; +} + +abstract class B { + constructor(a: number); + abstract f(a: number); + g(a: number); + abstract set a(a: number); +} +console.log(new B()); + +class C { + constructor(a: number); + f(a: number); +} +console.log(new C()); + +``` + +# Diagnostics +``` +warning[no_unused_variables/noUnusedVariables]: This parameter is unused. + ┌─ noUnusedVariables.ts:4:14 + │ +4 │ constructor(a: number) {} + │ - + +Suggested fix: Remove this parameter. + | @@ -1,7 +1,7 @@ +0 0 | // Invalid +1 1 | +2 2 | class D { +3 | - constructor(a: number) {} + 3 | + constructor() {} +4 4 | f(a: number) {} +5 5 | set a(a: number) {} +6 6 | } + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +warning[no_unused_variables/noUnusedVariables]: This parameter is unused. + ┌─ noUnusedVariables.ts:5:4 + │ +5 │ f(a: number) {} + │ - + +Suggested fix: Remove this parameter. + | @@ -2,7 +2,7 @@ +1 1 | +2 2 | class D { +3 3 | constructor(a: number) {} +4 | - f(a: number) {} + 4 | + f() {} +5 5 | set a(a: number) {} +6 6 | } +7 7 | console.log(new D()); + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + +``` +warning[no_unused_variables/noUnusedVariables]: This parameter is unused. + ┌─ noUnusedVariables.ts:6:8 + │ +6 │ set a(a: number) {} + │ - + += note: Unused variables usually are result of incomplete refactoring, typos and other source of bugs. + + +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 225dbbe65e50..d0e2ec24820c 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -15,6 +15,8 @@ pub struct Rules { #[serde(skip_serializing_if = "Option::is_none")] pub jsx: Option, #[serde(skip_serializing_if = "Option::is_none")] + pub no_unused_variables: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub regex: Option, #[serde(skip_serializing_if = "Option::is_none")] pub ts: Option, @@ -25,6 +27,7 @@ impl Default for Rules { recommended: Some(true), js: None, jsx: None, + no_unused_variables: None, regex: None, ts: None, } @@ -59,6 +62,15 @@ impl Rules { } else if self.is_recommended() { enabled_rules.extend(Jsx::RECOMMENDED_RULES); } + if let Some(group) = self.no_unused_variables.as_ref() { + if self.is_recommended() && group.is_recommended() { + enabled_rules.extend(&Js::RECOMMENDED_RULES); + } + enabled_rules.extend(&group.get_enabled_rules()); + disabled_rules.extend(&group.get_disabled_rules()); + } else if self.is_recommended() { + enabled_rules.extend(No_unused_variables::RECOMMENDED_RULES); + } if let Some(group) = self.regex.as_ref() { if self.is_recommended() && group.is_recommended() { enabled_rules.extend(&Js::RECOMMENDED_RULES); @@ -95,6 +107,15 @@ impl Rules { } else if self.is_recommended() { enabled_rules.extend(Jsx::RECOMMENDED_RULES); } + if let Some(group) = self.no_unused_variables.as_ref() { + if self.is_recommended() && group.is_recommended() { + enabled_rules.extend(&Js::RECOMMENDED_RULES); + } + enabled_rules.extend(&group.get_enabled_rules()); + disabled_rules.extend(&group.get_disabled_rules()); + } else if self.is_recommended() { + enabled_rules.extend(No_unused_variables::RECOMMENDED_RULES); + } if let Some(group) = self.regex.as_ref() { if self.is_recommended() && group.is_recommended() { enabled_rules.extend(&Js::RECOMMENDED_RULES); @@ -131,7 +152,7 @@ pub struct Js { } impl Js { const GROUP_NAME: &'static str = "js"; - pub(crate) const GROUP_RULES: [&'static str; 29] = [ + pub(crate) const GROUP_RULES: [&'static str; 28] = [ "noArguments", "noAsyncPromiseExecutor", "noCatchAssign", @@ -152,7 +173,6 @@ impl Js { "noUnnecessaryContinue", "noUnsafeNegation", "noUnusedTemplateLiteral", - "noUnusedVariables", "useBlockStatements", "useCamelCase", "useSimplifiedLogicExpression", @@ -162,7 +182,7 @@ impl Js { "useValidTypeof", "useWhile", ]; - const RECOMMENDED_RULES: [RuleFilter<'static>; 27] = [ + const RECOMMENDED_RULES: [RuleFilter<'static>; 26] = [ RuleFilter::Rule("js", Self::GROUP_RULES[0]), RuleFilter::Rule("js", Self::GROUP_RULES[1]), RuleFilter::Rule("js", Self::GROUP_RULES[2]), @@ -183,13 +203,12 @@ impl Js { RuleFilter::Rule("js", Self::GROUP_RULES[18]), RuleFilter::Rule("js", Self::GROUP_RULES[19]), RuleFilter::Rule("js", Self::GROUP_RULES[20]), - RuleFilter::Rule("js", Self::GROUP_RULES[21]), + RuleFilter::Rule("js", Self::GROUP_RULES[22]), RuleFilter::Rule("js", Self::GROUP_RULES[23]), RuleFilter::Rule("js", Self::GROUP_RULES[24]), RuleFilter::Rule("js", Self::GROUP_RULES[25]), RuleFilter::Rule("js", Self::GROUP_RULES[26]), RuleFilter::Rule("js", Self::GROUP_RULES[27]), - RuleFilter::Rule("js", Self::GROUP_RULES[28]), ]; pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } pub(crate) fn get_enabled_rules(&self) -> IndexSet { @@ -294,6 +313,64 @@ where } #[derive(Deserialize, Default, Serialize, Debug, Clone)] #[serde(rename_all = "camelCase", default, deny_unknown_fields)] +pub struct No_unused_variables { + #[doc = r" It enables the recommended rules for this group"] + #[serde(skip_serializing_if = "Option::is_none")] + pub recommended: Option, + #[doc = r" List of rules for the current group"] + #[serde( + skip_serializing_if = "IndexMap::is_empty", + deserialize_with = "deserialize_no_unused_variables_rules" + )] + pub rules: IndexMap, +} +impl No_unused_variables { + const GROUP_NAME: &'static str = "no_unused_variables"; + pub(crate) const GROUP_RULES: [&'static str; 1] = ["noUnusedVariables"]; + const RECOMMENDED_RULES: [RuleFilter<'static>; 1] = [RuleFilter::Rule( + "no_unused_variables", + Self::GROUP_RULES[0], + )]; + pub(crate) fn is_recommended(&self) -> bool { matches!(self.recommended, Some(true)) } + pub(crate) fn get_enabled_rules(&self) -> IndexSet { + IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { + if conf.is_enabled() { + Some(RuleFilter::Rule(Self::GROUP_NAME, key)) + } else { + None + } + })) + } + pub(crate) fn get_disabled_rules(&self) -> IndexSet { + IndexSet::from_iter(self.rules.iter().filter_map(|(key, conf)| { + if conf.is_disabled() { + Some(RuleFilter::Rule(Self::GROUP_NAME, key)) + } else { + None + } + })) + } +} +fn deserialize_no_unused_variables_rules<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: serde::de::Deserializer<'de>, +{ + let value: IndexMap = Deserialize::deserialize(deserializer)?; + for rule_name in value.keys() { + if !No_unused_variables::GROUP_RULES.contains(&rule_name.as_str()) { + return Err(serde::de::Error::custom(RomeError::Configuration( + ConfigurationError::DeserializationError(format!( + "Invalid rule name `{rule_name}`" + )), + ))); + } + } + Ok(value) +} +#[derive(Deserialize, Default, Serialize, Debug, Clone)] +#[serde(rename_all = "camelCase", default, deny_unknown_fields)] pub struct Regex { #[doc = r" It enables the recommended rules for this group"] #[serde(skip_serializing_if = "Option::is_none")] diff --git a/crates/tests_macros/src/lib.rs b/crates/tests_macros/src/lib.rs index dcbebac6d843..7cf72e94c640 100644 --- a/crates/tests_macros/src/lib.rs +++ b/crates/tests_macros/src/lib.rs @@ -164,7 +164,15 @@ impl Arguments { let path = path.as_ref(); let file_stem = path.file_stem()?; let file_stem = file_stem.to_str()?; - let test_name = file_stem.to_snake(); + let test_name = format!( + "{}{}", + file_stem.to_snake(), + if let Some(extension) = path.extension().and_then(|ext| ext.to_str()) { + format!("_{}", extension) + } else { + "".to_string() + } + ); let test_directory = path.parent().unwrap().display().to_string(); let test_full_path = path.display().to_string();