UnusedPrivateMethod flags base-class private helpers that subclasses call #990

New issue

Closed

opened 2026-10-11 14:42:45 +00:00 by jared · 1 comment

jared commented

2026-10-11 14:42:45 +00:00

Owner

Problem

The cop UnusedPrivateMethod reports a private helper as unused when only subclasses call it. The rule is mechanically a false positive here. The signal behind it is real, so the cop must not just go silent.

The cop is lib/os_sdlc/cops/unused_private_method.rb (plugin root plugins/os-sdlc). It collects private defs and call sites and symbols from the same file only.
CALLBACK_ALLOWLIST is fixed. The cop has no cross-file or inheritance awareness.
Observed in n8n-api: lib/n8n_api/resources/base.rb:14 build_page is private in Resources::Base. 9 subclass files call it. The cop reports "no call sites in this file".

Jared's ruling: a private inherited helper is a hidden parent/subclass contract. It can point to inverted responsibility. In n8n-api, the builder belonged on the built object.

Approach

Keep the signal. Do not exempt silently.

When a private method has no call sites in its file but the class has subclasses that call it, report a distinct message, for example "called only by subclasses".
Add a lint lesson for that message. It tells the reader to check if the responsibility is inverted, and to move the behavior to the object it builds.
Decide how the cop finds subclass callers (cross-file search or inheritance data). Record the choice.

Required work: none beyond the acceptance criteria.

Acceptance criteria

A private method called only by subclasses in other files gets a distinct message, not "no call sites in this file".
A private method with no callers anywhere still gets the original message.
A lesson exists for the new message and names the inverted-responsibility check.
A test covers the n8n-api shape: a base-class private helper called from subclass files.

Origin

Session: 44cf3646-a8fe-4f26-8550-e92296c57f86
Transcript: /home/jared/.claude/projects/-home-jared-dev-ruby-gems-n8n-api/44cf3646-a8fe-4f26-8550-e92296c57f86.jsonl
Where: /home/jared/dev/ruby-gems/n8n-api (filed from n8n-api; code lives in /home/jared/dev/cc-os/plugins/os-sdlc)

Discoverer: /home/jared/dev/ruby-gems/n8n-api, session 44cf3646-a8fe-4f26-8550-e92296c57f86, 2026-10-11. The cop flags Resources::Base#build_page, which 9 subclasses call.

## Problem The cop `UnusedPrivateMethod` reports a private helper as unused when only subclasses call it. The rule is mechanically a false positive here. The signal behind it is real, so the cop must not just go silent. 1. The cop is `lib/os_sdlc/cops/unused_private_method.rb` (plugin root `plugins/os-sdlc`). It collects private defs and call sites and symbols from the same file only. 2. `CALLBACK_ALLOWLIST` is fixed. The cop has no cross-file or inheritance awareness. 3. Observed in n8n-api: `lib/n8n_api/resources/base.rb:14 build_page` is private in `Resources::Base`. 9 subclass files call it. The cop reports "no call sites in this file". Jared's ruling: a private inherited helper is a hidden parent/subclass contract. It can point to inverted responsibility. In n8n-api, the builder belonged on the built object. ## Approach Keep the signal. Do not exempt silently. 1. When a private method has no call sites in its file but the class has subclasses that call it, report a distinct message, for example "called only by subclasses". 2. Add a lint lesson for that message. It tells the reader to check if the responsibility is inverted, and to move the behavior to the object it builds. 3. Decide how the cop finds subclass callers (cross-file search or inheritance data). Record the choice. Required work: none beyond the acceptance criteria. ## Acceptance criteria 1. A private method called only by subclasses in other files gets a distinct message, not "no call sites in this file". 2. A private method with no callers anywhere still gets the original message. 3. A lesson exists for the new message and names the inverted-responsibility check. 4. A test covers the n8n-api shape: a base-class private helper called from subclass files. ## Origin - Session: 44cf3646-a8fe-4f26-8550-e92296c57f86 - Transcript: /home/jared/.claude/projects/-home-jared-dev-ruby-gems-n8n-api/44cf3646-a8fe-4f26-8550-e92296c57f86.jsonl - Where: /home/jared/dev/ruby-gems/n8n-api (filed from n8n-api; code lives in /home/jared/dev/cc-os/plugins/os-sdlc) -------- **Discoverer:** /home/jared/dev/ruby-gems/n8n-api, session 44cf3646-a8fe-4f26-8550-e92296c57f86, 2026-10-11. The cop flags `Resources::Base#build_page`, which 9 subclasses call.

jared added the

labels

2026-10-11 14:42:45 +00:00

jared referenced this issue

2026-10-11 14:43:59 +00:00

Add lint lessons for the four Sdlc/Structural cops that n8n-api hit #991

jared referenced this issue

2026-10-11 14:44:32 +00:00

os-backlog reads only the first page of Forgejo labels, so issue-create rejects valid scope labels #992

jared commented

2026-10-12 11:19:25 +00:00

Author

Owner

Resolution

Done: Closed as covered (option B). The signal Jared asked to keep reaches the reader: the cop still flags subclass-only private helpers, and the lesson added by #991 names both causes (dead helper, or subclasses in other files call it) and the inverted-responsibility check. AC2 holds today; AC3 is done by #991. AC1 (distinct message) and AC4 are dropped: AC1 needs a cross-file search inside a cop that reads one file at a time.

Evidence: Lesson plugins/os-sdlc/lint-lessons/sdlc-structural-unused-private-method.yml (commit 6f63a09d, #991): sign_of names 'subclasses that call a private helper of their parent'; decision asks 'does this helper do work that belongs to the object it builds or acts on? If yes, move the work there'. The n8n-api case is fixed: commit 39b81c0 removed Resources::Base#build_page and lets each record class build itself.

Follow-ups: none

## Resolution **Done:** Closed as covered (option B). The signal Jared asked to keep reaches the reader: the cop still flags subclass-only private helpers, and the lesson added by #991 names both causes (dead helper, or subclasses in other files call it) and the inverted-responsibility check. AC2 holds today; AC3 is done by #991. AC1 (distinct message) and AC4 are dropped: AC1 needs a cross-file search inside a cop that reads one file at a time. **Evidence:** Lesson plugins/os-sdlc/lint-lessons/sdlc-structural-unused-private-method.yml (commit 6f63a09d, #991): sign_of names 'subclasses that call a private helper of their parent'; decision asks 'does this helper do work that belongs to the object it builds or acts on? If yes, move the work there'. The n8n-api case is fixed: commit 39b81c0 removed Resources::Base#build_page and lets each record class build itself. **Follow-ups:** none

jared closed this issue

2026-10-12 11:19:25 +00:00