Skip to content

fix: reduce false positives from local variable shadowing and early-return guards#687

Open
Akshay090 wants to merge 2 commits intoamilajack:mainfrom
Akshay090:fix/false-positives-and-early-return-guard
Open

fix: reduce false positives from local variable shadowing and early-return guards#687
Akshay090 wants to merge 2 commits intoamilajack:mainfrom
Akshay090:fix/false-positives-and-early-return-guard

Conversation

@Akshay090
Copy link
Contributor

@Akshay090 Akshay090 commented Mar 26, 2026

Fixes #688

Summary

Two related fixes that reduce false positives in the compat/compat rule:

1. Function parameter names matching browser APIs

The Identifier visitor that tracks locally declared names was missing ArrowFunctionExpression and FunctionExpression parent types. This caused false positives when callback parameters happened to share a name with a browser API.

Example — flagged incorrectly before this fix:

// 'scheduler' here is a domain entity, not the Window.scheduler browser API
schedulers.map(scheduler => scheduler.name);

The existing heuristic already handled FunctionDeclaration params, VariableDeclarator, Property, ClassDeclaration, and import specifiers. This adds the two missing function types for consistency.

2. Early-return guard pattern as feature detection

The existing isInsideIfStatement heuristic only suppressed errors for API usage inside an if block. The common early-return guard pattern was missed:

function setup() {
  if (!('serviceWorker' in navigator)) { return; }
  navigator.serviceWorker.register('/sw.js'); // was falsely flagged
}

The new isGuardedByEarlyReturn helper walks up from the flagged node to its containing block and checks preceding sibling if-statements for early exits (return/throw) whose test references the same API. Respects the ignoreConditionalChecks setting.

Test plan

  • Added 4 valid test cases for arrow function and function expression parameter shadowing
  • Added 4 valid test cases for early-return guard patterns (in operator, member expression, throw, bare identifier)
  • Added 2 invalid test cases (unrelated guard doesn't suppress; ignoreConditionalChecks overrides early-return detection)
  • All 118 tests pass, lint clean, build succeeds

…rowser APIs

The Identifier visitor that tracks locally declared names was missing
ArrowFunctionExpression and FunctionExpression parent types. This caused
false positives when callback parameters happened to share a name with a
browser API (e.g. `items.map(scheduler => scheduler.name)` flagged as
unsupported Window.scheduler usage).

Made-with: Cursor
The existing isInsideIfStatement heuristic only suppressed errors for
API usage inside an if block. The common early-return pattern was missed:

  if (!('serviceWorker' in navigator)) { return; }
  navigator.serviceWorker.register('/sw.js'); // falsely flagged

Now walks up from the flagged node to its containing block and checks
preceding sibling if-statements for early exits (return/throw) whose
test references the same API. Respects ignoreConditionalChecks setting.

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positives: local variables/parameters matching browser API names are flagged

1 participant