Skip to content

fix(extraction): reach Swift call arguments through call_suffix - #1896

Open
CaptainMittens wants to merge 1 commit into
DeusData:mainfrom
CaptainMittens:fix/swift-call-arguments
Open

fix(extraction): reach Swift call arguments through call_suffix#1896
CaptainMittens wants to merge 1 commit into
DeusData:mainfrom
CaptainMittens:fix/swift-call-arguments

Conversation

@CaptainMittens

Copy link
Copy Markdown
Contributor

What does this PR do?

Swift calls lost every argument, so no Swift HTTP call could ever produce an
HTTP_CALLS edge or a Route node.

handle_calls reads a call's arguments through one tree-sitter field lookup:

TSNode args = ts_node_child_by_field_name(node, TS_FIELD("arguments"));

The vendored Swift grammar declares no "arguments" field at all — its own
ts_field_names[] table has zero occurrences of the string, 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 everything downstream that needs the URL could not fire.
Alamofire, Moya and URLSession have been in the service-pattern table
(internal/cbm/service_patterns.c) the whole time and match the callee text
correctly — the URL simply never arrived.

Three changes, all in internal/cbm/extract_calls.c:

  1. swift_call_args() reaches the argument list through call_suffix, in the
    same shape as the existing objectscript_call_args() fallback, and is used
    from the same place. A trailing closure has no value_arguments, so it
    returns a null node and that call behaves exactly as before.
  2. extract_url_or_topic_arg() unwraps Swift's per-argument value_argument
    node, stepping past a leading value_argument_label. Without this,
    dataTask(with: "/api/v1/widgets") yields the label with rather than the
    path. PHP and C# already had the equivalent unwrap for their argument node.
  3. 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.

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 inside
a nested constructor — extract_positional_url handles a literal, a template
string, 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.request contains none. That is why the
pipeline test below uses URLSession.

No Package.swift handling is touched — that is #551, already scoped elsewhere.

Fixes #1892

Validation

Reproduce-first. With the fix reverted and the tests kept, all three fail:

swift_call_string_arg_issue1892           FAIL tests/test_extraction.c:3752: c->first_string_arg is NULL
swift_labeled_call_string_arg_issue1892   FAIL tests/test_extraction.c:3769: c->first_string_arg is NULL
pipeline_swift_http_call_makes_route_issue1892  FAIL tests/test_pipeline.c:5028: HTTP_CALLS (0) not >= 1

With the fix:

Suite Result
extraction 309 passed (307 before, plus the 2 new)
pipeline 256 passed (255 before, plus the 1 new)
full scripts/test.sh 7633 passed, 28 failed, 8 skipped (140 suites)

The 28 failures are all in tests/test_cli.c (client install/uninstall) and are
pre-existing in my environment, not caused by this change. Measured rather than
assumed: I reverted only internal/cbm/extract_calls.c to origin/main, kept
the new tests, rebuilt, and re-ran that suite — 263 passed, 28 failed, the same
count, 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-cppcheck and lint-no-suppress both clean.
clang-format reports no change wanted on any line this PR adds. I could not run
clang-tidy locally — it is not in my toolchain — so that one is unverified on
my side; CI's lint-ci covers cppcheck and clang-format, which I did run.

Built with -Wall -Wextra -Werror, no new warnings.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci) — cppcheck and clang-format run clean; clang-tidy not installed locally
  • New behavior is covered by a test (reproduce-first for bug fixes)

@github-actions

Copy link
Copy Markdown

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. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

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.

@CaptainMittens

Copy link
Copy Markdown
Contributor Author

#1976 builds on this one — it reaches the URL(string: "…")! constructor that wraps the string, which is the shape #1892 actually reported. Reviewing this PR first makes that one narrow to its own two commits, since GitHub shows this commit on it as well.

@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Reviewed and approved. This is the base of your stack and it holds up on its own.

swift_call_args is gated twice — on ctx->language == CBM_LANG_SWIFT, and on args already being null — so no other language can reach it. Following the objectscript_call_args shape that was already there for the same "grammar declares no arguments field" problem is the right call; it makes the next reader see two instances of one pattern rather than two ad-hoc special cases. Noting in the comment that a trailing closure yields a call_suffix with no value_arguments, and that this returns null and leaves behaviour unchanged, is the detail that makes the guard reviewable instead of just plausible.

One thing I checked rather than assumed, because it was the only part of this that is not language-gated: is_string_like is shared across every language, so adding line_string_literal to it could have changed behaviour well outside Swift. It does not — line_string_literal appears in exactly one of the 159 vendored grammars, and that one is Swift. So the widening is inert everywhere else. Worth stating explicitly since the diff does not make it obvious.

What happens next, so you are not left guessing: main does not currently compile — two changes landed the same type short-name index into type_registry.h and git merged them into duplicate struct members. Your green here predates that, so it is stale rather than wrong. #1993 is the repair. Once it lands I will re-run this PR's checks against a fixed main and merge it if it is green — there is nothing for you to do, and no rebase needed unless the re-run says otherwise.

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.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Sep 1, 2026
@CaptainMittens

Copy link
Copy Markdown
Contributor Author

Understood — nothing for me to do here, and I will leave the branch alone so the re-run tests what you reviewed.

On the is_string_like check: that was the one part I could not prove from inside the diff, so thank you for counting it. I had reasoned that line_string_literal was Swift-only from the grammar I was reading, which is not the same thing as knowing it appears in one of 159. Your check is the evidence; mine was an assumption that happened to be right.

main is now repaired — #1993 is in as 17786374, and this branch is 3 commits behind it with no conflict, so a re-run should be clean whenever you get to it.

@DeusData

DeusData commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Status update: your blocker has landed.

This was recorded on our side as approved and waiting on a fix to main. That fix is in — #1986 merged today as 620613ed, carrying the Swift scanner shift correction and its MANIFEST.md row.

The consequence is that this branch is now DIRTY and needs a rebase onto current main. main has taken well over a dozen merges today, so the conflict is expected rather than a sign of anything wrong with the change.

Worth knowing about the chain: #1976 ("reach a Swift URL built by a constructor") sits on top of this one and is CONFLICTING for the same reason. If you rebase this first, that one rebases cleanly on top. Either order works — they touch the same file and neither is large — I only want the sequencing to be deliberate rather than a surprise.

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>
@CaptainMittens
CaptainMittens force-pushed the fix/swift-call-arguments branch from 50eaeeb to c4e9028 Compare September 3, 2026 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Swift: no client-side Route or HTTP_CALLS emitted, so cross-repo-intelligence returns 0 edges (URLSession is already in the pattern table)

2 participants