Add new Rails/RefuteMethods cop

[koic]

Summary

This PR imports a custom cop from rails/rails#32441. The cop name has changed, but behavior is the same.

This cop checks for use of refute method series.

# bad
refute false
refute_empty [1, 2, 3]
refute_equal true, false

# good
assert_not false
assert_not_empty [1, 2, 3]
assert_not_equal true, false

assert_not methods are not minitest's memthods.
It is an aliases defined in Active Support (ActiveSupport::TestCase).
https://github.com/rails/rails/blob/v5.2.0/activesupport/lib/active_support/test_case.rb#L54-L68

So I put this cop in Rails department.

I think this cop has a useful opportunity in Rails applications.

Other Information

In the near future this PR aims to replace CustomCops/RefuteNot cop of rails/rails repo with this Rails/RefuseMethods cop. Thanks @y-yagi and @kamipo who suggested importing this cop, @composerinteralia is the original implementer.

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title
  and description in grammatically correct, complete sentences.
• Run rake default or rake parallel. It executes all tests and RuboCop for itself, and generates the documentation.

[koic]

In Rails applications, I think that it is often that RSpec is adopted. so I'm wondering if I should Enabled: false the default 🤔

Anyway, I think that there is affect on only **/*_test.rb, so Enabled: true is default.
If there is a pragmatic reason for this, I change it default to Enabled: false.

[bogdanvlviv]

note that, this pattern was changed in rails/rails to '**/test/**/*' see rails/rails#32648

[koic]

Thanks for the suggestion. I was wondering whether it was appropriate as a path pattern ('**/test/**/*') for Rails applications because the configuration differs from Rails framework repository layout that has a test subdirectory for each components. There is only a little worry about false positives due to misidentification of test subdirectory path, but this path pattern seems that it can be applied to the Rails applications without problem.

[bbatsov]

Cool! I didn't even know Rails was making use of custom cops!

[jrmhaig]

I realise that this PR is closed now but I only saw this after updating Rubocop. There doesn't appear to be an option to configure Rubocop to prefer refute methods over assert_not. Is this intentional?

[koic]

Yes. This PR imported a custom cop of Rails. It is over as an intention of import.
If you have an idea to provide an option to use refute methods instead of assert_not methods, you can consider such an extension.
However it will be an option as it is different from Rails' style.

[jrmhaig]

Thanks. I see that it is the preferred style in the Rails project itself but I understood that in Rubocop, where there are 'either-or' things like this, there is usually an EnforcedStyle option, such as here:

https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Rails/ActionFilter

[Supernats]

I just discovered this after updating our Rubocop version recently. I followed the discussion as well as I could and couldn't find any reason given for Rails preferring this, and why one is preferred over the other. If anyone has that information, would they mind sharing it with me?

In the absence of any rationale, it seems reasonable to have this disabled by default and allow the Rails project and all others that follow the convention to enable it for their projects.

@koic, would you mind sharing your insight and opinion?

[composerinteralia]

Not sure I can give you the full history here, but assert_not is a method defined in Rails
and refute comes from Minitest.

Rails:
https://github.com/rails/rails/blob/master/activesupport/lib/active_support/testing/assertions.rb#L8

Minitest:
https://github.com/seattlerb/minitest/blob/master/lib/minitest/assertions.rb#L532

[koic]

That coding style convention is documented in Rails Guide.

Use assert_not methods instead of refute.

http://guides.rubyonrails.org/contributing_to_ruby_on_rails.html#follow-the-coding-conventions

[bogdanvlviv]

There is info about assert_not that might answer for your questions. rails/rails@f75addd#commitcomment-2363903

[samjewell]

Am I right that we prefer assert_not over refute by default only because that's what the Rails team prefer? I think their reasons for preferring assert_not are outdated and no longer apply, even to Rails projects.

assert_not was originally added in Dec 2012 to replace assert !foo in this commit, when refute didn't exist (as far as I can tell). refute was then added to Minitest in April 2013 here.

That was 5 years ago. As refute has been in Minitest for over 5 years I think we're safe to move on, and that we don't especially need assert_not for backwards compatibility reasons - at least not as a default for all users of Rubocop with Rails. I also find refute nicer aesthetically, and I think it's preferable to use the method defined in Minitest over one defined in Rails, as it means that tests on Plain Old Ruby Objects can be run faster (without the dependency on Rails).