From 0ac4c04143e85756a661b07f4b09cf0b023ddb04 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sat, 19 Feb 2022 09:52:02 +0100 Subject: [PATCH] `Pathname.blank?` only returns true for `Pathname.new("")` Fix: https://github.com/rails/rails/issues/44452 We don't ignore blank path strings because as surprising as it is, they are valid paths: ```ruby >> path = Pathname.new(" ") => # >> path.write("test") => 4 >> path.exist? => true ``` Previously it would end up calling `Pathname#empty?` which returned true if the path existed and was an empty directory or file. That behavior was unlikely to be expected. --- activesupport/CHANGELOG.md | 9 +++++++++ .../lib/active_support/core_ext/pathname.rb | 1 + .../active_support/core_ext/pathname/blank.rb | 16 ++++++++++++++++ .../core_ext/pathname/existence.rb | 2 ++ .../test/core_ext/pathname/blank_test.rb | 12 ++++++++++++ 5 files changed, 40 insertions(+) create mode 100644 activesupport/lib/active_support/core_ext/pathname/blank.rb create mode 100644 activesupport/test/core_ext/pathname/blank_test.rb diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8507a00a05..0587735977 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,12 @@ +* `Pathname.blank?` only returns true for `Pathname.new("")` + + Previously it would end up calling `Pathname#empty?` which returned true + if the path existed and was an empty directory or file. + + That behavior was unlikely to be expected. + + *Jean Boussier* + * Deprecate `Notification::Event`'s `#children` and `#parent_of?` * Change default serialization format of `MessageEncryptor` from `Marshal` to `JSON` for Rails 7.1. diff --git a/activesupport/lib/active_support/core_ext/pathname.rb b/activesupport/lib/active_support/core_ext/pathname.rb index 611a43e7e0..10fa1903be 100644 --- a/activesupport/lib/active_support/core_ext/pathname.rb +++ b/activesupport/lib/active_support/core_ext/pathname.rb @@ -1,3 +1,4 @@ # frozen_string_literal: true +require "active_support/core_ext/pathname/blank" require "active_support/core_ext/pathname/existence" diff --git a/activesupport/lib/active_support/core_ext/pathname/blank.rb b/activesupport/lib/active_support/core_ext/pathname/blank.rb new file mode 100644 index 0000000000..a61994a356 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/pathname/blank.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require "pathname" + +class Pathname + # An Pathname is blank if it's empty: + # + # Pathname.new("").blank? # => true + # Pathname.new(" ").blank? # => false + # Pathname.new("test).blank? # => false + # + # @return [true, false] + def blank? + to_s.empty? + end +end diff --git a/activesupport/lib/active_support/core_ext/pathname/existence.rb b/activesupport/lib/active_support/core_ext/pathname/existence.rb index 9bb9b17ab3..848eb4dcec 100644 --- a/activesupport/lib/active_support/core_ext/pathname/existence.rb +++ b/activesupport/lib/active_support/core_ext/pathname/existence.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "pathname" + class Pathname # Returns the receiver if the named file exists otherwise returns +nil+. # pathname.existence is equivalent to diff --git a/activesupport/test/core_ext/pathname/blank_test.rb b/activesupport/test/core_ext/pathname/blank_test.rb new file mode 100644 index 0000000000..19c558803c --- /dev/null +++ b/activesupport/test/core_ext/pathname/blank_test.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require_relative "../../abstract_unit" +require "active_support/core_ext/pathname/blank" + +class PathnameBlankTest < ActiveSupport::TestCase + def test_blank + assert_predicate Pathname.new(""), :blank? + assert_not_predicate Pathname.new("test"), :blank? + assert_not_predicate Pathname.new(" "), :blank? + end +end