fix(pipeline): reject non-file-like IMPORTS targets in fallback strategies#771
Open
apappas1129 wants to merge 1 commit into
Open
fix(pipeline): reject non-file-like IMPORTS targets in fallback strategies#771apappas1129 wants to merge 1 commit into
apappas1129 wants to merge 1 commit into
Conversation
…egies A wildcard tsconfig path alias (e.g. "@lib/*" -> "./src/lib/*") matches any import specifier sharing that prefix, including an unrelated scoped npm package meant to resolve from node_modules. The direct match already failed cleanly (no node exists at the substituted path), but cbm_pipeline_resolve_import_node's Strategy 4 - a generic "strip a trailing path segment and retry" fallback applied to every language, not just its intended Rust crate::/self::/super:: case - re-resolves the truncated import string and can land on the tsconfig's OTHER, bare alias entry that maps straight to a directory. The resulting QN collides with that directory's real Folder node (cbm_pipeline_fqn_module and cbm_pipeline_fqn_folder both dot-join path segments with no type tag), and Strategy 4 accepted whatever node it found there. Strategy 3 (symbol-name fallback) already guards its matches with import_targetable_label(), which excludes Folder/Project/etc. Strategy 1b (resolve_sibling_file, used only for markup/build-tool sibling imports - SCSS/Just/BitBake/Meson/Pony, none of which are directory- module languages) and Strategy 4 lacked the same guard; both now use it too. Strategy 1 (the primary, direct resolution path) intentionally keeps no such filter: Go and Java are directory-module languages in this pipeline (cbm_lang_module_is_dir), so a bare package import correctly and deliberately resolves straight to a Folder node there - adding the guard to Strategy 1 broke that (verified via the Go/Java integration tests) before being scoped down to just the fallback strategies where the collision actually originates. Closes DeusData#767. Signed-off-by: Alexandros Pappas <11921291+apappas1129@users.noreply.github.com>
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.
What does this PR do?
Closes #767: a wildcard tsconfig path alias (e.g.
"@lib/*": ["./src/lib/*"])shares its prefix with an unrelated scoped npm package meant to resolve
normally from
node_modules. The engine invented anIMPORTSedge into thelocal
src/libFolder node instead of leaving the import unresolved.The direct match already failed cleanly today:
cbm_pipeline_resolve_moduleresolves
@lib/external-pkgto a QN that doesn't correspond to any realfile, and
cbm_gbuf_find_by_qncorrectly finds nothing. The phantom edgeactually comes from Strategy 4 in
cbm_pipeline_resolve_import_node(
src/pipeline/pass_pkgmap.c) — a generic "strip a trailing path segment andretry" fallback applied to every language (not just its intended Rust
crate::/self::/super::case). Truncating@lib/external-pkgdown to@libhits the tsconfig's other, bare (non-wildcard) alias entry that mapsstraight to a directory. The resulting QN collides with that directory's real
Folder node (
cbm_pipeline_fqn_moduleandcbm_pipeline_fqn_folderbothdot-join path segments with no type tag to distinguish them), and Strategy 4
accepted whatever node it found there.
Strategy 3 (symbol-name fallback) already guards its matches with
import_targetable_label(), which excludesFolder/Project/etc. Strategy1b (
resolve_sibling_file, used only for markup/build-tool sibling imports —SCSS/Just/BitBake/Meson/Pony, none of which are directory-module languages)
and Strategy 4 lacked the same guard; both now use it too.
Strategy 1 (the primary, direct resolution path) intentionally keeps no
such filter: Go and Java are directory-module languages in this pipeline
(
cbm_lang_module_is_dir), so a bare package import correctly anddeliberately resolves straight to a Folder node there. I initially added the
guard to Strategy 1 as well and it broke Go/Java import resolution — caught
by the existing
test_edge_imports.cintegration suite (ei_go_same_module_import,a documented GREEN-guard test) — before scoping the fix down to just the two
fallback strategies where the collision actually originates.
Checklist
git commit -s)make -f Makefile.cbm test) — full suite greenexcept one pre-existing, unrelated RSS-ceiling test in
test_incremental.c(~2.6GB vs 2048MB limit) on this machinemake -f Makefile.cbm lint-ci) — could not verifylocally, cppcheck/clang-format are not installed in this
environment; relying on CI
contract_edge_imports_alias_no_phantom_folder_edge_issue767(assertszero
IMPORTSedges for the collision case) andcontract_edge_imports_alias_resolves_real_file_issue767(regressionguard: a wildcard alias resolving to a real file must still produce its
edge) in
tests/test_lang_contract.c