Vendor CompactBlank cop

This commit is contained in:
Douglas Eichelberger 2024-01-26 13:24:32 -08:00
parent 6472aca1a6
commit f99d39faf9
7 changed files with 236 additions and 8 deletions

View File

@ -56,6 +56,10 @@ Homebrew/Blank:
# Core extensions are not available here: # Core extensions are not available here:
- "Homebrew/rubocops/**/*" - "Homebrew/rubocops/**/*"
- "Homebrew/startup/bootsnap.rb" - "Homebrew/startup/bootsnap.rb"
Homebrew/CompactBlank:
Exclude:
# `blank?` is not necessarily available here:
- "Homebrew/extend/enumerable.rb"
# only used internally # only used internally
Homebrew/MoveToExtendOS: Homebrew/MoveToExtendOS:
@ -216,8 +220,6 @@ Rails:
- "Homebrew/rubocops/**/*" - "Homebrew/rubocops/**/*"
# These relate to ActiveSupport and not other parts of Rails. # These relate to ActiveSupport and not other parts of Rails.
Rails/CompactBlank:
Enabled: true
Rails/Presence: Rails/Presence:
Enabled: true Enabled: true
Rails/SafeNavigationWithBlank: Rails/SafeNavigationWithBlank:

View File

@ -155,7 +155,7 @@ module Cask
end end
def to_args def to_args
@dsl_args.reject(&:blank?) @dsl_args.compact_blank
end end
end end
end end

View File

