feat(lsp): resolve cross-file base classes in the Python/TS cross-LSP - #1908
Merged
Conversation
`CBMDefinition.base_classes` carries the SOURCE SPELLING of each base
("Base", "django.db.Model"): extraction strips keywords and generic
arguments, but it cannot know where the name is declared. The Python and
TS cross-file registrars, however, consume `CBMLSPDef.embedded_types` as
fully-qualified names — py_lookup_attribute and ts_lookup_member feed each
entry straight into cbm_registry_lookup_type.
An unqualified spelling therefore matched nothing declared in ANOTHER
file. `class Child(Base)` in child.py never saw Base in base.py, so a
call to an inherited method through a typed receiver found no member and
fell through to the weak textual cascade, where the receiver-aware guard
(#592/#606) correctly kills it. The member-lookup walk over
embedded_types was already there in both languages; only the names it
walked were unusable across files.
cbm_pxc_collect_all_defs now resolves each base spelling to a project QN
using exactly the inputs pass_semantic uses to draw its INHERITS edge:
the project registry, the declaring module, and the file's import map.
Two properties follow. The LSP's inheritance view is the same relation
the graph records, so the two cannot diverge. And the binding is import-
or same-module-backed rather than a short-name guess, so the CALLS edge
it enables is a supported fact that the weak-member guard keeps.
Weak registry strategies (suffix_match / unique_name / field_type_hint /
fuzzy) are rejected for bases via an explicit drop-list: a base bound
because some project type happens to share its name is precisely the
fabricated relation #606 removed, and inheritance multiplies it — every
inherited member of the wrong base would become a callable target. An
unresolved base keeps its raw spelling and the behaviour that predates
this change, so stdlib and third-party bases are unaffected.
Scoped to the languages whose registrars read embedded_types as QNs
(Python, JS/TS/TSX). Go, JVM, C#, C++ and Rust already qualify their own
embedded types and keep the raw join.
Cost is O(defs) hash lookups plus ONE import map per file: no per-call-
site hierarchy walk, no registry scan, no per-file registry rebuild. The
complexity guard stays linear (nodes 1.93, edges 2.07, per-file defs
2.00 against a files x corpus coupling of ~4).
Tests:
- ts/S6 flips to its own documented condition: was `calls == 0` as a
tripwire recording the gap, now asserts calls >= 1 AND an lsp_ts_*
strategy. Measured: strategy=lsp_ts_method, confidence 0.95.
- python/S6b is new and covers what python/S6 structurally cannot: S6's
`def run(c)` parameter is un-annotated, so no receiver type exists to
inherit through and any binding there is a guess about a name (it
resolves via unique_name at 0.75). S6b supplies the receiver three ways
— annotated parameter, `self` in the subclass, constructor result — and
asserts INHERITS >= 1 plus an lsp_* strategy. Revert-check: all three
fall back to unique_name at 0.75 without this change, lsp_method at
0.90 with it.
- Both rows assert the STRATEGY, not just the edge count: a short-name
guess and a resolved inherited member produce the same count in a
two-file fixture, and only the strategy tells them apart.
- Corrects python/S6's stale comment citing a base_classes extraction
bug; extraction_inheritance is green for Python and TS, and
base_classes now holds clean names.
macOS full suite 7690 passed / 0 failed / 8 skipped (140 suites);
make lint-ci clean.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.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.
Makes the Python/TypeScript cross-LSP resolve cross-file base classes, so a call to an inherited method through a typed receiver resolves via an
lsp_*strategy instead of falling through to the weak textual cascade.The gap was one line, and it was not what the code comments claimed
The base-class walk already existed in both languages —
py_lookup_attribute_depth(py_lsp.c:1059) andts_lookup_member(ts_lsp.c:1702/:1854) both iterateCBMRegisteredType.embedded_types, the same inheritance channel C/C++, Java, PHP and Rust use.What was wrong were the names it walked.
pass_lsp_cross.c:286:base_classesholds what the source says —"Base"— whilepy_register_lsp_defs/ts_register_lsp_defsfeed each entry intocbm_registry_lookup_typeas a qualified name. A base declared in another file therefore never matched:class Child(Base)inchild.pycould not seeBaseinbase.py. Receiver unresolved → weak textual cascade → the member guard correctly kills it.The per-file registrars do qualify bases, but with the current file's module — wrong cross-file, and they never see imported files anyway.
pass_semanticalready resolves this correctly for INHERITS, viaresolve_as_class. The cross-LSP simply never received it, andlsp_crossruns beforesemantic, so those edges do not exist yet to reuse.The stale comment that sent everyone the wrong way
test_lsp_resolution_probe.ccited a known Pythonbase_classesextraction bug as the blocker. That bug is fixed:extraction_inheritanceis 9/9 green across Python, TypeScript and TSX, andbase_classesholds clean names —"Animal","Generic","django.db.Model","React.Component"— with parens,extends/implements, generic subscripts andmetaclass=kwargs already stripped. The names were never dirty. They were unqualified. That comment is replaced.The fix — one generic change, both languages
cbm_pxc_collect_all_defsnow resolves each base spelling to a project QN using exactly the inputspass_semanticuses for INHERITS: project registry, declaring module, and the file's import map. The LSP's inheritance view is therefore the same relation the graph records, so the two cannot diverge.Scoped to Python/JS/TS/TSX. Go, JVM, C#, C++ and Rust qualify their own embedded types and keep the raw join.
Weak strategies are rejected for bases —
suffix_match/unique_name/field_type_hint/fuzzyare dropped via an explicit list mirroring the member guard. A base bound by name-coincidence is the #606 failure multiplied across every inherited member, which is a far worse trade than the member-level case. Unresolved bases keep their raw spelling, so stdlib and third-party bases are untouched. In the fixtures, resolution goes through the import map, not a guess.The probes assert the STRATEGY, not the count
In a two-file fixture a lucky short-name guess and a genuinely resolved inherited member produce the same edge count. Only the strategy separates them, so a new helper counts edges by resolution strategy:
calls == 0tripwirecalls >= 1+lsp_tsassertedlsp_ts_method, conf 0.95INHERITS >= 1+lsp_assertedlsp_method, conf 0.90Revert-check: stashing the source change and rebuilding sends all three S6b receiver shapes — annotated parameter,
selfin the subclass, constructor result — back tounique_name/0.75. With the change they resolvelsp_method/0.90.python/S6 is left alone, and its flip-back note was wrong
python/S6assertscalls == 0and keeps doing so. Despite its name it is not an inheritance case: itsdef run(c)parameter is un-annotated, so no receiver type exists to inherit through, and any binding ofc.describe()is a guess about a name. Its previously-documented flip-back condition —>= 1once the cross-LSP resolves inheritance — was unattainable by construction, and that claim is corrected rather than inherited. S6b covers the real case by supplying the receiver type S6 deliberately withholds.Verification
lsp_resolution_probe88 passed (was 87 — the new test confirmed by name in output, count moved).complexity4/4: nodes 1.93, edges 2.07, per-file defs 2.00, shared-package leg 1.98/2.00 — against the ~4 that a files×corpus coupling produces. Cost is O(defs) hash lookups plus one import map per file; no per-call-site hierarchy walk, no registry scan, no per-file registry rebuild.make lint-ciclean.python/S6's comment, resolved by keeping fix(pipeline): suppress weak Python member calls #1903's assertion and tripwire while correcting its flip-back claim per the above.