Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add option to skip arrow function for explicit function return … #4233

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
36 changes: 36 additions & 0 deletions crates/biome_js_analyze/src/lint/nursery/use_explicit_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,10 @@ impl Rule for UseExplicitType {
return None;
}

if is_arrow_func(func) && is_function_used_in_object(func) {
return None;
}

if is_higher_order_function(func) {
return None;
}
Expand Down Expand Up @@ -362,6 +366,38 @@ fn is_function_used_in_argument_or_array(func: &AnyJsFunction) -> bool {
)
}

/// Check if the function is used in some object
///
/// # Examples
///
/// JS_OBJECT:
///
/// interface Behavior {
/// attribute: string;
/// func: () => string;
/// arrowFunc: () => string;
/// }
Comment on lines +375 to +379
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example seems not related to what we check. We could remove it.

///
/// function getObjectWithFunction(): Behavior {
/// return {
/// attribute: 'value',
/// func: function myFunc(): string { return "value" },
/// arrowFunc: () => {},
/// }
/// }
Comment on lines +381 to +387
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code example should be in code block.

Suggested change
/// function getObjectWithFunction(): Behavior {
/// return {
/// attribute: 'value',
/// func: function myFunc(): string { return "value" },
/// arrowFunc: () => {},
/// }
/// }
/// ```ts
/// function getObjectWithFunction(): Behavior {
/// return {
/// attribute: 'value',
/// func: function myFunc(): string { return "value" },
/// arrowFunc: () => {},
/// }
/// }
/// ```

///
fn is_function_used_in_object(func: &AnyJsFunction) -> bool {
matches!(
func.syntax().parent().kind(),
Some(JsSyntaxKind::JS_PROPERTY_OBJECT_MEMBER)
)
Comment on lines +390 to +393
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not enough because this will accept cases we want to reject.
For example, the following case should be rejected:

const x = {
  prop: () => {}
}

In contrast, the following could be accepted:

const X: Type  = {
  prop: () => {}
};
f({  prop: () => {} })

}

/// Check if a function is an arrow function
fn is_arrow_func(func: &AnyJsFunction) -> bool {
func.as_js_arrow_function_expression().is_some()
}
Comment on lines +396 to +399
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is to permissive. This accepts too much code.


/// Checks if a function is an IIFE (Immediately Invoked Function Expressions)
///
/// # Examples
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,20 @@ function fn() {
};
}

const x = { prop: () => {} }
const x = { bar: { prop: () => {} } }
Comment on lines -94 to -95
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These examples should be still invalid.

const x = { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} }
const x = { bar: { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} } }


// Returning object from function
interface Behavior {
attribute: string;
namedFunc: () => string;
arrowFunc: () => string;
}

function getObjectWithFunction(): Behavior {
return {
namedFunc: function myFunc() { return "value" },
arrowFunc: () => {},
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: invalid.ts
---
# Input
Expand Down Expand Up @@ -97,8 +98,24 @@ function fn() {
};
}

const x = { prop: () => {} }
const x = { bar: { prop: () => {} } }
const x = { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} }
const x = { bar: { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} } }


// Returning object from function
interface Behavior {
attribute: string;
namedFunc: () => string;
arrowFunc: () => string;
}

function getObjectWithFunction(): Behavior {
return {
namedFunc: function myFunc() { return "value" },
arrowFunc: () => {},
}
}

```

# Diagnostics
Expand Down Expand Up @@ -532,15 +549,69 @@ invalid.ts:85:2 lint/nursery/useExplicitType ━━━━━━━━━━━
```

```
invalid.ts:94:19 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:94:29 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Missing return type on function.

92 │ }
93 │
> 94 │ const x = { prop: () => {} }
│ ^^^^^^^^^
95 │ const x = { bar: { prop: () => {} } }
> 94 │ const x = { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} }
│ ^^^^^^^^^^^^^^^
95 │ const x = { bar: { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} } }
96 │

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:94:81 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Missing return type on function.

92 │ }
93 │
> 94 │ const x = { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} }
│ ^^^^^^^^^
95 │ const x = { bar: { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} } }
96 │

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:95:36 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Missing return type on function.

94 │ const x = { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} }
> 95 │ const x = { bar: { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} } }
│ ^^^^^^^^^^^^^^^
96 │

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:95:79 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Missing return type on function.

94 │ const x = { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} }
> 95 │ const x = { bar: { namedFunctions: function alpha () => {}, unNamedFunctions: function () => {} } }
│ ^^^^^^^^^^^^
96 │

i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.

Expand All @@ -550,13 +621,16 @@ invalid.ts:94:19 lint/nursery/useExplicitType ━━━━━━━━━━━
```

```
invalid.ts:95:26 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
invalid.ts:107:16 lint/nursery/useExplicitType ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

! Missing return type on function.

94 │ const x = { prop: () => {} }
> 95 │ const x = { bar: { prop: () => {} } }
│ ^^^^^^^^^
105 │ function getObjectWithFunction(): Behavior {
106 │ return {
> 107 │ namedFunc: function myFunc() { return "value" },
│ ^^^^^^^^^^^^^^^
108 │ arrowFunc: () => {},
109 │ }

i Declaring the return type makes the code self-documenting and can speed up TypeScript type checking.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,4 +102,17 @@ class Accumulator {
this.count += fn();
}
}
new Accumulator().accumulate(() => 1);
new Accumulator().accumulate(() => 1);

// Returning object from function
interface Behavior {
namedFunc: () => string;
arrowFunc: () => string;
}

function getObjectWithFunction(): Behavior {
return {
namedFunc: function myFunc(): string { return "value" },
arrowFunc: () => {},
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 86
expression: valid.ts
---
# Input
Expand Down Expand Up @@ -109,4 +110,18 @@ class Accumulator {
}
}
new Accumulator().accumulate(() => 1);

// Returning object from function
interface Behavior {
namedFunc: () => string;
arrowFunc: () => string;
}

function getObjectWithFunction(): Behavior {
return {
namedFunc: function myFunc(): string { return "value" },
arrowFunc: () => {},
}
}

```