rubocop-minitest icon indicating copy to clipboard operation
rubocop-minitest copied to clipboard

Add `{Assert,Refute}Match` `OnlyRegexpLiteral` config

Open sambostock opened this issue 4 months ago • 0 comments

AssertMatch & RefuteMatch enforce

-assert matcher.match?(object)
+assert_match(matcher, object)

and

-refute matcher.match?(object)
+refute_match(matcher, object)

respectively, which use =~ under the hood. This makes the cops highly susceptible to false positives, in cases where the matcher object responds to match?, but not to =~

class SuitColorMatcher
  BLACK = [:clubs,    :spades]
  RED   = [:diamonds, :hearts]
  SUITS = BLACK + RED

  def initialize(suit)
    raise ArgumentError, suit.inspect unless SUITS.include?(suit)
    @suit = suit
  end

  def match?(other)
    case @suit
    when *BLACK then BLACK.include?(other)
    when *RED   then RED  .include?(other)
    end
  end
end

SuitColorMatcher.new(:clubs).match?(:spades) # => true
SuitColorMatcher.new(:clubs) =~ :spades      # NoMethodError

or cases where the matcher object does also respond to =~, but in an incompatible way

"".match? "" # => true
"" =~ ""     # TypeError

Given RuboCop doesn't have runtime type information, the only case we can reliably identify is the case where a Regexp literal is used.

Therefore, this adds an OnlyRegexpLiteral config (false by default), which consumers can enable if their codebase has many false positives.

This provides a way for consumers to address #310, without changing the cop's behavior for everyone.

As part of making this change, a common module is extracted for the implementations of AssertMatch and RefuteMatch, which are largely identical.


Before submitting the PR make sure the following are checked:

  • [x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • [x] Wrote good commit messages.
  • ~Commit message starts with [Fix #issue-number] (if the related issue exists).~ I wasn't sure about this, since this PR isn't changing/fixing the default behavior, it's just adding an option which allows consumers to fix the behavior.
  • [x] Feature branch is up-to-date with master (if not - rebase it).
  • [x] Squashed related commits together.
  • [x] Added tests.
  • [x] Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • [x] Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details. Likewise, I tagged the change as "new", since it's an new option, and consumers have to opt-in to the new behavior. We could eventually make it the default though.

sambostock avatar Oct 04 '24 20:10 sambostock