Widen QueryMethodReachesWrite to follow calls into same-file classes #1449

Closed
opened 2026-10-27 21:57:24 +00:00 by jared · 6 comments
Owner

Problem

The cop Sdlc/Structural/QueryMethodReachesWrite follows calls only to same-file defs.
A query that calls a write through another same-file class is invisible to it.
The real site still writes from a query behind a class named "lookup": TrackerLookup.new(project).call.

Observed

Analysis: docs/lint-lessons/analyses/sdlc-structural-query-method-reaches-write.md, option B.
The cop sees names of same-file defs only. It does not follow a send into another class in the same file.

Reproduce

n/a: this is a widening of a cop; the matcher is plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb (helpers write_chain.rb, write_chain_offense.rb in the query_method_reaches_write/ folder).

Approach

#1380 shipped (main 311ab69a): QUERY_NAME and WRITE_NAME are anchored, and a def counts as a write when its body sends a write verb.
Follow a send into a class or module defined in the same file, and report when a method of that class reaches a write.

Value rule (judgment-tier ruling, 2026-10-28): the query promise belongs to the caller's name, not to the callee's class name. So the cop follows every same-file class or module, with no class-name list:

  • Const.new(...).m follows to the instance def m in the same-file class whose name is Const (last name segment).
  • Const.m follows to the singleton def self.m in the same-file class or module Const.
  • Inside the followed def, calls with no receiver or receiver self resolve as today.
  • A constant that names no class or module in this file is not followed.

Rule from Jared: every new hit must be a true positive, and no Exclude lists. Carve-outs go in the matcher.
The existing test test_passes_when_the_write_is_reached_only_through_another_class asserts no offense for a same-file TrackerService.new(...).resolve_and_persist. That shape is now a must-fire; the test changes to assert the offense.
WriteChain#body_mutates? and WriteChainOffense#concrete_write_call both scan a def body for a write send. If the change touches both, give that scan one owner.

Required work:

  1. Follow calls into same-file classes in the cop — check: present constructed_class in plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write/call_keys.rb

Acceptance criteria

  1. Given a query-named method that calls Lookup.new(x).call, where Lookup is a class in the same file and its instance method call calls a private method whose body sends save_tracker, when the cop runs, then it reports one offense on the query-named method, and the message names Lookup#call as the first call.
  2. Given a query-named method that calls Lookup.call(x), where Lookup is a module in the same file and its def self.call sends File.write, when the cop runs, then it reports one offense on the query-named method.
  3. Given the source WRITE_THROUGH_ANOTHER_CLASS_SOURCE in the cop test (IssueSource.for calls TrackerService.new(runner: runner).resolve_and_persist(project), same file), when the cop runs, then it reports one offense on IssueSource.for.

