Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(lint/noRedeclare): handle function and call overloads
Browse files Browse the repository at this point in the history
Conaclos committed Apr 26, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent b3ed181 commit 73aad56
Showing 10 changed files with 160 additions and 15 deletions.
38 changes: 38 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -68,6 +68,44 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

#### Bug fixes

- Fix [noRedeclare](https://biomejs.dev/linter/rules/no-redeclare/) that missed redeclarations in functions ([#2394](https://github.com/biomejs/biome/issues/2394)).

The rule was not able to detected redeclaraions of a parameter or a type parameter in the function body.
The following two redeclarations are now reported:

```ts
function f<T>(a) {
type T = number; // redeclaration
const a = 0; // redeclaration
}
```

Contributed by @Conaclos

- Fix [noRedeclare](https://biomejs.dev/linter/rules/no-redeclare/) that wrongly reported overloads in object types ([#2608](https://github.com/biomejs/biome/issues/2608)).

The rule no longer report redeclarations in the following code:

```ts
type Overloads = {
({ a }: { a: number }): number,
({ a }: { a: string }): string,
};
```

Contributed by @Conaclos

- Fix [noRedeclare](https://biomejs.dev/linter/rules/no-redeclare/) that didn't merge default function export declarations and types ([#2372](https://github.com/biomejs/biome/issues/2372)).

The following code is no longer reported as a redeclaration:

```ts
interface Foo {}
export default function Foo() {}
```

Contributed by @Conaclos

- [useConst](https://biomejs.dev/linter/rules/use-const/) now ignores a variable that is read before its assignment.

Previously, the rule reported the following example:
33 changes: 32 additions & 1 deletion crates/biome_js_analyze/src/lint/suspicious/no_redeclare.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,7 @@ use biome_analyze::{declare_rule, RuleSource};
use biome_console::markup;
use biome_js_semantic::Scope;
use biome_js_syntax::binding_ext::AnyJsBindingDeclaration;
use biome_js_syntax::TextRange;
use biome_js_syntax::{JsSyntaxKind, TextRange};
use biome_rowan::AstNode;
use rustc_hash::FxHashMap;

@@ -115,6 +115,37 @@ impl Rule for NoRedeclare {

fn check_redeclarations_in_single_scope(scope: &Scope, redeclarations: &mut Vec<Redeclaration>) {
let mut declarations = FxHashMap::<String, (TextRange, AnyJsBindingDeclaration)>::default();
if scope.syntax().kind() == JsSyntaxKind::JS_FUNCTION_BODY {
// Handle cases where a variable/type redeclares a parameter or type parameter.
// For example:
//
// ```js
// function f<T>(a) { type T = number; const a = 0; }
// ```
//
// I previously tried to remove the JsFunctionBody scope
// to directly add declarations of the function body in the function scope.
// Unfortunately, this is not a good idea because variables and types outside the function
// can be referenced in the parameters and type parameters of the function,
// then shadowed in the function body.
// Thus, using a distinct scope for the function body and the function makes sense.
// For example:
//
// ```js
// type U = string;
// function g() {}
// function f<T = U>(h = g) { type U = number; function g() {}; }
// ```
if let Some(function_scope) = scope.parent() {
for binding in function_scope.bindings() {
let id_binding = binding.tree();
if let Some(decl) = id_binding.declaration() {
let name = id_binding.text();
declarations.insert(name, (id_binding.syntax().text_trimmed_range(), decl));
}
}
}
}
for binding in scope.bindings() {
let id_binding = binding.tree();

Original file line number Diff line number Diff line change
@@ -532,4 +532,19 @@ invalid.jsonc:1:18 lint/suspicious/noRedeclare ━━━━━━━━━━━
function f(x) { var x = 5; }
```

# Diagnostics
```
invalid.jsonc:1:21 lint/suspicious/noRedeclare ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Shouldn't redeclare 'x'. Consider to delete it or rename it.
> 1 │ function f(x) { var x = 5; }
│ ^
i 'x' is defined here:
> 1 │ function f(x) { var x = 5; }
│ ^
```
Original file line number Diff line number Diff line change
@@ -6,3 +6,7 @@ class C {
}

function f<T, T>() {}

function g<T>() {
type T = number;
}
Original file line number Diff line number Diff line change
@@ -13,6 +13,9 @@ class C {

function f<T, T>() {}

function g<T>() {
type T = number;
}
```

# Diagnostics
@@ -45,21 +48,43 @@ invalid.ts:8:15 lint/suspicious/noRedeclare ━━━━━━━━━━━━
! Shouldn't redeclare 'T'. Consider to delete it or rename it.
6 │ }
7 │
> 8 │ function f<T, T>() {}
│ ^
9 │
6 │ }
7 │
> 8 │ function f<T, T>() {}
│ ^
9 │
10 │ function g<T>() {
i 'T' is defined here:
6 │ }
7 │
> 8 │ function f<T, T>() {}
│ ^
9 │
6 │ }
7 │
> 8 │ function f<T, T>() {}
│ ^
9 │
10 │ function g<T>() {
```

```
invalid.ts:11:10 lint/suspicious/noRedeclare ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! Shouldn't redeclare 'T'. Consider to delete it or rename it.
10 │ function g<T>() {
> 11 │ type T = number;
│ ^
12 │ }
i 'T' is defined here:
8 │ function f<T, T>() {}
9 │
> 10 │ function g<T>() {
│ ^
11 │ type T = number;
12 │ }
```
Original file line number Diff line number Diff line change
@@ -130,3 +130,13 @@ function g(A, { B }) {
export function h<a>(a: a) {
return a;
}

// Issue https://github.com/biomejs/biome/issues/2608
type Overloads = {
(e: "change-kind", { kind }: { kind: "a" }): void;
(e: "change-kind", { kind, extra }: { kind: "b"; extra: number }): void;
};

// Issue https://github.com/biomejs/biome/issues/2372
type F = number
export default function F() {}
Original file line number Diff line number Diff line change
@@ -137,6 +137,13 @@ export function h<a>(a: a) {
return a;
}

```

// Issue https://github.com/biomejs/biome/issues/2608
type Overloads = {
(e: "change-kind", { kind }: { kind: "a" }): void;
(e: "change-kind", { kind, extra }: { kind: "b"; extra: number }): void;
};

// Issue https://github.com/biomejs/biome/issues/2372
type F = number
export default function F() {}
```
3 changes: 3 additions & 0 deletions crates/biome_js_semantic/src/events.rs
Original file line number Diff line number Diff line change
@@ -289,6 +289,7 @@ impl SemanticEventExtractor {
#[inline]
pub fn enter(&mut self, node: &JsSyntaxNode) {
// If you push a scope for a given node type, don't forget to also update `Self::leave`.
// You should also edit [SemanticModelBuilder::push_node].
match node.kind() {
JS_IDENTIFIER_BINDING | TS_IDENTIFIER_BINDING | TS_TYPE_PARAMETER_NAME => {
self.enter_identifier_binding(&AnyJsIdentifierBinding::unwrap_cast(node.clone()));
@@ -334,6 +335,7 @@ impl SemanticEventExtractor {
| TS_TYPE_ALIAS_DECLARATION
| TS_DECLARE_FUNCTION_DECLARATION
| TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION
| TS_CALL_SIGNATURE_TYPE_MEMBER
| TS_METHOD_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_TYPE_MEMBER
| TS_INDEX_SIGNATURE_CLASS_MEMBER
@@ -634,6 +636,7 @@ impl SemanticEventExtractor {
| JS_STATIC_INITIALIZATION_BLOCK_CLASS_MEMBER
| TS_DECLARE_FUNCTION_DECLARATION
| TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION
| TS_CALL_SIGNATURE_TYPE_MEMBER
| TS_METHOD_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_TYPE_MEMBER
| TS_INDEX_SIGNATURE_CLASS_MEMBER
15 changes: 13 additions & 2 deletions crates/biome_js_semantic/src/semantic_model/builder.rs
Original file line number Diff line number Diff line change
@@ -76,16 +76,27 @@ impl SemanticModelBuilder {
| JS_CLASS_EXPORT_DEFAULT_DECLARATION
| JS_CLASS_EXPRESSION
| JS_FUNCTION_BODY
| JS_STATIC_INITIALIZATION_BLOCK_CLASS_MEMBER
| TS_MODULE_DECLARATION
| TS_EXTERNAL_MODULE_DECLARATION
| TS_INTERFACE_DECLARATION
| TS_ENUM_DECLARATION
| TS_TYPE_ALIAS_DECLARATION
| TS_FUNCTION_TYPE
| TS_DECLARE_FUNCTION_DECLARATION
| TS_DECLARE_FUNCTION_EXPORT_DEFAULT_DECLARATION
| TS_CALL_SIGNATURE_TYPE_MEMBER
| TS_METHOD_SIGNATURE_CLASS_MEMBER
| TS_METHOD_SIGNATURE_TYPE_MEMBER
| TS_INDEX_SIGNATURE_CLASS_MEMBER
| TS_INDEX_SIGNATURE_TYPE_MEMBER
| JS_BLOCK_STATEMENT
| JS_FOR_STATEMENT
| JS_FOR_OF_STATEMENT
| JS_FOR_IN_STATEMENT
| JS_SWITCH_STATEMENT
| JS_CATCH_CLAUSE => {
| JS_CATCH_CLAUSE
| TS_FUNCTION_TYPE
| TS_MAPPED_TYPE => {
self.node_by_range.insert(node.text_range(), node.clone());
}
_ => {}
1 change: 1 addition & 0 deletions crates/biome_js_syntax/src/binding_ext.rs
Original file line number Diff line number Diff line change
@@ -137,6 +137,7 @@ impl AnyJsBindingDeclaration {
| AnyJsBindingDeclaration::TsModuleDeclaration(_)
| AnyJsBindingDeclaration::TsTypeParameter(_),
AnyJsBindingDeclaration::JsFunctionDeclaration(_)
| AnyJsBindingDeclaration::JsFunctionExportDefaultDeclaration(_)
| AnyJsBindingDeclaration::JsArrayBindingPatternElement(_)
| AnyJsBindingDeclaration::JsArrayBindingPatternRestElement(_)
| AnyJsBindingDeclaration::JsObjectBindingPatternProperty(_)

0 comments on commit 73aad56

Please sign in to comment.