fix: extract variable names ignoring function calls#336
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a private helper Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
src/analyze.tstest/exports.test.ts
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
src/analyze.tstest/exports.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/exports.test.ts
There was a problem hiding this comment.
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
📒 Files selected for processing (2)
src/analyze.tstest/exports.test.ts
currently causing an issue with
unimportwhich is usingnamese.g.
resolves #303
Summary by CodeRabbit
Bug Fixes
Tests