feat: judge the cross-file population, and drain the top of the unmodelled-statement suppression - #153
Merged
Merged
Conversation
…al ids
LINT0004 interpolated a `ResolvedType` with `{:?}`, so a buffer mismatch
reached the user as `Buffer(SymbolId(7))` and a narrowing warning as
`Primitive(Decimal)`. Neither is a type a reader can look up in their own
source, and a symbol id shifts whenever symbol counts change.
Add `ResolvedType::display_abl`, a wrapper that borrows the symbol table and
the schema — the type alone cannot name itself, since a class or buffer carries
a symbol id and a table carries a schema id no symbol table maps. Primitives
render their ABL keyword, a buffer renders the table it is over, an array
renders its element type and extent, and the two lattice bottoms get named
renderings so a future interpolation cannot fall back to a debug print.
Both message sites in the rule go through one helper, and a new test file pins
the property two ways: a battery of sources whose messages must contain no
internal id, and a scan asserting no rule debug-formats a value outside its own
test module.
Closes #152
An inherited member's type was parked in `SymbolTable::inherited_member_types` so that attaching a workspace index could be merged on evidence that it changed no diagnostic. That evidence has been collected; the valve has outlived its purpose. The type now goes onto `Symbol::data_type` at the point the resolve pass synthesizes the member, exactly as a local declaration's would, and the side map and both its accessors are deleted rather than bypassed. A cross-file resolution was already shape-identical to a local one in `references`/`symbols`; it is now shape-identical in the one place the rules read, so an assignment through an inherited member is type-checked like any other. The analyze envelope keeps `data_type_source`, which answers what the type alone cannot — whether the declaration that supplied it is in this file or another one the index reached — and derives it from `symbol_origin` so the two fields cannot disagree about a row. Three suites asserted the silence and now state the answer: the inheritance sweep becomes an enumeration of which scenarios gain a finding and which stay silent, the mismatch test becomes the positive assertion, and the pipeline's firewall becomes the same enumeration one client down. New tests pin the promotion's edges — a `VOID` member stays untyped, a property's type travels the same path as a return type, a three-level chain carries the grandparent's type, and a `:`-qualified call stays deliberately unjudged. BREAKING CHANGE: `SymbolTable::inherited_member_type` and `record_inherited_member_type` are removed; read `Symbol::data_type`. Refs #102
A declaration whose `AS CLASS` name only the workspace index could find kept `ResolvedType::Unknown`, with the resolved class parked in `CrossFileState::indexed_receiver_class` where member resolution could read it and the type lattice could not. `check.rs` did the same job one level up, typing any index-synthesized class symbol as lattice bottom. Both existed so attaching an index could add no finding; both are deleted. The `Foreign` branch of `upgrade_class_types` now writes `ResolvedType::Class` exactly as the `Local` branch does, and `receiver_indexed_class` reaches the same answer through the declaration's own type — one channel, not two. So a primitive assigned into a workspace-class-typed variable, and a `NEW` of a workspace class assigned into a primitive, are the mismatches they always were. A synthesized class symbol also records the supertypes the index read from its header. Without that, `ClassLattice` sees a class that inherits nothing and reports a subclass assigned into its parent-typed variable — a false positive minted by attaching an index, on the one shape inheritance widening exists to allow. Its supertype spans point at the use site in this file, the header being in another one. The assignability suite's two cross-file tests asserted the mechanism, not just the silence, so both now state the mechanism that replaced it: each is still silent, and the lattice is why. BREAKING CHANGE: an index-synthesized class symbol types as `ResolvedType::Class` rather than `ResolvedType::Unknown`. Refs #102
The cross-file suites existed to pin what changed on the day the rules turned onto the cross-file population. Collecting that payoff: the `RUN`/`SHARED` sweep keeps its zero but compares exact codes and spans instead of per-rule counts, so a finding moving between codes can no longer be absorbed; the reason suite gains producer tests that assert which situation mints which reason through a real index, so "searched and absent", "not statically knowable", and "we did not look" cannot collapse into each other. `nothing_produces_the_new_reasons_yet` is renamed `no_index_produces_no_cross_file_reason` and keeps its body. It was misnamed rather than obsolete: it runs with no index attached and asserts that ordinary ABL mints no cross-file reason, which is the invariant the `External` case rests on and outlives the index landing. Module docs say what each suite now asserts, so a reader arriving at an enumeration does not have to reconstruct why it used to be a zero. Refs #102
… all four clients The `symbols` section goes to 4. No row key was added — `data_type` changed meaning for a cross-file row. Its absence used to be a reliable marker for "this row is a cross-file member", because the type was held off the symbol to keep it out of the type lattice; it is populated now, and a consumer branching on the absence would silently change behavior. `data_type_source` is the field to branch on instead, and the module docs say so. The parity table gains a row whose siblings make a finding **appear**. Every cross-file row so far could only get quieter when its siblings arrived — the removed `undefined-symbol` false positive — so `CrossFileEffect` gains a `Judged` arm and `expected_without_siblings` stops being a superset of `expected`. The new row declares both directions on one name: withheld, the inherited call is an `undefined-symbol`; supplied, that finding goes away and the parent's declared `INTEGER` assigned into a `LOGICAL` is a type mismatch. A resolver that stopped resolving trips the first arm; one that resolved without typing trips the second. The containment check keeps its teeth. An addition is allowed only where a `Judged` resolution declares it, and a declared addition that fails to arrive is also a failure — so a finding conjured out of a cross-file resolution nobody wrote down still fails the suite. All four legs, browser included, agree on the new row. Refs #102
`NotFoundInWorkspace` named four situations and only one of them was "absent": a genuine path-search miss, an inherited member the class declares but does not expose here, a member lookup that came up empty against a class the index did answer for, and a file that was located and could not be parsed. That was harmless while every rule skip-listed the reason. It stops being harmless the moment `undefined-symbol` reports an absent name, because the rule would render "no such symbol" over three situations where the symbol demonstrably exists — including one that is a gap in oxabl's own parser rather than anything about the user's code. So the reason splits at the producer: `AbsentFromWorkspace` for the searches that came back empty, `PresentButUnusable` for the other three. A rule then fires on a situation rather than on an enum variant, and no per-rule exception list is needed. Telling a broken file from a missing one needs the index seam to say so, which it could not: `IndexAnswer::NotFound` folded "located but unusable" in by documented design. It gains `Unusable`, and both backends answer it from the `parsed` flag they already had — the batch cache and the language server's salsa queries alike. `ClassLookup` gains the same distinction so the analyze envelope stops reporting a file that is sitting on disk as absent, and the `dependencies` section goes to 2 for the changed reason strings. R17 rides along, derived where `index_loaded` is: an index with no configured search path answers NotFound to everything without having looked anywhere, so a miss stays `External`. That is the truthful answer, and it keeps the browser — which has no filesystem — from disagreeing with the CLI about a diagnostic. Behavior-preserving by construction: both new reasons are skip-listed everywhere, and the A/B across this commit alone is empty. BREAKING CHANGE: `UnresolvedReason::NotFoundInWorkspace` is replaced by `AbsentFromWorkspace` and `PresentButUnusable`; `IndexAnswer` gains `Unusable` and `WorkspaceIndex` gains a defaulted `searches_any_path`. Refs #102
`undefined-symbol` now fires on `AbsentFromWorkspace` as well as `NotInScope`.
ABL cannot reference a symbol or a procedure whose code is not on the PROPATH, so
a `USING`, a `NEW`, or a literal `RUN` target that no configured path supplies is
genuinely undefined rather than merely unseen. The rule's positive `if let` on the
one reason it reported becomes an exhaustive wildcard-free match, so the next
reason has to be decided about here rather than absorbed silently, and the finding
carries a help line naming the search-path configuration — a missing source root
produces the same finding as missing code, and that line is its whole remediation.
The corpus said the first cut over-reported by more than twenty times, in classes
that are all correct ABL, so four narrowings landed with it. Each is a case where
a path search is not the mechanism the AVM would have used, which makes a miss
prove nothing:
- `RUN name IN handle` resolves an entry point in another *running* program;
no file could supply it, so it is `Unknowable`.
- An extension-less `RUN name` means an internal procedure first, including any
registered `SUPER PROCEDURE`'s. oxabl models neither, so the miss is
inconclusive and stays `External`.
- A bare class name searched under a shipped namespace — `USING
Progress.Json.ObjectModel.*` and `NEW SomeShippedClass()` — is checked against the
spellings actually *tried*, not the reference's own, so the AVM's class library
is carved out wherever it is reached from.
- A located file that does not visibly declare the class is `Unusable`, not
absent: the declaration may be spliced in from an `{include}` the index does
not expand, which is an ordinary ABL idiom.
Two real defects surfaced on the way and are fixed here rather than worked
around. A literal `RUN` target accepted a dotted second segment anywhere on the
same line, so `RUN write-header. RUN build-list.` produced the target
`write-header. RUN`; adjacency to the period now distinguishes a dotted name from
a statement terminator. And the name search applied the *walk's* extension set to
a target the author spelled out, so `RUN util/row-count.pp` reported a file
that plainly exists as absent — a `RUN` target may now carry any extension but
`.i`. An index's self-exclusion answers `Unusable` too, so a program that runs
itself persistently is not told its own path is missing.
`check --json` goes to 3: a diagnostic row carries its `help`.
Refs #102
`DELETE OBJECT` was one of the ~30 recognized-but-unmodelled forms: matched by
keyword, skipped to the statement end, and every identifier it passed over
harvested lexically and marked `TOUCHED_BY_UNMODELLED_STATEMENT`. That mark is
per-symbol and file-wide, so one `DELETE OBJECT` silenced the three count-gated
rules for names elsewhere in the file — measured across a large real-world
codebase, it is one of the two forms that dominate the suppression.
It skipped for a real reason: the operand is an expression, so
`Delete { buffer: Identifier }` could not hold `DELETE OBJECT ttbl:HANDLE.` or
`DELETE OBJECT hArray[i].`. Hence a new `StatementKind::DeleteObject` carrying
the target as an `Expression` plus the `NO-ERROR` flag. The resolve pass walks it
like any other expression, so the handle is credited an ordinary read, and
nothing in the statement is marked.
Worth noting what this removes: `DELETE PROCEDURE`, `DELETE WIDGET`, and
`DELETE SERVER` already fell through to a real `Delete` node. Only the `OBJECT`
spelling skipped.
`docs/design/ast-invariants.md` §8 describes the new node in the same commit, per
the repo's standing obligation for a public `oxabl_ast` change.
BREAKING CHANGE: `StatementKind` gains a `DeleteObject` variant; an exhaustive
match over it must handle the new arm.
Refs #102, #136
`COMPILE some/path.p SAVE.` was skipped like any other unmodelled form, which harvested every identifier-shaped token it passed over — the path segments and `SAVE` — and marked each one `TOUCHED_BY_UNMODELLED_STATEMENT`. None of them is a symbol reference: the operand is a file path. So the harvest credited nothing true and suppressed the count-gated rules for any real variable whose name collided with a path word or with `SAVE`, file-wide. Measured over a large real-world codebase, this is the second of the two forms that dominate the suppression, and the one whose share of it is entirely spurious. Head-parsing it would be worse than deleting the harvest: crediting reads inside a file path would invent references that do not exist. So the form keeps emitting `Skipped` — it *was* recognized, which is a different fact from a parse failure — with an empty name list, through a narrow `skipped_stmt_no_names` constructor beside the existing helpers rather than an `Option`-ised parameter at thirty call sites. The `#[must_use]` discipline on the skip helpers is intact. `ast-invariants.md` §8 now records both shapes #136 chooses between: symbol-shaped operands earn a head-parse, path-shaped ones earn an empty name list. Refs #102, #136
…s measured tail The status section and the handoff both described a resolver deliberately walled off from the rules. That wall is gone: three valves deleted, `undefined-symbol` reporting names absent from the configured search paths, and two statement forms drained out of the unmodelled-statement suppression. `CLAUDE.md` now says what the four unresolved reasons license a consumer to claim, which envelope sections moved and why (`symbols` 4 for a *changed meaning* rather than a new key, `dependencies` 2 for split reason strings), and which single valve stays closed — `:`-qualified member access, the larger half of the population and its own piece of work. `HANDOFF.md` gains the session's own section, including the part most worth carrying forward: the first cut of the absent-name rule over-reported by more than twenty times, and every one of those findings was correct ABL. The four narrowings that fixed it are each a case where a path search is not the mechanism the AVM would have used, and the two real defects found on the way are named so nobody re-derives them. #136 keeps its scope but loses its guesswork — the issue carries a measured ranking of the remaining forms and the two-shape taxonomy that decides each one's fix. Refs #102, #136
The comment, test fixture, and handoff examples added alongside the `RUN` target and extension-policy fixes carried incidental names rather than obviously synthetic ones. Swapped for neutral placeholders, which is what every other fixture in these crates uses. No behavior change.
ABL on UNIX takes backslash as an alternative escape character, and it is legal outside a quoted literal — so `ab\cd` names the identifier `abcd`. The lexer treated a backslash there as `Invalid`, which turned an ordinary identifier into a parse error and cost the whole statement. Escape markers are removed from the compiler-visible name while the authored span is preserved, so a diagnostic still underlines what the author wrote. The existing tilde escape and the backslash line continuation are untouched.
Four corrections to the newly-judged cross-file population, each a case where a name was judged on evidence that did not apply to it. A wildcard import of a namespace the AVM ships no longer exempts every type name in the file. The carve-out exists because a *bare* name under such an import could be a shipped class, so a path-search miss proves nothing; a qualified spelling can never be supplied by a wildcard, so one `USING Progress.Lang.*` was silently exempting unrelated qualified typos. `USING ... FROM ASSEMBLY` and `FROM PROPATH` are carried on the AST instead of being parsed and discarded. An assembly-supplied type has no source on any path, so searching for one and reporting the miss made `undefined-symbol` fire at error severity on code whose author had already said where the type comes from. An imported supertype resolves by symbol identity rather than by the spelling in the header, so `CLASS child INHERITS base` under `USING pkg.base` reaches the same class the reference does. Matching by name meant the inheritance edge was missing, and a legal widening assignment was reported as a type mismatch. An overload set is judged by the return type its members agree on. Only a genuine disagreement goes to the lattice bottom. The previous rule erased the type whenever a name appeared more than once in the reachable surface, which is an `OVERRIDE` or an inherited interface contract far more often than an overload — so it unjudged most real OO-ABL, including the inherited-member typing this population was opened to reach.
CLAUDE.md picks up the corrected cross-file behavior, and the agent guides referenced from it (issue tracker, triage labels, domain) are added so those references resolve.
…ides The A/B instrument collected five rules. LINT0004 was not among them — the script was written for suppression work, before cross-file typing existed — so the rule that cross-file type resolution moves reported no delta rather than reporting zero. Adding it surfaced both a 44% increase from the type valves and four findings that were being judged on an arbitrary overload's return type, neither of which was visible before. The second half is about inputs. A collection's most consequential input is the `oxabl.toml` the run discovers, which need not be named on the command line and, for a corpus kept outside the repo, is typically untracked. Two collections days apart can therefore be driven by different configs, and nothing in the resulting JSONL shows it: the ratio still computes, still looks plausible, and attributes a config change to the code change. Each `collect` now writes a manifest of the inputs it ran under — rule set, config hash, an environment-override hash, corpus revision — and `diff` exits 5 rather than comparing two sides that disagree. A side with no manifest still diffs, with a warning, so an older collection is not stranded. The manifest carries only hashes, counts and revisions, never a path or a corpus fact, so it can be published beside a ratio to make that ratio reproducible. Same failure class as the defaulted coverage-key lookup this script already guards: silence that reads as a measurement.
This was referenced Aug 3, 2026
The recorded LINT0001 ratio came from a run driven by an out-of-repo `oxabl.toml` that is discovered by walking up from each analyzed file, so it never appeared on the command line, and was untracked. Two runs days apart used different include paths and nothing in the output said so. Re-measured against a pinned config, and the section now says which figure to cite and why the old one is gone. The instrument enforces this now rather than relying on the reader.
Merged
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
1 participant
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.
Cross-file resolution landed silent by design (#146–#148): an inherited member resolved but its type was parked in a side map, a class-typed declaration resolved but kept
Unknown, and a name searched for across every configured path was skip-listed beside names nobody had looked for. Every one of those valves existed so that attaching a workspace index could be merged on evidence that it changed no diagnostic. That evidence was collected. This opens the valves, judges what comes through, and drains the top of the unmodelled-statement suppression that was blinding the count-gated rules.Ten commits, four phases, each phase gated on an A/B over a large real-world ABL codebase kept outside this repo. Findings keyed on path, line, column, and code; message compared separately, since a message-only change must not read as one removal plus one addition.
What changed, phase by phase
Phase 0 — make the messages readable first. LINT0004 interpolated a
ResolvedTypewith{:?}, so a buffer mismatch reached the user asBuffer(SymbolId(7)).ResolvedType::display_ablpairs a type with the symbol table and the schema — a class or buffer carries aSymbolIdonly the former resolves, a table aTableIdonly the latter does — and rendersCHARACTER,DECIMAL EXTENT 3,buffer Customer. This had to be first: a symbol id in a message shifts whenever symbol counts change, and this PR changes them throughout. Closes #152.A/B: zero behavior deltas. Every rule ×1.000, no location added or removed, every LINT0004 message re-rendered in place.
Phase A — the type valves. An inherited member's declared type goes onto
Symbol::data_type; a declaration whoseAS CLASSname resolves through the index carriesResolvedType::Class;check.rstypes an index-synthesized class as its class. All three side channels are deleted, not bypassed. One addition was needed to make that safe: a synthesized class symbol now records the supertypes the index read from its header, becauseClassLatticeclimbs throughSymbolTable::supertypes, and a class with no entry looks like a class that inherits nothing — which reported a subclass assigned into its parent-typed variable, a false positive on the one shape inheritance widening exists to allow.A/B: one group, no singleton tail —
LONGCHARintoCHARACTERthrough an inherited method, spread over ~0.5% of files. Adjudicated by reading members: each is an inheritedLONGCHAR-returning method called unqualified in a subclass and assigned into a localCHARACTER, where the widening ladder makes the valueLONGCHARand the target truncates. True positives, one reason, atwarn— no file newly carries an error. LINT0004 ×1.429; every other rule ×1.000.Phase A2 — reporting a genuinely absent name. Two commits on purpose. First a pure refactor:
NotFoundInWorkspacenamed four situations and only one was "absent", which is harmless while every rule skip-lists a reason and false the instant one reports it. It splits intoAbsentFromWorkspace(a genuine path-search miss) andPresentButUnusable(an inaccessible inherited member, a member lookup that failed against a class the index did answer for, a file that could not be read or parsed, a file that does not visibly declare its class, and the analysed file itself). Telling a broken file from a missing one required the seam to say so, soIndexAnswergainsUnusable, answered from theparsedflag both backends already had. R17 rides along: an index with no configured path answersNotFoundto everything without having looked anywhere, so a miss there staysExternal— derived besideindex_loadedfrom a defaultedWorkspaceIndex::searches_any_path, which also keeps the browser's file-less export from diverging from the CLI. The A/B across that commit alone is empty, by construction.Then the rule flip:
undefined-symbolfires onAbsentFromWorkspace, at error severity, with a help line naming[workspace.sources].include_paths. The reason match is exhaustive and wildcard-free, so the next variant is a compile error in the rule that would have to judge it.The first cut over-reported by more than twenty times, and every one of those findings was correct ABL. Rather than ship the wave, four narrowings landed with it — each a case where a path search is not the mechanism the AVM itself would use, so a miss proves nothing:
RUN name IN handlereaches an entry point in another running program →Unknowable.RUN namemeans an internal procedure first, including any registeredSUPER PROCEDURE's. oxabl models neither →External.Unusable— its declaration is usually spliced in from an{include}, which the index does not expand.Two real defects surfaced on the way and are fixed rather than worked around: a literal
RUNtarget accepted a dotted second segment anywhere on the same line, soRUN write-header. RUN build-list.produced the targetwrite-header. RUN(adjacency to the period now separates a dotted name from a terminator); and the name search applied the walk's extension set to a path the author spelled out, so a.ppprogram that plainly exists was reported absent (search::ExtensionPolicy— aRUNtarget may carry any extension but.i). An index's self-exclusion answersUnusabletoo, so a program thatRUNs itself persistently is not told its own path is missing.A/B: LINT0001 ×1.019, every other rule ×1.000. All 202 additions were adjudicated, not sampled, by checking whether each named target resolves at its spelled path, exists elsewhere in the tree, or exists nowhere. Zero resolve at their spelled path — there is no case of the search failing to find code that a configured path supplies. 190 of 202 (94%) name code that exists nowhere in the tree: a literal
RUNtarget in 196 of the 202 sites, aNEWof an undeclared class in 5. The remaining 12 name code that exists at a different path than the one written — some a bare name whose file sits one directory below a configured root, some a directory prefix that does not match where the file lives (a stale target, or a PROPATH entry that remaps it). Those are the diagnosable-misconfiguration shape the help line exists for, not mis-resolution. Second leg, one real source root removed from the search paths: the shape is diagnosable — findings cluster by the first path segment of the missing name, so a whole absent directory is visible in the grouping. That leg also exposed thatcheck --jsondroppedhelp, so a machine consumer saw half the diagnostic; added, version 3.Phase B — draining the suppression.
DELETE OBJECTis head-parsed intoStatementKind::DeleteObject { target: Expression, no_error }— its own variant because the operand may bettbl:HANDLEorhArray[i], which is exactly why it skipped, and noteDELETE PROCEDURE/WIDGET/SERVERnever did.COMPILEkeeps emittingSkippedwith an empty name list: its operand is a file path, so head-parsing would credit reads that do not exist, while harvesting suppressed real variables whose names collided with a path segment or withSAVE.A/B: unjudged symbols fell by 55%. Finding deltas confined to LINT0002/LINT0005/LINT0006 — 28 findings, every one a symbol that was previously unjudgeable — with no LINT0004 movement, which would have meant a type valve moved, and no LINT0001 movement.
What is deliberately still silent
Each of these is pinned by a test that names the reason, so the day one opens the suite says what changed:
:-qualified member and method-call expressions.check.rstypes everyMethodCallandMemberAccessasUnknownregardless of what it resolved to, so a cross-file type reaches the lattice only through an unqualified reference. That is the larger half of the population and wants its own evidence — folding it in here would have made this PR's A/Bs unreadable.AS CLASS pkg.Missingdeclaration spelling.DataType::Classis a bareStringwith no span, so there is nothing to underline.AbsentFromWorkspace.External,PresentButUnusable, andUnknowablestay skip-listed by every rule.Evidence and docs
CrossFileEffectgainsJudged(a finding cross-file resolution produces — the withheld answer is no longer a superset of the supplied one) andResolvedFromWorkspaceMiss(a finding that exists only because a path was searched, and so does not arrive on the browser's export). A finding may appear only where aJudgedeffect declares it, and a declared one that fails to arrive is also a failure. All four legs agree on the new rows, browser included.symbols→ 4 with no new key —data_typechanged meaning for a cross-file row, from reliably absent to populated, so a consumer detecting cross-file rows by that absence must branch ondata_type_sourceinstead.dependencies→ 2 for the split reason strings.docs/design/ast-invariants.md§8 records the new node and the two-shape taxonomy in the same commit as the AST change.PUT≈ 73% of what is left) plus the taxonomy that decides each one's fix — symbol-shaped operands earn a head-parse, path-shaped ones earn a deleted harvest.CLAUDE.mdandHANDOFF.mdupdated.Gates:
cargo test(2017 tests),cargo fmt --check,cargo clippy -D warningsall clean. Warm language-server single-edit cycle 2.83ms against the 50ms interactivity gate.Closes #152. Refs #102, #136.