Skip to content

fix(lint): visibility-bare-identifier reports the unwrapped occurrence beside a has() guard - #16413

Merged
baozhoutao merged 3 commits into
mainfrom
claude/issue-16118-visibility-bare-identifier-has-occurrence
Sep 6, 2026
Merged

fix(lint): visibility-bare-identifier reports the unwrapped occurrence beside a has() guard#16413
baozhoutao merged 3 commits into
mainfrom
claude/issue-16118-visibility-bare-identifier-has-occurrence

Conversation

@claude

@claude claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Fixes #16118

What changed

visibility-bare-identifier now reports an identifier written bare beside a has() guard in the same visibility predicate. The exclusion a has() argument earns is unchanged in intent — it is a legitimate select target — but it is now keyed to that occurrence instead of spreading over the predicate.

One rule id, one severity, no new rule, no change to visibility-predicate-syntax, visibility-predicate-unknown-function, visibility-predicate-over-budget or the RHS-position rule.

The mechanism, measured — the card's hypothesis is falsified

The card and the triage both read the silence as an exclusion "keyed on the identifier NAME across the whole predicate". Measured on origin/main at de75e407e: there is no such exclusion in the rule at all. namespaceRoots only declares receivers (a.b, a?.b, a['b'], a.exists(…)), and a has() argument is not a receiver — the rule had no has() handling of any kind.

The silence came from the shared oracle. firstUndeclaredReference reads the first error cel-js's checker reports and acts only on a Unknown variable: X message. A bare has(x)has() applied to something that is not a select — fails that check with has() invalid argument, a different class, so everything behind it in the same predicate went unjudged.

Probe against the strict environment the helper builds, raw checker verdict beside the helper's answer:

source checker's first error firstUndeclaredReference
status == "qualified" Unknown variable: status "status"
has(status) && status == "qualified" has() invalid argument null
has(status) && other == "x" has() invalid argument null
other == "x" && has(status) Unknown variable: other "other"
has(record.status) && status == "qualified" Unknown variable: status "status"

Row 3 is what tells the two mechanisms apart: the masked name is not the guarded one. The masking is positional, not name-keyed. Row 4 shows it is order-dependent — the same predicate with its arms swapped reports.

The fix

maskHasCalls(source, ast) replaces every has(…) call with a same-width true literal, using the canonical AST's own start / end spans, and the masked source is what reaches the checker. That does both jobs at once: the argument occurrence is gone (the exclusion, kept, now per-occurrence), and no first error of another class is left to mask the rest.

Three properties the shape is chosen for:

  • It can remove a finding, never invent one. The rewrite only deletes source, so every name the checker can now report is an identifier written outside a has() call in the author's own bytes.
  • The rest of the predicate reaches the checker byte-identical, so the shapes the checker owns — comprehension-macro variables above all — keep the verdict they have with no guard written.
  • A span it cannot locate exactly turns the whole rewrite off, so an AST shape this has not measured costs coverage instead of producing a wrong span.

namespaceRoots and bareRhsOnlyIdentifiers still read the original AST, so the declared list is unmoved.

Red-first

Eight pins added on formStack, run on the tests-only commit before any source change:

× row 2 — the SAME identifier unwrapped beside `has(status)` is reported
× a bare `has()` does not mask a DIFFERENT name behind it either
Tests  2 failed | 6 passed | 144 skipped (152)

The six that passed include both controls the triage asked for: row 1 (status == "qualified", must keep firing) and row 3 (has(status) alone, must stay silent).

Ablation

Implementation committed first, then the occurrence keying reverted in place (maskHasCalls(source, ast) back to source) with a trap-guarded restore:

--- before: with-mask count = 1 ; without-mask count = 0
--- after mutation: with-mask count = 0 ; without-mask count = 1
--- mutated blob = cd7360ff8f22dd3792723515bffee0c886673885 (HEAD was 5fdac4f82049e3524096cd40f793c3547c3905bb)
× row 2 — the SAME identifier unwrapped beside `has(status)` is reported
× a bare `has()` does not mask a DIFFERENT name behind it either
Tests  2 failed | 6 passed | 144 skipped (152)
RESTORED ok (hash 5fdac4f82049e3524096cd40f793c3547c3905bb, git diff HEAD empty)

Exactly the two rows the fix is for go red; rows 1 and 3 and the five other pins stay green, so the pins discriminate and the negative control is not carried by the fix. No rebuild leg: the suite imports the mutated module by relative path inside packages/lint/src, so it resolves to source and not to dist/.

Positive control on a real app

objectstack validate over examples/app-showcase, with the predicate at task.view.ts:368 swapped from record.priority == 'urgent' to has(priority) && priority == 'urgent':

validate exit = 1
  • view "views[0]" · formViews.edit: visibility predicate references `priority` as a bare identifier. …
      rule: visibility-bare-identifier  at views[0].formViews.edit.sections[0].fields[6]
RESTORED ok (hash 660ca568f0f6222cd4eb1decde5475ca4907cd6b, git diff HEAD empty)

That is the card's own shape reaching the real CLI. Unmutated, all four example apps validate at exit 0 — the fix surfaces no pre-existing defect in the examples.

