Repository navigation
fix(ts): tsconfig alias imports to dotted basenames, and calls through constructor-injected fields - #2
Open
thientrile wants to merge 4 commits into
Open
fix(ts): tsconfig alias imports to dotted basenames, and calls through constructor-injected fields#2thientrile wants to merge 4 commits into
thientrile wants to merge 4 commits into
Conversation
…ases The alias branch passed the already extension-stripped target to cbm_pipeline_fqn_module, which stripped `.service` from `src/conversations/conversations.service` a second time. The module was missed and the import bound to an unrelated file mentioning the class, so every aliased import in a NestJS `x.service.ts` layout pointed at the wrong file. Resolve it like the relative branch (DeusData#1682) does, and strip the same extension list there. Signed-off-by: Liguni <thientrile2003@gmail.com>
`constructor(private readonly svc: Svc)` declares the field `svc`, but only public_field_definition was collected, so `this.svc.method()` had no receiver type and produced no CALLS edge. NestJS injects every service this way, so impact analysis missed most cross-class callers. Signed-off-by: Liguni <thientrile2003@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d5d33686
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Signed-off-by: Liguni <thientrile2003@gmail.com>
Signed-off-by: Liguni <thientrile2003@gmail.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two TypeScript resolution bugs from a NestJS repo (40k nodes, re-indexed with
mode=full,tsconfig"paths": { "src/*": ["src/*"] }).1. Alias imports to
x.service.tsresolve to the wrong fileimport { ConversationsService } from 'src/conversations/conversations.service'bound tosrc/tasks/capabilities/task-write-capabilities.spec.ts.The alias branch of
cbm_pipeline_resolve_modulepassed the already extension-stripped target tocbm_pipeline_fqn_module, which stripped.servicea second time. The module QN missed, and resolution fell back to an unrelated file that mentions the class. The relative branch had the same bug and was fixed in DeusData#1682. The alias branch now resolves the same way, andstrip_resolved_extstrips the same extension list as the relative resolver (.mjs,.cts,.json, …), since nothing strips after it any more.2.
this.<injected>.method()produces no CALLS edgeconstructor(private readonly svc: Svc)declares the fieldsvc, butast_sweep_shapescollected onlypublic_field_definition.this.svchad no type, so the call never resolved. Parameters with an accessibility modifier,readonlyoroverrideare now registered as typed fields. Plain constructor parameters stay local.Verification
ei_typescript_dotted_alias_import_targets_source_module(edge_imports) andtslsp_constructor_parameter_property_resolution(ts_lsp) fail onmainand pass with the fix.test-runner edge_imports ts_lsp→ 368 passed, 0 failed (gcc 14, ASan).gcc:14container:extract_spill_round_trip_keeps_every_fieldfails atcbm_result_spill_open(Python spill dir, untouched by this PR). This looks environmental; CI on this PR settles it.