Follow-up to e9bc620f786d7da9afb82ec13ca3ec249f856c0c and
3ec629784cac7a8b518feb402475153465cd8e96.
`assert_not_deprecated` without any arguments will test against
`ActiveSupport::Deprecation.instance`, whereas `Enumerable#sum` would
use `ActiveSupport.deprecator`. However, the `Enumerable#sum` override
was removed in 3ec629784cac7a8b518feb402475153465cd8e96, so we can
simply remove `assert_not_deprecated`.
Use case
A very common pattern in Ruby, especially in testing is to save the value of an attribute, set a new value, and then restore the old value in an `ensure` clause.
e.g. in unit tests
```ruby
def test_something_when_enabled
enabled_was, SomeLibrary.enabled = SomeLibrary.enabled, true
# test things
ensure
SomeLibrary.enabled = enabled_was
end
```
Or sometime in actual APIs:
```ruby
def with_something_enabled
enabled_was = @enabled
@enabled = true
yield
ensure
@enabled = enabled_was
end
```
There is no inherent problem with this pattern, but it can be easy to make a mistake, for instance the unit test example:
```ruby
def test_something_when_enabled
some_call_that_may_raise
enabled_was, SomeLibrary.enabled = SomeLibrary.enabled, true
# test things
ensure
SomeLibrary.enabled = enabled_was
end
```
In the above if `some_call_that_may_raise` actually raises, `SomeLibrary.enabled` is set back to `nil` rather than its original value. I've seen this mistake quite frequently.
Object#with
I think it would be very useful to have a method on Object to implement this pattern in a correct and easy to use way.
NB: `public_send` is used because I don't think such method should be usable if the accessors are private.
With usage:
```ruby
def test_something_when_enabled
SomeLibrary.with(enabled: true) do
# test things
end
end
```
```ruby
GC.with(measure_total_time: true, auto_compact: false) do
# do something
end
```
Lots of tests in Rails's codebase could be simplified, e.g.:
- Changing `Thread.report_on_exception`: 2d2fdc941e/activerecord/test/cases/connection_pool_test.rb (L583-L595)
- Changing a class attribute: 2d2fdc941e/activerecord/test/cases/associations/belongs_to_associations_test.rb (L136-L150)
Method delegation with `...` argument is known to be slow because it allocates
redundant Array and Hash objects each time when being called.
see: https://bugs.ruby-lang.org/issues/19165
Current implementation of `delegate` defines all delegation methods in this
slow form regardless of the original method's arity, but in case the delegation
target is a Class or a Module, we can investigate the arity of the original
method in the definition timing, then we can define the delegation in proper
minimal arity.
This makes 3.5 times faster zero arity method delegation as follows:
Warming up --------------------------------------
old 811.534k i/100ms
new 1.807M i/100ms
Calculating -------------------------------------
old 9.809M (± 3.4%) i/s - 49.504M in 5.053355s
new 34.360M (± 0.8%) i/s - 173.465M in 5.048692s
Comparison:
new: 34360408.4 i/s
old: 9809157.4 i/s - 3.50x (± 0.00) slower
This commit adds `ActiveSupport.deprecator` and replaces all usages of
`ActiveSupport::Deprecation.warn` in `activesupport/lib` with
`ActiveSupport.deprecator`.
Additionally, this commit adds `ActiveSupport.deprecator` to
`Rails.application.deprecators` so that it can be configured using e.g.
`config.active_support.report_deprecations`.
Prior to this commit, when `Time#change` or `Time#advance` constructed a
time inside the final stretch of Daylight Saving Time (DST), the non-DST
offset would always be chosen for local times:
```ruby
# DST ended just before 2021-11-07 2:00:00 AM in US/Eastern.
ENV["TZ"] = "US/Eastern"
time = Time.local(2021, 11, 07, 00, 59, 59) + 1
# => 2021-11-07 01:00:00 -0400
time.change(day: 07)
# => 2021-11-07 01:00:00 -0500
time.advance(seconds: 0)
# => 2021-11-07 01:00:00 -0500
time = Time.local(2021, 11, 06, 01, 00, 00)
# => 2021-11-06 01:00:00 -0400
time.change(day: 07)
# => 2021-11-07 01:00:00 -0500
time.advance(days: 1)
# => 2021-11-07 01:00:00 -0500
```
And the DST offset would always be chosen for times with a `TimeZone`
object:
```ruby
Time.zone = "US/Eastern"
time = Time.new(2021, 11, 07, 02, 00, 00, Time.zone) - 3600
# => 2021-11-07 01:00:00 -0500
time.change(day: 07)
# => 2021-11-07 01:00:00 -0400
time.advance(seconds: 0)
# => 2021-11-07 01:00:00 -0400
time = Time.new(2021, 11, 8, 01, 00, 00, Time.zone)
# => 2021-11-08 01:00:00 -0500
time.change(day: 07)
# => 2021-11-07 01:00:00 -0400
time.advance(days: -1)
# => 2021-11-07 01:00:00 -0400
```
This commit fixes `Time#change` and `Time#advance` to choose the offset
that matches the original time's offset when possible:
```ruby
ENV["TZ"] = "US/Eastern"
time = Time.local(2021, 11, 07, 00, 59, 59) + 1
# => 2021-11-07 01:00:00 -0400
time.change(day: 07)
# => 2021-11-07 01:00:00 -0400
time.advance(seconds: 0)
# => 2021-11-07 01:00:00 -0400
time = Time.local(2021, 11, 06, 01, 00, 00)
# => 2021-11-06 01:00:00 -0400
time.change(day: 07)
# => 2021-11-07 01:00:00 -0400
time.advance(days: 1)
# => 2021-11-07 01:00:00 -0400
Time.zone = "US/Eastern"
time = Time.new(2021, 11, 07, 02, 00, 00, Time.zone) - 3600
# => 2021-11-07 01:00:00 -0500
time.change(day: 07)
# => 2021-11-07 01:00:00 -0500
time.advance(seconds: 0)
# => 2021-11-07 01:00:00 -0500
time = Time.new(2021, 11, 8, 01, 00, 00, Time.zone)
# => 2021-11-08 01:00:00 -0500
time.change(day: 07)
# => 2021-11-07 01:00:00 -0500
time.advance(days: -1)
# => 2021-11-07 01:00:00 -0500
```
It's worth noting that there is a somewhat dubious case when calling
`advance` with a mix of calendar and clock units (e.g. months + hours).
`advance` adds calendar units first, then adds clock units. So a
non-DST time may first be advanced to a date within DST before any clock
units are applied. For example:
```ruby
# DST began on 2021-03-14 in US/Eastern.
Time.zone = "US/Eastern"
non_dst = Time.new(2021, 03, 07, 00, 00, 00, Time.zone)
# => 2021-03-07 00:00:00 -0500
non_dst.advance(months: 8, hours: 1)
# => 2021-11-07 01:00:00 -0400
```
One could argue that the expected result is `2021-11-07 01:00:00 -0500`.
However, the difference between that and the result for `hours: 0` is
greater than 1 hour:
```ruby
adjusted_result = non_dst.advance(months: 8, hours: 1) + 3600
# => 2021-11-07 01:00:00 -0500
adjusted_result - non_dst.advance(months: 8, hours: 0)
# => 7200.0
```
Which might also be unexpected.
Furthermore, it may be difficult (or expensive) to apply such an
adjustment in a consistent way. For example, the result for `hours: 2`
does have the expected `-0500` offset, so it might seem no adjustment is
necessary, but if we didn't adjust it too, it would conflict with the
adjusted `hours: 1` result:
```ruby
non_dst.advance(months: 8, hours: 2)
# => 2021-11-07 01:00:00 -0500
```
Therefore, the approach in this commit (which produces a `-0400` offset
for `hours: 1`) seems like a reasonable compromise.
Fixes#45055.
Closes#45556.
Closes#46248.
Co-authored-by: Kevin Hall <bigtoe416@yahoo.com>
Co-authored-by: Takayoshi Nishida <takayoshi.nishida@gmail.com>
When calling `#descendants` on a non-reloadable class with reloadable
descendants, it can return classes that were unloaded but not yet
garbage collected.
`DescendantsTracker` have been dealing with this for a long time
but the core_ext versions of these methods were never made reloader
aware.
This also refactor `DescendantsTracker` to not be so different when
there is no native `#subclasses` method.
Rubinius has not been maintained since May 2020 and based on the
discussion at https://github.com/rails/rails/pull/44984 ,
I think we can remove Rubinius specific code from Rails.
Rails 5 added String#upcase_first, which converts the first letter of a
string to uppercase (returning a duplicate). This commit adds the
corollary method, converting the first letter to lowercase.
This provides a basic level of protection against different threads
trying to mutate a shared default object. It is not a bulletproof
solution, because the default may contain nested non-frozen objects, but
it should cover common cases.
This makes the value supplied to the `default` option of
`thread_mattr_accessor` to be set in descendant classes
as well as in any new Thread that starts.
Previously, the `default` value provided was set only at the
moment of defining the attribute writer, which would cause
the attribute to be uninitialized in descendants and in other threads.
For instance:
```ruby
class Processor
thread_mattr_accessor :mode, default: :smart
end
class SubProcessor < Processor
end
SubProcessor.mode # => :smart (was `nil` prior to this commit)
Thread.new do
Processor.mode # => :smart (was `nil` prior to this commit)
end.join
```
If a non-`nil` default has been specified, there is a small (~7%)
performance decrease when reading non-`nil` values, and a larger (~45%)
performance decrease when reading `nil` values.
Benchmark script:
```ruby
# frozen_string_literal: true
require "benchmark/ips"
require "active_support"
require "active_support/core_ext/module/attribute_accessors_per_thread"
class MyClass
thread_mattr_accessor :default_value, default: "default"
thread_mattr_accessor :string_value, default: "default"
thread_mattr_accessor :nil_value, default: "default"
end
MyClass.string_value = "string"
MyClass.nil_value = nil
Benchmark.ips do |x|
x.report("default_value") { MyClass.default_value }
x.report("string_value") { MyClass.string_value }
x.report("nil_value") { MyClass.nil_value }
end
```
Before this commit:
```
default_value 2.075M (± 0.7%) i/s - 10.396M in 5.010585s
string_value 2.103M (± 0.7%) i/s - 10.672M in 5.074624s
nil_value 1.777M (± 0.9%) i/s - 8.924M in 5.023058s
```
After this commit:
```
default_value 2.008M (± 0.7%) i/s - 10.187M in 5.072990s
string_value 1.967M (± 0.7%) i/s - 9.891M in 5.028570s
nil_value 1.144M (± 0.5%) i/s - 5.770M in 5.041630s
```
If no default or a `nil` default is specified, there is no performance
impact.
Fixes#43312.
Co-authored-by: Jonathan Hefner <jonathan@hefner.pro>
Add the method ERB::Util.xml_name_escape to escape dangerous characters
in names of tags and names of attributes, following the specification of
XML.
Use that method in the tag helpers of ActionView::Helpers. Rename the option
:escape_attributes to :escape, to simplify by applying the option to the whole
tag.
- `#first` raises an error when called on a beginless range.
Using `#begin` to get the first element instead.
- Adding additional equality condition to cover the case when both
ranges are beginless
With Ruby 2.4+ the default for +to_time+ changed from converting to the
local system time to preserving the offset of the receiver. At the time
Rails supported older versions of Ruby so a compatibility layer was
added to assist in the migration process. From Rails 5.0 new applications
have defaulted to the Ruby 2.4+ behavior and since Rails 7.0 now only
supports Ruby 2.7+ this compatibility layer can be safely removed.
To minimize any noise generated the deprecation warning only appears when
the setting is configured to `false` as that is the only scenario where
the removal of the compatibility layer has any effect.
Ref: https://github.com/aws/aws-sdk-ruby/pull/2670
Some gems like aws-sdk-core use `Object#extend(Enumerable)`.
It's not a very good pattern, but it's somehwat handled ok by Ruby.
However if Enumerable has constants, then any time the module is
extended, the global constant cache is flushed and this has a very
negative impact on performance for the virtual machine, and even
worse for JITs.
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(" ")
=> #<Pathname: >
>> 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.
