[htarnacki]

What does this PR do?

Closes #2154. Stacked on #2360: the first commit here (bb3f1cdd) is #2360, only the second commit is this PR. Once #2360 merges I'll rebase and the diff collapses to that one commit.

A Scala class Foo / trait Foo / enum Foo and its companion object Foo share a source name, so both containers and their methods got the same QN and overwrote each other. The companion now gets scalac's Foo$ suffix wherever a class QN is computed (extract_defs.c, extract_unified.c, via cbm_scala_is_companion_object in helpers.c). Detection is O(1) per object after one scan of its scope: the scope's class/trait/enum names are recorded once as byte ranges into the source (no copies) in a per-file cache that lives in the scratch arena, and every later query in that scope is a hash probe.

On the question from the issue — standalone object Foo: only companions get the $. A standalone object has nothing to collide with, and suffixing it would rename every Scala object in existing graphs for no gain. With this rule the only QNs that change are the ones that were previously lost to a collision. Test extract_scala_trait_companion_and_standalone_object pins both sides.

Because the pair is one name at the source level, resolution has to treat it as such:

• pass_pkgmap.c: import pkg.Foo no longer fails closed as "ambiguous" between Foo and Foo$ (the class stands for the pair); import pkg.Foo.member looks under Foo$ first — only object members are importable.
• registry.c: Foo.make() reaches Foo$.make through import_map, same_module, the qualified-tail disambiguator and receiver_chain_admits. Every $ probe is gated on the registry's own record of Scala companions: cbm_registry_add_def records Foo$ only for a type-like definition in a .scala file, and companion_known() is the single check in front of each probe. A Java or JS/TS class that happens to be named Foo$ is plain text — Foo.m() next to it resolves exactly as on main (pinned by resolve_java_ts_dollar_class_next_to_class_is_unchanged).

Class-body val/var keep their module-level QN like every other language (parent_class already records the owner). My original branch qualified them by owner; measured on finagle that dropped 608 same_module CALLS (the resolver probes module.name), so I removed it.

Before / after on twitter/finagle (ca472de, 1,898 .scala files)

#2360 head vs this branch, fresh CBM_CACHE_DIR each run. Edge identity is compared with $ stripped, so a plain QN rename does not count as churn.

	before	after	delta
nodes	28,121	28,987	+866
companion (Foo$) Class nodes	0	815	every one has its class sibling
DEFINES	24,738	25,604	+866 (members that used to overwrite each other)
DEFINES_METHOD	10,198	10,248	+50
USAGE	56,068	56,461	+393
CALLS	30,329	30,323	−6
IMPORTS	6,343	6,341	−2

CALLS into companion members after: 4,938. Strategy mix is stable (import_map 2,730 → 2,724; same_module 1,419 → 1,418; qualified_suffix 1,139 → 1,139) — that is what the registry changes are for; without them same_module fell to 1,323 and qualified_suffix to 388.

Of the 151 removed / 141 added CALLS (normalized), 138 / 120 are the same call re-attributed to a different caller node in the same file — e.g. in DestinationTracing.scala the override def apply inside object ServerDestTracingFilter { def module = new Module1 { … } } used to be filed under the class's ServerDestTracingFilter.apply (L40) because the object and the class were one node; now that scope has no def and the call lands on the file node. The 13 genuinely dropped / 21 genuinely new pairs are mostly self-targeting fixes: Rng.scala nextDouble used to bind to BackoffTest.ZeroRng.nextDouble, now binds to Rng.nextDouble; Announcer.announce moved from AnnouncerTest.TestAnnouncer.announce to Announcer.announce; MessageEncoder.toBuf from postgresql.FrontendMessage.toBuf to its own toBuf; BindingFactory / NameInterpreterTest bind moved from the NameInterpreter trait to LoadedNameInterpreter. One loss I could not recover: HttpServer.java → HttpMuxer.withHandler (Java caller into a Scala companion, resolved by the Java path).

