fix(apex): resolve a qualified heritage clause to its tail type (#3277) - #3278
fix(apex): resolve a qualified heritage clause to its tail type (#3277)#3278Soumyadip2003-AI wants to merge 3 commits into
Conversation
…Labs#3217) `method_re` matched the return type with `[\w<>\[\]]+`, so any signature whose return type carried a dot, a comma, or a space never matched and the method was dropped with no node, no edges, and no warning. That covers the two most common shapes in real Apex: `Database.QueryLocator` (every `Database.Batchable.start()`) and `Map<String, Object>` (the `@AuraEnabled` controller convention), plus nested generics like `List<Map<String, Id>>`. Widen the return type to a single `_TYPE` pattern that admits a namespace-qualified name and generic arguments. Whitespace is admitted only adjacent to a comma, so a statement such as `insert new Account(...)` or `Integer a = 1, b = compute();` still cannot be read as a declaration - extraction of the existing fixtures is byte-identical. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEfivKVYzq1e5E6hJHiuKi
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. No changes could be formally verified in this run.
Graphify review — findings
Adds namespace-qualified and generic type support to the Apex extractor's heritage and method matching. extract_apex now resolves an extends/implements type to its tail segment (Database.Batchable<sObject> → Batchable) via heritage_names, which splits the clause on top-level commas only so generic argument lists like Map<String, Object> stay intact rather than being shredded into fake nodes, and applies the same _TYPE shape to method return types so dotted/generic returns are recognized while statements like insert new Account(...) are no longer misread as method declarations.
Worth a look
- method regex with permissive _TYPE misreads statements like
insert new Account(...)as methods —graphify/extractors/apex.py:116· 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 — 564 functions depend on the 564 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_apex()— 19 callers, 5 callees
Verification — 564 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: 564 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_apex.
The verifier did not have enough to check extract\_apex, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 more finding(s) on lines outside this diff (see the check run).
… type
Review follow-up. The first pass admitted whitespace only next to a comma, so a
return type spaced around the angle brackets themselves - all legal Apex - was
still dropped:
public Map <String, Object> spaceBeforeAngle() # dropped
public List< Account > spacesInside() # dropped
public List < Map< String, Id > > roomy() # dropped
Read the type as segments joined by the type punctuators `<`, `>` and `,`, with
whitespace allowed only adjacent to one of them and never between two bare
words. That separator rule is what still bars an ordinary statement from being
read as a declaration, so a bare `<`/`>` used as a comparison
(`if (a > b) { doIt(x); }`, `while (i < list.size()) { next(); }`) fabricates
nothing; the negative test grew those cases plus a generic cast, and extraction
of the existing fixtures is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UEfivKVYzq1e5E6hJHiuKi
…hify-Labs#3277) `extends (\w+)` and `implements ([\w,\s]+)` both stop at the first `.`, so a namespace-qualified base was truncated to its namespace. Two silent consequences: a node was fabricated for the namespace (`Database`, `Outer`) and the heritage edge pointed at it, and because the capture never reached the comma, every remaining interface in the list was dropped - `implements Database.Batchable<sObject>, Schedulable` lost `Schedulable` entirely. That shape is the canonical scheduled batch job in Salesforce, and the fabricated `Database` node collected an edge from every batch class in the repo. Reuse the `_TYPE` expression for both clauses and route each through a `heritage_names()` helper that splits on top-level commas only - so a generic argument list (`Map<String, Object>`) is not shredded into fragments - then drops generic arguments and takes the tail segment. This is the tail-name treatment Kotlin (Graphify-Labs#1793) and Scala (Graphify-Labs#1794) already use. Interface `extends` goes through the same helper. Extraction of the existing fixtures is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UEfivKVYzq1e5E6hJHiuKi
a5c8ecb to
07f910d
Compare
|
Rebased on the updated #3276, which now carries a whitespace-tolerant On the advisory finding — public class Claim {
public void real() {
insert new Account(Name = 'x');
insert new List<Account>{ a, b };
upsert new Contact(LastName = 'y');
update new Account(Id = i);
}
}Only The mechanism, since "permissive" is the reasonable prior for a widened regex: Same note as on #3276: the trailing "1 more finding(s) on lines outside this diff" is not present in the check-run output or the review-comment payload, so I cannot read it — happy to address it if it can be surfaced. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Fixes Apex heritage and return-type parsing so namespace-qualified and generic types resolve correctly. Heritage clauses now split on top-level commas only and reduce each entry to its tail type name, so implements Database.Batchable<sObject>, Schedulable yields Batchable and Schedulable instead of a spurious Database node and a dropped Schedulable; the shared _TYPE pattern likewise lets method matching accept qualified, generic, array, and whitespace-padded return types (Database.QueryLocator, Map <String, Object>) while still refusing to read statements like insert new Account(...) as declarations.
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 564 functions depend on the 564 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract_apex()— 19 callers, 5 callees
Verification — 564 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: 564 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_apex.
The verifier did not have enough to check extract\_apex, 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: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 more finding(s) on lines outside this diff (see the check run).
Fixes #3277.
The bug
The class and interface regexes match the heritage clause with patterns that admit no dot:
rf"(?:\s+extends\s+(\w+))?(?:\s+implements\s+([\w,\s]+))?\s*\{{?"Both stop at the first
., with two silent consequences:Database,Outer) and the heritage edge points at it. A namespace is not a type, and the real base type gets no node at all.implements Database.Batchable<sObject>, Schedulableis the canonical shape of a scheduled batch job in Salesforce, so this is not an edge case in a real org — and the fabricatedDatabasenode collects animplementsedge from every batch class in the repo, which is mild hub pollution on top of the lost data.Repro
The fix
Both clauses reuse the
_TYPEexpression, and each is routed through oneheritage_names()helper that:</>depth — so an interface list containing a generic argument (implements Map<String, Object>, Schedulable) is not shredded intoMap<String/Object>fragments;The tail-name choice follows the treatment Kotlin (#1793) and Scala (#1794) received, and the unqualified-tail convention the C++ qualified-base handling already uses, so the edge can bind to a locally declared type of that name. Interface
extendsgoes through the same helper rather than keeping its own comma split.Verification
tests/fixtures/sample.cls,sample.trigger) is unchanged — same nodes, same 18 / 2 edges.tests/test_languages.py:test_apex_qualified_heritage_uses_tail_type— a qualifiedextendsplus a three-entryimplementslist containing a comma-bearing generic, asserting the three real bases are linked and thatDatabase,Outer,sObjectandObjectare not fabricated as nodes.test_apex_interface_qualified_extends_uses_tail_type— the same for an interface'sextendslist.5301 passed, 12 skipped. The one failure,tests/test_labeling.py::test_label_communities_batches_when_over_batch_size, reproduces identically on a clean checkout ofv8and is unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01UEfivKVYzq1e5E6hJHiuKi