@ -314,7 +314,7 @@ class GitHubPackages
"org.opencontainers.image.url" => bottle_hash["formula"]["homepage"], "org.opencontainers.image.url" => bottle_hash["formula"]["homepage"],
"org.opencontainers.image.vendor" => org, "org.opencontainers.image.vendor" => org,
"org.opencontainers.image.version" => version, "org.opencontainers.image.version" => version,
}.reject { |_, v| v.blank? } }.compact_blank
manifests = [] manifests = []
end end
@ -369,7 +369,7 @@ class GitHubPackages
architecture: architecture, architecture: architecture,
os: os, os: os,
"os.version" => os_version, "os.version" => os_version,
}.reject { |_, v| v.blank? } }.compact_blank
tar_sha256 = Digest::SHA256.new tar_sha256 = Digest::SHA256.new
Zlib::GzipReader.open(local_file) do |gz| Zlib::GzipReader.open(local_file) do |gz|
@ -391,7 +391,7 @@ class GitHubPackages
"sh.brew.bottle.glibc.version" => glibc_version, "sh.brew.bottle.glibc.version" => glibc_version,
"sh.brew.bottle.size" => local_file_size.to_s, "sh.brew.bottle.size" => local_file_size.to_s,
"sh.brew.tab" => tab.to_json, "sh.brew.tab" => tab.to_json,
}.reject { |_, v| v.blank? } }.compact_blank
annotations_hash = formula_annotations_hash.merge(descriptor_annotations_hash).merge( annotations_hash = formula_annotations_hash.merge(descriptor_annotations_hash).merge(
{ {
@ -399,7 +399,7 @@ class GitHubPackages
"org.opencontainers.image.documentation" => documentation, "org.opencontainers.image.documentation" => documentation,
"org.opencontainers.image.title" => "#{formula_full_name} #{tag}", "org.opencontainers.image.title" => "#{formula_full_name} #{tag}",
}, },
).reject { |_, v| v.blank? }.sort.to_h ).compact_blank.sort.to_h
image_manifest = { image_manifest = {
schemaVersion: 2, schemaVersion: 2,

View File

@ -105,7 +105,7 @@ module Homebrew
end end
content = [content] unless content.is_a?(Array) content = [content] unless content.is_a?(Array)
content.reject(&:blank?).map do |release| content.compact_blank.map do |release|
next if release["draft"] || release["prerelease"] next if release["draft"] || release["prerelease"]
value = T.let(nil, T.untyped) value = T.let(nil, T.untyped)

View File

@ -3,6 +3,7 @@
require_relative "../extend/array" require_relative "../extend/array"
require_relative "blank" require_relative "blank"
require_relative "compact_blank"
require_relative "io_read" require_relative "io_read"
require_relative "move_to_extend_os" require_relative "move_to_extend_os"
require_relative "present" require_relative "present"

View File

@ -0,0 +1,109 @@
# typed: true
# frozen_string_literal: true
module RuboCop
module Cop
module Homebrew
# Checks if collection can be blank-compacted with `compact_blank`.
#
# @note
# It is unsafe by default because false positives may occur in the
# blank check of block arguments to the receiver object.
#
# For example, `[[1, 2], [3, nil]].reject { |first, second| second.blank? }` and
# `[[1, 2], [3, nil]].compact_blank` are not compatible. The same is true for `blank?`.
# This will work fine when the receiver is a hash object.
#
# And `compact_blank!` has different implementations for `Array`, `Hash`, and
# `ActionController::Parameters`.
# `Array#compact_blank!`, `Hash#compact_blank!` are equivalent to `delete_if(&:blank?)`.
# `ActionController::Parameters#compact_blank!` is equivalent to `reject!(&:blank?)`.
# If the cop makes a mistake, autocorrected code may get unexpected behavior.
#
# @example
#
# # bad
# collection.reject(&:blank?)
# collection.reject { |_k, v| v.blank? }
#
# # good
# collection.compact_blank
#
# # bad
# collection.delete_if(&:blank?) # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
# collection.delete_if { |_, v| v.blank? } # Same behavior as `Array#compact_blank!` and `Hash#compact_blank!`
# collection.reject!(&:blank?) # Same behavior as `ActionController::Parameters#compact_blank!`
# collection.reject! { |_k, v| v.blank? } # Same behavior as `ActionController::Parameters#compact_blank!`
#
# # good
# collection.compact_blank!
#
class CompactBlank < Base
include RangeHelp
extend AutoCorrector
MSG = "Use `%<preferred_method>s` instead."
RESTRICT_ON_SEND = [:reject, :delete_if, :reject!].freeze
def_node_matcher :reject_with_block?, <<~PATTERN
(block
(send _ {:reject :delete_if :reject!})
$(args ...)
(send
$(lvar _) :blank?))
PATTERN
def_node_matcher :reject_with_block_pass?, <<~PATTERN
(send _ {:reject :delete_if :reject!}
(block_pass
(sym :blank?)))
PATTERN
def on_send(node)
return unless bad_method?(node)
range = offense_range(node)
preferred_method = preferred_method(node)
add_offense(range, message: format(MSG, preferred_method: preferred_method)) do |corrector|
corrector.replace(range, preferred_method)
end
end
private
def bad_method?(node)
return true if reject_with_block_pass?(node)
if (arguments, receiver_in_block = reject_with_block?(node.parent))
return use_single_value_block_argument?(arguments, receiver_in_block) ||
use_hash_value_block_argument?(arguments, receiver_in_block)
end
false
end
def use_single_value_block_argument?(arguments, receiver_in_block)
arguments.length == 1 && arguments[0].source == receiver_in_block.source
end
def use_hash_value_block_argument?(arguments, receiver_in_block)
arguments.length == 2 && arguments[1].source == receiver_in_block.source
end
def offense_range(node)
end_pos = if node.parent&.block_type? && node.parent&.send_node == node
node.parent.source_range.end_pos
else
node.source_range.end_pos
end
range_between(node.loc.selector.begin_pos, end_pos)
end
def preferred_method(node)
node.method?(:reject) ? "compact_blank" : "compact_blank!"
end
end
end
end
end

View File

@ -0,0 +1,116 @@
# frozen_string_literal: true
require "rubocops/compact_blank"
RSpec.describe RuboCop::Cop::Homebrew::CompactBlank do
subject(:cop) { described_class.new }
it "registers and corrects an offense when using `reject { |e| e.blank? }`" do
expect_offense(<<~RUBY)
collection.reject { |e| e.blank? }
^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank` instead.
RUBY
expect_correction(<<~RUBY)
collection.compact_blank
RUBY
end
it "registers and corrects an offense when using `reject(&:blank?)`" do
expect_offense(<<~RUBY)
collection.reject(&:blank?)
^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank` instead.
RUBY
expect_correction(<<~RUBY)
collection.compact_blank
RUBY
end
it "registers and corrects an offense when using `delete_if { |e| e.blank? }`" do
expect_offense(<<~RUBY)
collection.delete_if { |e| e.blank? }
^^^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead.
RUBY
expect_correction(<<~RUBY)
collection.compact_blank!
RUBY
end
it "registers and corrects an offense when using `delete_if(&:blank?)`" do
expect_offense(<<~RUBY)
collection.delete_if(&:blank?)
^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead.
RUBY
expect_correction(<<~RUBY)
collection.compact_blank!
RUBY
end
it "registers and corrects an offense when using `reject! { |e| e.blank? }`" do
expect_offense(<<~RUBY)
collection.reject! { |e| e.blank? }
^^^^^^^^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead.
RUBY
expect_correction(<<~RUBY)
collection.compact_blank!
RUBY
end
it "registers and corrects an offense when using `reject!(&:blank?)`" do
expect_offense(<<~RUBY)
collection.reject!(&:blank?)
^^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank!` instead.
RUBY
expect_correction(<<~RUBY)
collection.compact_blank!
RUBY
end
it "registers and corrects an offense when using `reject(&:blank?)` in block" do
expect_offense(<<~RUBY)
hash.transform_values { |value| value.reject(&:blank?) }
^^^^^^^^^^^^^^^^ Homebrew/CompactBlank: Use `compact_blank` instead.
RUBY
expect_correction(<<~RUBY)
hash.transform_values { |value| value.compact_blank }
RUBY
end
it "does not register an offense when using `compact_blank`" do
expect_no_offenses(<<~RUBY)
collection.compact_blank
RUBY
end
it "does not register an offense when using `compact_blank!`" do
expect_no_offenses(<<~RUBY)
collection.compact_blank!
RUBY
end
it "does not register an offense when using `reject { |k, v| k.blank? }`" do
expect_no_offenses(<<~RUBY)
collection.reject { |k, v| k.blank? }
RUBY
end
it "does not register an offense when using the receiver of `blank?` is not a block variable" do
expect_no_offenses(<<~RUBY)
def foo(arg)
collection.reject { |_| arg.blank? }
end
RUBY
end
it "does not register an offense when using `reject { |e| e.empty? }`" do
expect_no_offenses(<<~RUBY)
collection.reject { |e| e.empty? }
RUBY
end
end