From 44b16ef79d5396e70e2ddcbb66a68c8f7b74f0e0 Mon Sep 17 00:00:00 2001 From: Jelle van der Waa Date: Mon, 13 May 2024 11:23:01 +0200 Subject: [PATCH] feat(linter/eslint): Implement max-classes-per-file (#3241) Rule Detail: [link](https://eslint.org/docs/latest/rules/max-classes-per-file) --- crates/oxc_linter/src/rules.rs | 2 + .../src/rules/eslint/max_classes_per_file.rs | 196 ++++++++++++++++++ .../src/snapshots/max_classes_per_file.snap | 67 ++++++ crates/oxc_semantic/src/class/table.rs | 4 + 4 files changed, 269 insertions(+) create mode 100644 crates/oxc_linter/src/rules/eslint/max_classes_per_file.rs create mode 100644 crates/oxc_linter/src/snapshots/max_classes_per_file.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index ebde285ca..27a9708ee 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -44,6 +44,7 @@ mod eslint { pub mod for_direction; pub mod getter_return; pub mod guard_for_in; + pub mod max_classes_per_file; pub mod max_lines; pub mod max_params; pub mod no_array_constructor; @@ -404,6 +405,7 @@ oxc_macros::declare_all_lint_rules! { eslint::for_direction, eslint::getter_return, eslint::guard_for_in, + eslint::max_classes_per_file, eslint::max_lines, eslint::max_params, eslint::no_ternary, diff --git a/crates/oxc_linter/src/rules/eslint/max_classes_per_file.rs b/crates/oxc_linter/src/rules/eslint/max_classes_per_file.rs new file mode 100644 index 000000000..226b8889b --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/max_classes_per_file.rs @@ -0,0 +1,196 @@ +use oxc_ast::AstKind; +use oxc_diagnostics::OxcDiagnostic; + +use oxc_syntax::class::ClassId; +use serde_json::Value; + +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule}; + +fn max_classes_per_file_diagnostic(total: usize, max: usize, span1: Span) -> OxcDiagnostic { + OxcDiagnostic::warning(format!( + "eslint(max-classes-per-file): File has too many classes ({total}). Maximum allowed is {max}", + )) + .with_help("Reduce the number of classes in this file") + .with_labels([span1.into()]) +} + +#[derive(Debug, Default, Clone)] +pub struct MaxClassesPerFile(Box); + +#[derive(Debug, Clone)] +pub struct MaxClassesPerFileConfig { + pub max: usize, + pub ignore_expressions: bool, +} + +impl std::ops::Deref for MaxClassesPerFile { + type Target = MaxClassesPerFileConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl Default for MaxClassesPerFileConfig { + fn default() -> Self { + Self { max: 1, ignore_expressions: false } + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforce a maximum number of classes per file + /// + /// ### Why is this bad? + /// + /// Files containing multiple classes can often result in a less navigable and poorly + /// structured codebase. Best practice is to keep each file limited to a single responsibility. + /// + /// ### Example + /// ```javascript + /// class Foo {} + /// class Bar {} + /// ``` + MaxClassesPerFile, + pedantic, +); + +impl Rule for MaxClassesPerFile { + fn from_configuration(value: serde_json::Value) -> Self { + let config = value.get(0); + if let Some(max) = config + .and_then(Value::as_number) + .and_then(serde_json::Number::as_u64) + .and_then(|v| usize::try_from(v).ok()) + { + Self(Box::new(MaxClassesPerFileConfig { max, ignore_expressions: false })) + } else { + let max = value + .get(0) + .and_then(|config| config.get("max")) + .and_then(serde_json::Value::as_number) + .and_then(serde_json::Number::as_u64) + .map_or(1, |v| usize::try_from(v).unwrap_or(1)); + + let ignore_expressions = value + .get(0) + .and_then(|config| config.get("ignoreExpressions")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + Self(Box::new(MaxClassesPerFileConfig { max, ignore_expressions })) + } + } + + fn run_once(&self, ctx: &LintContext<'_>) { + let mut class_count = ctx.semantic().classes().declarations.len(); + + if self.ignore_expressions { + let class_expressions = ctx + .semantic() + .classes() + .iter_enumerated() + .filter(|(_class_id, node_id)| !ctx.nodes().kind(**node_id).is_declaration()) + .count(); + class_count -= class_expressions; + } + + if class_count <= self.max { + return; + } + + let ast_node_id = ctx.semantic().classes().get_node_id(ClassId::from(self.max)); + let span = if let AstKind::Class(class) = ctx.nodes().kind(ast_node_id) { + class.span + } else { + Span::new(0, 0) + }; + + ctx.diagnostic(max_classes_per_file_diagnostic(class_count, self.max, span)); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("class Foo {}", None), + ("var x = class {};", None), + ("var x = 5;", None), + ("class Foo {}", Some(serde_json::json!([1]))), + ( + "class Foo {} + class Bar {}", + Some(serde_json::json!([2])), + ), + ("class Foo {}", Some(serde_json::json!([{ "max": 1 }]))), + ( + "class Foo {} + class Bar {}", + Some(serde_json::json!([{ "max": 2 }])), + ), + ( + " + class Foo {} + const myExpression = class {} + ", + Some(serde_json::json!([{ "ignoreExpressions": true, "max": 1 }])), + ), + ( + " + class Foo {} + class Bar {} + const myExpression = class {} + ", + Some(serde_json::json!([{ "ignoreExpressions": true, "max": 2 }])), + ), + ]; + + let fail = vec![ + ( + "class Foo {} + class Bar {}", + None, + ), + ( + "class Foo {} + const myExpression = class {}", + None, + ), + ( + "var x = class {}; + var y = class {};", + None, + ), + ( + "class Foo {} + var x = class {};", + None, + ), + ("class Foo {} class Bar {}", Some(serde_json::json!([1]))), + ("class Foo {} class Bar {} class Baz {}", Some(serde_json::json!([2]))), + ( + " + class Foo {} + class Bar {} + const myExpression = class {} + ", + Some(serde_json::json!([{ "ignoreExpressions": true, "max": 1 }])), + ), + ( + " + class Foo {} + class Bar {} + class Baz {} + const myExpression = class {} + ", + Some(serde_json::json!([{ "ignoreExpressions": true, "max": 2 }])), + ), + ]; + + Tester::new(MaxClassesPerFile::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/max_classes_per_file.snap b/crates/oxc_linter/src/snapshots/max_classes_per_file.snap new file mode 100644 index 000000000..be00edabe --- /dev/null +++ b/crates/oxc_linter/src/snapshots/max_classes_per_file.snap @@ -0,0 +1,67 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: max_classes_per_file +--- + ⚠ eslint(max-classes-per-file): File has too many classes (2). Maximum allowed is 1 + ╭─[max_classes_per_file.tsx:2:4] + 1 │ class Foo {} + 2 │ class Bar {} + · ──────────── + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (2). Maximum allowed is 1 + ╭─[max_classes_per_file.tsx:2:25] + 1 │ class Foo {} + 2 │ const myExpression = class {} + · ──────── + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (2). Maximum allowed is 1 + ╭─[max_classes_per_file.tsx:2:12] + 1 │ var x = class {}; + 2 │ var y = class {}; + · ──────── + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (2). Maximum allowed is 1 + ╭─[max_classes_per_file.tsx:2:12] + 1 │ class Foo {} + 2 │ var x = class {}; + · ──────── + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (2). Maximum allowed is 1 + ╭─[max_classes_per_file.tsx:1:14] + 1 │ class Foo {} class Bar {} + · ──────────── + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (3). Maximum allowed is 2 + ╭─[max_classes_per_file.tsx:1:27] + 1 │ class Foo {} class Bar {} class Baz {} + · ──────────── + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (2). Maximum allowed is 1 + ╭─[max_classes_per_file.tsx:3:20] + 2 │ class Foo {} + 3 │ class Bar {} + · ──────────── + 4 │ const myExpression = class {} + ╰──── + help: Reduce the number of classes in this file + + ⚠ eslint(max-classes-per-file): File has too many classes (3). Maximum allowed is 2 + ╭─[max_classes_per_file.tsx:4:20] + 3 │ class Bar {} + 4 │ class Baz {} + · ──────────── + 5 │ const myExpression = class {} + ╰──── + help: Reduce the number of classes in this file diff --git a/crates/oxc_semantic/src/class/table.rs b/crates/oxc_semantic/src/class/table.rs index 4a474c8e1..b2f8ce180 100644 --- a/crates/oxc_semantic/src/class/table.rs +++ b/crates/oxc_semantic/src/class/table.rs @@ -57,6 +57,10 @@ impl ClassTable { std::iter::successors(Some(class_id), |class_id| self.parent_ids.get(class_id).copied()) } + pub fn len(&self) -> usize { + self.declarations.raw.len() + } + pub fn iter_enumerated(&self) -> impl Iterator + '_ { self.declarations.iter_enumerated() }