From 495d88db7159865d96e090ad2c2a2efdd029e8a7 Mon Sep 17 00:00:00 2001 From: YaBoiSkinnyP Date: Fri, 19 Jul 2024 03:55:12 -0400 Subject: [PATCH 1/4] feature --- crates/oxc_linter/src/rules.rs | 2 + .../rules/typescript/no_extraneous_class.rs | 354 ++++++++++++++++++ .../src/snapshots/no_extraneous_class.snap | 103 +++++ 3 files changed, 459 insertions(+) create mode 100644 crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs create mode 100644 crates/oxc_linter/src/snapshots/no_extraneous_class.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index c9d85f32933bc..5e38ded477a7a 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -141,6 +141,7 @@ mod typescript { pub mod no_empty_interface; pub mod no_explicit_any; pub mod no_extra_non_null_assertion; + pub mod no_extraneous_class; pub mod no_import_type_side_effects; pub mod no_misused_new; pub mod no_namespace; @@ -571,6 +572,7 @@ oxc_macros::declare_all_lint_rules! { typescript::no_non_null_asserted_nullish_coalescing, typescript::no_confusing_non_null_assertion, typescript::no_dynamic_delete, + typescript::no_extraneous_class, jest::consistent_test_it, jest::expect_expect, jest::max_expects, diff --git a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs new file mode 100644 index 0000000000000..608584dc8e55b --- /dev/null +++ b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs @@ -0,0 +1,354 @@ +use oxc_ast::{ + ast::{ClassElement, MethodDefinitionKind}, + AstKind +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule, AstNode}; + +#[derive(Debug, Default, Clone)] +pub struct NoExtraneousClass { + allow_constructor_only: bool, + allow_empty: bool, + allow_static_only: bool, + allow_with_decorator: bool +} + +declare_oxc_lint!( + /// ### What it does + /// + /// This rule reports when a class has no non-static members, + /// such as for a class used exclusively as a static namespace. + /// This rule also reports classes that have only a constructor and no fields. + /// Those classes can generally be replaced with a standalone function. + /// + /// ### Why is this bad? + /// + /// Users who come from a OOP paradigm may wrap their utility functions in an extra class, + /// instead of putting them at the top level of an ECMAScript module. + /// Doing so is generally unnecessary in JavaScript and TypeScript projects. + /// + /// Wrapper classes add extra cognitive complexity to code without adding any structural improvements + /// + /// Whatever would be put on them, such as utility functions, are already organized by virtue of being in a module. + /// + /// As an alternative, you can import * as ... the module to get all of them in a single object. + /// IDEs can't provide as good suggestions for static class or namespace imported properties when you start typing property names + /// + /// It's more difficult to statically analyze code for unused variables, etc. + /// when they're all on the class (see: Finding dead code (and dead types) in TypeScript). + /// + /// ### Example + /// ```javascript + /// class StaticConstants { + /// static readonly version = 42; + /// + /// static isProduction() { + /// return process.env.NODE_ENV === 'production'; + /// } + /// } + /// + /// class HelloWorldLogger { + /// constructor() { + /// console.log('Hello, world!'); + /// } + /// } + /// + /// abstract class Foo {} + /// ``` + NoExtraneousClass, + suspicious, // TODO: change category to `correctness`, `suspicious`, `pedantic`, `perf`, `restriction`, or `style` + // See for details +); + +fn empty_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn(format!( + "typescript-eslint(no-extraneous-class): Unexpected empty class." + )) + .with_label(span) +} + +fn only_static_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn(format!( + "typescript-eslint(no-extraneous-class): Unexpected class with only static properties." + )) + .with_label(span) +} + +fn only_constructor_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn(format!( + "typescript-eslint(no-extraneous-class): Unexpected class with only a constructor." + )) + .with_label(span) +} + +impl Rule for NoExtraneousClass { + fn from_configuration(value: serde_json::Value) -> Self { + Self { + allow_constructor_only: value + .get(0) + .and_then(|x| x.get("allowConstructorOnly")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + allow_empty: value + .get(0) + .and_then(|x| x.get("allowEmpty")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + allow_static_only: value + .get(0) + .and_then(|x| x.get("allowStaticOnly")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + allow_with_decorator: value + .get(0) + .and_then(|x| x.get("allowWithDecorator")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false), + } + } + + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + match node.kind() { + AstKind::Class(class) => { + if class.super_class.is_some() { + return + } + if self.allow_with_decorator && class.decorators.len() > 0 { + return + } + if class.body.body.len() == 0 { + if self.allow_empty { + return + } + ctx.diagnostic(empty_no_extraneous_class_diagnostic(class.span)); + return + } + + let only_constructor = class.body.body.iter().all(|prop| { + if let ClassElement::MethodDefinition(method_definition) = prop { + method_definition.kind == MethodDefinitionKind::Constructor && !method_definition.value.params.items.iter().any(|param| { + param.is_public() + }) + } else { + false + } + }); + if only_constructor && !self.allow_constructor_only { + ctx.diagnostic(only_constructor_no_extraneous_class_diagnostic(class.span)); + return + } + let only_static = class.body.body.iter().all(|prop| { + prop.r#static() && !prop.is_abstract() + }); + if only_static && !self.allow_static_only { + ctx.diagnostic(only_static_no_extraneous_class_diagnostic(class.span)); + return + } + } + _ => {} + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ( + " + class Foo { + public prop = 1; + constructor() {} + } + ", + None, + ), + ( + " + export class CClass extends BaseClass { + public static helper(): void {} + private static privateHelper(): boolean { + return true; + } + constructor() {} + } + ", + None, + ), + ( + " + class Foo { + constructor(public bar: string) {} + } + ", + None, + ), + ("class Foo {}", Some(serde_json::json!([{ "allowEmpty": true }]))), + ( + " + class Foo { + constructor() {} + } + ", + Some(serde_json::json!([{ "allowConstructorOnly": true }])), + ), + ( + " + export class Bar { + public static helper(): void {} + private static privateHelper(): boolean { + return true; + } + } + ", + Some(serde_json::json!([{ "allowStaticOnly": true }])), + ), + ( + " + export default class { + hello() { + return 'I am foo!'; + } + } + ", + None, + ), + ( + " + @FooDecorator + class Foo {} + ", + Some(serde_json::json!([{ "allowWithDecorator": true }])), + ), + ( + " + @FooDecorator + class Foo { + constructor(foo: Foo) { + foo.subscribe(a => { + console.log(a); + }); + } + } + ", + Some(serde_json::json!([{ "allowWithDecorator": true }])), + ), + ( + " + abstract class Foo { + abstract property: string; + } + ", + None, + ), + ( + " + abstract class Foo { + abstract method(): string; + } + ", + None, + ), + ]; + + let fail = vec![ + ("class Foo {}", None), + ( + " + class Foo { + public prop = 1; + constructor() { + class Bar { + static PROP = 2; + } + } + } + export class Bar { + public static helper(): void {} + private static privateHelper(): boolean { + return true; + } + } + ", + None, + ), + ( + " + class Foo { + constructor() {} + } + ", + None, + ), + ( + " + export class AClass { + public static helper(): void {} + private static privateHelper(): boolean { + return true; + } + constructor() { + class nestedClass {} + } + } + ", + None, + ), + ( + " + export default class { + static hello() {} + } + ", + None, + ), + ( + " + @FooDecorator + class Foo {} + ", + Some(serde_json::json!([{ "allowWithDecorator": false }])), + ), + ( + " + @FooDecorator + class Foo { + constructor(foo: Foo) { + foo.subscribe(a => { + console.log(a); + }); + } + } + ", + Some(serde_json::json!([{ "allowWithDecorator": false }])), + ), + ( + " + abstract class Foo {} + ", + None, + ), + ( + " + abstract class Foo { + static property: string; + } + ", + None, + ), + ( + " + abstract class Foo { + constructor() {} + } + ", + None, + ), + ]; + + Tester::new(NoExtraneousClass::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_extraneous_class.snap b/crates/oxc_linter/src/snapshots/no_extraneous_class.snap new file mode 100644 index 0000000000000..8bb6f0312149c --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_extraneous_class.snap @@ -0,0 +1,103 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ typescript-eslint(no-extraneous-class): Unexpected empty class. + ╭─[no_extraneous_class.tsx:1:1] + 1 │ class Foo {} + · ──────────── + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. + ╭─[no_extraneous_class.tsx:5:8] + 4 │ constructor() { + 5 │ ╭─▶ class Bar { + 6 │ │ static PROP = 2; + 7 │ ╰─▶ } + 8 │ } + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. + ╭─[no_extraneous_class.tsx:10:11] + 9 │ } + 10 │ ╭─▶ export class Bar { + 11 │ │ public static helper(): void {} + 12 │ │ private static privateHelper(): boolean { + 13 │ │ return true; + 14 │ │ } + 15 │ ╰─▶ } + 16 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only a constructor. + ╭─[no_extraneous_class.tsx:2:4] + 1 │ + 2 │ ╭─▶ class Foo { + 3 │ │ constructor() {} + 4 │ ╰─▶ } + 5 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected empty class. + ╭─[no_extraneous_class.tsx:8:8] + 7 │ constructor() { + 8 │ class nestedClass {} + · ──────────────────── + 9 │ } + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. + ╭─[no_extraneous_class.tsx:2:19] + 1 │ + 2 │ ╭─▶ export default class { + 3 │ │ static hello() {} + 4 │ ╰─▶ } + 5 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected empty class. + ╭─[no_extraneous_class.tsx:2:4] + 1 │ + 2 │ ╭─▶ @FooDecorator + 3 │ ╰─▶ class Foo {} + 4 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only a constructor. + ╭─[no_extraneous_class.tsx:2:4] + 1 │ + 2 │ ╭─▶ @FooDecorator + 3 │ │ class Foo { + 4 │ │ constructor(foo: Foo) { + 5 │ │ foo.subscribe(a => { + 6 │ │ console.log(a); + 7 │ │ }); + 8 │ │ } + 9 │ ╰─▶ } + 10 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected empty class. + ╭─[no_extraneous_class.tsx:2:4] + 1 │ + 2 │ abstract class Foo {} + · ───────────────────── + 3 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. + ╭─[no_extraneous_class.tsx:2:4] + 1 │ + 2 │ ╭─▶ abstract class Foo { + 3 │ │ static property: string; + 4 │ ╰─▶ } + 5 │ + ╰──── + + ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only a constructor. + ╭─[no_extraneous_class.tsx:2:4] + 1 │ + 2 │ ╭─▶ abstract class Foo { + 3 │ │ constructor() {} + 4 │ ╰─▶ } + 5 │ + ╰──── From 26c9ef03a7db7667fdf8f04354ef47ab5fbd03ab Mon Sep 17 00:00:00 2001 From: YaBoiSkinnyP Date: Fri, 19 Jul 2024 04:14:08 -0400 Subject: [PATCH 2/4] format and lint --- .../rules/typescript/no_extraneous_class.rs | 189 +++++++++--------- 1 file changed, 93 insertions(+), 96 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs index 608584dc8e55b..d1f124ef85997 100644 --- a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs +++ b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs @@ -1,6 +1,6 @@ use oxc_ast::{ - ast::{ClassElement, MethodDefinitionKind}, - AstKind + ast::{ClassElement, FormalParameter, MethodDefinitionKind}, + AstKind, }; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; @@ -10,53 +10,53 @@ use crate::{context::LintContext, rule::Rule, AstNode}; #[derive(Debug, Default, Clone)] pub struct NoExtraneousClass { - allow_constructor_only: bool, - allow_empty: bool, - allow_static_only: bool, - allow_with_decorator: bool + allow_constructor_only: bool, + allow_empty: bool, + allow_static_only: bool, + allow_with_decorator: bool, } declare_oxc_lint!( /// ### What it does - /// + /// /// This rule reports when a class has no non-static members, - /// such as for a class used exclusively as a static namespace. - /// This rule also reports classes that have only a constructor and no fields. - /// Those classes can generally be replaced with a standalone function. + /// such as for a class used exclusively as a static namespace. + /// This rule also reports classes that have only a constructor and no fields. + /// Those classes can generally be replaced with a standalone function. /// /// ### Why is this bad? - /// + /// /// Users who come from a OOP paradigm may wrap their utility functions in an extra class, - /// instead of putting them at the top level of an ECMAScript module. - /// Doing so is generally unnecessary in JavaScript and TypeScript projects. - /// - /// Wrapper classes add extra cognitive complexity to code without adding any structural improvements - /// - /// Whatever would be put on them, such as utility functions, are already organized by virtue of being in a module. - /// - /// As an alternative, you can import * as ... the module to get all of them in a single object. - /// IDEs can't provide as good suggestions for static class or namespace imported properties when you start typing property names - /// - /// It's more difficult to statically analyze code for unused variables, etc. - /// when they're all on the class (see: Finding dead code (and dead types) in TypeScript). + /// instead of putting them at the top level of an ECMAScript module. + /// Doing so is generally unnecessary in JavaScript and TypeScript projects. + /// + /// Wrapper classes add extra cognitive complexity to code without adding any structural improvements + /// + /// Whatever would be put on them, such as utility functions, are already organized by virtue of being in a module. + /// + /// As an alternative, you can import * as ... the module to get all of them in a single object. + /// IDEs can't provide as good suggestions for static class or namespace imported properties when you start typing property names + /// + /// It's more difficult to statically analyze code for unused variables, etc. + /// when they're all on the class (see: Finding dead code (and dead types) in TypeScript). /// /// ### Example /// ```javascript - /// class StaticConstants { - /// static readonly version = 42; - /// - /// static isProduction() { - /// return process.env.NODE_ENV === 'production'; - /// } - /// } - /// - /// class HelloWorldLogger { - /// constructor() { - /// console.log('Hello, world!'); - /// } - /// } - /// - /// abstract class Foo {} + /// class StaticConstants { + /// static readonly version = 42; + /// + /// static isProduction() { + /// return process.env.NODE_ENV === 'production'; + /// } + /// } + /// + /// class HelloWorldLogger { + /// constructor() { + /// console.log('Hello, world!'); + /// } + /// } + /// + /// abstract class Foo {} /// ``` NoExtraneousClass, suspicious, // TODO: change category to `correctness`, `suspicious`, `pedantic`, `perf`, `restriction`, or `style` @@ -64,93 +64,90 @@ declare_oxc_lint!( ); fn empty_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { - OxcDiagnostic::warn(format!( - "typescript-eslint(no-extraneous-class): Unexpected empty class." - )) - .with_label(span) + OxcDiagnostic::warn("typescript-eslint(no-extraneous-class): Unexpected empty class.") + .with_label(span) } fn only_static_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { - OxcDiagnostic::warn(format!( - "typescript-eslint(no-extraneous-class): Unexpected class with only static properties." - )) + OxcDiagnostic::warn( + "typescript-eslint(no-extraneous-class): Unexpected class with only static properties.", + ) .with_label(span) } fn only_constructor_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { - OxcDiagnostic::warn(format!( - "typescript-eslint(no-extraneous-class): Unexpected class with only a constructor." - )) + OxcDiagnostic::warn( + "typescript-eslint(no-extraneous-class): Unexpected class with only a constructor.", + ) .with_label(span) } impl Rule for NoExtraneousClass { - fn from_configuration(value: serde_json::Value) -> Self { - Self { - allow_constructor_only: value + fn from_configuration(value: serde_json::Value) -> Self { + Self { + allow_constructor_only: value .get(0) .and_then(|x| x.get("allowConstructorOnly")) .and_then(serde_json::Value::as_bool) .unwrap_or(false), - allow_empty: value + allow_empty: value .get(0) .and_then(|x| x.get("allowEmpty")) .and_then(serde_json::Value::as_bool) .unwrap_or(false), - allow_static_only: value + allow_static_only: value .get(0) .and_then(|x| x.get("allowStaticOnly")) .and_then(serde_json::Value::as_bool) .unwrap_or(false), - allow_with_decorator: value + allow_with_decorator: value .get(0) .and_then(|x| x.get("allowWithDecorator")) .and_then(serde_json::Value::as_bool) .unwrap_or(false), - } - } + } + } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - match node.kind() { - AstKind::Class(class) => { - if class.super_class.is_some() { - return - } - if self.allow_with_decorator && class.decorators.len() > 0 { - return - } - if class.body.body.len() == 0 { - if self.allow_empty { - return - } - ctx.diagnostic(empty_no_extraneous_class_diagnostic(class.span)); - return - } - - let only_constructor = class.body.body.iter().all(|prop| { - if let ClassElement::MethodDefinition(method_definition) = prop { - method_definition.kind == MethodDefinitionKind::Constructor && !method_definition.value.params.items.iter().any(|param| { - param.is_public() - }) - } else { - false - } - }); - if only_constructor && !self.allow_constructor_only { - ctx.diagnostic(only_constructor_no_extraneous_class_diagnostic(class.span)); - return - } - let only_static = class.body.body.iter().all(|prop| { - prop.r#static() && !prop.is_abstract() - }); - if only_static && !self.allow_static_only { - ctx.diagnostic(only_static_no_extraneous_class_diagnostic(class.span)); - return - } - } - _ => {} - } - } + if let AstKind::Class(class) = node.kind() { + if class.super_class.is_some() { + return; + } + if self.allow_with_decorator && class.decorators.len() > 0 { + return; + } + if class.body.body.len() == 0 { + if self.allow_empty { + return; + } + ctx.diagnostic(empty_no_extraneous_class_diagnostic(class.span)); + return; + } + + let only_constructor = class.body.body.iter().all(|prop| { + if let ClassElement::MethodDefinition(method_definition) = prop { + method_definition.kind == MethodDefinitionKind::Constructor + && !method_definition + .value + .params + .items + .iter() + .any(FormalParameter::is_public) + } else { + false + } + }); + if only_constructor && !self.allow_constructor_only { + ctx.diagnostic(only_constructor_no_extraneous_class_diagnostic(class.span)); + return; + } + let only_static = + class.body.body.iter().all(|prop| prop.r#static() && !prop.is_abstract()); + if only_static && !self.allow_static_only { + ctx.diagnostic(only_static_no_extraneous_class_diagnostic(class.span)); + } + } + } } #[test] From adaab47fbc5aab4b9a79cfbc9f0a2e28fe47ad61 Mon Sep 17 00:00:00 2001 From: YaBoiSkinnyP Date: Fri, 19 Jul 2024 15:40:05 -0400 Subject: [PATCH 3/4] cleaned up and fixed span --- crates/oxc_ast/src/ast_impl/js.rs | 5 + .../rules/typescript/no_extraneous_class.rs | 95 +++++++++---------- .../src/snapshots/no_extraneous_class.snap | 76 ++++++--------- 3 files changed, 79 insertions(+), 97 deletions(-) diff --git a/crates/oxc_ast/src/ast_impl/js.rs b/crates/oxc_ast/src/ast_impl/js.rs index 4fdab1343fc05..20e1243c58a6e 100644 --- a/crates/oxc_ast/src/ast_impl/js.rs +++ b/crates/oxc_ast/src/ast_impl/js.rs @@ -1045,6 +1045,11 @@ impl<'a> FormalParameter<'a> { pub fn is_public(&self) -> bool { matches!(self.accessibility, Some(TSAccessibility::Public)) } + + #[inline] + pub fn has_modifier(&self) -> bool { + self.accessibility.is_some() || self.readonly || self.r#override + } } impl FormalParameterKind { diff --git a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs index d1f124ef85997..3a8b7d4ccde1c 100644 --- a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs +++ b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs @@ -1,5 +1,5 @@ use oxc_ast::{ - ast::{ClassElement, FormalParameter, MethodDefinitionKind}, + ast::{ClassElement, FormalParameter}, AstKind, }; use oxc_diagnostics::OxcDiagnostic; @@ -59,8 +59,7 @@ declare_oxc_lint!( /// abstract class Foo {} /// ``` NoExtraneousClass, - suspicious, // TODO: change category to `correctness`, `suspicious`, `pedantic`, `perf`, `restriction`, or `style` - // See for details + suspicious ); fn empty_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic { @@ -84,69 +83,61 @@ fn only_constructor_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic impl Rule for NoExtraneousClass { fn from_configuration(value: serde_json::Value) -> Self { + use serde_json::Value; + let Some(config) = value.get(0).and_then(Value::as_object) else { + return Self::default(); + }; Self { - allow_constructor_only: value - .get(0) - .and_then(|x| x.get("allowConstructorOnly")) - .and_then(serde_json::Value::as_bool) + allow_constructor_only: config + .get("allowConstructorOnly") + .and_then(Value::as_bool) .unwrap_or(false), - allow_empty: value - .get(0) - .and_then(|x| x.get("allowEmpty")) - .and_then(serde_json::Value::as_bool) + allow_empty: config + .get("allowEmpty") // lb + .and_then(Value::as_bool) .unwrap_or(false), - allow_static_only: value - .get(0) - .and_then(|x| x.get("allowStaticOnly")) - .and_then(serde_json::Value::as_bool) + allow_static_only: config + .get("allowStaticOnly") + .and_then(Value::as_bool) .unwrap_or(false), - allow_with_decorator: value - .get(0) - .and_then(|x| x.get("allowWithDecorator")) - .and_then(serde_json::Value::as_bool) + allow_with_decorator: config + .get("allowWithDecorator") + .and_then(Value::as_bool) .unwrap_or(false), } } fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { - if let AstKind::Class(class) = node.kind() { - if class.super_class.is_some() { - return; - } - if self.allow_with_decorator && class.decorators.len() > 0 { - return; - } - if class.body.body.len() == 0 { - if self.allow_empty { - return; + let AstKind::Class(class) = node.kind() else { + return; + }; + if class.super_class.is_some() + || (self.allow_with_decorator && !class.decorators.is_empty()) + { + return; + } + let span = class.id.as_ref().map(|id| id.span).unwrap_or(class.span); + let body = &class.body.body; + match body.as_slice() { + [] => { + if !self.allow_empty { + ctx.diagnostic(empty_no_extraneous_class_diagnostic(class.span)); } - ctx.diagnostic(empty_no_extraneous_class_diagnostic(class.span)); - return; } - - let only_constructor = class.body.body.iter().all(|prop| { - if let ClassElement::MethodDefinition(method_definition) = prop { - method_definition.kind == MethodDefinitionKind::Constructor - && !method_definition - .value - .params - .items - .iter() - .any(FormalParameter::is_public) - } else { - false + [ClassElement::MethodDefinition(constructor)] if constructor.kind.is_constructor() => { + let only_constructor = + !constructor.value.params.items.iter().any(FormalParameter::has_modifier); + if only_constructor && !self.allow_constructor_only { + ctx.diagnostic(only_constructor_no_extraneous_class_diagnostic(span)); } - }); - if only_constructor && !self.allow_constructor_only { - ctx.diagnostic(only_constructor_no_extraneous_class_diagnostic(class.span)); - return; } - let only_static = - class.body.body.iter().all(|prop| prop.r#static() && !prop.is_abstract()); - if only_static && !self.allow_static_only { - ctx.diagnostic(only_static_no_extraneous_class_diagnostic(class.span)); + _ => { + let only_static = body.iter().all(|prop| prop.r#static() && !prop.is_abstract()); + if only_static && !self.allow_static_only { + ctx.diagnostic(only_static_no_extraneous_class_diagnostic(span)); + } } - } + }; } } diff --git a/crates/oxc_linter/src/snapshots/no_extraneous_class.snap b/crates/oxc_linter/src/snapshots/no_extraneous_class.snap index 8bb6f0312149c..4a8793cb361f1 100644 --- a/crates/oxc_linter/src/snapshots/no_extraneous_class.snap +++ b/crates/oxc_linter/src/snapshots/no_extraneous_class.snap @@ -8,33 +8,27 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. - ╭─[no_extraneous_class.tsx:5:8] - 4 │ constructor() { - 5 │ ╭─▶ class Bar { - 6 │ │ static PROP = 2; - 7 │ ╰─▶ } - 8 │ } + ╭─[no_extraneous_class.tsx:5:14] + 4 │ constructor() { + 5 │ class Bar { + · ─── + 6 │ static PROP = 2; ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. - ╭─[no_extraneous_class.tsx:10:11] - 9 │ } - 10 │ ╭─▶ export class Bar { - 11 │ │ public static helper(): void {} - 12 │ │ private static privateHelper(): boolean { - 13 │ │ return true; - 14 │ │ } - 15 │ ╰─▶ } - 16 │ + ╭─[no_extraneous_class.tsx:10:17] + 9 │ } + 10 │ export class Bar { + · ─── + 11 │ public static helper(): void {} ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only a constructor. - ╭─[no_extraneous_class.tsx:2:4] - 1 │ - 2 │ ╭─▶ class Foo { - 3 │ │ constructor() {} - 4 │ ╰─▶ } - 5 │ + ╭─[no_extraneous_class.tsx:2:10] + 1 │ + 2 │ class Foo { + · ─── + 3 │ constructor() {} ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected empty class. @@ -63,18 +57,12 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only a constructor. - ╭─[no_extraneous_class.tsx:2:4] - 1 │ - 2 │ ╭─▶ @FooDecorator - 3 │ │ class Foo { - 4 │ │ constructor(foo: Foo) { - 5 │ │ foo.subscribe(a => { - 6 │ │ console.log(a); - 7 │ │ }); - 8 │ │ } - 9 │ ╰─▶ } - 10 │ - ╰──── + ╭─[no_extraneous_class.tsx:3:10] + 2 │ @FooDecorator + 3 │ class Foo { + · ─── + 4 │ constructor(foo: Foo) { + ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected empty class. ╭─[no_extraneous_class.tsx:2:4] @@ -85,19 +73,17 @@ source: crates/oxc_linter/src/tester.rs ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties. - ╭─[no_extraneous_class.tsx:2:4] - 1 │ - 2 │ ╭─▶ abstract class Foo { - 3 │ │ static property: string; - 4 │ ╰─▶ } - 5 │ + ╭─[no_extraneous_class.tsx:2:19] + 1 │ + 2 │ abstract class Foo { + · ─── + 3 │ static property: string; ╰──── ⚠ typescript-eslint(no-extraneous-class): Unexpected class with only a constructor. - ╭─[no_extraneous_class.tsx:2:4] - 1 │ - 2 │ ╭─▶ abstract class Foo { - 3 │ │ constructor() {} - 4 │ ╰─▶ } - 5 │ + ╭─[no_extraneous_class.tsx:2:19] + 1 │ + 2 │ abstract class Foo { + · ─── + 3 │ constructor() {} ╰──── From 7a4dd7475de9ce98d50a67f041a643ae85ab96ca Mon Sep 17 00:00:00 2001 From: YaBoiSkinnyP Date: Fri, 19 Jul 2024 15:48:26 -0400 Subject: [PATCH 4/4] clippy --- crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs index 3a8b7d4ccde1c..2d6c5a02ae338 100644 --- a/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs +++ b/crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs @@ -116,7 +116,7 @@ impl Rule for NoExtraneousClass { { return; } - let span = class.id.as_ref().map(|id| id.span).unwrap_or(class.span); + let span = class.id.as_ref().map_or(class.span, |id| id.span); let body = &class.body.body; match body.as_slice() { [] => {