Skip to content

linter: unicorn/consistent-function-scoping false negative when an inner function references a top-level import #22381

@matsu3m

Description

@matsu3m

What version of Oxlint are you using?

1.63.0

What command did you run?

oxlint path/to/file.tsx

What does your .oxlintrc.json (or oxlint.config.ts) config file look like?

{
  "categories": {
    "correctness": "off"
  },
  "plugins": ["unicorn"],
  "rules": {
    "unicorn/consistent-function-scoping": "error"
  }
}

What happened?

Summary

The unicorn/consistent-function-scoping rule doesn't seem to flag inner functions if their body references an identifier imported at the top level, yet it works as expected when those same functions reference browser or Node globals. This behavior is inconsistent with the original eslint-plugin-unicorn, which correctly reports the issue in both scenarios.

Minimal reproduction

import { notifications } from "some-module";

export const Outer = () => {
  // (A) references an imported binding -> NOT reported (bug)
  const usesImport = () => {
    notifications.show({ message: "x" });
  };

  // (B) references only a browser global (no resolved symbol) -> reported as expected
  const usesGlobal = () => {
    window.open("/", "_blank");
  };

  return [usesImport, usesGlobal];
};

When I run oxlint on this file, it only flags usesGlobal and seems to completely ignore usesImport. This is unexpected because usesImport doesn't actually rely on anything defined within Outer, so it could just as easily be moved to the top level of the module without losing access to notifications.

Expected behavior

Both inner functions should be reported to stay consistent with the upstream eslint-plugin-unicorn. There's no real need to keep a function nested just because it points to a module-level import. Since those imports are still accessible once the function is hoisted to the top level, they shouldn't be getting in the way.

Suspected root cause

In crates/oxc_linter/src/rules/unicorn/consistent_function_scoping.rs L270-L287, parent_scope_ids is built by walking every ancestor scope, including the top-level module scope.

let parent_scope_ids = {
    let mut current_scope_id = function_scope_id;
    let mut parent_scope_ids = FxHashSet::default();
    while let Some(parent_scope_id) = ctx.scoping().scope_parent_id(current_scope_id) {
        parent_scope_ids.insert(parent_scope_id);
        current_scope_id = parent_scope_id;
    }
    parent_scope_ids
};

for reference_id in function_body_var_references {
    let reference = ctx.scoping().get_reference(reference_id);
    let Some(symbol_id) = reference.symbol_id() else { continue };
    let scope_id = ctx.scoping().symbol_scope_id(symbol_id);
    if parent_scope_ids.contains(&scope_id) && symbol_id != function_declaration_symbol_id {
        return; // <- early return when an import binding is referenced
    }
}

Since import bindings live in the top scope, they are naturally included in parent_scope_ids. This means that any reference to an imported symbol will match the contains(&scope_id) check and trigger an early return, which ultimately suppresses the diagnostic for the enclosing inner function. On the other hand, browser globals like window do not have a resolved symbol_id, so they hit the continue branch instead. This explains why they do not trigger the early return and continue to be reported as expected.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Priority

    None yet

    Effort

    None yet

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions