Replace Rails/NegateInclude with Inverse/Invertible cops

This commit is contained in:
Douglas Eichelberger 2024-01-19 13:54:52 -08:00
parent e55c14e596
commit 9d081a67cc
7 changed files with 370 additions and 11 deletions

View File

@ -51,6 +51,11 @@ FormulaAuditStrict:
Homebrew:
Enabled: true
Homebrew/Blank:
Exclude:
# Core extensions are not available here:
- "Homebrew/rubocops/**/*"
# only used internally
Homebrew/MoveToExtendOS:
Enabled: false
@ -210,19 +215,10 @@ Rails:
- "Homebrew/rubocops/**/*"
# These relate to ActiveSupport and not other parts of Rails.
Rails/Blank:
Enabled: true
Rails/CompactBlank:
Enabled: true
Rails/NegateInclude:
Enabled: true
Rails/Presence:
Enabled: true
Rails/Present:
Enabled: true
Exclude:
# `present?` is defined as `!blank?` wihin this file
- "Homebrew/extend/blank.rb"
Rails/SafeNavigationWithBlank:
Enabled: true
@ -355,6 +351,15 @@ Style/HashAsLastArrayItem:
- "/**/Formula/**/*.rb"
- "**/Formula/**/*.rb"
Style/InverseMethods:
Exclude:
# Core extensions are not available here:
- "Homebrew/standalone/init.rb"
- "Homebrew/rubocops/**/*"
InverseMethods:
:blank?: :present?
:exclude?: :include?
Style/InvertibleUnlessCondition:
Enabled: true
InverseMethods:
@ -364,6 +369,7 @@ Style/InvertibleUnlessCondition:
:zero?:
# Don't require non-standard `exclude?` (for now at least) - it's not available in every file
:include?:
:blank?: :present?
Style/MutableConstant:
# would rather freeze too much than too little

View File

@ -2,8 +2,10 @@
# frozen_string_literal: true
require_relative "../extend/array"
require_relative "blank"
require_relative "io_read"
require_relative "move_to_extend_os"
require_relative "present"
require_relative "shell_commands"
# formula audit cops

View File

@ -0,0 +1,73 @@
# typed: true
# frozen_string_literal: true
module RuboCop
module Cop
module Homebrew
# Checks for code that can be written with simpler conditionals
# using `Object#blank?`.
#
# @note
# This cop is unsafe autocorrection, because `' '.empty?` returns false,
# but `' '.blank?` returns true. Therefore, autocorrection is not compatible
# if the receiver is a non-empty blank string, tab, or newline meta characters.
#
# @example
# # Converts usages of `nil? || empty?` to `blank?`
#
# # bad
# foo.nil? || foo.empty?
# foo == nil || foo.empty?
#
# # good
# foo.blank?
class Blank < Base
extend AutoCorrector
MSG_NIL_OR_EMPTY = "Use `%<prefer>s` instead of `%<current>s`."
# `(send nil $_)` is not actually a valid match for an offense. Nodes
# that have a single method call on the left hand side
# (`bar || foo.empty?`) will blow up when checking
# `(send (:nil) :== $_)`.
def_node_matcher :nil_or_empty?, <<~PATTERN
(or
{
(send $_ :!)
(send $_ :nil?)
(send $_ :== nil)
(send nil :== $_)
}
{
(send $_ :empty?)
(send (send (send $_ :empty?) :!) :!)
}
)
PATTERN
def on_or(node)
nil_or_empty?(node) do |var1, var2|
return if var1 != var2
message = format(MSG_NIL_OR_EMPTY, prefer: replacement(var1), current: node.source)
add_offense(node, message: message) do |corrector|
autocorrect(corrector, node)
end
end
end
private
def autocorrect(corrector, node)
variable1, _variable2 = nil_or_empty?(node)
range = node.source_range
corrector.replace(range, replacement(variable1))
end
def replacement(node)
node.respond_to?(:source) ? "#{node.source}.blank?" : "blank?"
end
end
end
end
end

View File

@ -0,0 +1,76 @@
# typed: true
# frozen_string_literal: true
module RuboCop
module Cop
module Homebrew
# Checks for code that can be written with simpler conditionals
# using `Object#present?`.
#
# @example
# # Converts usages of `!nil? && !empty?` to `present?`
#
# # bad
# !foo.nil? && !foo.empty?
#
# # bad
# foo != nil && !foo.empty?
#
# # good
# foo.present?
class Present < Base
extend AutoCorrector
MSG_EXISTS_AND_NOT_EMPTY = "Use `%<prefer>s` instead of `%<current>s`."
def_node_matcher :exists_and_not_empty?, <<~PATTERN
(and
{
(send (send $_ :nil?) :!)
(send (send $_ :!) :!)
(send $_ :!= nil)
$_
}
{
(send (send $_ :empty?) :!)
}
)
PATTERN
def on_and(node)
exists_and_not_empty?(node) do |var1, var2|
return if var1 != var2
message = format(MSG_EXISTS_AND_NOT_EMPTY, prefer: replacement(var1), current: node.source)
add_offense(node, message: message) do |corrector|
autocorrect(corrector, node)
end
end
end
def on_or(node)
exists_and_not_empty?(node) do |var1, var2|
return if var1 != var2
add_offense(node, message: MSG_EXISTS_AND_NOT_EMPTY) do |corrector|
autocorrect(corrector, node)
end
end
end
def autocorrect(corrector, node)
variable1, _variable2 = exists_and_not_empty?(node)
range = node.source_range
corrector.replace(range, replacement(variable1))
end
private
def replacement(node)
node.respond_to?(:source) ? "#{node.source}.present?" : "present?"
end
end
end
end
end

