Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement ActiveSupport's Object#blank? directly #16259

Merged
merged 8 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,13 @@
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/hash/except.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/hash/keys.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/hash/slice.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/object/blank.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/object/deep_dup.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/object/duplicable.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/string/exclude.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/string/filters.rb
!**/vendor/bundle/ruby/*/gems/activesupport-*/lib/active_support/core_ext/string/indent.rb

# Ignore partially included gems where we don't need all files
**/vendor/bundle/ruby/*/gems/concurrent-ruby-*/lib/*/*.jar
**/vendor/bundle/ruby/*/gems/i18n-*/lib/i18n/tests*
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The i8n cleanup is unrelated, it should have been included in #15311
Let me know if you'd like a cleaner diff, and I can move this to a separate PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dduugg Fine with me as-is, thanks!

**/vendor/gems/mechanize-*/.*
**/vendor/gems/mechanize-*/*.md
**/vendor/gems/mechanize-*/*.rdoc
Expand All @@ -102,12 +99,14 @@
**/vendor/bundle/ruby/*/gems/coderay-*/
**/vendor/bundle/ruby/*/gems/colorize-*/
**/vendor/bundle/ruby/*/gems/commander-*/
**/vendor/bundle/ruby/*/gems/concurrent-ruby-*/
**/vendor/bundle/ruby/*/gems/diff-lcs-*/
**/vendor/bundle/ruby/*/gems/docile-*/
**/vendor/bundle/ruby/*/gems/ecma-re-validator-*/
**/vendor/bundle/ruby/*/gems/hana-*/
**/vendor/bundle/ruby/*/gems/highline-*/
**/vendor/bundle/ruby/*/gems/hpricot-*/
**/vendor/bundle/ruby/*/gems/i18n-*/
**/vendor/bundle/ruby/*/gems/jaro_winkler-*/
**/vendor/bundle/ruby/*/gems/json-*/
**/vendor/bundle/ruby/*/gems/json_schemer-*/
Expand Down
3 changes: 3 additions & 0 deletions Library/.rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ Rails/Presence:
Enabled: true
Rails/Present:
Enabled: true
Exclude:
# `present?` is defined as `!blank?` wihin this file
- "Homebrew/extend/blank.rb"
Rails/RelativeDateConstant:
Enabled: true
Rails/SafeNavigation:
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# typed: strict
# frozen_string_literal: true

require "concurrent/map"
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved

class Object
# An object is blank if it's false, empty, or a whitespace string.
# For example, +nil+, '', ' ', [], {}, and +false+ are all blank.
Expand All @@ -13,15 +12,13 @@ class Object
# to
#
# address.blank?
#
# @return [true, false]
sig { returns(T::Boolean) }
def blank?
respond_to?(:empty?) ? !!empty? : !self
respond_to?(:empty?) ? !!T.unsafe(self).empty? : false
dduugg marked this conversation as resolved.
Show resolved Hide resolved
carlocab marked this conversation as resolved.
Show resolved Hide resolved
end

# An object is present if it's not blank.
#
# @return [true, false]
sig { returns(T::Boolean) }
def present?
!blank?
end
Expand All @@ -40,8 +37,7 @@ def present?
# becomes
#
# region = params[:state].presence || params[:country].presence || 'US'
#
# @return [Object]
sig { returns(T.nilable(T.self_type)) }
def presence
self if present?
end
Expand All @@ -51,33 +47,45 @@ class NilClass
# +nil+ is blank:
#
# nil.blank? # => true
#
# @return [true]
sig { returns(TrueClass) }
def blank?
true
end

sig { returns(FalseClass) }
def present? # :nodoc:
false
end
end

class FalseClass
# +false+ is blank:
#
# false.blank? # => true
#
# @return [true]
sig { returns(TrueClass) }
def blank?
true
end

sig { returns(FalseClass) }
def present? # :nodoc:
false
end
end

class TrueClass
# +true+ is not blank:
#
# true.blank? # => false
#
# @return [false]
sig { returns(FalseClass) }
def blank?
false
end

sig { returns(TrueClass) }
def present? # :nodoc:
true
end
end

class Array
Expand All @@ -87,7 +95,12 @@ class Array
# [1,2,3].blank? # => false
#
# @return [true, false]
alias_method :blank?, :empty?
alias blank? empty?

sig { returns(T::Boolean) }
def present? # :nodoc:
!empty?
end
end

class Hash
Expand All @@ -97,14 +110,35 @@ class Hash
# { key: 'value' }.blank? # => false
#
# @return [true, false]
alias_method :blank?, :empty?
alias blank? empty?

