Skip to content

chore(rubocop): configure Naming/PredicateMethod instead of 35 inline disables #477

Description

@ivndev001

What to build

Naming/PredicateMethod is disabled inline 35 times
(grep -rn "Naming/PredicateMethod" lib app test bench), the single most-disabled cop in the repo.
Replace those directives with one bounded configuration block — bounded is the whole point; see the
constraint below, which is what separates this change from switching the cop off.

The 35 sites fall into three classes:

  • Sidekiq-spec'd public API whose boolean return is the contract and whose name is fixed by the
    spec: lib/wurk/queue.rb:65 pause!, :72 unpause!, :105 clear; lib/wurk/job_set.rb:53
    clear, :173 delete_by_value, :182 delete_by_jid; lib/wurk/dead_set.rb:32 trim,
    :68 kill; lib/wurk/job_record.rb:95 delete; lib/wurk/status.rb:97 delete, :112
    write; lib/wurk/unique.rb:101 enable!; lib/wurk/encryption.rb:92 enable;
    lib/wurk/web/enterprise.rb:95 reset; lib/wurk/client/buffered.rb:502 reliable_push!.
  • Bang methods (pause!, unpause!, enable!, reset_after_fork!, reliable_push!).
  • Test/bench polling helpers named wait_for_* / wait_until*11 sites, not 13, across
    test/integration/*.rb and bench/swarm_boot.rb:54. (11 + 24 = the 35 total.)

Six sites are NOT spec-fixed public API — do not claim they are

An earlier version of this issue put all 24 non-wait_ sites in the spec-fixed class. Their own doc
comments refute that for several, so the "none of which can be renamed" claim was wrong:

  • lib/wurk/client/buffered.rb:534 reliable_push_drainer — its comment says "Spec for
    reliable_push (sidekiq-pro.md §5) only requires drain on next push — this is a Wurk extension
    for issue Client outage buffer: local queue while Redis is down #19"
  • lib/wurk/swarm/restart.rb:56 claim_exit — "Reaper hook", referenced only from swarm.rb
  • lib/wurk/web/enterprise.rb:173 set_paused — internal web helper, three references, all in-file
  • lib/wurk/dead_set.rb:55 kill_raw, lib/wurk/flow.rb:179 abandon,
    lib/wurk/leader.rb:92 acquire, lib/wurk/queue_slot.rb:205 acquire / :223 release,
    lib/wurk/client/buffered.rb:154 reset_after_fork! — a word-boundary grep of
    docs/target/sidekiq-{free,pro,ent}.md returns zero hits for each

These keep their site-scoped # rubocop:disable with the existing reason comment. No method is
renamed in this issue; if any site genuinely warrants a rename, file it separately. That closes the
old body's "or, if it is not public API, a method to rename" branch, which contradicted
## Affected paths and would have made this slice runtime-affecting.

The constraint that makes this safe

AllowedMethods matches a bare method name with no path or scope qualifier
(allowed_method?(node.method_name)), so listing generic names like clear, delete, write,
reset, acquire or release exempts those names repo-wide, for every method written from now
on
. AllowBangMethods: true does the same for every future ! method. That turns 35 reviewable,
site-scoped disables into an unbounded silent exemption on a required status checklint (rubocop) is in the live main-protection ruleset, and this repo auto-merges on required checks
alone.

So scope the new block with Include: to the files holding the cited sites, and state the intended
blast radius in the PR ("this exempts N names across M files, not the repo").

The block

Insert after Naming/VariableNumber (.rubocop.yml:100-102). Two mechanical traps:

  1. Enabled: false is NOT an acceptable implementation of this issue, even though
    Minitest/MultipleAssertions: Enabled: false sits twelve lines below as a copyable template. The
    acceptance criteria below are written to catch that.
  2. The cop's default AllowedMethods is ['call'], an array, and .rubocop.yml declares no
    inherit_mode, so your list REPLACES it and drops call. There are ~30 def call sites in
    lib//app/ (middleware chains). Declare inherit_mode: {merge: [AllowedMethods]} or re-list
    call.

Then delete the inline directives for the allowlisted sites only and run rubocop. Check
bundle exec rubocop -V first: AllowBangMethods arrived in rubocop 1.78 and the Gemfile does not
pin rubocop — if the installed version predates it, use an AllowedPatterns entry ending !\z.

Acceptance criteria

  • .rubocop.yml gains a Naming/PredicateMethod: block that does not contain
    Enabled: false, and bundle exec rubocop --show-cops Naming/PredicateMethod reports
    Enabled: true with the AllowBangMethods / AllowedPatterns / AllowedMethods keys set
  • the cop still fires: a scratch def frobnicate; 1 == 1; end added to a linted file raises
    exactly one Naming/PredicateMethod offense, and removing it clears the offense
  • the six non-spec sites named above keep their inline # rubocop:disable and their reason
    comments
  • grep -rn "Naming/PredicateMethod" lib app test bench bin Rakefile returns only those
    remaining site-scoped directives
  • bundle exec rubocop --parallel reports zero offenses
  • no public method is renamed — the Sidekiq::* alias surface and docs/target/*.md are
    untouched, and git diff --name-only origin/main...HEAD lists no file under docs/target/
  • gates green: rubocop, rake test, rake test:parity (bin/check exit 0)

AFK / HITL

AFK.

Affected paths

.rubocop.yml, lib/wurk/**/.rb (directive lines only), test/integration/.rb (directive lines only),
bench/swarm_boot.rb

Verified against

42cc4e4

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    afkagent ships it solo via worktree+PRarea/backendenhancementNew feature or requestsize/4h~half a day — substantial, design work

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions