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

Deprecate starts_with? and ends_with? for String core extensions #39152

Merged
merged 1 commit into from May 5, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion actionmailer/test/asset_host_test.rb
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion actionpack/lib/action_dispatch/http/cache.rb
Expand Up @@ -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/)
Expand Down
11 changes: 5 additions & 6 deletions 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions actionview/lib/action_view/helpers/asset_url_helper.rb
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion actionview/lib/action_view/helpers/form_tag_helper.rb
Expand Up @@ -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]
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/test_fixtures.rb
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions activestorage/test/models/blob_test.rb
Expand Up @@ -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
Expand All @@ -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
Expand Down
5 changes: 5 additions & 0 deletions 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
Expand Down
Expand Up @@ -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
9 changes: 4 additions & 5 deletions activesupport/lib/active_support/dependencies.rb
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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

Expand Down
14 changes: 7 additions & 7 deletions activesupport/test/core_ext/string_ext_test.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion guides/source/active_record_validations.md
Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions guides/source/active_support_core_extensions.md
Expand Up @@ -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.
Expand Down
11 changes: 5 additions & 6 deletions railties/test/application/configuration_test.rb
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down