devise
devise copied to clipboard
avoid modify of frozen string in validatable.rb
frozen string was modified in assert_validations_api. Use multiline string instead.
The error is hard to reproduce but trivial: As frozen_string_literal is set, the concatenation of the error message in line 50 is modifying a frozen string. It's easy to use a multiline string instead.
Hey @mameier, rhis does seem like a valid fix to me. I think commit 2e521bffcd93752cd08c32b41a05e3298b51eb02 is unrelated and should be dropped from this PR.
Also I think we need to assert the error message here, to make sure it's a proper fix: https://github.com/heartcombo/devise/blob/main/test/models/validatable_test.rb#L113-L115
Hey @p8 , I admit that commit 2e521bf is a different issue. I did'n mean to include it in this pull request. I moved the commit to a new branch but don'n know how to remove it from this pull request on github.
The behavior has changed slightly in Ruby 3 so that interpolated strings are no longer frozen:
% cat x.rb
# frozen_string_literal: true
blah = "BLAH"
raise "Could not use :validatable module since #{blah} does not respond " <<
"to the following methods: LOL"
# on Ruby 2.7.7
% be ruby x.rb
Traceback (most recent call last):
x.rb:5:in `<main>': can't modify frozen String: "Could not use :validatable module since BLAH does not respond " (FrozenError)
# on Ruby 3.2.0
% be ruby x.rb
x.rb:5:in `<main>': Could not use :validatable module since BLAH does not respond to the following methods: LOL (RuntimeError)
See https://bugs.ruby-lang.org/issues/17104 and https://scriptday.com/blog/2020/09/16/ruby-3-0-interpolated-strings-are-no-longer-frozen for some references.
Funny enough, our tests were passing because FrozenError
actually inherits from RuntimeError
:
>> FrozenError.ancestors
=> [FrozenError, RuntimeError, StandardError, Exception, Object ...]
But if I change the test to check for the message, it passes on 3.2 but fails on Ruby 2.7 as expected:
Failure:
ValidatableTest#test_should_not_be_included_in_objects_with_invalid_API [test/models/validatable_test.rb:118]:
Expected /Could not use :validatable module since .* does not respond to the following methods: validates_presence_of/ to match # encoding: ASCII-8BIT
# valid: true
"can't modify frozen String: \"Could not use :validatable module since #<Class:0x00000001315d98c8> does not respond \"".
In order to keep it compatible to the Ruby versions we currently support, I have cherry-picked the commit and expanded the tests in https://github.com/heartcombo/devise/pull/5563. Thanks @mameier.