dan/oxc
1
0
Fork 0
mirror of https://github.com/danbulant/oxc synced 2026-05-19 04:08:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

feat(linter): add typescript-eslint/no-extraneous-class (#4357)

Browse source 
Added rule for https://typescript-eslint.io/rules/no-extraneous-class/

Also, I chose to make the match the node against the class and derive
the body from the node, rather than matching against the body and using
the context to go back up to the parent class node as in the original
source.
This commit is contained in:
Jaden Rodriguez 2024-07-19 15:56:30 -04:00 committed by GitHub
parent ea33f9470b
commit 3c0c7093b5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 438 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
crates/oxc_ast/src/ast_impl/js.rs
View file

@ -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 {

2
crates/oxc_linter/src/rules.rs
View file

@ -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,

342
crates/oxc_linter/src/rules/typescript/no_extraneous_class.rs Normal file
View file

@ -0,0 +1,342 @@
use oxc_ast::{
ast::{ClassElement, FormalParameter},
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
);
fn empty_no_extraneous_class_diagnostic(span: Span) -> OxcDiagnostic {
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(
"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(
"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 {
use serde_json::Value;
let Some(config) = value.get(0).and_then(Value::as_object) else {
return Self::default();
};
Self {
allow_constructor_only: config
.get("allowConstructorOnly")
.and_then(Value::as_bool)
.unwrap_or(false),
allow_empty: config
.get("allowEmpty") // lb
.and_then(Value::as_bool)
.unwrap_or(false),
allow_static_only: config
.get("allowStaticOnly")
.and_then(Value::as_bool)
.unwrap_or(false),
allow_with_decorator: config
.get("allowWithDecorator")
.and_then(Value::as_bool)
.unwrap_or(false),
}
}
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
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_or(class.span, |id| id.span);
let body = &class.body.body;
match body.as_slice() {
[] => {
if !self.allow_empty {
ctx.diagnostic(empty_no_extraneous_class_diagnostic(class.span));
}
}
[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));
}
}
_ => {
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));
}
}
};
}
}
#[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();
}

89
crates/oxc_linter/src/snapshots/no_extraneous_class.snap Normal file
View file

@ -0,0 +1,89 @@
---
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: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: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:10]
1 │
2 │ class Foo {
· ───
3 │ constructor() {}
╰────
⚠ 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: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]
1 │
2 │ abstract class Foo {}
· ─────────────────────
3 │
╰────
⚠ typescript-eslint(no-extraneous-class): Unexpected class with only static properties.
╭─[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:19]
1 │
2 │ abstract class Foo {
· ───
3 │ constructor() {}
╰────