Deprecate starts_with? and ends_with? for String core extensions

In the past, we sometimes hit missing `Symbol#start_with?` and `Symbol#end_with?`. rails@63256bc rails@a8e8129 So I proposed `Symbol#start_with?` and `Symbol#end_with?` to allow duck typing that methods for String and Symbol, then now it is available in Ruby 2.7. https://bugs.ruby-lang.org/issues/16348 Using `String#starts_with?` and `String#ends_with?` could not be gained that conveniency, so it is preferable to not use these in the future.
shunyama · May 5, 2020 · 6a4395f · 6a4395f
1 parent bd87b37
commit 6a4395f
Show file tree

Hide file tree

Showing 16 changed files with 40 additions and 47 deletions.
diff --git a/actionmailer/test/asset_host_test.rb b/actionmailer/test/asset_host_test.rb
@@ -29,7 +29,7 @@ def test_asset_host_as_string
 
   def test_asset_host_as_one_argument_proc
     AssetHostMailer.config.asset_host = Proc.new { |source|
-      if source.starts_with?("/images")
+      if source.start_with?("/images")
         "http://images.example.com"
       end
     }

diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb
@@ -114,7 +114,7 @@ def etag?; etag; end
 
         # True if an ETag is set and it's a weak validator (preceded with W/)
         def weak_etag?
-          etag? && etag.starts_with?('W/"')
+          etag? && etag.start_with?('W/"')
         end
 
         # True if an ETag is set and it isn't a weak validator (not preceded with W/)

diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require "singleton"
-require "active_support/core_ext/string/starts_ends_with"
 
 module Mime
   class Mimes
@@ -117,7 +116,7 @@ def self.sort!(list)
             type = list[idx]
             break if type.q < app_xml.q
 
-            if type.name.ends_with? "+xml"
+            if type.name.end_with? "+xml"
               list[app_xml_idx], list[idx] = list[idx], app_xml
               app_xml_idx = idx
             end
@@ -306,15 +305,15 @@ def to_ary; end
       def to_a; end
 
       def method_missing(method, *args)
-        if method.to_s.ends_with? "?"
+        if method.to_s.end_with? "?"
           method[0..-2].downcase.to_sym == to_sym
         else
           super
         end
       end
 
       def respond_to_missing?(method, include_private = false)
-        (method.to_s.ends_with? "?") || super
+        (method.to_s.end_with? "?") || super
       end
   end
 
@@ -349,11 +348,11 @@ def ref; end
 
     private
       def respond_to_missing?(method, _)
-        method.to_s.ends_with? "?"
+        method.to_s.end_with? "?"
       end
 
       def method_missing(method, *args)
-        false if method.to_s.ends_with? "?"
+        false if method.to_s.end_with? "?"
       end
   end
 end

diff --git a/actionview/lib/action_view/helpers/asset_url_helper.rb b/actionview/lib/action_view/helpers/asset_url_helper.rb
@@ -80,7 +80,7 @@ module Helpers #:nodoc:
     # absolute path of the asset, for example "/assets/rails.png".
     #
     #    ActionController::Base.asset_host = Proc.new { |source|
-    #      if source.ends_with?('.css')
+    #      if source.end_with?('.css')
     #        "http://stylesheets.example.com"
     #      else
     #        "http://assets.example.com"
@@ -206,7 +206,7 @@ def asset_path(source, options = {})
 
         relative_url_root = defined?(config.relative_url_root) && config.relative_url_root
         if relative_url_root
-          source = File.join(relative_url_root, source) unless source.starts_with?("#{relative_url_root}/")
+          source = File.join(relative_url_root, source) unless source.start_with?("#{relative_url_root}/")
         end
 
         if host = compute_asset_host(source, options)

diff --git a/actionview/lib/action_view/helpers/form_tag_helper.rb b/actionview/lib/action_view/helpers/form_tag_helper.rb
@@ -134,7 +134,7 @@ def form_tag(url_for_options = {}, options = {}, &block)
       #   #    <option selected="selected">MasterCard</option></select>
       def select_tag(name, option_tags = nil, options = {})
         option_tags ||= ""
-        html_name = (options[:multiple] == true && !name.to_s.ends_with?("[]")) ? "#{name}[]" : name
+        html_name = (options[:multiple] == true && !name.to_s.end_with?("[]")) ? "#{name}[]" : name
 
         if options.include?(:include_blank)
           include_blank = options[:include_blank]

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb
@@ -9,7 +9,7 @@ def indexes(table_name)
           exec_query("PRAGMA index_list(#{quote_table_name(table_name)})", "SCHEMA").map do |row|
             # Indexes SQLite creates implicitly for internal use start with "sqlite_".
             # See https://www.sqlite.org/fileformat2.html#intschema
