Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
feat(rome_js_analyze): noUselessTypeConstraint (#4484)
Browse files Browse the repository at this point in the history
Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Victorien ELVINGER <victorien@elvinger.fr>
  • Loading branch information
3 people committed May 18, 2023
1 parent 61abdc3 commit 7f72d15
Show file tree
Hide file tree
Showing 16 changed files with 678 additions and 17 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ the correct rules to apply [#4502](https://github.com/rome/tools/issues/4502)

### Linter

#### New rules
- [`noUselessTypeConstraint`](https://docs.rome.tools/lint/rules/noUselessTypeConstraint/)

#### Other changes

- `noInnerDeclarations`: allow function declarations in nested block inside an _ES module_ [#4492](https://github.com/rome/tools/compare/main...Conaclos:noInnerDeclarations/4492?expand=1).
Expand Down Expand Up @@ -54,7 +57,6 @@ the correct rules to apply [#4502](https://github.com/rome/tools/issues/4502)
- Fix an issue where the `noAssignInExpressions` rule replaced the operator with an invalid token, which caused other lint rules to crash. [#4464](https://github.com/rome/tools/issues/4464)
- Fix an issue that [`noUnusedVariables`](https://docs.rome.tools/lint/rules/nounusedvariables/) rule did not correctly detect exports when a variable and an `interface` had the same name [#4468](https://github.com/rome/tools/pull/4468)


## 12.1.0

### CLI
Expand Down
1 change: 1 addition & 0 deletions crates/rome_diagnostics_categories/src/categories.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ define_categories! {
"lint/complexity/noUselessLabel":"https://docs.rome.tools/lint/rules/noUselessLabel",
"lint/complexity/noUselessRename": "https://docs.rome.tools/lint/rules/noUselessRename",
"lint/complexity/noUselessSwitchCase": "https://docs.rome.tools/lint/rules/noUselessSwitchCase",
"lint/complexity/noUselessTypeConstraint": "https://docs.rome.tools/lint/rules/noUselessTypeConstraint",
"lint/complexity/noWith": "https://docs.rome.tools/lint/rules/noWith",
"lint/complexity/useFlatMap": "https://docs.rome.tools/lint/rules/useFlatMap",
"lint/complexity/useOptionalChain": "https://docs.rome.tools/lint/rules/useOptionalChain",
Expand Down
3 changes: 2 additions & 1 deletion crates/rome_js_analyze/src/analyzers/complexity.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
use rome_analyze::context::RuleContext;
use rome_analyze::{declare_rule, ActionCategory, Ast, Rule, RuleDiagnostic};
use rome_console::markup;

use rome_diagnostics::Applicability;
use rome_js_syntax::TsTypeConstraintClause;
use rome_rowan::{AstNode, BatchMutationExt};

use crate::JsRuleAction;

declare_rule! {
/// Disallow using `any` or `unknown` as type constraint.
///
/// Generic type parameters (`<T>`) in TypeScript may be **constrained** with [`extends`](https://www.typescriptlang.org/docs/handbook/generics.html#generic-constraints).
/// A supplied type must then be a subtype of the supplied constraint.
/// All types are subtypes of `any` and `unknown`.
/// It is thus useless to extend from `any` or `unknown`.
///
/// Source: https://typescript-eslint.io/rules/no-unnecessary-type-constraint/
///
/// ## Examples
///
/// ### Invalid
///
/// ```ts,expect_diagnostic
/// interface FooAny<T extends any> {}
/// ```
/// ```ts,expect_diagnostic
/// type BarAny<T extends any> = {};
/// ```
/// ```ts,expect_diagnostic
/// class BazAny<T extends any> {
/// }
/// ```
/// ```ts,expect_diagnostic
/// class BazAny {
/// quxAny<U extends any>() {}
/// }
/// ```
/// ```ts,expect_diagnostic
/// const QuuxAny = <T extends any>() => {};
/// ```
/// ```ts,expect_diagnostic
/// function QuuzAny<T extends any>() {}
/// ```
///
/// ```ts,expect_diagnostic
/// interface FooUnknown<T extends unknown> {}
/// ```
/// ```ts,expect_diagnostic
/// type BarUnknown<T extends unknown> = {};
/// ```
/// ```ts,expect_diagnostic
/// class BazUnknown<T extends unknown> {
/// }
/// ```ts,expect_diagnostic
/// class BazUnknown {
/// quxUnknown<U extends unknown>() {}
/// }
/// ```
/// ```ts,expect_diagnostic
/// const QuuxUnknown = <T extends unknown>() => {};
/// ```
/// ```ts,expect_diagnostic
/// function QuuzUnknown<T extends unknown>() {}
/// ```
///
/// ### Valid
///
/// ```ts
/// interface Foo<T> {}
///
/// type Bar<T> = {};
///```
pub(crate) NoUselessTypeConstraint {
version: "next",
name: "noUselessTypeConstraint",
recommended: true,
}
}

impl Rule for NoUselessTypeConstraint {
type Query = Ast<TsTypeConstraintClause>;
type State = ();
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Option<Self::State> {
let node = ctx.query();
let ty = node.ty().ok()?;

if ty.as_ts_any_type().is_some() || ty.as_ts_unknown_type().is_some() {
Some(())
} else {
None
}
}

fn diagnostic(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<RuleDiagnostic> {
let node = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
node.syntax().text_trimmed_range(),
markup! {
"Constraining a type parameter to "<Emphasis>"any"</Emphasis>" or "<Emphasis>"unknown"</Emphasis>" is useless."
},
)
.note(markup! {
"All types are subtypes of "<Emphasis>"any"</Emphasis>" and "<Emphasis>"unknown"</Emphasis>"."
}),
)
}

fn action(ctx: &RuleContext<Self>, _state: &Self::State) -> Option<JsRuleAction> {
let node = ctx.query();
let mut mutation = ctx.root().begin();
mutation.remove_node(node.clone());

Some(JsRuleAction {
category: ActionCategory::QuickFix,
applicability: Applicability::MaybeIncorrect,
message: markup! { "Remove the constraint." }.to_owned(),
mutation,
})
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
interface FooAny1<T extends any> {
field: T;
}

interface FooAny2<T extends unknown> {
field: T;
}

class BazAny<T extends any> {
quxAny<U extends any>() {}
}

const QuuxAny = <T extends any>() => {};

function QuuzAny<T extends any>() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
assertion_line: 96
expression: invalid.ts
---
# Input
```js
interface FooAny1<T extends any> {
field: T;
}

interface FooAny2<T extends unknown> {
field: T;
}

class BazAny<T extends any> {
quxAny<U extends any>() {}
}

const QuuxAny = <T extends any>() => {};

function QuuzAny<T extends any>() {}

```

# Diagnostics
```
invalid.ts:1:21 lint/complexity/noUselessTypeConstraint FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Constraining a type parameter to any or unknown is useless.
> 1 │ interface FooAny1<T extends any> {
│ ^^^^^^^^^^^
2 │ field: T;
3 │ }
i All types are subtypes of any and unknown.
i Suggested fix: Remove the constraint.
1 │ interface·FooAny1<T·extends·any>·{
│ -----------
```

```
invalid.ts:5:21 lint/complexity/noUselessTypeConstraint FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Constraining a type parameter to any or unknown is useless.
3 │ }
4 │
> 5 │ interface FooAny2<T extends unknown> {
│ ^^^^^^^^^^^^^^^
6 │ field: T;
7 │ }
i All types are subtypes of any and unknown.
i Suggested fix: Remove the constraint.
5 │ interface·FooAny2<T·extends·unknown>·{
│ ---------------
```

```
invalid.ts:9:16 lint/complexity/noUselessTypeConstraint FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Constraining a type parameter to any or unknown is useless.
7 │ }
8 │
> 9 │ class BazAny<T extends any> {
│ ^^^^^^^^^^^
10 │ quxAny<U extends any>() {}
11 │ }
i All types are subtypes of any and unknown.
i Suggested fix: Remove the constraint.
9 │ class·BazAny<T·extends·any>·{
│ -----------
```

```
invalid.ts:10:12 lint/complexity/noUselessTypeConstraint FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Constraining a type parameter to any or unknown is useless.
9 │ class BazAny<T extends any> {
> 10 │ quxAny<U extends any>() {}
│ ^^^^^^^^^^^
11 │ }
12 │
i All types are subtypes of any and unknown.
i Suggested fix: Remove the constraint.
10 │ ··quxAny<U·extends·any>()·{}
│ -----------
```

```
invalid.ts:13:20 lint/complexity/noUselessTypeConstraint FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Constraining a type parameter to any or unknown is useless.
11 │ }
12 │
> 13 │ const QuuxAny = <T extends any>() => {};
│ ^^^^^^^^^^^
14 │
15 │ function QuuzAny<T extends any>() {}
i All types are subtypes of any and unknown.
i Suggested fix: Remove the constraint.
13 │ const·QuuxAny·=·<T·extends·any>()·=>·{};
│ -----------
```

```
invalid.ts:15:20 lint/complexity/noUselessTypeConstraint FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Constraining a type parameter to any or unknown is useless.
13 │ const QuuxAny = <T extends any>() => {};
14 │
> 15 │ function QuuzAny<T extends any>() {}
│ ^^^^^^^^^^^
16 │
i All types are subtypes of any and unknown.
i Suggested fix: Remove the constraint.
15 │ function·QuuzAny<T·extends·any>()·{}
│ -----------
```


Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
interface FooAny0<T> {
field: T;
}

interface FooNotAny0<T extends string> {
field: T;
}

type Bar<T> = {};

type Bar2<T extends string> = {};
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: valid.ts
---
# Input
```js
interface FooAny0<T> {
field: T;
}

interface FooNotAny0<T extends string> {
field: T;
}

type Bar<T> = {};

type Bar2<T extends string> = {};

```


0 comments on commit 7f72d15

Please sign in to comment.