decidim-dev/lib/decidim/dev/rubocop/cop/decidim/admin_callout_antipattern.rb (2)

8-9: extend AutoCorrector declared but no autocorrector provided.

add_offense on line 43 is called without a corrector block, so the cop advertises autocorrect support but doesn't actually correct anything. Either remove extend AutoCorrector or provide a corrector (e.g., a no-op or a TODO).

Proposed fix

-      class AdminCalloutAntipattern < RuboCop::Cop::Base
-        extend AutoCorrector
+      class AdminCalloutAntipattern < RuboCop::Cop::Base

---

48-58: Redundant guard: line 53 is subsumed by line 55.

return true if is_single_word && is_too_short (line 53) can never be reached without line 55 (return true if is_too_short) also being true. The is_single_word conjunct adds nothing—every too-short text is already caught unconditionally two lines later.

If the intent is that short multi-word strings should not be flagged (only short single-word strings), then move line 55 above line 54 and add the is_single_word guard. Otherwise, remove line 53 as dead code.

Option A: Remove redundant line (current behavior preserved)

       def antipattern_text?(text)
         stripped_text = text.gsub(/[[:punct:]]/, "")
         is_single_word = text.strip !~ /\s/
         is_too_short = stripped_text.length < MIN_LENGTH_THRESHOLD

-        return true if is_single_word && is_too_short
         return true if is_single_word && SINGLE_WORD_ANTI_PATTERNS.include?(stripped_text.downcase)
         return true if is_too_short

         false
       end

Option B: If multi-word short strings should be allowed

       def antipattern_text?(text)
         stripped_text = text.gsub(/[[:punct:]]/, "")
         is_single_word = text.strip !~ /\s/
-        is_too_short = stripped_text.length < MIN_LENGTH_THRESHOLD
 
-        return true if is_single_word && is_too_short
+        return true if is_single_word && stripped_text.length < MIN_LENGTH_THRESHOLD
         return true if is_single_word && SINGLE_WORD_ANTI_PATTERNS.include?(stripped_text.downcase)
-        return true if is_too_short
 
         false
       end

decidim-dev/spec/rubocop/decidim/admin_callout_antipattern_spec.rb (1)

10-28: Add a few more edge-case tests to nail down intended behavior.

The happy paths are covered, but some important boundary cases are missing:

1. Short multi-word string (e.g., "all done") — 8 chars stripped, currently flagged by the is_too_short check. Is that intentional? A test would document this decision.
2. Long single word not in the list (e.g., "congratulations") — 15 chars, single word, not in SINGLE_WORD_ANTI_PATTERNS. Should pass; a test would confirm.
3. Single word in the list that is exactly at the threshold (e.g., "successfully" is 12 chars, so < 12 is false — it's caught only by the list match). Worth an explicit boundary test.
4. Non-string argument (e.g., a variable have_admin_callout(msg)) — should be silently ignored.

Suggested additional tests

it "registers an offense for short multi-word callouts" do
  expect_offense(<<~RUBY)
    expect(page).to have_admin_callout("all done")
                                       ^^^^^^^^^^ Anti-pattern detected: avoid generic single-word or very short text in have_admin_callout. Use the full admin flash message, e.g. 'Meeting successfully published'.
  RUBY
end

it "accepts long single-word callouts not in the anti-pattern list" do
  expect_no_offenses(<<~RUBY)
    expect(page).to have_admin_callout("congratulations")
  RUBY
end

it "does not flag non-string arguments" do
  expect_no_offenses(<<~RUBY)
    expect(page).to have_admin_callout(flash_message)
  RUBY
end