-            next if row["name"].starts_with?("sqlite_")
+            next if row["name"].start_with?("sqlite_")
 
             index_sql = query_value(<<~SQL, "SCHEMA")
               SELECT sql

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -26,7 +26,7 @@ def sqlite3_connection(config)
       # Allow database path relative to Rails.root, but only if the database
       # path is not the special path that tells sqlite to build a database only
       # in memory.
-      if ":memory:" != config[:database] && !config[:database].to_s.starts_with?("file:")
+      if ":memory:" != config[:database] && !config[:database].to_s.start_with?("file:")
         config[:database] = File.expand_path(config[:database], Rails.root) if defined?(Rails.root)
         dirname = File.dirname(config[:database])
         Dir.mkdir(dirname) unless File.directory?(dirname)

diff --git a/activerecord/lib/active_record/test_fixtures.rb b/activerecord/lib/active_record/test_fixtures.rb
@@ -44,7 +44,7 @@ def fixtures(*fixture_set_names)
         if fixture_set_names.first == :all
           raise StandardError, "No fixture path found. Please set `#{self}.fixture_path`." if fixture_path.blank?
           fixture_set_names = Dir[::File.join(fixture_path, "{**,*}/*.{yml}")].uniq
-          fixture_set_names.reject! { |f| f.starts_with?(file_fixture_path.to_s) } if defined?(file_fixture_path) && file_fixture_path
+          fixture_set_names.reject! { |f| f.start_with?(file_fixture_path.to_s) } if defined?(file_fixture_path) && file_fixture_path
           fixture_set_names.map! { |f| f[fixture_path.to_s.size..-5].delete_prefix("/") }
         else
           fixture_set_names = fixture_set_names.flatten.map(&:to_s)

diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb
@@ -139,8 +139,8 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
       blob.open do |file|
         assert file.binmode?
         assert_equal 0, file.pos
-        assert File.basename(file.path).starts_with?("ActiveStorage-#{blob.id}-")
-        assert file.path.ends_with?(".jpg")
+        assert File.basename(file.path).start_with?("ActiveStorage-#{blob.id}-")
+        assert file.path.end_with?(".jpg")
         assert_equal file_fixture("racecar.jpg").binread, file.read, "Expected downloaded file to match fixture file"
       end
     end
@@ -161,7 +161,7 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
       assert file.binmode?
       assert_equal 0, file.pos
       assert_match(/\.jpg\z/, file.path)
-      assert file.path.starts_with?(tmpdir)
+      assert file.path.start_with?(tmpdir)
       assert_equal file_fixture("racecar.jpg").binread, file.read, "Expected downloaded file to match fixture file"
     end
   end

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Deprecate `starts_with?` and `ends_with?` for String core extensions.
+    Use the native `start_with?` and `end_with?` instead.
+
+    *Ryuta Kamizono*
+
 *   Add override of unary plus for `ActiveSupport::Duration`.
 
     `+ 1.second` is now identical to `+1.second` to prevent errors

diff --git a/activesupport/lib/active_support/core_ext/string/starts_ends_with.rb b/activesupport/lib/active_support/core_ext/string/starts_ends_with.rb
@@ -3,4 +3,6 @@
 class String
   alias_method :starts_with?, :start_with?
   alias_method :ends_with?, :end_with?
+  deprecate starts_with?: :start_with?
+  deprecate ends_with?: :end_with?
 end
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
@@ -12,7 +12,6 @@
 require "active_support/core_ext/kernel/reporting"
 require "active_support/core_ext/load_error"
 require "active_support/core_ext/name_error"
-require "active_support/core_ext/string/starts_ends_with"
 require "active_support/dependencies/interlock"
 require "active_support/inflector"
 
@@ -453,7 +452,7 @@ def loadable_constants_for_path(path, bases = autoload_paths)
 
     # Search for a file in autoload_paths matching the provided suffix.
     def search_for_file(path_suffix)
-      path_suffix += ".rb" unless path_suffix.ends_with?(".rb")
+      path_suffix += ".rb" unless path_suffix.end_with?(".rb")
 
       autoload_paths.each do |root|
         path = File.join(root, path_suffix)
