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
[Fix #9328] Recognize shareable_constant_value magic comment in Style/MutableConstant #9410
[Fix #9328] Recognize shareable_constant_value magic comment in Style/MutableConstant #9410
Conversation
@@ -103,23 +125,29 @@ | |||
|
|||
include_examples( | |||
'magic comment', | |||
'# -*- encoding: ASCII-8BIT; frozen_string_literal: true -*-', | |||
'# -*- encoding: ASCII-8BIT; frozen_string_literal: true; ' \ | |||
'shareable_constant_value: literal -*-', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This had to be split into 2 lines because of line length, but it makes the magic comment less readable. 🤔
34627fb
to
53476f0
Compare
53476f0
to
6b78bcb
Compare
@@ -83,6 +84,7 @@ def on_assignment(value) | |||
end | |||
|
|||
def strict_check(value) | |||
return if shareable_constant_value? | |||
return if immutable_literal?(value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it surprising that the tests started failing when i changed immutable_literal?(value)
to !mutable_literal?(value)
.
@marcandre Please take a look? |
I can't have a thorough look right now, but # shareable_constant_value: literal
X = [1, 2, 3]
# shareable_constant_value: false
Y = [1, 2, 3] And raise on offense for |
Ping @caalberts |
Yes. X is tested in https://github.com/rubocop-hq/rubocop/pull/9410/files#diff-6e9b3e5cdbc957813be5790f5b8c0850c30c3f1bf0eef7576009023cad7c3f25R160. I'll add a test for Y on ruby 3.0. |
6b78bcb
to
40e63ec
Compare
@@ -166,7 +172,7 @@ def extract_frozen_string_literal | |||
end | |||
|
|||
def extract_shareable_constant_value | |||
match('shareable[_-]constant[_-]values') | |||
match('shareable[_-]constant[_-]value') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method didn't match shareable_constant_value: *
Closing in favour of #9929 that builds on top of this PR. |
Ruby 3.0 adds
shareable_constant_value
magic comment. This PR changesStyle/MutableConstant
to be aware ofshareable_constant_value
magic comment.Fixes #9328.
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}.md
if the new code introduces user-observable changes. See changelog entry format for details.