jared/cc-os

Watch

Fork 0

UnusedPrivateMethod cop flags private hook methods that only a superclass calls #1091

New issue

Closed

opened 2026-10-17 16:49:05 +00:00 by jared · 4 comments

jared commented

2026-10-17 16:49:05 +00:00

Owner

Problem

Sdlc/Structural/UnusedPrivateMethod flags a private method of a superclass when only its subclasses call it. Example: plugin_key is private in cc-architect Operations::Base (plugins/cc-architect/scripts/plugin_config/operations/base.rb:23), and disable.rb:15, enable.rb:12 and audit.rb:15 (all < Base) call it. The cop reports it. This is the last Sdlc offense that blocks the cc-architect worklist, apart from RequireRelativeDensity (#1462).

The other direction already works: a private hook in a subclass that the superclass calls is not flagged (tests/cops/unused_private_method_test.rb:164, :168). The original repro (CommandController and the crc_*/command.rb hooks) no longer exists.

The lookup also needs a root. SuperclassCalls finds the root with [%r{\A.*/lib(?=/|\z)}]. A file with no lib/ ancestor, such as cc-architect scripts/, gets no root, so no related file is read.

Observed

cd plugins/os-sdlc && bundle exec rubocop --cache false --only Sdlc/Structural/UnusedPrivateMethod /home/jared/dev/cc-os/plugins/cc-architect/scripts/plugin_config/operations/base.rb

base.rb:23:7: C: Sdlc/Structural/UnusedPrivateMethod: plugin_key is a private method with no call sites in this file
1 offense detected

Reproduce

cd plugins/os-sdlc && bundle exec rubocop --cache false --only Sdlc/Structural/UnusedPrivateMethod /home/jared/dev/cc-os/plugins/cc-architect/scripts/plugin_config/operations/base.rb

Approach

Extend the existing related-class lookup in cops/structural/unused_private_method/ so it answers in both directions. UnusedDefs#unused_private_defs keeps its one @superclass_calls.call?(def_node) reject.

Subclass direction: a class in the lookup root whose parent_class short name equals the short name of the def's owner class is a subclass. Its CallSites names count as callers of the superclass's private methods. Use CallSites unchanged, the same as the superclass direction.
Cost: read each candidate file as text first, and parse only a file whose text contains < <OwnerShortName>. Do not parse every file under the root.
Root: the nearest lib ancestor, as today. With no lib ancestor, the root is the file's own directory, and the glob is not recursive (<dir>/*.rb). Both directions use the same root.
Message: the cop now reads other files, so "no call sites in this file" is wrong. Change it to "no call sites in this file, its superclass or its subclasses".

Name matching stays by short name, as in the superclass direction today. Qualified-constant matching is out of scope.

Required work:

Change the offense message — check: present its superclass or its subclasses in plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method.rb
Fall back to the file's own directory when no lib ancestor exists — check: absent return false unless @lib_root in plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method/superclass_calls.rb

Acceptance criteria

Given a superclass under lib/ with a private method, and a subclass file in the same root that calls that method, when the cop runs on the superclass file, then it reports nothing.
Given a superclass with no lib/ ancestor and a subclass in the same directory that calls the superclass's private method, when the cop runs on the superclass file, then it reports nothing.
Given a private method in the superclass that no file in the root calls, when the cop runs, then it reports the method, and the message says "no call sites in this file, its superclass or its subclasses".
Given a superclass with no lib/ ancestor and a subclass in a subdirectory of it that calls the superclass's private method, when the cop runs on the superclass file, then it reports the method (the fallback root is not recursive).
Given cc-architect plugins/cc-architect/scripts/plugin_config/operations/base.rb, when the cop runs, then plugin_key is not reported.

Owner sketch

module RuboCop::Cop::Sdlc::Structural
  class UnusedPrivateMethod
    class SuperclassCalls
      def initialize(file_path); end
      def call?(def_node); end   # superclass calls the subclass's private def, or a subclass calls the superclass's private def
    end

    class SuperclassDefinition    # existing: parent class files by name under the root
      def call_names; end
    end

    class SubclassDefinition      # new file, one constant per file: classes under the root whose parent short name is the owner
      def initialize(root_files, owner_name); end
      def call_names; end         # text pre-filter "< Owner", then CallSites over the matching class nodes
    end
  end
end

Retired tests

plugins/os-sdlc/tests/cops/unused_private_method_test.rb#test_flags_private_method_never_called_anywhere_in_file

Non-goals

Qualified-constant matching (namespaced Base collisions).
String-named send calls (#1091 comment 1, item 2).
The cc-architect RequireRelativeDensity offense (#1462).

Origin

Trigger: design audit of #1090, 2026-10-17; re-scoped 2026-10-28 by /os-backlog:ticket-decompose (session 6ee3ad76)
Improvised this session: protected visibility instead of private on CommandController#line and the options hooks (code since removed)
Chain: plugin_key flagged in cc-architect ← cop finds only superclass callers, and only under a lib/ root ← DESIGN (plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method/superclass_calls.rb)
Root candidate: this ticket
Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method/superclass_calls.rb
Session: f1eaf768-77b4-48c7-afde-b0344f519273
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/f1eaf768-77b4-48c7-afde-b0344f519273.jsonl

VERDICT: CREATE
REASON: Existing design defect in the cop giving a false positive on template-method hooks; #1090's protected is only a workaround.
VERDICT: CREATE (2026-10-28 re-check) — the fault is live at cc-architect operations/base.rb:23; no other open ticket has this mechanism.

## Problem `Sdlc/Structural/UnusedPrivateMethod` flags a private method of a superclass when only its subclasses call it. Example: `plugin_key` is private in cc-architect `Operations::Base` (`plugins/cc-architect/scripts/plugin_config/operations/base.rb:23`), and `disable.rb:15`, `enable.rb:12` and `audit.rb:15` (all `< Base`) call it. The cop reports it. This is the last Sdlc offense that blocks the cc-architect worklist, apart from RequireRelativeDensity (#1462). The other direction already works: a private hook in a subclass that the superclass calls is not flagged (`tests/cops/unused_private_method_test.rb:164`, `:168`). The original repro (`CommandController` and the `crc_*/command.rb` hooks) no longer exists. The lookup also needs a root. `SuperclassCalls` finds the root with `[%r{\A.*/lib(?=/|\z)}]`. A file with no `lib/` ancestor, such as cc-architect `scripts/`, gets no root, so no related file is read. ## Observed ``` cd plugins/os-sdlc && bundle exec rubocop --cache false --only Sdlc/Structural/UnusedPrivateMethod /home/jared/dev/cc-os/plugins/cc-architect/scripts/plugin_config/operations/base.rb ``` ``` base.rb:23:7: C: Sdlc/Structural/UnusedPrivateMethod: plugin_key is a private method with no call sites in this file 1 offense detected ``` ## Reproduce ``` cd plugins/os-sdlc && bundle exec rubocop --cache false --only Sdlc/Structural/UnusedPrivateMethod /home/jared/dev/cc-os/plugins/cc-architect/scripts/plugin_config/operations/base.rb ``` ## Approach Extend the existing related-class lookup in `cops/structural/unused_private_method/` so it answers in both directions. `UnusedDefs#unused_private_defs` keeps its one `@superclass_calls.call?(def_node)` reject. 1. Subclass direction: a class in the lookup root whose `parent_class` short name equals the short name of the def's owner class is a subclass. Its `CallSites` names count as callers of the superclass's private methods. Use `CallSites` unchanged, the same as the superclass direction. 2. Cost: read each candidate file as text first, and parse only a file whose text contains `< <OwnerShortName>`. Do not parse every file under the root. 3. Root: the nearest `lib` ancestor, as today. With no `lib` ancestor, the root is the file's own directory, and the glob is not recursive (`<dir>/*.rb`). Both directions use the same root. 4. Message: the cop now reads other files, so "no call sites in this file" is wrong. Change it to "no call sites in this file, its superclass or its subclasses". Name matching stays by short name, as in the superclass direction today. Qualified-constant matching is out of scope. Required work: 1. Change the offense message — check: present `its superclass or its subclasses` in `plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method.rb` 2. Fall back to the file's own directory when no lib ancestor exists — check: absent `return false unless @lib_root` in `plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method/superclass_calls.rb` ## Acceptance criteria 1. Given a superclass under `lib/` with a private method, and a subclass file in the same root that calls that method, when the cop runs on the superclass file, then it reports nothing. 2. Given a superclass with no `lib/` ancestor and a subclass in the same directory that calls the superclass's private method, when the cop runs on the superclass file, then it reports nothing. 3. Given a private method in the superclass that no file in the root calls, when the cop runs, then it reports the method, and the message says "no call sites in this file, its superclass or its subclasses". 4. Given a superclass with no `lib/` ancestor and a subclass in a subdirectory of it that calls the superclass's private method, when the cop runs on the superclass file, then it reports the method (the fallback root is not recursive). 5. Given cc-architect `plugins/cc-architect/scripts/plugin_config/operations/base.rb`, when the cop runs, then `plugin_key` is not reported. ## Owner sketch ```ruby module RuboCop::Cop::Sdlc::Structural class UnusedPrivateMethod class SuperclassCalls def initialize(file_path); end def call?(def_node); end # superclass calls the subclass's private def, or a subclass calls the superclass's private def end class SuperclassDefinition # existing: parent class files by name under the root def call_names; end end class SubclassDefinition # new file, one constant per file: classes under the root whose parent short name is the owner def initialize(root_files, owner_name); end def call_names; end # text pre-filter "< Owner", then CallSites over the matching class nodes end end end ``` ## Retired tests plugins/os-sdlc/tests/cops/unused_private_method_test.rb#test_flags_private_method_never_called_anywhere_in_file ## Non-goals - Qualified-constant matching (namespaced `Base` collisions). - String-named `send` calls (#1091 comment 1, item 2). - The cc-architect RequireRelativeDensity offense (#1462). ## Origin - Trigger: design audit of #1090, 2026-10-17; re-scoped 2026-10-28 by `/os-backlog:ticket-decompose` (session 6ee3ad76) - Improvised this session: protected visibility instead of private on CommandController#line and the options hooks (code since removed) - Chain: `plugin_key` flagged in cc-architect ← cop finds only superclass callers, and only under a `lib/` root ← DESIGN (plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method/superclass_calls.rb) - Root candidate: this ticket - Where: plugins/os-sdlc/lib/os_sdlc/cops/structural/unused_private_method/superclass_calls.rb - Session: f1eaf768-77b4-48c7-afde-b0344f519273 - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/f1eaf768-77b4-48c7-afde-b0344f519273.jsonl VERDICT: CREATE REASON: Existing design defect in the cop giving a false positive on template-method hooks; #1090's protected is only a workaround. VERDICT: CREATE (2026-10-28 re-check) — the fault is live at cc-architect operations/base.rb:23; no other open ticket has this mechanism.

jared added the

labels

2026-10-17 16:49:05 +00:00

jared referenced this issue

2026-10-17 16:51:41 +00:00

Move method to owner: CommandController superclass for the seven commands (copied:defn:8a8c27d3) #1090

jared referenced this issue from a commit

2026-10-17 16:55:28 +00:00

refactor(os-sdlc): CommandController owns the entry contract and the CommandLine build (#1090)

jared referenced this issue

2026-10-25 16:44:44 +00:00

Audit every Sdlc cop against the zero-false-positive standard #1325

jared commented

2026-10-25 21:19:35 +00:00

Author

Owner

Occurrence: Sdlc/Structural/UnusedPrivateMethod: count subclass and string-named calls across the project
Seen: plugins/os-sdlc/lib/os_sdlc/cops/unused_private_method.rb (found by the #1325 zero-false-positive audit)
Run: Session eb98cdcd-6783-46a0-b7df-cf18a8fe2a56
Adds: (1) parent lookup searches only under the nearest lib dir, so hooks called by a parent outside lib/ fire: operations/validate.rb:45,49 via schema_checks.rb:19,28; test/test_list.rb:36 via test/support/plugin_config_test_case.rb:96. (2) send(var) over a string list: plugins/os-adr/lib/adr/record/command_line.rb:8,30-34; fix: count str names as call sites. (3) Parent private helpers called by subclasses: backlog/cli/commands.rb:14-36, cc-architect operations/base.rb:23,62,71.

Occurrence: Sdlc/Structural/UnusedPrivateMethod: count subclass and string-named calls across the project Seen: plugins/os-sdlc/lib/os_sdlc/cops/unused_private_method.rb (found by the #1325 zero-false-positive audit) Run: Session eb98cdcd-6783-46a0-b7df-cf18a8fe2a56 Adds: (1) parent lookup searches only under the nearest lib dir, so hooks called by a parent outside lib/ fire: operations/validate.rb:45,49 via schema_checks.rb:19,28; test/test_list.rb:36 via test/support/plugin_config_test_case.rb:96. (2) send(var) over a string list: plugins/os-adr/lib/adr/record/command_line.rb:8,30-34; fix: count str names as call sites. (3) Parent private helpers called by subclasses: backlog/cli/commands.rb:14-36, cc-architect operations/base.rb:23,62,71.

jared referenced this issue

2026-10-25 21:26:25 +00:00

Audit every Sdlc cop against the zero-false-positive standard #1325

jared referenced this issue from a commit

2026-10-25 21:44:55 +00:00

docs(os-sdlc): zero-false-positive audit of the Sdlc cops (#1325)

jared commented

2026-10-27 21:21:52 +00:00

Author

Owner

Occurrence: UnusedPrivateMethod flags a base-class private helper that only subclasses call
Seen: plugins/cc-architect/scripts/plugin_config/operations/base.rb:27 Base#plugin_key (callers: operations/audit.rb, disable.rb, enable.rb)
Run: fix-lints on target cc-architect, 2026-10-27, session 9f415790-a6bd-4850-8d70-3dd45004ce2b (main f138ee07)
Adds: a second plugin with the superclass-helper direction; this is the last Sdlc offense besides RequireRelativeDensity that keeps the cc-architect worklist from clear. Left in place, no disable.

Occurrence: UnusedPrivateMethod flags a base-class private helper that only subclasses call Seen: plugins/cc-architect/scripts/plugin_config/operations/base.rb:27 Base#plugin_key (callers: operations/audit.rb, disable.rb, enable.rb) Run: fix-lints on target cc-architect, 2026-10-27, session 9f415790-a6bd-4850-8d70-3dd45004ce2b (main f138ee07) Adds: a second plugin with the superclass-helper direction; this is the last Sdlc offense besides RequireRelativeDensity that keeps the cc-architect worklist from clear. Left in place, no disable.

jared referenced this issue

2026-10-28 07:28:39 +00:00

Gate: full-tree lint sweep as a hard suite-gate check #1418

jared commented

2026-10-28 15:54:06 +00:00

Author

Owner

Work started 2026-10-28 via /os-sdlc:implement on branch ticket-1091 (session 6ee3ad76). Body re-scoped by /os-backlog:ticket-decompose first: original repro is gone; scope is the subclass-calls-superclass-private direction plus a non-recursive fallback root.

jared commented

2026-10-28 16:02:44 +00:00

Author

Owner

Resolution

Done: UnusedPrivateMethod now counts receiverless calls in subclasses as callers of a superclass private method (SubclassDefinition, text pre-filter before parse); with no lib ancestor the lookup root is the file's own directory, non-recursive; message names its superclass or its subclasses. Implementation 212, commit bfe8955f on ticket-1091.

Evidence: Pipeline: 10 dispatches, all gates pass, suite-check green (44.9 s suite), contract-auditor pass. rubocop --only Sdlc/Structural/UnusedPrivateMethod on plugins/cc-architect/scripts/plugin_config/operations/base.rb: no offenses (was base.rb:23 plugin_key).

Follow-ups: none; qualified-constant matching and string-named send calls are recorded as Non-goals in the body; the cc-architect RequireRelativeDensity offense is #1462.

## Resolution **Done:** UnusedPrivateMethod now counts receiverless calls in subclasses as callers of a superclass private method (SubclassDefinition, text pre-filter before parse); with no lib ancestor the lookup root is the file's own directory, non-recursive; message names its superclass or its subclasses. Implementation 212, commit bfe8955f on ticket-1091. **Evidence:** Pipeline: 10 dispatches, all gates pass, suite-check green (44.9 s suite), contract-auditor pass. rubocop --only Sdlc/Structural/UnusedPrivateMethod on plugins/cc-architect/scripts/plugin_config/operations/base.rb: no offenses (was base.rb:23 plugin_key). **Follow-ups:** none; qualified-constant matching and string-named send calls are recorded as Non-goals in the body; the cc-architect RequireRelativeDensity offense is #1462.

jared closed this issue

2026-10-28 16:02:44 +00:00

jared referenced this issue from a commit

2026-10-28 16:03:45 +00:00

feat(os-sdlc): Audited POODR boundaries for #1091: subclass-direction lookup placed in a nested SubclassDefinition; contracts protected; both WORK items present. (#1091)

jared referenced this issue from a commit

2026-10-28 16:03:45 +00:00

docs(os-sdlc): #1091 History row