Skip to content

Commit

Permalink
fix(linter): fix some cases on AssignmentExpression for ```unic…
Browse files Browse the repository at this point in the history 
…orn/consistent-function-scoping``` (#5675)

This is to fix the cases mentioned in the comment section of #5365.

In short, it will report these as PASS test cases:

```js
let inner;

function foo1() {
  inner = function() {}
}
function foo2() {
  inner = function() {}
}
```

```js
let inner;

function outer() {
  inner = function() {}
}
```

And report these below as FAIL test cases:

```js
let inner;

function foo1() {
  inner = function smth() {}
}
function foo2() {
  inner = function bar() {}
}
```

```js
let inner;

function outer() {
  inner = function inner() {}
}
```

The case below was already done in #5312 but mentioned in #5365. It is a
FAIL case as well:
```js
function outer() {
  const inner = function inner() {}
}
```

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Don Isaac <[email protected]>
  • Loading branch information
@Arian94 @autofix-ci @DonIsaac
3 people authored Sep 16, 2024
1 parent dc10eaf commit 737ba1d
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 43 deletions.
112 changes: 69 additions & 43 deletions crates/oxc_linter/src/rules/unicorn/consistent_function_scoping.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,52 +159,59 @@ impl Rule for ConsistentFunctionScoping {
}

fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let (function_declaration_symbol_id, function_body, reporter_span) = match node.kind() {
AstKind::Function(function) => {
if let Some(AstKind::AssignmentExpression(_)) = ctx.nodes().parent_kind(node.id()) {
return;
}

if function.is_typescript_syntax() {
return;
}

// NOTE: function.body will always be some here because of
// checks in `is_typescript_syntax`
let Some(function_body) = &function.body else { return };

if let Some(binding_ident) = get_function_like_declaration(node, ctx) {
(
binding_ident.symbol_id.get().unwrap(),
function_body,
function
.id
.as_ref()
.map_or(Span::sized(function.span.start, 8), |func_binding_ident| {
func_binding_ident.span
}),
)
} else if let Some(function_id) = &function.id {
let Some(symbol_id) = function_id.symbol_id.get() else {
let (function_declaration_symbol_id, function_body, reporter_span) =
match node.kind() {
AstKind::Function(function) => {
if function.is_typescript_syntax() {
return;
};
(symbol_id, function_body, function_id.span())
} else {
return;
}

if let Some(func_scope_id) = function.scope_id.get() {
if let Some(parent_scope_id) = ctx.scopes().get_parent_id(func_scope_id) {
// Example: const foo = function bar() {};
// The bar function scope id is 1. In order to ignore this rule,
// its parent's scope id (in this case `foo`'s scope id is 0 and is equal to root scope id)
// should be considered.
if parent_scope_id == ctx.scopes().root_scope_id() {
return;
}
}
}

// NOTE: function.body will always be some here because of
// checks in `is_typescript_syntax`
let Some(function_body) = &function.body else { return };

if let Some(binding_ident) = get_function_like_declaration(node, ctx) {
(
binding_ident.symbol_id.get().unwrap(),
function_body,
function.id.as_ref().map_or(
Span::sized(function.span.start, 8),
|func_binding_ident| func_binding_ident.span,
),
)
} else if let Some(function_id) = &function.id {
let Some(symbol_id) = function_id.symbol_id.get() else {
return;
};
(symbol_id, function_body, function_id.span())
} else {
return;
}
}
}
AstKind::ArrowFunctionExpression(arrow_function) if self.check_arrow_functions => {
let Some(binding_ident) = get_function_like_declaration(node, ctx) else {
return;
};
let Some(symbol_id) = binding_ident.symbol_id.get() else {
return;
};
AstKind::ArrowFunctionExpression(arrow_function) if self.check_arrow_functions => {
let Some(binding_ident) = get_function_like_declaration(node, ctx) else {
return;
};
let Some(symbol_id) = binding_ident.symbol_id.get() else {
return;
};

(symbol_id, &arrow_function.body, binding_ident.span())
}
_ => return,
};
(symbol_id, &arrow_function.body, binding_ident.span())
}
_ => return,
};

// if the function is declared at the root scope, we don't need to check anything
if ctx.symbols().get_scope_id(function_declaration_symbol_id)
Expand Down Expand Up @@ -588,6 +595,17 @@ fn test() {
("module.exports = function foo() {};", None),
("module.exports.foo = function foo() {};", None),
("foo.bar.func = function foo() {};", None),
(
"let inner;
function foo1() {
inner = function() {}
}
function foo2() {
inner = function() {}
}",
None,
),
("if(f) function f(){}", None),
];

Expand Down Expand Up @@ -626,6 +644,14 @@ fn test() {
),
("function foo() { function bar() { return <JSX/>; } }", None),
("function doFoo(Foo) { const doBar = () => arguments; return doBar(); };", None),
(
"let inner;
function outer() {
inner = function inner() {}
}",
None,
),
// end of cases that eslint-plugin-unicorn passes, but we fail.
(
"function doFoo(foo) {
Expand Down
9 changes: 9 additions & 0 deletions crates/oxc_linter/src/snapshots/consistent_function_scoping.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,15 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: Move this function to the outer scope.

⚠ eslint-plugin-unicorn(consistent-function-scoping): Function does not capture any variables from the outer scope.
╭─[consistent_function_scoping.tsx:4:34]
3 │ function outer() {
4 │ inner = function inner() {}
· ─────
5 │ }
╰────
help: Move this function to the outer scope.

⚠ eslint-plugin-unicorn(consistent-function-scoping): Function does not capture any variables from the outer scope.
╭─[consistent_function_scoping.tsx:2:26]
1 │ function doFoo(foo) {
Expand Down

0 comments on commit 737ba1d

Please sign in to comment.