sig { returns(T::Boolean) }
def present? # :nodoc:
!empty?
end
end

class Symbol
# A Symbol is blank if it's empty:
#
# :''.blank? # => true
# :symbol.blank? # => false
alias blank? empty?

sig { returns(T::Boolean) }
def present? # :nodoc:
!empty?
end
end

class String
BLANK_RE = /\A[[:space:]]*\z/
ENCODED_BLANKS = Concurrent::Map.new do |h, enc|
BLANK_RE = /\A[[:space:]]*\z/.freeze
# This is a cache that is intentionally mutable
# rubocop:disable Style/MutableConstant
ENCODED_BLANKS_ = T.let(Hash.new do |h, enc|
h[enc] = Regexp.new(BLANK_RE.source.encode(enc), BLANK_RE.options | Regexp::FIXEDENCODING)
end
end, T::Hash[Encoding, Regexp])
# rubocop:enable Style/MutableConstant

# A string is blank if it's empty or contains whitespaces only:
#
Expand All @@ -116,8 +150,7 @@ class String
# Unicode whitespace is supported:
#
# "\u00a0".blank? # => true
#
# @return [true, false]
sig { returns(T::Boolean) }
def blank?
# The regexp that matches blank strings is expensive. For the case of empty
# strings we can speed up this method (~3.5x) with an empty? call. The
Expand All @@ -126,30 +159,43 @@ def blank?
begin
BLANK_RE.match?(self)
rescue Encoding::CompatibilityError
ENCODED_BLANKS[self.encoding].match?(self)
T.must(ENCODED_BLANKS_[encoding]).match?(self)
end
end

sig { returns(T::Boolean) }
def present? # :nodoc:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR uses the latest version of the code, bc we're no longer contained by version compatibility of ActiveSupport as a whole. This includes changes such as rails/rails#49909, captured here.

!blank?
end
end

class Numeric #:nodoc:
class Numeric # :nodoc:
# No number is blank:
#
# 1.blank? # => false
# 0.blank? # => false
#
# @return [false]
sig { returns(FalseClass) }
def blank?
false
end

sig { returns(TrueClass) }
def present?
true
end
end

class Time #:nodoc:
class Time # :nodoc:
# No Time is blank:
#
# Time.now.blank? # => false
#
# @return [false]
sig { returns(FalseClass) }
def blank?
false
end

sig { returns(TrueClass) }
def present?
true
end
end
6 changes: 0 additions & 6 deletions Library/Homebrew/extend/object.rbi

This file was deleted.

2 changes: 1 addition & 1 deletion Library/Homebrew/global.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
require "active_support/core_ext/hash/deep_merge"
require "active_support/core_ext/hash/except"
require "active_support/core_ext/hash/keys"
require "active_support/core_ext/object/blank"
require "active_support/core_ext/string/exclude"
require "active_support/core_ext/string/filters"
require "active_support/core_ext/string/indent"
Expand Down Expand Up @@ -75,6 +74,7 @@
HOMEBREW_BOTTLES_EXTNAME_REGEX = /\.([a-z0-9_]+)\.bottle\.(?:(\d+)\.)?tar\.gz$/.freeze

require "extend/module"
require "extend/blank"
require "env_config"
require "macos_version"
require "os"
Expand Down
50 changes: 50 additions & 0 deletions Library/Homebrew/test/extend/blank_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

require "extend/blank"

describe Object do
let(:empty_true) do
Class.new(described_class) do
def empty?
0
end
end
end
let(:empty_false) do
Class.new(described_class) do
def empty?
nil
end
end
end
let(:blank) { [empty_true.new, nil, false, "", " ", " \n\t \r ", " ", "\u00a0", [], {}] }
let(:present) { [empty_false.new, described_class.new, true, 0, 1, "a", [nil], { nil => 0 }, Time.now] }

describe ".blank?" do
it "checks if an object is blank" do
blank.each { |v| expect(v.blank?).to be true }
present.each { |v| expect(v.blank?).to be false }
end

it "checks if an object is blank with bundled string encodings" do
Encoding.list.reject(&:dummy?).each do |encoding|
expect(" ".encode(encoding).blank?).to be true
expect("a".encode(encoding).blank?).to be false
end
end
end

describe ".present?" do
it "checks if an object is present" do
blank.each { |v| expect(v.present?).to be false }
present.each { |v| expect(v.present?).to be true }
end
end

describe ".presence" do
it "returns the object if present, or nil" do
blank.each { |v| expect(v.presence).to be_nil }
present.each { |v| expect(v.presence).to be v }
end
end
end

This file was deleted.

This file was deleted.

Loading
Loading