fix(extraction): reach Swift call arguments through call_suffix - #1896
fix(extraction): reach Swift call arguments through call_suffix#1896CaptainMittens wants to merge 1 commit into
Conversation
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Reviewed and approved. This is the base of your stack and it holds up on its own.
One thing I checked rather than assumed, because it was the only part of this that is not language-gated: What happens next, so you are not left guessing: That also unblocks the sequencing on #1976, which narrows to its own commit once this is in. Thanks for tracing this to the grammar's shape rather than special-casing the symptom — "Swift declares no arguments field at all" is the sentence that explains why every Swift call was missing its URL, and it took finding to write. |
|
Understood — nothing for me to do here, and I will leave the branch alone so the re-run tests what you reviewed. On the
|
|
Status update: your blocker has landed. This was recorded on our side as approved and waiting on a fix to The consequence is that this branch is now Worth knowing about the chain: #1976 ("reach a Swift URL built by a constructor") sits on top of this one and is The change itself is approved and needs nothing further from you beyond the rebase. Its three current red checks are against a stale base and will be re-run once it is refreshed. |
handle_calls reads a call's arguments through one tree-sitter field lookup, ts_node_child_by_field_name(node, "arguments"). The vendored Swift grammar declares no "arguments" field at all -- its own ts_field_names[] table has zero occurrences, against one in Go and two in TypeScript. Swift models a call as a target expression plus a call_suffix, and the arguments hang off the suffix as value_arguments. So args was null for every Swift call ever parsed, first_string_arg was never populated, and no Swift HTTP call could raise an HTTP_CALLS edge or a Route node. Alamofire, Moya and URLSession have been in the service-pattern table the whole time and match the callee text correctly; the URL simply never arrived. Three changes, all in extract_calls.c: - swift_call_args() reaches the argument list through call_suffix, in the same shape as the existing objectscript_call_args() fallback and used from the same place. A trailing closure has no value_arguments, so it returns a null node and that call behaves as before. - extract_url_or_topic_arg() unwraps Swift's per-argument value_argument node, stepping past a leading value_argument_label. Without it, dataTask(with: "/api/v1/widgets") yields the label "with" rather than the path. PHP and C# already had the same unwrap for their own "argument" node. - is_string_like() gains line_string_literal, which is what Swift calls an ordinary "..." literal. The list already held raw_string_literal, so only Swift's common case was missing. This is the layer underneath DeusData#1892 rather than the whole of it. A literal URL argument now arrives; a literal nested inside a constructor, as in URLSession.shared.data(from: URL(string: "...")!), still does not, because extract_positional_url reads a literal, a template string, a concatenation or a named constant and that shape is none of them. Reproduce-first: all three tests fail without the fix, two on a null first_string_arg and one on HTTP_CALLS being 0. Fixes DeusData#1892 Signed-off-by: Joshua Richter <jrichter5781@gmail.com>
50eaeeb to
c4e9028
Compare
What does this PR do?
Swift calls lost every argument, so no Swift HTTP call could ever produce an
HTTP_CALLSedge or aRoutenode.handle_callsreads a call's arguments through one tree-sitter field lookup:The vendored Swift grammar declares no
"arguments"field at all — its ownts_field_names[]table has zero occurrences of the string, against one in Goand two in TypeScript. Swift models a call as a
targetexpression plus acall_suffix, and the arguments hang off the suffix asvalue_arguments.So
argswas null for every Swift call ever parsed,first_string_argwasnever populated, and everything downstream that needs the URL could not fire.
Alamofire,MoyaandURLSessionhave been in the service-pattern table(
internal/cbm/service_patterns.c) the whole time and match the callee textcorrectly — the URL simply never arrived.
Three changes, all in
internal/cbm/extract_calls.c:swift_call_args()reaches the argument list throughcall_suffix, in thesame shape as the existing
objectscript_call_args()fallback, and is usedfrom the same place. A trailing closure has no
value_arguments, so itreturns a null node and that call behaves exactly as before.
extract_url_or_topic_arg()unwraps Swift's per-argumentvalue_argumentnode, stepping past a leading
value_argument_label. Without this,dataTask(with: "/api/v1/widgets")yields the labelwithrather than thepath. PHP and C# already had the equivalent unwrap for their
argumentnode.is_string_like()gainsline_string_literal, which is what Swift calls anordinary
"…"literal. The list already heldraw_string_literal, so onlySwift's common case was missing.
What this does not do
It makes a literal URL argument reachable. It does not resolve
URLSession.shared.data(from: URL(string: "…")!), where the literal sits insidea nested constructor —
extract_positional_urlhandles a literal, a templatestring, a concatenation, or a named constant, and that nested shape is none of
those. Issue #1892 is therefore not fully closed by this change; it is the
blocking layer underneath it.
Also worth recording for whoever picks that up: Alamofire's
AF.request(…)shorthand does not match the pattern table, because the table matches the
library name in the callee text and
AF.requestcontains none. That is why thepipeline test below uses
URLSession.No
Package.swifthandling is touched — that is #551, already scoped elsewhere.Fixes #1892
Validation
Reproduce-first. With the fix reverted and the tests kept, all three fail:
With the fix:
extractionpipelinescripts/test.shThe 28 failures are all in
tests/test_cli.c(client install/uninstall) and arepre-existing in my environment, not caused by this change. Measured rather than
assumed: I reverted only
internal/cbm/extract_calls.ctoorigin/main, keptthe new tests, rebuilt, and re-ran that suite —
263 passed, 28 failed, the samecount, and every failure recorded in the full run fails on the baseline too. If
they are green on your runners, they are environmental here and worth ignoring
for this PR.
Lint:
make -f Makefile.cbm lint-cppcheckandlint-no-suppressboth clean.clang-formatreports no change wanted on any line this PR adds. I could not runclang-tidylocally — it is not in my toolchain — so that one is unverified onmy side; CI's
lint-cicovers cppcheck and clang-format, which I did run.Built with
-Wall -Wextra -Werror, no new warnings.Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)make -f Makefile.cbm lint-ci) — cppcheck and clang-format run clean; clang-tidy not installed locally