fix(pipeline): classify native fetch() as HTTP_CALLS, not local shadows#927
Open
apappas1129 wants to merge 1 commit into
Open
fix(pipeline): classify native fetch() as HTTP_CALLS, not local shadows#927apappas1129 wants to merge 1 commit into
apappas1129 wants to merge 1 commit into
Conversation
Closes DeusData#856: native `fetch()` calls (Node 18+ built-in, and the identical browser/WHATWG API — no static way to tell them apart, and no need to: both make a real outbound HTTP request) were invisible to cross-repo intelligence. A bare `fetch(url)` has no import and typically no local definition, so registry resolution comes back empty and the call was silently dropped instead of becoming an HTTP_CALLS edge. The obvious fix — add "fetch" to the http_libraries substring table in service_patterns.c — is wrong: that table is consulted by an early, unconditional check in both pass_calls.c and pass_parallel.c that runs before/regardless of whether the callee resolves to a real local definition (by design, so `axios.get(url)` still classifies even when the registry mis-binds bare `get` to an unrelated local method). That's safe for "axios"/"requests" because nobody names their own function that; it is not safe for "fetch", which collides with a plausible local identifier (`function fetch(){}` or `const fetch = () => {}`). Instead, cbm_service_pattern_is_global_fetch() is a new, narrow, exact- match check consulted only in the *empty-resolution* fallback in both files — the existing DeusData#523 path for unindexed external libraries. By the time that branch runs, the registry has already had its chance to resolve "fetch" to a local/imported definition; only a genuine miss reaches the new check. This also means a member call like `repo.fetch()` is naturally excluded (its callee_name is "repo.fetch", not the bare "fetch" this checks for). pass_parallel.c's empty-resolution branch calls the low-level emit_http_async_service_edge() directly rather than going through emit_service_edge(), which re-derives its own classification from res->qualified_name via cbm_service_pattern_match() — a call with "fetch" would come back CBM_SVC_NONE there and silently fall through to a plain CALLS edge. Three new tests in test_pipeline.c: - bare fetch() -> HTTP_CALLS, sequential path (< 50 files) - bare fetch() -> HTTP_CALLS, forced through the parallel resolver (>= 50 files) - a local `function fetch(){}` shadowing the global -> plain CALLS to the local definition, zero HTTP_CALLS (the false-positive this PR must not introduce), bundled with a `repo.fetch()` member-call check in the first test Verified end-to-end, not just by inspection: prod build clean under -Wall -Wextra -Werror; test build clean under ASan+UBSan; full suite 5936 passed, 1 skipped (pre-existing, unrelated), 0 failed; clang-format clean on all touched files; lint-cppcheck's pre-existing failures (extract_defs.c, compat_regex.c) reproduced identically on unmodified main via stash/pop, confirming they predate this change. Refs DeusData#592. 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 #856: native
fetch()calls (Node 18+'s built-in, and the identical browser/WHATWG API — no static way to tell them apart, and no need to: both make a real outbound HTTP request) were invisible to cross-repo intelligence. A barefetch(url)has no import and typically no local definition, so registry resolution comes back empty and the call was silently dropped instead of becoming anHTTP_CALLSedge.The obvious fix — add
"fetch"to thehttp_librariessubstring table inservice_patterns.c— is wrong: that table is consulted by an early, unconditional check in bothpass_calls.candpass_parallel.cthat runs before/regardless of whether the callee resolves to a real local definition (by design, soaxios.get(url)still classifies even when the registry mis-binds baregetto an unrelated local method — the #523 bypass). That's safe for"axios"/"requests"because nobody names their own function that; it is not safe for"fetch", which collides with a plausible local identifier (function fetch(){}orconst fetch = () => {}).Instead,
cbm_service_pattern_is_global_fetch()is a new, narrow, exact-match check consulted only in the empty-resolution fallback in both files — the existing #523 path for unindexed external libraries. By the time that branch runs, the registry has already had its chance to resolve"fetch"to a local/imported definition; only a genuine miss reaches the new check. This also means a member call likerepo.fetch()is naturally excluded — itscallee_nameis"repo.fetch", not the bare"fetch"this checks for.pass_parallel.c's empty-resolution branch calls the low-levelemit_http_async_service_edge()directly rather than going throughemit_service_edge(), which re-derives its own classification fromres->qualified_nameviacbm_service_pattern_match()— a call with"fetch"would come backCBM_SVC_NONEthere and silently fall through to a plainCALLSedge, quietly losing the fix.Three new tests in
test_pipeline.c:fetch()→HTTP_CALLS, sequential path (< 50 files)fetch()→HTTP_CALLS, forced through the parallel resolver (≥ 50 files) — the two call-resolution implementations are independent, so both need direct coveragefunction fetch(){}shadowing the global → plainCALLSto the local definition, zeroHTTP_CALLS(the false-positive this PR must not introduce), bundled with arepo.fetch()member-call check in the first testRefs #592.
Checklist
git commit -s)make -f Makefile.cbm test) — full suite: 5936 passed, 1 skipped (pre-existing, unrelated), 0 failedmake -f Makefile.cbm lint-ci) — clang-format clean on all touched files;lint-cppcheck's pre-existing failures (extract_defs.c,compat_regex.c) reproduce identically on unmodifiedmain(verified via stash/pop), confirming they predate this change and aren't introduced by it