Repository navigation
feat(ts): type this.field receivers from class field declarations - #4231
rohit-jsfreaky wants to merge 2 commits into
Conversation
The TS receiver table behind `this.repo.findById()` held constructor parameter properties, `new` locals and typed parameters, but no class fields, which is how most NestJS, Angular and TypeORM classes hold their collaborators (`protected repo: Repo`, `private logger = new Logger()`, `#root: Node`). Those calls got no edge. _ts_receiver_type_table now records non-static class fields: `name: T`, `name?: T`, `T | undefined` / `T | null`, an untyped `new T()` initializer, and `#private` names keyed as the extractor records the receiver. Arrays, generics and two-class unions stay untyped; the Graphify-Labs#2553 origin gate and first-binding-wins are unchanged. JS/TS files bypass the AST cache, so no schema change. Against the TypeScript compiler, 0.9.80 vs this change: NestJS this.x.method() 47.9% -> 77.0% (+102 edges), TypeORM 64.5% -> 79.6% (+91), all 193 confirmed at the call site, 0 lost; changed-files rebuild == full build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8gwEpjajiWHVW3c9smuKf
|
Thanks for the pull request, @rohit-jsfreaky. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. PR-changed functions: 0/1 verified (0 proven, 0 may-equivalent, 0 distinguished) · 1 not verified (1 vacuous).
Not verified on this run: \_ts\_receiver\_type\_table (vacuous: never exercised).
Graphify review — findings
Extends _ts_receiver_type_table so this.field.m() calls resolve when the field is declared on the class, not only injected through the constructor. Instance fields are typed from a bare annotation, a T | undefined/T | null union via _optional_type_ident, or an untyped new Cls() initializer, and #private fields are included. Static fields, arrays, generics and multi-type unions stay untyped and emit no edge.
Worth a look
- Class field receiver types leak across classes —
graphify/extractors/engine.py:2452· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 834 functions depend on the 296 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_extract_generic()— 18 callers, 34 callees - new:
extract_js()— 87 callers, 5 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 10 callees - new:
extract_cpp()— 36 callers, 5 callees - new:
extract_julia()— 19 callers, 8 callees - new:
extract_svelte()— 14 callers, 8 callees - new:
extract_php()— 33 callers, 3 callees - …and 11 more — each is listed as a finding
Verification — 834 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 735 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
30 of 344 test file(s) selected (9%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_build.py— impacttests/test_cjs_module_extension.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_dotnet.py— impacttests/test_extract.py— impacttests/test_extract_php_closures.py— impacttests/test_import_extension_resolution.py— impacttests/test_indirect_call_block_scoped_shadow.py— impacttests/test_indirect_dispatch.py— impacttests/test_indirect_dispatch_assign_return.py— impacttests/test_indirect_dispatch_getattr.py— impacttests/test_js_exported_scalar_bindings.py— impacttests/test_languages.py— impacttests/test_multilang.py— impacttests/test_python_underscore_resolution.py— impacttests/test_rationale.py— impacttests/test_ruby_resolution.py— impacttests/test_scala_context_bounds.py— impacttests/test_scala_self_type.py— impacttests/test_scala_top_level_binding.py— impacttests/test_scala_type_definition.py— impacttests/test_svelte_extraction.py— impacttests/test_swift_computed_properties.py— impacttests/test_swift_protocol_requirements.py— impacttests/test_trailing_newline_not_a_syntax_error.py— impacttests/test_ts_new_expression_calls.py— impacttests/test_ts_receiver_member_calls.py— impact, changed-testtests/test_typescript_module_extensions.py— impacttests/test_vue_extraction.py— impact
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Formal verification
Could not verify: Could not verify \_ts\_receiver\_type\_table.
The verifier did not have enough to check \_ts\_receiver\_type\_table, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 233 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly AttributeError — names the real obstacle, not a sampling gap)
· 19 more finding(s) on lines outside this diff (see the check run).
Class fields went into the per-file receiver table, so two classes in one file with a same-named field of different types typed each other's `this.repo` (`A.run` bound to `RepoB.save`), and a local named like a field borrowed the field's type. Fields now go to a per-class table (`ts_type_table["fields"][Class][field]`), and only a `this.x.m()` call, marked `ts_this_field` on its raw call, reads it, through the caller's own class. Every other receiver uses the file table exactly as before. NestJS +102 and TypeORM +89 edges, all confirmed by the TypeScript compiler at the call site, 0 lost; changed-files rebuild == full build. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B8gwEpjajiWHVW3c9smuKf
Confirmed and fixed in 898cae6. Repro: |
|
Landed in v0.9.81 via an authorship-preserving cherry-pick, so your commit is on |
What does this PR do?
Closes #4230.
_ts_receiver_type_table(graphify/extractors/engine.py) now also reads class fields, sothis.repo.findById()resolves whenrepois declared on the class, not only when it is a constructor parameter property. Fields go into a per-class table (ts_type_table["fields"][Class][field]), not the per-file name table. Only athis.x.m()call (markedts_this_fieldon its raw call) reads it, through the caller's own class, so two classes in one file with a same-named field keep their own types. Field types read:repo: Repo,repo?: Repo,declare readonly repo: Repo->Repo(the existing single-type_identifierrule);repo: Repo | undefined/| null->Repo;logger = new Logger(...)(no annotation) ->Logger;#root: Node-> keyed#root, which is the receiver the extractor already records forthis.#root.insert();staticfields are skipped (this.xinside an instance method never reads them).Arrays, generics and two-class unions stay untyped. Every other receiver (locals, parameters, constructor parameter properties) uses the file table exactly as before. The #2553 origin gate and the confidence (
INFERRED, 0.85) are unchanged.Type of change
Verification & Invariants
Invariant: a
this.x.m()edge is emitted only whenxhas one plain declared class type that the caller's file can see; anything else emits nothing, as before.Persisted state it could invalidate: none. JS/TS files bypass the AST cache (
_JS_CACHE_BYPASS_SUFFIXES), so there is no cache entry to retire.Measured against the TypeScript compiler (ground truth:
checker.getSymbolAtLocationat every call inside every named function), 0.9.80 vs this branch, coldgraphify update <dir> --no-cluster --force:this.x.method()foundpackages/)packages/typeorm/src)src/)"Correct": 95 + 84 match a (caller, callee) pair in the compiler's list directly; the other 7 + 5 did not match a pair by line, so each was checked with the compiler at the exact call site: same file, class and method as the new edge.
Changed-files rebuild vs full build (
watch._rebuild_code, NestJS, changingpackages/core/injector/module-ref.ts): 2173 / 2173 / 2173callsedges, identical.Build time: no difference measurable above this machine's noise. TS extractor CPU time over all 664 NestJS files, 6 passes each: 0.9.80 5.03-6.66 s per pass, this branch 4.86-6.34 s. Full cold build, 7 interleaved runs: medians 9.9 vs 10.7 s with overlapping ranges (9.5-16.4 vs 9.3-16.9 s). The change adds one node-type check to a walk that already visits every node.
this.xlookup).test_typed_class_fields_type_this_field_calls(typed, optional,#private,new-initialized anddeclarefields) fails on 0.9.80;test_class_fields_the_table_cannot_type_emit_no_edge(static, array, union, generic) andtest_class_field_of_an_unimported_type_emits_no_edge(origin gate) pass on both;test_same_named_fields_in_two_classes_of_one_file_keep_their_own_type(A.repo: RepoA,B.repo: RepoBin one file) fails on 0.9.80, where neither call resolves.Limitations:
this.x!.m()(non-null assertion) records no receiver at all, before and after this change, so it stays unresolved;this.x?.m()resolves. Getters (get repo() { ... }) and fields typed by a generic or a type alias are not typed. A field declared on a base class is not used forthis.xin a subclass (fields are read through the caller's own class only).How was this tested?
Project venv (Python 3.12.12, graphify from this branch via PYTHONPATH), Windows 11; TypeScript 5.9.3 for the ground truth:
Graphify-specific checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01B8gwEpjajiWHVW3c9smuKf