Skip to content

Commit

Permalink
Refactor and rename check_statements_for_function_return to is_first_…
Browse files Browse the repository at this point in the history 
…statement_function_return
  • Loading branch information
@kaykdm
kaykdm committed Sep 30, 2024
1 parent dd41388 commit ed3ec30
Show file tree
Hide file tree
Showing 5 changed files with 152 additions and 83 deletions.
78 changes: 59 additions & 19 deletions crates/biome_js_analyze/src/lint/nursery/use_explicit_function_return_type.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ declare_lint_rule! {
/// const func = (value: number) => ({ type: 'X', value }) as any;
/// ```
///
/// The following pattern is considered incorrect code for a higher-order function, as the returned function does not specify a return type:
///
/// ```ts,expect_diagnostic
/// const arrowFn = () => () => {};
/// ```
Expand All @@ -79,6 +81,28 @@ declare_lint_rule! {
/// }
/// ```
///
/// The following pattern is considered incorrect code for a higher-order function because the function body contains multiple statements. We only check whether the first statement is a function return.
///
/// ```ts,expect_diagnostic
/// // A function has multiple statements in the body
/// function f() {
/// if (x) {
/// return 0;
/// }
/// return (): void => {}
/// }
/// ```
///
/// ```ts,expect_diagnostic
/// // A function has multiple statements in the body
/// function f() {
/// let str = "test";
/// return (): string => {
/// str;
/// }
/// }
/// ```
///
/// ### Valid
/// ```ts
/// // No return value should be expected (void)
Expand Down Expand Up @@ -108,10 +132,14 @@ declare_lint_rule! {
/// }
/// ```
///
/// The following patterns are considered correct code for a function immediately returning a value with `as const`:
///
/// ```ts
/// const func = (value: number) => ({ foo: 'bar', value }) as const;
/// ```
///
/// The following patterns are considered correct code for a function allowed within specific expression contexts, such as an IIFE, a function passed as an argument, or a function inside an array:
///
/// ```ts
/// // Callbacks without return types
/// setTimeout(function() { console.log("Hello!"); }, 1000);
Expand All @@ -122,13 +150,23 @@ declare_lint_rule! {
/// ```
///
/// ```ts
/// // a function inside an array
/// [function () {}, () => {}];
/// ```
///
/// The following pattern is considered correct code for a higher-order function, where the returned function explicitly specifies a return type and the function body contains only one statement:
///
/// ```ts
/// // the outer function returns an inner function that has a `void` return type
/// const arrowFn = () => (): void => {};
/// ```
///
/// ```ts
/// // the outer function returns an inner function that has a `void` return type
/// const arrowFn = () => {
/// return (): void => { };
/// }
/// ```
///
pub UseExplicitFunctionReturnType {
version: "next",
Expand Down Expand Up @@ -294,9 +332,6 @@ fn is_function_used_in_argument_or_expression_list(func: &AnyJsFunction) -> bool
/// Checks whether the given function is a higher-order function, i.e., a function
/// that returns another function either directly in its body or as an expression.
///
/// A higher-order function is one that returns either a regular function or an arrow
/// function from within its body.
///
/// # Arguments
///
/// * `func` - A reference to an `AnyJsFunction` that represents the JavaScript function to inspect.
Expand All @@ -309,8 +344,8 @@ fn is_function_used_in_argument_or_expression_list(func: &AnyJsFunction) -> bool
/// # Note
///
/// This function currently **does not support** detecting a return of a function
/// inside other statements like `if` statements or `switch` statements. It only detects
/// direct returns of functions or function returns in a straightforward function body.
/// inside other statements, such as `if` statements or `switch` statements. It only checks
/// whether the first statement is a return of a function in a straightforward function body.
fn is_higher_order_function(func: &AnyJsFunction) -> bool {
match func.body().ok() {
Some(AnyJsFunctionBody::AnyJsExpression(expr)) => {
Expand All @@ -321,13 +356,13 @@ fn is_higher_order_function(func: &AnyJsFunction) -> bool {
)
}
Some(AnyJsFunctionBody::JsFunctionBody(func_body)) => {
check_statements_for_function_return(func_body.statements())
is_first_statement_function_return(func_body.statements())
}
_ => false,
}
}

/// Checks whether the given list of JavaScript statements contains a return statement
/// Checks whether the first statement in the given list of JavaScript statements is a return statement
/// that returns a function expression (either a regular function or an arrow function).
///
/// # Arguments
Expand All @@ -338,17 +373,22 @@ fn is_higher_order_function(func: &AnyJsFunction) -> bool {
///
/// * `true` if the list contains a return statement with a function expression as its argument.
/// * `false` if no such return statement is found or if the list is empty.
fn check_statements_for_function_return(statements: JsStatementList) -> bool {
statements.into_iter().any(|statement| {
if let AnyJsStatement::JsReturnStatement(return_stmt) = statement {
if let Some(args) = return_stmt.argument() {
return matches!(
args,
AnyJsExpression::JsFunctionExpression(_)
| AnyJsExpression::JsArrowFunctionExpression(_)
);
fn is_first_statement_function_return(statements: JsStatementList) -> bool {
statements
.into_iter()
.next()
.and_then(|stmt| {
if let AnyJsStatement::JsReturnStatement(return_stmt) = stmt {
return_stmt.argument()
} else {
None
}
}
false
})
})
.map_or(false, |args| {
matches!(
args,
AnyJsExpression::JsFunctionExpression(_)
| AnyJsExpression::JsArrowFunctionExpression(_)
)
})
}
21 changes: 15 additions & 6 deletions crates/biome_js_analyze/tests/specs/nursery/useExplicitFunctionReturnType/invalid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,12 @@ export default function () {}
// check higher order functions
const arrowFn = () => () => {};
const arrowFn = () => function() {}
function fn() {
let str = "hey";
return function () {
return str;
};
}
const arrowFn = () => {
return () => { };
}

// does not support detecting a return of a function inside other statements like if, switch, etc.
// we check only the first statment
const arrowFn = (a: number) => {
if (a === 1) {
return (): void => { };
Expand All @@ -80,4 +75,18 @@ const arrowFn = (a: number) => {
return (): void => { };
}
}
}

function f() {
if (x) {
return 0;
}
return (): void => {}
}

function fn() {
let str = "hey";
return function (): string {
return str;
};
}
124 changes: 78 additions & 46 deletions crates/biome_js_analyze/tests/specs/nursery/useExplicitFunctionReturnType/invalid.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,12 @@ export default function () {}
// check higher order functions
const arrowFn = () => () => {};
const arrowFn = () => function() {}
function fn() {
let str = "hey";
return function () {
return str;
};
}
const arrowFn = () => {
return () => { };
}

// does not support detecting a return of a function inside other statements like if, switch, etc.
// we check only the first statment
const arrowFn = (a: number) => {
if (a === 1) {
return (): void => { };
Expand All @@ -87,6 +82,20 @@ const arrowFn = (a: number) => {
}
}
}

function f() {
if (x) {
return 0;
}
return (): void => {}
}

function fn() {
let str = "hey";
return function (): string {
return str;
};
}
```

# Diagnostics
Expand Down Expand Up @@ -380,7 +389,7 @@ invalid.ts:49:23 lint/nursery/useExplicitFunctionReturnType ━━━━━━
> 49 │ const arrowFn = () => () => {};
│ ^^^^^^^^
50 │ const arrowFn = () => function() {}
51 │ function fn() {
51 │ const arrowFn = () => {
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
Expand All @@ -398,8 +407,8 @@ invalid.ts:50:23 lint/nursery/useExplicitFunctionReturnType ━━━━━━
49 │ const arrowFn = () => () => {};
> 50 │ const arrowFn = () => function() {}
│ ^^^^^^^^^^^^^
51 │ function fn() {
52 │ let str = "hey";
51 │ const arrowFn = () => {
52 │ return () => { };
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
Expand All @@ -409,19 +418,16 @@ invalid.ts:50:23 lint/nursery/useExplicitFunctionReturnType ━━━━━━
```

```
invalid.ts:53:10 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:52:10 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Missing return type on function.
51 │ function fn() {
52 │ let str = "hey";
> 53 │ return function () {
│ ^^^^^^^^^^^^^
> 54 │ return str;
> 55 │ };
│ ^
56 │ }
57 │ const arrowFn = () => {
50 │ const arrowFn = () => function() {}
51 │ const arrowFn = () => {
> 52 │ return () => { };
│ ^^^^^^^^^
53 │ }
54 │
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
Expand All @@ -431,16 +437,21 @@ invalid.ts:53:10 lint/nursery/useExplicitFunctionReturnType ━━━━━━
```

```
invalid.ts:58:10 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:57:17 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Missing return type on function.
56 │ }
57 │ const arrowFn = () => {
> 58 │ return () => { };
│ ^^^^^^^^^
59 │ }
60 │
55 │ // does not support detecting a return of a function inside other statements like if, switch, etc.
56 │ // we check only the first statment·
> 57 │ const arrowFn = (a: number) => {
│ ^^^^^^^^^^^^^^^^
> 58 │ if (a === 1) {
...
> 64 │ }
> 65 │ }
│ ^
66 │ const arrowFn = (a: number) => {
67 │ switch (a) {
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
Expand All @@ -450,20 +461,21 @@ invalid.ts:58:10 lint/nursery/useExplicitFunctionReturnType ━━━━━━
```

```
invalid.ts:62:17 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:66:17 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Missing return type on function.
61 │ // does not support detecting a return of a function inside other statements like if, switch, etc.
> 62 │ const arrowFn = (a: number) => {
64 │ }
65 │ }
> 66 │ const arrowFn = (a: number) => {
│ ^^^^^^^^^^^^^^^^
> 63 │ if (a === 1) {
> 67 │ switch (a) {
...
> 69 │ }
> 70 │ }
> 77 │ }
> 78 │ }
│ ^
71 │ const arrowFn = (a: number) => {
72 │ switch (a) {
79 │
80 │ function f() {
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
Expand All @@ -473,21 +485,41 @@ invalid.ts:62:17 lint/nursery/useExplicitFunctionReturnType ━━━━━━
```

```
invalid.ts:71:17 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:78:2 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Missing return type on function.
69 │ }
70 │ }
> 71 │ const arrowFn = (a: number) => {
│ ^^^^^^^^^^^^^^^^
> 72 │ switch (a) {
...
> 80 │ return (): void => { };
> 81 │ }
> 82 │ }
> 83 │ }
│ ^
76 │ }
77 │ }
> 78 │ }
│
> 79 │
> 80 │ function f() {
│ ^^^^^^^^^^
81 │ if (x) {
82 │ return 0;
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
i Add a return type annotation.
```

```
invalid.ts:85:2 lint/nursery/useExplicitFunctionReturnType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Missing return type on function.
83 │ }
84 │ return (): void => {}
> 85 │ }
│
> 86 │
> 87 │ function fn() {
│ ^^^^^^^^^^^
88 │ let str = "hey";
89 │ return function (): string {
i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.
Expand Down
Loading

0 comments on commit ed3ec30

Please sign in to comment.