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

feat(rome_js_analyze): noUnsafeDeclarationMerging #4736

Merged
merged 2 commits into from
Jul 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ if no error diagnostics are emitted.

This rule recommends using `Number.isNaN` instead of the global and unsafe `isNaN` that attempts a type coercion.

- Add [`noUnsafeDeclarationMerging`](https://docs.rome.tools/lint/rules/noUnsafeDeclarationMerging/)

This rule disallows declaration merging between an interface and a class.

- Add [`useArrowFunction`](https://docs.rome.tools/lint/rules/usearrowfunction/)

This rule proposes turning function expressions into arrow functions.
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 @@ -94,6 +94,7 @@ define_categories! {
"lint/nursery/noRedundantRoles": "https://docs.rome.tools/lint/rules/noRedundantRoles",
"lint/nursery/noSelfAssign": "https://docs.rome.tools/lint/rules/noSelfAssign",
"lint/nursery/noStaticOnlyClass": "https://docs.rome.tools/lint/rules/noStaticOnlyClass",
"lint/nursery/noUnsafeDeclarationMerging": "https://docs.rome.tools/lint/rules/noUnsafeDeclarationMerging",
"lint/nursery/noUselessEmptyExport": "https://docs.rome.tools/lint/rules/noUselessEmptyExport",
"lint/nursery/noVoid": "https://docs.rome.tools/lint/rules/noVoid",
"lint/nursery/useAriaPropTypes": "https://docs.rome.tools/lint/rules/useAriaPropTypes",
Expand Down
2 changes: 2 additions & 0 deletions crates/rome_js_analyze/src/semantic_analyzers/nursery.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,108 @@
use crate::semantic_services::Semantic;
use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use rome_console::markup;
use rome_js_syntax::{
binding_ext::AnyJsBindingDeclaration, TsDeclareStatement, TsExportDeclareClause,
TsInterfaceDeclaration,
};
use rome_rowan::{AstNode, TextRange};

declare_rule! {
/// Disallow unsafe declaration merging between interfaces and classes.
///
/// _TypeScript_'s [declaration merging](https://www.typescriptlang.org/docs/handbook/declaration-merging.html) supports merging separate declarations with the same name.
///
/// _Declaration merging_ between classes and interfaces is unsafe.
/// The _TypeScript Compiler_ doesn't check whether properties defined in the interface are initialized in the class.
/// This can cause lead to _TypeScript_ not detecting code that will cause runtime errors.
///
/// Source: https://typescript-eslint.io/rules/no-unsafe-declaration-merging/
///
/// ## Examples
///
/// ### Invalid
///
/// ```ts,expect_diagnostic
/// interface Foo {
/// f(): void
/// }
///
/// class Foo {}
///
/// const foo = new Foo();
/// foo.f(); // Runtime Error: Cannot read properties of undefined.
/// ```
///
/// ## Valid
///
/// ```ts
/// interface Foo {}
/// class Bar implements Foo {}
/// ```
///
/// ```ts
/// namespace Baz {}
/// namespace Baz {}
/// enum Baz {}
/// ```
pub(crate) NoUnsafeDeclarationMerging {
version: "next",
name: "noUnsafeDeclarationMerging",
recommended: true,
}
}

impl Rule for NoUnsafeDeclarationMerging {
type Query = Semantic<TsInterfaceDeclaration>;
type State = TextRange;
type Signals = Option<Self::State>;
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let ts_interface = ctx.query();
let model = ctx.model();
let interface_binding = ts_interface.id().ok()?;
let interface_name = interface_binding.name_token().ok()?;
let scope = model.scope(ts_interface.syntax()).parent()?;
for binding in scope.bindings() {
if let Some(AnyJsBindingDeclaration::JsClassDeclaration(class)) =
binding.tree().declaration()
{
// This is not unsafe of merging an interface and an ambient class.
if class.parent::<TsDeclareStatement>().is_none()
&& class.parent::<TsExportDeclareClause>().is_none()
{
if let Ok(id) = class.id() {
if let Some(id) = id.as_js_identifier_binding() {
if let Ok(name) = id.name_token() {
if name.text() == interface_name.text() {
return Some(name.text_trimmed_range());
}
}
}
}
}
}
}
None
}

fn diagnostic(ctx: &RuleContext<Self>, class_range: &Self::State) -> Option<RuleDiagnostic> {
let ts_interface = ctx.query();
Some(
RuleDiagnostic::new(
rule_category!(),
class_range,
markup! {
"This "<Emphasis>"class"</Emphasis>" is unsafely merged with an "<Emphasis>"interface"</Emphasis>"."
},
)
.detail(ts_interface.id().ok()?.range(), markup! {
"The "<Emphasis>"interface"</Emphasis>" is declared here."
})
.note(markup! {
"The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class."
}),
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
interface Foo {}
class Foo {}

class Foo2 {}
interface Foo2 {}

function f(){
interface Foo3 {}
class Foo3 {}
}

{
interface Foo4 {}
class Foo4 {}
}

export {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: invalid.ts
---
# Input
```js
interface Foo {}
class Foo {}

class Foo2 {}
interface Foo2 {}

function f(){
interface Foo3 {}
class Foo3 {}
}

{
interface Foo4 {}
class Foo4 {}
}

export {}

```

# Diagnostics
```
invalid.ts:2:7 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This class is unsafely merged with an interface.

1 │ interface Foo {}
> 2 │ class Foo {}
│ ^^^
3 │
4 │ class Foo2 {}

i The interface is declared here.

> 1 │ interface Foo {}
│ ^^^
2 │ class Foo {}
3 │

i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.


```

```
invalid.ts:4:7 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This class is unsafely merged with an interface.

2 │ class Foo {}
3 │
> 4 │ class Foo2 {}
│ ^^^^
5 │ interface Foo2 {}
6 │

i The interface is declared here.

4 │ class Foo2 {}
> 5 │ interface Foo2 {}
│ ^^^^
6 │
7 │ function f(){

i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.


```

```
invalid.ts:9:11 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This class is unsafely merged with an interface.

7 │ function f(){
8 │ interface Foo3 {}
> 9 │ class Foo3 {}
│ ^^^^
10 │ }
11 │

i The interface is declared here.

7 │ function f(){
> 8 │ interface Foo3 {}
│ ^^^^
9 │ class Foo3 {}
10 │ }

i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.


```

```
invalid.ts:14:11 lint/nursery/noUnsafeDeclarationMerging ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! This class is unsafely merged with an interface.

12 │ {
13 │ interface Foo4 {}
> 14 │ class Foo4 {}
│ ^^^^
15 │ }
16 │

i The interface is declared here.

12 │ {
> 13 │ interface Foo4 {}
│ ^^^^
14 │ class Foo4 {}
15 │ }

i The TypeScript compiler doesn't check whether properties defined in the interface are initialized in the class.


```


Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
interface Foo {}
class Bar implements Foo {}

namespace Foo2 {}
namespace Foo2 {}

enum Foo3 {}
namespace Foo3 {}

namespace Foo4 {}
function Foo4() {}

const Foo5 = class {};

interface Foo6 {}
function f6(){
class Foo6 {};
}

class Foo7 {}
function f7(){
interface Foo7 {}
}

interface Foo8 {}
declare class Foo8 {}

export interface Foo9 {}
export declare class Foo9 {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
expression: valid.ts
---
# Input
```js
interface Foo {}
class Bar implements Foo {}

namespace Foo2 {}
namespace Foo2 {}

enum Foo3 {}
namespace Foo3 {}

namespace Foo4 {}
function Foo4() {}

const Foo5 = class {};

interface Foo6 {}
function f6(){
class Foo6 {};
}

class Foo7 {}
function f7(){
interface Foo7 {}
}

interface Foo8 {}
declare class Foo8 {}

export interface Foo9 {}
export declare class Foo9 {}

```


Loading