Skip to content
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
42 changes: 33 additions & 9 deletions crates/oxc_linter/src/rules/eslint/no_shadow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,6 @@ impl Rule for NoShadow {
continue;
}

if let Some(shadowed_span) =
Self::function_expression_name_shadow_span(ctx, symbol_id, symbol_name_str)
{
ctx.diagnostic(no_shadow_diagnostic(symbol_span, symbol_name_str, shadowed_span));
continue;
}

if let Some(shadowed_symbol_id) = Self::find_shadowed_symbol_id(ctx, symbol_id) {
if !self.should_ignore_shadowed_symbol(ctx, symbol_id, shadowed_symbol_id) {
let shadowed_span = scoping.symbol_span(shadowed_symbol_id);
Expand All @@ -126,6 +119,13 @@ impl Rule for NoShadow {
continue;
}

if let Some(shadowed_span) =
Self::function_expression_name_shadow_span(ctx, symbol_id, symbol_name_str)
{
ctx.diagnostic(no_shadow_diagnostic(symbol_span, symbol_name_str, shadowed_span));
continue;
}

let symbol_flags = scoping.symbol_flags(symbol_id);
if self.is_builtin_global_shadow(ctx, symbol_id, symbol_name_str, symbol_flags) {
ctx.diagnostic(no_shadow_global_diagnostic(symbol_span, symbol_name_str));
Expand Down Expand Up @@ -485,6 +485,8 @@ impl NoShadow {

let outer_declaration_id = ctx.scoping().symbol_declaration(outer_symbol_id);
let outer_symbol_span = ctx.scoping().symbol_span(outer_symbol_id);
let is_outer_formal_parameter = Self::is_formal_parameter_symbol(ctx, outer_declaration_id);

let Some(initializer) =
Self::find_initializer_expression(ctx, outer_declaration_id, outer_symbol_span)
else {
Expand All @@ -499,8 +501,16 @@ impl NoShadow {
return false;
}

let unwrapped_expression_id = Self::unwrap_expression(ctx, inner_expression_id);
initializer.address() == ctx.nodes().kind(unwrapped_expression_id).address()
if is_outer_formal_parameter {
let unwrapped_expression_id = Self::unwrap_expression(ctx, inner_expression_id);
return initializer.address() == ctx.nodes().kind(unwrapped_expression_id).address();
}

let inner_scope_id = ctx.scoping().symbol_scope_id(inner_symbol_id);
let outer_scope_id = ctx.scoping().symbol_scope_id(outer_symbol_id);
ctx.scoping()
.scope_parent_id(inner_scope_id)
.is_some_and(|parent_scope_id| parent_scope_id == outer_scope_id)
}

fn is_init_pattern_node(
Expand Down Expand Up @@ -642,6 +652,20 @@ impl NoShadow {
None
}

fn is_formal_parameter_symbol(ctx: &LintContext, declaration_id: NodeId) -> bool {
for node_id in
std::iter::once(declaration_id).chain(ctx.nodes().ancestor_ids(declaration_id))
{
match ctx.nodes().kind(node_id) {
AstKind::FormalParameter(_) => return true,
kind if is_initializer_sentinel(kind) => break,
_ => {}
}
}

false
}

fn unwrap_expression(ctx: &LintContext, expression_id: NodeId) -> NodeId {
let mut current_id = expression_id;

Expand Down
47 changes: 31 additions & 16 deletions crates/oxc_linter/src/rules/eslint/no_shadow/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1719,28 +1719,31 @@ fn test_eslint() {
None,
None,
), // { "ecmaVersion": 6 },
(
"let x = false; export const a = wrap(function a() { if (!x) { x = true; a(); } });",
Some(serde_json::json!([{ "hoist": "all" }])),
None,
None,
), // { "ecmaVersion": 6, "sourceType": "module" },
("const a = wrap(function a() {});", None, None, None), // { "ecmaVersion": 6 },
("const a = foo || wrap(function a() {});", None, None, None), // { "ecmaVersion": 6 },
("const { a = wrap(function a() {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
("const { a = foo || wrap(function a() {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
// The following tests are disabled as the behaviour of eslint vs typescript-eslint
// differs. We are following typescript-eslint's behaviour, but these tests are left
// here for clarity.
// (
// "let x = false; export const a = wrap(function a() { if (!x) { x = true; a(); } });",
// Some(serde_json::json!([{ "hoist": "all" }])),
// None,
// None,
// ), // { "ecmaVersion": 6, "sourceType": "module" },
// ("const a = wrap(function a() {});", None, None, None), // { "ecmaVersion": 6 },
// ("const a = foo || wrap(function a() {});", None, None, None), // { "ecmaVersion": 6 },
// ("const { a = wrap(function a() {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
// ("const { a = foo || wrap(function a() {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
("const { a = foo, b = function a() {} } = {}", None, None, None), // { "ecmaVersion": 6 },
("const { A = Foo, B = class A {} } = {}", None, None, None), // { "ecmaVersion": 6 },
("function foo(a = wrap(function a() {})) {}", None, None, None), // { "ecmaVersion": 6 },
("function foo(a = foo || wrap(function a() {})) {}", None, None, None), // { "ecmaVersion": 6 },
("const A = wrap(class A {});", None, None, None), // { "ecmaVersion": 6 },
("const A = foo || wrap(class A {});", None, None, None), // { "ecmaVersion": 6 },
("const { A = wrap(class A {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
("const { A = foo || wrap(class A {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
// ("const A = wrap(class A {});", None, None, None), // { "ecmaVersion": 6 },
// ("const A = foo || wrap(class A {});", None, None, None), // { "ecmaVersion": 6 },
// ("const { A = wrap(class A {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
// ("const { A = foo || wrap(class A {}) } = obj;", None, None, None), // { "ecmaVersion": 6 },
("function foo(A = wrap(class A {})) {}", None, None, None), // { "ecmaVersion": 6 },
("function foo(A = foo || wrap(class A {})) {}", None, None, None), // { "ecmaVersion": 6 },
("var a = function a() {} ? foo : bar", None, None, None),
("var A = class A {} ? foo : bar", None, None, None), // { "ecmaVersion": 6, },
// ("var a = function a() {} ? foo : bar", None, None, None),
// ("var A = class A {} ? foo : bar", None, None, None), // { "ecmaVersion": 6, },
(
"(function Array() {})",
Some(serde_json::json!([{ "builtinGlobals": true }])),
Expand Down Expand Up @@ -2304,6 +2307,18 @@ fn test_typescript_eslint() {
None,
None,
),
(
"
import { memo } from 'react';
const FooBarComponent = memo(function FooBarComponent() {
return <div>Foo</div>;
});
",
None,
None,
Some(PathBuf::from("test.tsx")),
),
(
"
function foo<T extends (id: string, ...args: any[]) => any>(
Expand Down
99 changes: 0 additions & 99 deletions crates/oxc_linter/src/snapshots/eslint_no_shadow.snap
Original file line number Diff line number Diff line change
Expand Up @@ -664,51 +664,6 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Consider renaming 'x' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:29]
1let x = false; export const a = wrap(function a() { if (!x) { x = true; a(); } });
· ┬ ┬
· │ ╰── 'a' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:7]
1const a = wrap(function a() {});
· ┬ ┬
· │ ╰── 'a' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:7]
1const a = foo || wrap(function a() {});
· ┬ ┬
· │ ╰── 'a' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:9]
1const { a = wrap(function a() {}) } = obj;
· ┬ ┬
· │ ╰── 'a' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:9]
1const { a = foo || wrap(function a() {}) } = obj;
· ┬ ┬
· │ ╰── 'a' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:9]
1const { a = foo, b = function a() {} } = {}
Expand Down Expand Up @@ -745,42 +700,6 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'A' is already declared in the upper scope.
╭─[no_shadow.tsx:1:7]
1const A = wrap(class A {});
· ┬ ┬
· │ ╰── 'A' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'A' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'A' is already declared in the upper scope.
╭─[no_shadow.tsx:1:7]
1const A = foo || wrap(class A {});
· ┬ ┬
· │ ╰── 'A' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'A' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'A' is already declared in the upper scope.
╭─[no_shadow.tsx:1:9]
1const { A = wrap(class A {}) } = obj;
· ┬ ┬
· │ ╰── 'A' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'A' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'A' is already declared in the upper scope.
╭─[no_shadow.tsx:1:9]
1const { A = foo || wrap(class A {}) } = obj;
· ┬ ┬
· │ ╰── 'A' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'A' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'A' is already declared in the upper scope.
╭─[no_shadow.tsx:1:14]
1function foo(A = wrap(class A {})) {}
Expand All @@ -799,24 +718,6 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Consider renaming 'A' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'a' is already declared in the upper scope.
╭─[no_shadow.tsx:1:5]
1var a = function a() {} ? foo : bar
· ┬ ┬
· │ ╰── 'a' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'a' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'A' is already declared in the upper scope.
╭─[no_shadow.tsx:1:5]
1var A = class A {} ? foo : bar
· ┬ ┬
· │ ╰── 'A' is declared here
· ╰── shadowed declaration is here
╰────
help: Consider renaming 'A' to avoid shadowing the variable from the outer scope.

eslint(no-shadow): 'Array' is already a global variable.
╭─[no_shadow.tsx:1:11]
1 │ (function Array() {})
Expand Down
Loading