View File

@ -2,7 +2,7 @@
# frozen_string_literal: true
# Disable Rails cops, as we haven't required active_support yet.
# rubocop:disable Rails
# rubocop:disable Rails, Homebrew/Blank
homebrew_bootsnap_enabled = ENV["HOMEBREW_NO_BOOTSNAP"].nil? && !ENV["HOMEBREW_BOOTSNAP"].nil?
# we need some development tools to build bootsnap native code
@ -49,4 +49,4 @@ if homebrew_bootsnap_enabled
$stderr.puts "Error: HOMEBREW_BOOTSNAP could not `require \"bootsnap\"`!\n\n"
end
end
# rubocop:enable Rails
# rubocop:enable Rails, Homebrew/Blank

View File

@ -0,0 +1,108 @@
# frozen_string_literal: true
require "rubocops/blank"
describe RuboCop::Cop::Homebrew::Blank do
subject(:cop) { described_class.new }
shared_examples "offense" do |source, correction, message|
it "registers an offense and corrects" do
expect_offense(<<~RUBY, source: source, message: message)
#{source}
^{source} #{message}
RUBY
expect_correction(<<~RUBY)
#{correction}
RUBY
end
end
it "accepts checking nil?" do
expect_no_offenses("foo.nil?")
end
it "accepts checking empty?" do
expect_no_offenses("foo.empty?")
end
it "accepts checking nil? || empty? on different objects" do
expect_no_offenses("foo.nil? || bar.empty?")
end
# Bug: https://github.com/rubocop/rubocop/issues/4171
it "does not break when RHS of `or` is a naked falsiness check" do
expect_no_offenses("foo.empty? || bar")
end
it "does not break when LHS of `or` is a naked falsiness check" do
expect_no_offenses("bar || foo.empty?")
end
# Bug: https://github.com/rubocop/rubocop/issues/4814
it "does not break when LHS of `or` is a send node with an argument" do
expect_no_offenses("x(1) || something")
end
context "when nil or empty" do
it_behaves_like "offense", "foo.nil? || foo.empty?",
"foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of `foo.nil? || foo.empty?`."
it_behaves_like "offense", "nil? || empty?", "blank?", "Homebrew/Blank: Use `blank?` instead of `nil? || empty?`."
it_behaves_like "offense", "foo == nil || foo.empty?",
"foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of `foo == nil || foo.empty?`."
it_behaves_like "offense", "nil == foo || foo.empty?",
"foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of `nil == foo || foo.empty?`."
it_behaves_like "offense", "!foo || foo.empty?", "foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of `!foo || foo.empty?`."
it_behaves_like "offense", "foo.nil? || !!foo.empty?",
"foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of `foo.nil? || !!foo.empty?`."
it_behaves_like "offense", "foo == nil || !!foo.empty?",
"foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of " \
"`foo == nil || !!foo.empty?`."
it_behaves_like "offense", "nil == foo || !!foo.empty?",
"foo.blank?",
"Homebrew/Blank: Use `foo.blank?` instead of " \
"`nil == foo || !!foo.empty?`."
end
context "when checking all variable types" do
it_behaves_like "offense", "foo.bar.nil? || foo.bar.empty?",
"foo.bar.blank?",
"Homebrew/Blank: Use `foo.bar.blank?` instead of " \
"`foo.bar.nil? || foo.bar.empty?`."
it_behaves_like "offense", "FOO.nil? || FOO.empty?",
"FOO.blank?",
"Homebrew/Blank: Use `FOO.blank?` instead of `FOO.nil? || FOO.empty?`."
it_behaves_like "offense", "Foo.nil? || Foo.empty?",
"Foo.blank?",
"Homebrew/Blank: Use `Foo.blank?` instead of `Foo.nil? || Foo.empty?`."
it_behaves_like "offense", "Foo::Bar.nil? || Foo::Bar.empty?",
"Foo::Bar.blank?",
"Homebrew/Blank: Use `Foo::Bar.blank?` instead of " \
"`Foo::Bar.nil? || Foo::Bar.empty?`."
it_behaves_like "offense", "@foo.nil? || @foo.empty?",
"@foo.blank?",
"Homebrew/Blank: Use `@foo.blank?` instead of `@foo.nil? || @foo.empty?`."
it_behaves_like "offense", "$foo.nil? || $foo.empty?",
"$foo.blank?",
"Homebrew/Blank: Use `$foo.blank?` instead of `$foo.nil? || $foo.empty?`."
it_behaves_like "offense", "@@foo.nil? || @@foo.empty?",
"@@foo.blank?",
"Homebrew/Blank: Use `@@foo.blank?` instead of " \
"`@@foo.nil? || @@foo.empty?`."
it_behaves_like "offense", "foo[bar].nil? || foo[bar].empty?",
"foo[bar].blank?",
"Homebrew/Blank: Use `foo[bar].blank?` instead of " \
"`foo[bar].nil? || foo[bar].empty?`."
it_behaves_like "offense", "foo(bar).nil? || foo(bar).empty?",
"foo(bar).blank?",
"Homebrew/Blank: Use `foo(bar).blank?` instead of " \
"`foo(bar).nil? || foo(bar).empty?`."
end
end

