[Fix rubocop#9328] Honour shareable_constant_value magic comment #9929
Conversation
|
@marcandre @koic - Please take a look when free. |
|
Good PR @thearjunmdas ! 👍 |
Thanks a lot for the quick review. Will address the comments and get back. |
|
#9929 (review) - Added all the test cases mentioned here. Please take a look and feel free to add more comments if we could make it better. |
|
Have added some comments, please review them - so that I can proceed with the changes. |
|
Please do take a look, @marcandre |
|
ping @marcandre |
|
@koic @marcandre - can you folks please take a look? lets try to merge this before it becomes stale? |
| @@ -0,0 +1 @@ | |||
| * [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][]) | |||
There was a problem hiding this comment.
If you wanted to add a second person here, you could do something like this:
| * [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][]) | |
| * [#9328](https://github.com/rubocop/rubocop/issues/9328): Recognize shareable_constant_value magic comment. ([@thearjunmdas][], [@caalberts][]) |
There was a problem hiding this comment.
@dvandersluis thanks for the tip. Worked perfectly.
|
@thearjunmdas Please, also document the support for this magic comment in the cop's description, so people would know what to expect. |
|
You'll also have to rebase on top of |
|
Oh, so sorry I missed the pings 😅 |
|
Thanks for reviving the PR @marcandre and @bbatsov. I will update on the comments. P.S. Cleaning the commit history - squashing my commits, rebasing - please ignore intermittent commit history change. Will clean it up. |
[Fix rubocop#9328] Resolve review comments [Fix rubocop#9328] Removing nil check, refactoring spec [Fix rubocop#9328] Tests for code next to inline comments [Fix rubocop#9328] Move module to mutable_constant
|
@bbatsov @marcandre rebased on top of master, added documentation. Please let me know, if anything else needs to be done. |
|
|
||
| it 'raises offense when shareable_constant_value is specified as an inline comment' do | ||
| expect_offense(<<~RUBY) | ||
| X = [1, 2, 3] # shareable_constant_value: literal |
There was a problem hiding this comment.
At some point we'll need a lint cop to check for this. :D
There was a problem hiding this comment.
Makes, sense. Since it's an experimental feature, maybe we need not do it right away.
| # NOTE: This special directive helps to create constants | ||
| # that hold only immutable objects, or Ractor-shareable | ||
| # constants. - ruby docs | ||
| # |
There was a problem hiding this comment.
@bbatsov - Added some examples. Also added an excerpt from ruby docs so that shareable_constant_value doesn't turn out to be an alternative to .freeze
|
@bbatsov - please take a look. |
|
Looks good. Thanks! |
Ruby 3.0 supports shareable_constant_value magic comment. This PR updates Style/MutableConstant to honour shareable_constant_value magic comment.
Fixes #9328
Took PR 9410 as reference. Tried to add @caalberts as co-author in changelog, but changelog spec fails.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.