@@ -473,9 +472,9 @@ def autoloadable_module?(path_suffix)
     end
 
     def load_once_path?(path)
-      # to_s works around a ruby issue where String#starts_with?(Pathname)
+      # to_s works around a ruby issue where String#start_with?(Pathname)
       # will raise a TypeError: no implicit conversion of Pathname into String
-      autoload_once_paths.any? { |base| path.starts_with? base.to_s }
+      autoload_once_paths.any? { |base| path.start_with?(base.to_s) }
     end
 
     # Attempt to autoload the provided module name by searching for a directory
@@ -585,7 +584,7 @@ def load_missing_constant(from_mod, const_name)
       end
 
       name_error = NameError.new("uninitialized constant #{qualified_name}", const_name)
-      name_error.set_backtrace(caller.reject { |l| l.starts_with? __FILE__ })
+      name_error.set_backtrace(caller.reject { |l| l.start_with?(__FILE__) })
       raise name_error
     end
 

diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb
@@ -239,15 +239,15 @@ def test_ord
     assert_equal 97, "abc".ord
   end
 
-  def test_starts_ends_with_alias
+  def test_deprecated_starts_ends_with_alias
     s = "hello"
-    assert s.starts_with?("h")
-    assert s.starts_with?("hel")
-    assert_not s.starts_with?("el")
+    assert_deprecated { assert s.starts_with?("h") }
+    assert_deprecated { assert s.starts_with?("hel") }
+    assert_deprecated { assert_not s.starts_with?("el") }
 
-    assert s.ends_with?("o")
-    assert s.ends_with?("lo")
-    assert_not s.ends_with?("el")
+    assert_deprecated { assert s.ends_with?("o") }
+    assert_deprecated { assert s.ends_with?("lo") }
+    assert_deprecated { assert_not s.ends_with?("el") }
   end
 
   def test_string_squish

diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md
@@ -985,7 +985,7 @@ and performs the validation on it. The custom validator is called using the
 ```ruby
 class MyValidator < ActiveModel::Validator
   def validate(record)
-    unless record.name.starts_with? 'X'
+    unless record.name.start_with? 'X'
       record.errors.add :name, "Need a name starting with X please!"
     end
   end

diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md
@@ -1212,17 +1212,6 @@ The `inquiry` method converts a string into a `StringInquirer` object making equ
 
 NOTE: Defined in `active_support/core_ext/string/inquiry.rb`.
 
-### `starts_with?` and `ends_with?`
-
-Active Support defines 3rd person aliases of `String#start_with?` and `String#end_with?`:
-
-```ruby
-"foo".starts_with?("f") # => true
-"foo".ends_with?("o")   # => true
-```
-
-NOTE: Defined in `active_support/core_ext/string/starts_ends_with.rb`.
-
 ### `strip_heredoc`
 
 The method `strip_heredoc` strips indentation in heredocs.

diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
@@ -4,7 +4,6 @@
 require "rack/test"
 require "env_helpers"
 require "set"
-require "active_support/core_ext/string/starts_ends_with"
 
 class ::MyMailInterceptor
   def self.delivering_email(email); email; end
@@ -1710,8 +1709,8 @@ def index
     test "autoload paths do not include asset paths" do
       app "development"
       ActiveSupport::Dependencies.autoload_paths.each do |path|
-        assert_not_operator path, :ends_with?, "app/assets"
-        assert_not_operator path, :ends_with?, "app/javascript"
+        assert_not_operator path, :end_with?, "app/assets"
+        assert_not_operator path, :end_with?, "app/javascript"
       end
     end
 
@@ -1722,8 +1721,8 @@ def index
       app "development"
 
       ActiveSupport::Dependencies.autoload_paths.each do |path|
-        assert_not_operator path, :ends_with?, "app/assets"
-        assert_not_operator path, :ends_with?, "app/webpack"
+        assert_not_operator path, :end_with?, "app/assets"
+        assert_not_operator path, :end_with?, "app/webpack"
       end
     end
 
@@ -1733,7 +1732,7 @@ def index
       # Action Mailer modifies AS::Dependencies.autoload_paths in-place.
       autoload_paths = ActiveSupport::Dependencies.autoload_paths
       autoload_paths_from_app_and_engines = autoload_paths.reject do |path|
-        path.ends_with?("mailers/previews")
+        path.end_with?("mailers/previews")
       end
       assert_equal true, Rails.configuration.add_autoload_paths_to_load_path
       assert_empty autoload_paths_from_app_and_engines - $LOAD_PATH