Skip to content

fix: extract variable names ignoring function calls#336

Merged
pi0 merged 4 commits intomainfrom
fix/extra-names
Mar 4, 2026
Merged

fix: extract variable names ignoring function calls#336
pi0 merged 4 commits intomainfrom
fix/extra-names

Conversation

@danielroe
Copy link
Copy Markdown
Member

@danielroe danielroe commented Mar 4, 2026

currently causing an issue with unimport which is using names

e.g.

export const useSleep = () => useAsyncData('sleep', async () => { return null })

// returns names: ["useSleep", "async"]

resolves #303

Summary by CodeRabbit

  • Bug Fixes

    • Improved parsing of comma-separated export names to correctly handle nested constructs, generics, and string literals, ensuring all exported identifiers are detected.
  • Tests

    • Added tests for async exports, middleware-style exports, typed and function-typed declarations, multi-identifier exports, and escaped-string edge cases.

@danielroe danielroe requested a review from pi0 March 4, 2026 19:58
@danielroe danielroe self-assigned this Mar 4, 2026
@danielroe danielroe added the bug Something isn't working label Mar 4, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 4, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46681810-c6b7-4cf9-93e7-0873dd07e259

📥 Commits

Reviewing files that changed from the base of the PR and between b24552c and 1650fd5.

📒 Files selected for processing (2)
  • src/analyze.ts
  • test/exports.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/analyze.ts

📝 Walkthrough

Walkthrough

Adds a private helper _extractExtraNames() in src/analyze.ts to robustly parse comma-separated extra export names while respecting nested (), [], {} and string literals; replaces prior inline extraction in findExports() and extends test/exports.test.ts with additional declaration cases covering multi-identifier and typed exports. Fixes comma-in-generic mis-parsing.

Changes

Cohort / File(s) Summary
Export Analysis Refactor
src/analyze.ts
Adds internal _extractExtraNames(extraNamesStr) to parse comma-separated names safely (tracks nesting and string state). Replaces ad-hoc/regex parsing in findExports() and appends returned names to declaredExport.names when present. No public API changes.
Test Coverage — export declarations
test/exports.test.ts
Adds multiple new declaration tests validating typed declarations, multi-identifier exports, generics, and function-typed exports (e.g., useSleep, handler, middleware, a,b pair, foo variants, foo, bar).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble commas, dodge each quote and brace,
Hopping through generics in a careful pace,
I pull the names from tangled strings,
Tidy exports—no surprise it brings,
Tests applaud my little, nimble trace.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Title accurately describes the primary fix: preventing incorrect identifiers (like 'async' from function calls or 'number' from generics) from being extracted as export variable names.
Linked Issues check ✅ Passed The code changes implement the required fix by adding _extractExtraNames helper to properly parse extra export names while tracking nesting depth and string state, preventing spurious identifiers from commas in type generics or function calls.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the extra name extraction issue: new helper function in analyze.ts, integration into findExports logic, and corresponding test cases for the fix.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/extra-names

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
test/exports.test.ts (1)

109-124: Nice regression coverage; add one mixed-separator case for completeness.

Please add a case where a function-call comma and a real declarator comma coexist, e.g. export const a = fn('x', async () => {}), b = 1, and assert names are ["a", "b"]. This locks in the helper’s core contract.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/exports.test.ts` around lines 109 - 124, Add a new mixed-separator test
case to the exports tests that ensures a function-call comma and a declarator
comma coexist: add the string "export const a = fn('x', async () => {}), b = 1"
as a case similar to the existing entries and assert its parsed result is {
type: "declaration", name: "a", names: ["a", "b"] } so the helper correctly
returns both declarator names; update the test array alongside the entries for
useSleep/handler/middleware.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/analyze.ts`:
- Around line 406-412: The string-parsing logic using inString and extraNamesStr
misclassifies quotes preceded by an even number of backslashes because it only
checks extraNamesStr[i - 1] !== "\\"; update the condition where you toggle
inString (the block that checks if char === inString) to count consecutive
backslashes immediately before index i (walking backwards from i-1) and only
treat the quote as escaped when the count is odd; use the existing variables
(inString, char, i, extraNamesStr) so the quote closes the string when the
preceding-backslash count is even (including zero).

---

Nitpick comments:
In `@test/exports.test.ts`:
- Around line 109-124: Add a new mixed-separator test case to the exports tests
that ensures a function-call comma and a declarator comma coexist: add the
string "export const a = fn('x', async () => {}), b = 1" as a case similar to
the existing entries and assert its parsed result is { type: "declaration",
name: "a", names: ["a", "b"] } so the helper correctly returns both declarator
names; update the test array alongside the entries for
useSleep/handler/middleware.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6643a2c3-1660-4da0-b86e-3964cf0da30a

📥 Commits

Reviewing files that changed from the base of the PR and between ce07479 and 632c778.

📒 Files selected for processing (2)
  • src/analyze.ts
  • test/exports.test.ts

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/analyze.ts`:
- Around line 424-437: The comma-separation logic in _extractExtraNames
incorrectly treats commas inside TypeScript generics as top-level separators;
update the loop that tracks bracket nesting (currently using depth and
extraNamesStr) to also track angle bracket nesting by adding an angleDepth
counter that increments on '<' and decrements on '>', and change the comma check
to only treat a comma as a real separator when depth === 0 and angleDepth === 0;
make sure to reference the same loop and variables (_extractExtraNames,
extraNamesStr, depth) so the new angleDepth is updated alongside existing
bracket handling.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4eff45e5-1e2e-4bbe-ab08-52431e25d8c0

📥 Commits

Reviewing files that changed from the base of the PR and between 632c778 and 359bd31.

📒 Files selected for processing (2)
  • src/analyze.ts
  • test/exports.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/exports.test.ts

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/analyze.ts`:
- Around line 431-433: The code currently sets inTypeAnnotation = false on any
top-level '=' (char === "=") which incorrectly closes type annotations for arrow
types like '=>'; update the condition so '=' only ends the type annotation when
it is not part of an arrow token. Concretely, when evaluating char === "=", also
check neighboring characters (e.g., the previous and next characters via
source[index-1] and source[index+1] or whatever context variables exist) and
skip setting inTypeAnnotation = false if the '=' is part of '=>'. Keep the
original depth and angleDepth checks and only suppress the state change for
arrow '=>' sequences.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a7a9c78-7e67-4e54-8eab-5c2de4c15ca9

📥 Commits

Reviewing files that changed from the base of the PR and between 359bd31 and b24552c.

📒 Files selected for processing (2)
  • src/analyze.ts
  • test/exports.test.ts

@pi0 pi0 merged commit f9efe9d into main Mar 4, 2026
5 checks passed
@pi0 pi0 deleted the fix/extra-names branch March 4, 2026 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Commas in export can cause findExports / findExportNames to add incorrect export

2 participants