The −2 IMPORTS (3 removed / 1 added): import Mux.Server.SessionF and import Types.Name.Unnamed used to bind because the member lived under the class QN; it now lives under the inner owner's companion (Types.Name$.Unnamed, Mux.Server$.SessionF). The Owner$.member probe only suffixes the top-level owner, so the class-scoped lookup finds nothing and fails closed per #2360's rule. Left as is — probing $ at every nesting level would need the same treatment in the registry, which is out of scope here; tracked in #2426. The third pair is a Java test's already-wrong import of a A type parameter retargeting from Parameter.A to Parameter$.NullParameter.A.

Non-Scala files: 5 added / 7 removed edges, all SEMANTICALLY_RELATED ranking noise between thrift-generated Java (same as in #2360).

Tests

• extraction (393): class+companion owners and methods are distinct; trait companion gets $; standalone object does not; class-body vals keep module QN; extract_scala_companion_detection_is_scoped — an object Foo whose class Foo sits in a different scope (nested inside another object) is not a companion, and a same-scope pair still is.
• pipeline (298): import numeric.Rational + Rational.make(1, 2) → Rational$.make via import_map; import numeric.Rational.fromInt + fromInt(2) → Rational$.fromInt. Red without the pkgmap/registry changes (the member lookup under Rational$ is what finds fromInt); the parallel-resolver twin (pipeline_scala_companion_parallel_import_and_factory_call) pins the same edges through pass_parallel.
• registry (71): Rational.make reaches Rational$.make via import_map, same_module and qualified_suffix; a $-less owner still requires an exact segment; resolve_dollar_owner_is_companion_only_when_registered_from_scala — the plain cbm_registry_add and cbm_registry_add_def from a .java file record no companion, so none of the $ probes fire; resolve_java_ts_dollar_class_next_to_class_is_unchanged — a Java/TS Foo$ class next to Foo leaves Foo.m() resolution byte-identical to main for all three strategies.
• edge_imports (72) unchanged.

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (extraction, pipeline, registry, edge_imports suites; full make -f Makefile.cbm test left to CI)
• Lint passes (lint-format with clang-format 21, lint-no-suppress, lint-memory-core; cppcheck 2.17.1 with the lint-cppcheck flag set clean on the touched files)
• New behavior is covered by a test (reproduce-first for bug fixes)

[github-actions]

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.

[htarnacki]

Rebased onto the reworked #2360 head (09f704d2); this PR is still the single commit 0d720d52 on top of it.

What changed in the rebase:

• pass_pkgmap.c: the scala_symbol_in_namespace companion special-case is gone with that function. The companion object now enters feat(scala): parse import selectors and resolve package-aware imports (#2153) #2360's top-level index under pkg.Foo$, so import pkg.Foo binds the class without any extra rule; only the import pkg.Owner.member lookup keeps the Owner$ → Owner scope order (object members are the importable ones).
• registry.c: the lint-cppcheck duplicateCondition at resolve_import_map — the companion probe and the import_map_suffix fallback were two consecutive if (suffix && suffix[0]) blocks; they are one block now. cppcheck 2.17.1 with the Makefile's flag set reports nothing on the touched files.

Re-measured on finagle against the new #2360 head (table in the PR body updated): 815 companion nodes, DEFINES +866, CALLS 30,317 → 30,311, strategy mix unchanged (import_map 2,711 → 2,705, same_module 1,419 → 1,418, qualified_suffix 1,141 → 1,141). New since the last measurement: IMPORTS −2 — import Types.Name.Unnamed / import Mux.Server.SessionF, where the $ is on the inner owner (Name$, Server$) and the probe only suffixes the top-level one; with #2360's fail-closed member rule they no longer bind to the class. Noted in the body, left as is.

Tests: extraction 386, pipeline 293, registry 69, edge_imports 70, all green; the pipeline companion tests still go red when the Owner$ scope probe is removed.

[DeusData]

Hi @htarnacki, thank you for this one too, and for the clear rebase notes. Using scalac's Foo$ only for real companions is exactly the conservative shape we like: standalone objects keep their QNs, and existing graphs change only where nodes used to collide. Comparing edges with $ stripped, plus the case-by-case account of the re-attributions, answered most of our questions before we asked them. Moving the companion into #2360's top-level index instead of a special case is a nice simplification, and the cppcheck fix is right.

Two requests:

1. Companion detection cost (helpers.c:173). cbm_scala_is_companion_object scans every named sibling in the scope and copies each class/trait/enum sibling's name into the arena. It runs about three times per object_definition (in extract_class_def, and in compute_class_qn in both extract_defs.c and extract_unified.c). That is O(objects x definitions in the scope) per file, with an allocation per comparison. It's fine for hand-written code, but quadratic on large generated files, such as a sealed hierarchy with thousands of case classes and objects in one scope. We keep extraction free of lookup-inside-loop patterns. Could you compare the names in place against the source byte range, and collect a scope's class/trait/enum names once (for example, cached on the walk frame) instead of rescanning for every object?
2. The $ tolerance applies to every language (registry.c:1143-1175, the $ probes in resolve_import_map and resolve_same_module, and receiver_chain_admits). These paths serve every language without an LSP. $ is a legal identifier character in Java and JS/TS (think of the RxJS user$ convention), so the assumption that only Scala emits a $-terminated owner is likely true, but it isn't enforced. We keep changes to cbm_registry_resolve language-scoped for this reason. Could you either gate it to Scala (for example, record companion owner QNs at registration and accept Foo$ only for a known Scala companion), or add a guard test showing that a JS/TS or Java class Foo$ next to Foo leaves Foo.m() resolution unchanged?

Leaving the inner-owner $ case (Types.Name$.Unnamed, Mux.Server$.SessionF) failing closed is fine. Thank you for pointing it out; a small follow-up issue would help us track it. The one red CI job (daemon_runtime_connection_cap_covers_slow_hello_and_stopping_is_terminal on macos-15-intel) is a known timing-sensitive daemon test on our side and unrelated to your change. Since this stacks on #2360, one rework plus one rebase of the stack is probably the easiest path. Thanks again!

[htarnacki]

Thanks — both points are in the new head 8116eaaa (rebased onto #2360's bb3f1cdd, still one commit on top of it).

1. Companion detection cost. cbm_scala_is_companion_object no longer scans or copies. On the first query in a scope it walks that scope's named children once and records every class / trait / enum name as a (scope node id, byte offset, length) triple in a small open-addressing table (CBMScalaCompanionCache in cbm.h, scc_* in helpers.c); the name bytes stay in the source and comparisons are memcmp against the range. A (scope, "") marker records that the scope was scanned, so a scope without any class is not rescanned either. Every later query in that scope — from extract_class_def or either compute_class_qn — is one hash probe, so per file it is O(definitions) total instead of O(objects × definitions). The table lives in the extraction context's scratch arena (falls back to the file arena when there is none), nothing in a CBMFileResult points at it, and it is reset with the context per file. Test: extract_scala_companion_detection_is_scoped — an object Foo whose only class Foo sits inside a different enclosing object is not a companion, while the same-scope pair still is (i.e. the cache is keyed by scope, not by name alone).

2. $ tolerance gated to Scala. Went with the registration route. cbm_registry_add_def (the variant the passes call, which knows the defining file) records a QN in a companions table only when it ends in $, the label is type-like, and the file is .scala by cbm_language_for_filename. companion_known(owner) — one cbm_ht_has on <owner>$ — is now the single gate in front of every $ probe: resolve_import_map (exact and the import_map_suffix walk), resolve_same_module, and qn_seg_matches under qn_tail_matches / receiver_chain_admits. The table is NULL until the first companion, so a Java/JS/TS/Python registry never allocates it and each probe costs one NULL test. Tests:

• resolve_dollar_owner_is_companion_only_when_registered_from_scala — the same Rational / Rational$ / Rational$.make shape as the positive test, but added through the plain cbm_registry_add (no defining file, so nothing is recorded as a companion): none of import_map, same_module, qualified_suffix fire. The $ character alone buys nothing.
• resolve_java_ts_dollar_class_next_to_class_is_unchanged — Foo, Foo$, Foo$.m registered via cbm_registry_add_def from a/Foo.java and, in a second pass, from a/Foo.ts, next to an unrelated Bar.m: Foo.m() gives the exact verdicts main gives (suffix_match on the import path via the bare-name scorer; nothing from same_module; nothing from the qualified-tail path, also with Foo$.m as the unique candidate) — the main baseline for those verdicts was taken with a probe test before the change.

Re-measured on finagle against the new #2360 head (table in the body updated; $-stripped comparison as before): 815 companion nodes, DEFINES +866, USAGE +393, CALLS 30,329 → 30,323, IMPORTS 6,343 → 6,341, strategy mix unchanged (import_map 2,730 → 2,724; same_module 1,419 → 1,418; qualified_suffix 1,139 → 1,139). Of the 151 removed / 141 added CALLS, 138 / 120 are the same call re-attributed to another caller node in the same file; the rest are the self-targeting fixes listed in the body (Rng.nextDouble, Announcer.announce, MessageEncoder.toBuf, LoadedNameInterpreter.bind). The two inner-owner $ imports (Types.Name$.Unnamed, Mux.Server$.SessionF) still fail closed as discussed; follow-up issue: #2426 (with a three-file repro that shows the lost IMPORTS edge and the import_map → unique_name downgrade of the calls).

Tests: extraction 393, pipeline 298 (a parallel-resolver twin of the companion test added), registry 71, edge_imports 72, parallel 74, incremental 163 — green; lint-memory-core and clang-format clean. #2366 is rebased on this head.

[htarnacki]

Rebased onto the new #2360 head (ba1946ce): 8116eaaa → 8109aec1, clean cherry-pick, no code change.

Suites registry 73 / extraction 399 / pipeline 308 / edge_imports 72 / parallel 76 / incremental 163 / importance 13 all pass; memory-core and clang-format clean.

[htarnacki]

CI status on 8109aec1: lint / lint failed, which cascaded into ci-ok — infrastructure, not this change.

• The job never reached a linter: the Install build deps step (apt) hung for 30 minutes and was cancelled by the job timeout; every later step (LLVM, cppcheck, scripts/lint.sh --ci) is skipped. It's the second apt-related runner failure on this stack (the previous one on feat(scala): parse import selectors and resolve package-aware imports (#2153) #2360 was a 403 from packages.microsoft.com).

Locally on this head clang-format on touched files and lint-memory-core (no file grew) are clean. The same lint / lint job passed on both neighbours of this commit in the stack: #2360 (ba1946ce, its parent) and #2366 (a5652eee, which contains this commit verbatim). A maintainer re-run of lint should clear it with no change to the branch.

[htarnacki]

Rebased onto the new #2360 head (14d43883, itself on main 268a9d88): 8109aec1 → 726ee32a, no functional change.

One conflict, in internal/cbm/cbm.h: main added the doc_memo / doc_pod_index fields to CBMExtractCtx where this PR adds scala_companions. Both kept.

Suites registry 75 / extraction 414 / pipeline 325 / edge_imports 78 / parallel 76 / incremental 164 / importance 13 all pass; memory-core and clang-format clean.

[DeusData]

Thanks for addressing the caching and companion-name review points and keeping the branch current. The series direction question on #2360 needs a maintainer answer before more routine rebases; no additional rework is requested here.

[DeusData]

Thanks again for the companion-identity fix and the completed review updates. The series decision on #2360 remains ours; no further routine rebase is requested while that is pending. Sorry for the delay—I’ll follow up soon.