This repository was archived by the owner on Aug 31, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 657
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(rome_js_analyze):
noVoidTypeReturn
(#3806)
- Loading branch information
Showing
13 changed files
with
656 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
151 changes: 151 additions & 0 deletions
151
crates/rome_js_analyze/src/analyzers/nursery/no_void_type_return.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,151 @@ | ||
use rome_analyze::context::RuleContext; | ||
use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic}; | ||
use rome_console::markup; | ||
use rome_js_syntax::{ | ||
JsArrowFunctionExpression, JsFunctionDeclaration, JsFunctionExportDefaultDeclaration, | ||
JsFunctionExpression, JsGetterClassMember, JsGetterObjectMember, JsMethodClassMember, | ||
JsMethodObjectMember, JsReturnStatement, TsAnyReturnType, | ||
}; | ||
use rome_rowan::{declare_node_union, AstNode}; | ||
|
||
use crate::control_flow::JsAnyControlFlowRoot; | ||
|
||
declare_rule! { | ||
/// Disallow returning a value from a function with the return type 'void' | ||
/// | ||
/// 'void' signals the absence of value. The returned value is likely to be ignored by the caller. | ||
/// Thus, returning a value when the return type of function is 'void', is undoubtedly an error. | ||
/// | ||
/// Only returning without a value is allowed, as it’s a control flow statement. | ||
/// | ||
/// ## Examples | ||
/// | ||
/// ### Invalid | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// class A { | ||
/// f(): void { | ||
/// return undefined; | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// const a = { | ||
/// f(): void { | ||
/// return undefined; | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// function f(): void { | ||
/// return undefined; | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// export default function(): void { | ||
/// return undefined; | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// const g = (): void => { | ||
/// return undefined; | ||
/// }; | ||
/// ``` | ||
/// | ||
/// ```ts,expect_diagnostic | ||
/// const h = function(): void { | ||
/// return undefined; | ||
/// }; | ||
/// ``` | ||
/// | ||
/// ### Valid | ||
/// | ||
/// ```js | ||
/// class A { | ||
/// f() { | ||
/// return undefined; | ||
/// } | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts | ||
/// class B { | ||
/// f(): void {} | ||
/// } | ||
/// ``` | ||
/// | ||
/// ```ts | ||
/// function f(): void { | ||
/// return; | ||
/// } | ||
/// ``` | ||
/// | ||
/// ``` | ||
pub(crate) NoVoidTypeReturn { | ||
version: "11.0.0", | ||
name: "noVoidTypeReturn", | ||
recommended: false, | ||
} | ||
} | ||
|
||
declare_node_union! { | ||
pub(crate) JsFunctionMethod = JsArrowFunctionExpression | JsFunctionDeclaration | JsFunctionExportDefaultDeclaration | JsFunctionExpression | JsGetterClassMember | JsGetterObjectMember | JsMethodClassMember | JsMethodObjectMember | ||
} | ||
|
||
pub(crate) fn return_type(func: &JsFunctionMethod) -> Option<TsAnyReturnType> { | ||
match func { | ||
JsFunctionMethod::JsArrowFunctionExpression(func) => { | ||
func.return_type_annotation()?.ty().ok() | ||
} | ||
JsFunctionMethod::JsFunctionDeclaration(func) => func.return_type_annotation()?.ty().ok(), | ||
JsFunctionMethod::JsFunctionExportDefaultDeclaration(func) => { | ||
func.return_type_annotation()?.ty().ok() | ||
} | ||
JsFunctionMethod::JsFunctionExpression(func) => func.return_type_annotation()?.ty().ok(), | ||
JsFunctionMethod::JsGetterClassMember(func) => { | ||
Some(TsAnyReturnType::TsType(func.return_type()?.ty().ok()?)) | ||
} | ||
JsFunctionMethod::JsGetterObjectMember(func) => { | ||
Some(TsAnyReturnType::TsType(func.return_type()?.ty().ok()?)) | ||
} | ||
JsFunctionMethod::JsMethodClassMember(func) => func.return_type_annotation()?.ty().ok(), | ||
JsFunctionMethod::JsMethodObjectMember(func) => func.return_type_annotation()?.ty().ok(), | ||
} | ||
} | ||
|
||
impl Rule for NoVoidTypeReturn { | ||
type Query = Ast<JsReturnStatement>; | ||
type State = JsFunctionMethod; | ||
type Signals = Option<Self::State>; | ||
type Options = (); | ||
|
||
fn run(ctx: &RuleContext<Self>) -> Self::Signals { | ||
let ret = ctx.query(); | ||
// Do not take arg-less returns into account | ||
let _arg = ret.argument()?; | ||
let func = ret | ||
.syntax() | ||
.ancestors() | ||
.find(|x| JsAnyControlFlowRoot::can_cast(x.kind())) | ||
.and_then(JsFunctionMethod::cast)?; | ||
let ret_type = return_type(&func)?; | ||
ret_type.as_ts_type()?.as_ts_void_type().and(Some(func)) | ||
} | ||
|
||
fn diagnostic(ctx: &RuleContext<Self>, func: &Self::State) -> Option<RuleDiagnostic> { | ||
let ret = ctx.query(); | ||
Some(RuleDiagnostic::new( | ||
rule_category!(), | ||
ret.range(), | ||
markup! { | ||
"The function should not "<Emphasis>"return"</Emphasis>" a value because its return type is "<Emphasis>"void"</Emphasis>"." | ||
}, | ||
).detail(func.range(), "The function is here:").note( | ||
"'void' signals the absence of value. The returned value is likely to be ignored by the caller." | ||
)) | ||
} | ||
} |
23 changes: 23 additions & 0 deletions
23
crates/rome_js_analyze/tests/specs/nursery/noVoidTypeReturn/invalid.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
class A { | ||
f(): void { | ||
return undefined; | ||
} | ||
} | ||
|
||
const a = { | ||
f(): void { | ||
return undefined; | ||
} | ||
} | ||
|
||
function f(): void { | ||
return undefined; | ||
} | ||
|
||
export default function(): void { | ||
return undefined; | ||
} | ||
|
||
const g = (): void => { | ||
return undefined; | ||
}; |
174 changes: 174 additions & 0 deletions
174
crates/rome_js_analyze/tests/specs/nursery/noVoidTypeReturn/invalid.ts.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
--- | ||
source: crates/rome_js_analyze/tests/spec_tests.rs | ||
assertion_line: 73 | ||
expression: invalid.ts | ||
--- | ||
# Input | ||
```js | ||
class A { | ||
f(): void { | ||
return undefined; | ||
} | ||
} | ||
|
||
const a = { | ||
f(): void { | ||
return undefined; | ||
} | ||
} | ||
|
||
function f(): void { | ||
return undefined; | ||
} | ||
|
||
export default function(): void { | ||
return undefined; | ||
} | ||
|
||
const g = (): void => { | ||
return undefined; | ||
}; | ||
|
||
``` | ||
|
||
# Diagnostics | ||
``` | ||
invalid.ts:3:3 lint/nursery/noVoidTypeReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
! The function should not return a value because its return type is void. | ||
1 │ class A { | ||
2 │ f(): void { | ||
> 3 │ return undefined; | ||
│ ^^^^^^^^^^^^^^^^^ | ||
4 │ } | ||
5 │ } | ||
i The function is here: | ||
1 │ class A { | ||
> 2 │ f(): void { | ||
│ ^^^^^^^^^^^ | ||
> 3 │ return undefined; | ||
> 4 │ } | ||
│ ^ | ||
5 │ } | ||
6 │ | ||
i 'void' signals the absence of value. The returned value is likely to be ignored by the caller. | ||
``` | ||
|
||
``` | ||
invalid.ts:9:3 lint/nursery/noVoidTypeReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
! The function should not return a value because its return type is void. | ||
7 │ const a = { | ||
8 │ f(): void { | ||
> 9 │ return undefined; | ||
│ ^^^^^^^^^^^^^^^^^ | ||
10 │ } | ||
11 │ } | ||
i The function is here: | ||
7 │ const a = { | ||
> 8 │ f(): void { | ||
│ ^^^^^^^^^^^ | ||
> 9 │ return undefined; | ||
> 10 │ } | ||
│ ^ | ||
11 │ } | ||
12 │ | ||
i 'void' signals the absence of value. The returned value is likely to be ignored by the caller. | ||
``` | ||
|
||
``` | ||
invalid.ts:14:2 lint/nursery/noVoidTypeReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
! The function should not return a value because its return type is void. | ||
13 │ function f(): void { | ||
> 14 │ return undefined; | ||
│ ^^^^^^^^^^^^^^^^^ | ||
15 │ } | ||
16 │ | ||
i The function is here: | ||
11 │ } | ||
12 │ | ||
> 13 │ function f(): void { | ||
│ ^^^^^^^^^^^^^^^^^^^^ | ||
> 14 │ return undefined; | ||
> 15 │ } | ||
│ ^ | ||
16 │ | ||
17 │ export default function(): void { | ||
i 'void' signals the absence of value. The returned value is likely to be ignored by the caller. | ||
``` | ||
|
||
``` | ||
invalid.ts:18:2 lint/nursery/noVoidTypeReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
! The function should not return a value because its return type is void. | ||
17 │ export default function(): void { | ||
> 18 │ return undefined; | ||
│ ^^^^^^^^^^^^^^^^^ | ||
19 │ } | ||
20 │ | ||
i The function is here: | ||
15 │ } | ||
16 │ | ||
> 17 │ export default function(): void { | ||
│ ^^^^^^^^^^^^^^^^^^ | ||
> 18 │ return undefined; | ||
> 19 │ } | ||
│ ^ | ||
20 │ | ||
21 │ const g = (): void => { | ||
i 'void' signals the absence of value. The returned value is likely to be ignored by the caller. | ||
``` | ||
|
||
``` | ||
invalid.ts:22:2 lint/nursery/noVoidTypeReturn ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ | ||
! The function should not return a value because its return type is void. | ||
21 │ const g = (): void => { | ||
> 22 │ return undefined; | ||
│ ^^^^^^^^^^^^^^^^^ | ||
23 │ }; | ||
24 │ | ||
i The function is here: | ||
19 │ } | ||
20 │ | ||
> 21 │ const g = (): void => { | ||
│ ^^^^^^^^^^^^^ | ||
> 22 │ return undefined; | ||
> 23 │ }; | ||
│ ^ | ||
24 │ | ||
i 'void' signals the absence of value. The returned value is likely to be ignored by the caller. | ||
``` | ||
|
||
|
15 changes: 15 additions & 0 deletions
15
crates/rome_js_analyze/tests/specs/nursery/noVoidTypeReturn/valid.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
class A { | ||
f() { | ||
return undefined; | ||
} | ||
} | ||
|
||
class B { | ||
f(): void { | ||
return; | ||
} | ||
} | ||
|
||
function f(): void { | ||
return; | ||
} |
Oops, something went wrong.