Verification at efd084c42

  • pnpm --filter @objectstack/lint test — 100 files, 3425 passed / 5 skipped, VERDICT command-exit 0
  • pnpm --filter @objectstack/lint typecheckVERDICT command-exit 0
  • pnpm --filter @objectstack/metadata-protocol test — 2414 passed / 10 skipped, VERDICT command-exit 0
  • pnpm --filter @objectstack/cli exec vitest run --project unit — 2453 passed / 6 expected fail, VERDICT command-exit 0. The integration tier is declared to CI: the diff touches no tier-integration file, no bin/ entry and no spawn helper.
  • pnpm lint (repo-wide eslint . --no-inline-config) — exit 0 at this head sha
  • All 54 gate families dispatch-gates.mjs derives for this diff — run, --ran reconciles 54 derived, 54 run, 0 NOT-MEASURED, 0 UNRUN. Two needed a prerequisite before they measured anything (check:docs-transcript-drift and check:dual-build-cjs-loads, both exit 3 / PREREQUISITE NOT MET until the build; green after). check:type-check-debt needs its own documented 6144 MB ceiling — exit 3 under a tighter one, exit 0 at 6144 with none above its recorded number.

The 54 downstream consumers of @objectstack/lint are effectively the whole workspace; CI runs the farm.

Behaviour change

Predicates that used to publish clean now gate: a guarded-but-unprefixed visibleWhen on a view, page component or form section is refused at build, validate and lint alike. That is the fail-open shape the rule exists to catch — such a predicate never evaluates for any record, and an unevaluable visibleWhen on a form surface renders the field and carries its required: true into the console's submit check (card 688). @objectstack/lint patch.

Out of scope

  • The oracle's own masking is filed as formula: firstUndeclaredReference returns null for every undeclared reference behind a first checker error of another class #16412, unassigned — firstUndeclaredReference returns null for every undeclared reference behind a first checker error of another class, and flow-variable-scope.ts:273 plus validate.ts:688 / :713 read it too. This PR repairs only the visibility rule's call site.
  • Open question for the maintainer: a bare has(x) is itself refused by the canonical checker (has() invalid argument) yet parses, so no rule in this family reports it — row 3 is silent because nothing judges it, not because something decided it was fine. The triage pinned row 3 as the negative control and this PR keeps that verdict unchanged; whether some rule should own that source is a separate decision.
  • hotcrm card 1625's local sweep is untouched — that is hotcrm's call.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8


Generated by Claude Code

…tifier`

Red-first pins for the reachability gap: an identifier written bare beside a
`has()` guard in the same visibility predicate publishes clean, while the same
identifier alone gates. Two of the eight rows fail on this commit — the guarded
row and the different-name row that tells positional masking apart from a
name-keyed exclusion — and the `has(status)`-alone row is the negative control
that must stay silent after the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
…to the occurrence

`firstUndeclaredReference` reads the FIRST error cel-js's checker reports and
acts only on `Unknown variable: X`. A bare `has(x)` fails that check with
`has() invalid argument` instead, so a first error of a different class masked
every undeclared reference behind it in the same predicate — the same name and
any other one alike.

Mask each `has(…)` call out of the source before the checker sees it, using the
canonical AST's own spans and a same-width `true` literal. The argument
occurrence is excluded, which is the exclusion it earns as a select target,
while every other occurrence is judged exactly as it would be with no guard
written beside it. The rewrite only deletes source, so it can remove a finding
but never invent one, and the rest of the predicate reaches the checker
byte-identical.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
…identifier fix

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8
@github-actions github-actions Bot added the size/m label Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json de75e407e53af5a54b265261bc886e167626ce98packageMentionDocs.

Which tree this was computed on

This run read content/docs from cba3123a0805d5acdcf44a872f8ae70c75750848 — the merge of head efd084c428c0e6ff7985648a9ff7f532c5c42d3d into base de75e407e53af5a54b265261bc886e167626ce98, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin cba3123a0805d5acdcf44a872f8ae70c75750848 && git checkout cba3123a0805d5acdcf44a872f8ae70c75750848
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin de75e407e53af5a54b265261bc886e167626ce98 efd084c428c0e6ff7985648a9ff7f532c5c42d3d && git checkout -B drift-repro de75e407e53af5a54b265261bc886e167626ce98 && git merge --no-ff efd084c428c0e6ff7985648a9ff7f532c5c42d3d

node scripts/docs-audit/affected-docs.mjs --json de75e407e53af5a54b265261bc886e167626ce98

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Queue note — second group run; one honest shard-5 red so far, no removal — PM seat domain:devx @ objectstack (#6023), session session_01Vbw3RPgdtqesx4azk9SbW8, 2026-09-06T21:41Z.

group run created Test Core (5/6) aggregate outcome
34059823700 21:02:34Z cancelled at 30m15s (21:33:09Z) failure 21:33Z (the #16316 attestation gate) regrouped
34060488115 21:15:07Z running (started 21:15:30Z)

Not this PR's failure: the diff is one lint rule + its test + a changeset; every PR-side row at efd084c42 was green (shard 5 included). Shard 5 is the CLI slice whose predicted time is stale (#16173, pm:awaiting-maintainer for the dataset refresh — the only lever). The queue regrouped rather than removed, so the seat's one re-queue stays unspent (a removal ⇒ one re-queue; a second removal ⇒ park citing #16173). ⛔ No test skipped, no shard retried by hand, no empty commit. Next read 21:50Z.


Generated by Claude Code

Merged via the queue into main with commit 60ff091 Sep 6, 2026
35 checks passed
@baozhoutao
baozhoutao deleted the claude/issue-16118-visibility-bare-identifier-has-occurrence branch September 6, 2026 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lint: visibility-bare-identifier goes silent for an identifier that also appears inside a has() in the same predicate

2 participants