View File

@ -0,0 +1,94 @@
# frozen_string_literal: true
require "rubocops/present"
describe RuboCop::Cop::Homebrew::Present do
subject(:cop) { described_class.new }
shared_examples "offense" do |source, correction, message|
it "registers an offense and corrects" do
expect_offense(<<~RUBY, source: source, message: message)
#{source}
^{source} #{message}
RUBY
expect_correction(<<~RUBY)
#{correction}
RUBY
end
end
it "accepts checking nil?" do
expect_no_offenses("foo.nil?")
end
it "accepts checking empty?" do
expect_no_offenses("foo.empty?")
end
it "accepts checking nil? || empty? on different objects" do
expect_no_offenses("foo.nil? || bar.empty?")
end
it "accepts checking existence && not empty? on different objects" do
expect_no_offenses("foo && !bar.empty?")
end
it_behaves_like "offense", "foo && !foo.empty?",
"foo.present?",
"Homebrew/Present: Use `foo.present?` instead of `foo && !foo.empty?`."
it_behaves_like "offense", "!foo.nil? && !foo.empty?",
"foo.present?",
"Homebrew/Present: Use `foo.present?` instead of `!foo.nil? && !foo.empty?`."
it_behaves_like "offense", "!nil? && !empty?", "present?",
"Homebrew/Present: Use `present?` instead of `!nil? && !empty?`."
it_behaves_like "offense", "foo != nil && !foo.empty?",
"foo.present?",
"Homebrew/Present: Use `foo.present?` instead of `foo != nil && !foo.empty?`."
it_behaves_like "offense", "!!foo && !foo.empty?",
"foo.present?",
"Homebrew/Present: Use `foo.present?` instead of `!!foo && !foo.empty?`."
context "when checking all variable types" do
it_behaves_like "offense", "!foo.nil? && !foo.empty?",
"foo.present?",
"Homebrew/Present: Use `foo.present?` instead of " \
"`!foo.nil? && !foo.empty?`."
it_behaves_like "offense", "!foo.bar.nil? && !foo.bar.empty?",
"foo.bar.present?",
"Homebrew/Present: Use `foo.bar.present?` instead of " \
"`!foo.bar.nil? && !foo.bar.empty?`."
it_behaves_like "offense", "!FOO.nil? && !FOO.empty?",
"FOO.present?",
"Homebrew/Present: Use `FOO.present?` instead of " \
"`!FOO.nil? && !FOO.empty?`."
it_behaves_like "offense", "!Foo.nil? && !Foo.empty?",
"Foo.present?",
"Homebrew/Present: Use `Foo.present?` instead of " \
"`!Foo.nil? && !Foo.empty?`."
it_behaves_like "offense", "!@foo.nil? && !@foo.empty?",
"@foo.present?",
"Homebrew/Present: Use `@foo.present?` instead of " \
"`!@foo.nil? && !@foo.empty?`."
it_behaves_like "offense", "!$foo.nil? && !$foo.empty?",
"$foo.present?",
"Homebrew/Present: Use `$foo.present?` instead of " \
"`!$foo.nil? && !$foo.empty?`."
it_behaves_like "offense", "!@@foo.nil? && !@@foo.empty?",
"@@foo.present?",
"Homebrew/Present: Use `@@foo.present?` instead of " \
"`!@@foo.nil? && !@@foo.empty?`."
it_behaves_like "offense", "!foo[bar].nil? && !foo[bar].empty?",
"foo[bar].present?",
"Homebrew/Present: Use `foo[bar].present?` instead of " \
"`!foo[bar].nil? && !foo[bar].empty?`."
it_behaves_like "offense", "!Foo::Bar.nil? && !Foo::Bar.empty?",
"Foo::Bar.present?",
"Homebrew/Present: Use `Foo::Bar.present?` instead of " \
"`!Foo::Bar.nil? && !Foo::Bar.empty?`."
it_behaves_like "offense", "!foo(bar).nil? && !foo(bar).empty?",
"foo(bar).present?",
"Homebrew/Present: Use `foo(bar).present?` instead of " \
"`!foo(bar).nil? && !foo(bar).empty?`."
end
end