Orchestrator check after the run (not a test): run the cop over all of plugins/, including extensionless scripts; every new hit is a true positive and is fixed in this unit. Must-not-fire regression cases (a same-file Lookup#call that only computes; a constant from another file) are added by the orchestrator after the run.

Owner sketch

class RuboCop::Cop::Sdlc::Structural::QueryMethodReachesWrite
  def on_new_investigation          # builds the same-file def index, as today
end

class QueryMethodReachesWrite::WriteChain
  def find                          # BFS over calls; now also follows Const.new(...).m and Const.m
end                                 # into defs of same-file classes/modules

Origin

  • Trigger: Jared ruled "B for four cops" on the inc 7 widenings of the lint-cops track (handoff .cc-os/os/handoffs/2026-10-27-1542-lint-cops-track.md).
  • Improvised this session: none
  • Chain: cop misses a write behind a same-file class ← cop checks only same-file def names ← DESIGN (docs/lint-lessons/analyses/sdlc-structural-query-method-reaches-write.md)
  • Root candidate: none found; searched QueryMethodReachesWrite, query_method_reaches_write, same-file class
  • Where: plugins/os-sdlc/lib/os_sdlc/cops/query_method_reaches_write.rb
  • Session: 9f415790-a6bd-4850-8d70-3dd45004ce2b
  • Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/9f415790-a6bd-4850-8d70-3dd45004ce2b.jsonl
    VERDICT: CREATE
## Problem The cop `Sdlc/Structural/QueryMethodReachesWrite` follows calls only to same-file defs. A query that calls a write through another same-file class is invisible to it. The real site still writes from a query behind a class named "lookup": `TrackerLookup.new(project).call`. ## Observed Analysis: `docs/lint-lessons/analyses/sdlc-structural-query-method-reaches-write.md`, option B. The cop sees names of same-file defs only. It does not follow a send into another class in the same file. ## Reproduce n/a: this is a widening of a cop; the matcher is `plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write.rb` (helpers `write_chain.rb`, `write_chain_offense.rb` in the `query_method_reaches_write/` folder). ## Approach #1380 shipped (main 311ab69a): `QUERY_NAME` and `WRITE_NAME` are anchored, and a def counts as a write when its body sends a write verb. Follow a send into a class or module defined in the same file, and report when a method of that class reaches a write. Value rule (judgment-tier ruling, 2026-10-28): the query promise belongs to the caller's name, not to the callee's class name. So the cop follows every same-file class or module, with no class-name list: - `Const.new(...).m` follows to the instance def `m` in the same-file class whose name is `Const` (last name segment). - `Const.m` follows to the singleton def `self.m` in the same-file class or module `Const`. - Inside the followed def, calls with no receiver or receiver `self` resolve as today. - A constant that names no class or module in this file is not followed. Rule from Jared: every new hit must be a true positive, and no Exclude lists. Carve-outs go in the matcher. The existing test `test_passes_when_the_write_is_reached_only_through_another_class` asserts no offense for a same-file `TrackerService.new(...).resolve_and_persist`. That shape is now a must-fire; the test changes to assert the offense. `WriteChain#body_mutates?` and `WriteChainOffense#concrete_write_call` both scan a def body for a write send. If the change touches both, give that scan one owner. Required work: 1. Follow calls into same-file classes in the cop — check: present `constructed_class` in `plugins/os-sdlc/lib/os_sdlc/cops/structural/query_method_reaches_write/call_keys.rb` ## Acceptance criteria 1. Given a query-named method that calls `Lookup.new(x).call`, where `Lookup` is a class in the same file and its instance method `call` calls a private method whose body sends `save_tracker`, when the cop runs, then it reports one offense on the query-named method, and the message names `Lookup#call` as the first call. 2. Given a query-named method that calls `Lookup.call(x)`, where `Lookup` is a module in the same file and its `def self.call` sends `File.write`, when the cop runs, then it reports one offense on the query-named method. 3. Given the source `WRITE_THROUGH_ANOTHER_CLASS_SOURCE` in the cop test (`IssueSource.for` calls `TrackerService.new(runner: runner).resolve_and_persist(project)`, same file), when the cop runs, then it reports one offense on `IssueSource.for`. Orchestrator check after the run (not a test): run the cop over all of `plugins/`, including extensionless scripts; every new hit is a true positive and is fixed in this unit. Must-not-fire regression cases (a same-file `Lookup#call` that only computes; a constant from another file) are added by the orchestrator after the run. ## Owner sketch ```ruby class RuboCop::Cop::Sdlc::Structural::QueryMethodReachesWrite def on_new_investigation # builds the same-file def index, as today end class QueryMethodReachesWrite::WriteChain def find # BFS over calls; now also follows Const.new(...).m and Const.m end # into defs of same-file classes/modules ``` ## Origin - Trigger: Jared ruled "B for four cops" on the inc 7 widenings of the lint-cops track (handoff `.cc-os/os/handoffs/2026-10-27-1542-lint-cops-track.md`). - Improvised this session: none - Chain: cop misses a write behind a same-file class ← cop checks only same-file def names ← DESIGN (docs/lint-lessons/analyses/sdlc-structural-query-method-reaches-write.md) - Root candidate: none found; searched QueryMethodReachesWrite, query_method_reaches_write, same-file class - Where: plugins/os-sdlc/lib/os_sdlc/cops/query_method_reaches_write.rb - Session: 9f415790-a6bd-4850-8d70-3dd45004ce2b - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/9f415790-a6bd-4850-8d70-3dd45004ce2b.jsonl VERDICT: CREATE
Author
Owner

Blocked by #1380 (verb-position anchoring on the same matchers). Remove waiting when #1380 closes.

Blocked by #1380 (verb-position anchoring on the same matchers). Remove `waiting` when #1380 closes.
Author
Owner

#1380 is closed (main 311ab69a): QUERY_NAME and WRITE_NAME are anchored, and a chain def also counts as a write when its body sends an anchored write verb. #1449 is unblocked.

#1380 is closed (main 311ab69a): QUERY_NAME and WRITE_NAME are anchored, and a chain def also counts as a write when its body sends an anchored write verb. #1449 is unblocked.
Author
Owner

Work started on branch ticket-1449 (worktree .claude/worktrees/ticket-1449), os-sdlc pipeline, target os-sdlc. Ticket body corrected first: the required-work path now has the structural/ folder; ACs are must-fire only; value rule recorded in Approach.

Work started on branch ticket-1449 (worktree .claude/worktrees/ticket-1449), os-sdlc pipeline, target os-sdlc. Ticket body corrected first: the required-work path now has the structural/ folder; ACs are must-fire only; value rule recorded in Approach.
Author
Owner

Follow-up dropped by ticket-skeptic: IssueSource.for still reaches a write through TrackerLookup#call (cross-file, so the widened cop stays silent), but IssueSource.for has no production caller and open #1216 deletes it. TrackerLookup#call still persists for its live caller tracker/forgejo.rb:11; fold that note into #1216.

Follow-up dropped by ticket-skeptic: IssueSource.for still reaches a write through TrackerLookup#call (cross-file, so the widened cop stays silent), but IssueSource.for has no production caller and open #1216 deletes it. TrackerLookup#call still persists for its live caller tracker/forgejo.rb:11; fold that note into #1216.
Author
Owner

Follow-up dropped by ticket-skeptic (speculative hardening): known cop gaps after #1449, none a regression against main and no real site missed. Data.define/Struct.new block defs get owner Object; DefKey keys by last name segment so A::Helper and B::Helper collide (main had the same flaw); module_function/extend self, self.class.m and Const.new into initialize are not followed.

Follow-up dropped by ticket-skeptic (speculative hardening): known cop gaps after #1449, none a regression against main and no real site missed. Data.define/Struct.new block defs get owner Object; DefKey keys by last name segment so A::Helper and B::Helper collide (main had the same flaw); module_function/extend self, self.class.m and Const.new into initialize are not followed.
Author
Owner

Resolution

Done: QueryMethodReachesWrite follows Const.m and Const.new(...).m into same-file classes and modules (value rule: follow every same-file class/module, no class-name list). Defs keyed by owner/kind/name (DefKey, SameFileDefs, CallKeys); a bare/self call falls back to the name-only match main used, so mixin and superclass helpers still resolve. Lesson signal and analysis row updated.

Evidence: main 5ad0d5f1 pushed. os-sdlc suite 2877 runs, 0 failures. Lint clean for the change (5 pre-existing RawFileContentAssertion offenses on main, filed #1456). Probe of the cop over 2752 plugins/ files incl. extensionless scripts: 0 offenses, 0 errors. 18 cop tests incl. must-not-fire (computing Lookup#call, constant from another file) and regressions (class << self helper, Data.define block def crash, mixin/superclass, top-level label). Pipeline impl 208 escalated on a brittle required-work check, hand-finished with one design audit; run abandoned.

Follow-ups: #1456 (lint red on main). Dropped by skeptic, noted on this ticket: IssueSource.for/TrackerLookup write (fold into #1216); cop keying gaps (Data.define owners, last-segment collisions, module_function).

## Resolution **Done:** QueryMethodReachesWrite follows Const.m and Const.new(...).m into same-file classes and modules (value rule: follow every same-file class/module, no class-name list). Defs keyed by owner/kind/name (DefKey, SameFileDefs, CallKeys); a bare/self call falls back to the name-only match main used, so mixin and superclass helpers still resolve. Lesson signal and analysis row updated. **Evidence:** main 5ad0d5f1 pushed. os-sdlc suite 2877 runs, 0 failures. Lint clean for the change (5 pre-existing RawFileContentAssertion offenses on main, filed #1456). Probe of the cop over 2752 plugins/ files incl. extensionless scripts: 0 offenses, 0 errors. 18 cop tests incl. must-not-fire (computing Lookup#call, constant from another file) and regressions (class << self helper, Data.define block def crash, mixin/superclass, top-level label). Pipeline impl 208 escalated on a brittle required-work check, hand-finished with one design audit; run abandoned. **Follow-ups:** #1456 (lint red on main). Dropped by skeptic, noted on this ticket: IssueSource.for/TrackerLookup write (fold into #1216); cop keying gaps (Data.define owners, last-segment collisions, module_function).
jared closed this issue 2026-10-28 07:26:10 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
jared/cc-os#1449
No description provided.