Repository navigation
fix(resolution): a comment before an anonymous export default stays anonymous - #618
Conversation
…nonymous The classifier read `export default /* c */ () => 2` as an expression, so the default_fn_class fallback could name the module's first exported function. Skip whitespace and JS block/line comments between `export default` and the head before classifying. Adds a kernel unit test (red before) and an extraction regression fixture.
…acOS The installer returns realpath'd file paths; the test compared the raw mkdtemp paths, so every find and array equality failed on macOS (/var -> /private/var). Realpath the temp base before joining the documented file. Docker/linux suite: 641 passed, 8 skipped.
![bompus-ai-review[bot]](https://avatars.githubusercontent.com/u/253635?s=60&v=4){kind=small}
| return true; | ||
| }; | ||
| let mut rest = &line[m.end()..]; | ||
| let mut rest = skip_js_trivia(&line[m.end()..]); |
There was a problem hiding this comment.
medium: Require keyword boundaries after skipping comments · ref db0043
skip_js_trivia(...) exposes the expression head to re!(r"^(?:function|class)"), which matches identifier prefixes as well as keywords.
Failure: With an exported Model and a local classify(x) { return x; }, export default /* c */ classify(Model) is classified as anonymous. A call through a default import consequently loses its edge to Model, which the previous expression fallback resolved.
Suggested fix: Recognize complete JavaScript keyword tokens for function, class, and async, and add a regression test for a commented expression whose callee starts with one of those words.
| return true; | ||
| }; | ||
| let mut rest = &line[m.end()..]; | ||
| let mut rest = skip_js_trivia(&line[m.end()..]); |
There was a problem hiding this comment.
medium: Ignore comments when balancing a parenthesized expression · ref a06475
After skip_js_trivia(...) exposes a (, the classifier scans every character and treats any ) followed by => or : as the end of an arrow parameter list.
Failure: For export default /* c */ (/* ) => */ memo(Model)), the ) inside the second comment closes the scan early, so the expression is classified as anonymous. With an exported Model and a local identity memo, calls through the default import lose their previously resolved edge to Model.
Suggested fix: Skip comments and other JavaScript literals while balancing parentheses, or classify the head from the syntax tree; cover this parenthesized expression in a regression test.
| return t; | ||
| } | ||
| if let Some(rest) = t.strip_prefix("//") { | ||
| match rest.find('\n') { |
There was a problem hiding this comment.
medium: Stop line comments at every JavaScript line terminator · ref afb019
The new line-comment scanner uses only rest.find('\n'), although JavaScript also ends line comments at a standalone carriage return, U+2028, and U+2029.
Failure: If a file contains export default // c followed by U+2028, memo(Model);, a newline, and function later() {}, the scanner skips the real expression and sees function later as the export head. It marks the default anonymous and drops the previously resolved edge to exported Model.
Suggested fix: End the comment at any JavaScript line terminator, advancing by the matched character's UTF-8 width, and test a default expression after U+2028 or U+2029.
Addresses the three review findings: the function/class keyword needs a word boundary after the comment skip; a comment inside the parenthesized head is skipped while balancing so its ) is not a closer; and a line comment ends at every JavaScript line terminator (LF, CR, U+2028, U+2029), not just LF. The unit test covers all three; golden dumps unchanged; precision gin/json held.
|
|
||
| for (const location of locations) { | ||
| const file = () => path.join(location === 'global' ? dirs.home() : dirs.cwd(), ...c[location]!); | ||
| const baseDir = () => fs.realpathSync(location === 'global' ? dirs.home() : dirs.cwd()); |
There was a problem hiding this comment.
medium: Keep global expected paths consistent with the test HOME · ref 465003
baseDir() realpaths dirs.home(), but useTmpDirs passes the original tmpHome to setHome. Global targets build and return paths from os.homedir(), which retains that original spelling.
Failure: When CODEGRAPH_TEST_TMPDIR is long enough for the test setup to create its /tmp/cgt-* symlink, the global targets return paths through the symlink while file() expects paths through its real destination. The JSON target path assertions fail even though the files were written successfully. The same mismatch occurs when macOS temp paths use /var rather than /private/var.
Suggested fix: Realpath tmpHome before passing it to setHome, or preserve the original home spelling when constructing global expected paths.
Global targets build paths from HOME as spelled; the JSON-target expectations already used the real path, so a symlinked temp root (or macOS /var) disagreed. Set HOME to the real temp path. Installer suite: 641 passed, 8 skipped.
…macOS paths The remaining macOS failures were the other target blocks: mkTmpDir returned a symlinked /tmp spelling while the targets return real paths. Realpath the mkdtemp result once for every helper; the JSON-target HOME realpath stays as a no-op. Installer suite: 641 passed, 8 skipped.
Summary
export default /* comment */ () => 2was classified as an expression default, so thedefault_fn_classfallback could name the module's first exported function. Skip whitespace and JS block/line comments betweenexport defaultand the head before classifying, including afterasync.The second commit fixes the macOS-only failure in the batch-4 JSON agent target tests: the installer returns realpath'd file paths, while the tests compared the raw
mkdtemppaths, so every lookup and array equality failed on macOS (/varvs/private/var). The test now realpaths the temp base before joining the documented file.Verification
export_default_head_skips_comments: red before, green after.default-export-declaration+kernel-golden-dumps: 20 passed; golden dumps unchanged.installer-targets: 641 passed, 8 skipped locally after the realpath fix.