Skip to content

Conversation

@eregon
Copy link
Member

@eregon eregon commented Sep 8, 2019

@byroot
Copy link
Member

byroot commented Sep 25, 2019

@eregon seems like Matz is ok for shipping this as an experiment.

So this can be merged now?

If so do you think we could also experiment with returning a frozen string for Module#name (#2175) ?

@eregon
Copy link
Member Author

eregon commented Sep 25, 2019

@byroot Yes, I'll merge this soon before the next preview, I just need to mark the change as experimental.

If so do you think we could also experiment with returning a frozen string for Module#name (#2175) ?

I think that would require matz's approval. Could you reply on https://bugs.ruby-lang.org/issues/16150#note-18 to ask that?

* Always the same frozen String for a given Symbol.
* Avoids extra allocations whenever calling Symbol#to_s.
* See [Feature #16150]
@eregon eregon merged commit 6ffc045 into ruby:master Sep 26, 2019
@eregon eregon deleted the frozen-symbol-to_s branch September 26, 2019 08:23
eregon added a commit to eregon/rails that referenced this pull request Sep 26, 2019
* String#+@ is available since Ruby 2.3.
* See the upstream Ruby change making Symbol#to_s return a frozen String
  for efficiency: ruby/ruby#2437
koic added a commit to koic/rubocop-rails that referenced this pull request Oct 12, 2019
Follow ruby/ruby#2437.

This PR fixes the following `FrozenError` for save_bang_spec.rb

```Failures:
% ruby -v
ruby 2.7.0dev (2019-10-09T16:08:42Z master 42edb05626) [x86_64-darwin17]
% bundle exec rspec ./spec/rubocop/cop/rails/save_bang_spec.rb
(snip)

  1) RuboCop::Cop::Rails::SaveBang update behaves like
checks_common_offense when using update without arguments
     Failure/Error: inspect_source(method.to_s)

     FrozenError:
       can't modify frozen String: "update"
     Shared Example Group: "checks_common_offense" called from
./spec/rubocop/cop/rails/save_bang_spec.rb:541
     # ./spec/rubocop/cop/rails/save_bang_spec.rb:58:in `block (3 levels)
in <top (required)>'
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants