-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[#11076] Add autocorrect for hash in Lint/LiteralInInterpolation
#11475
[#11076] Add autocorrect for hash in Lint/LiteralInInterpolation
#11475
Conversation
Lint/LiteralInInterpolation
ba2d48f
to
0c65b94
Compare
|
||
def autocorrected_value_in_hash_for_symbol(node) | ||
# TODO: We need to detect symbol unacceptable names more correctly | ||
if / |"|'/.match?(node.value.to_s) |
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.
A gem unparser provides more simple (and better) way to generate code strings for autocorrect result.
Should we add it to the list of dependencies?
if / |"|'/.match?(node.value.to_s) | |
Unparser.unparse(node).gsub('"') { '\"' } |
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.
Probably that'd be an overkill for a single usage. I'm always wary of adding additional runtime deps.
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 aimed to separate codes to unparse hash from the rubocop to make its project scope more clear.
But after more research I realized that the unparser gem
sometimes generate results not exact same to Ruby's one.
Thus I agree to cancel this suggestion.
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.
@KessaPassa Unparser only guarantess: parse(code) == parse(unparse(parse(code)))
, so it guarantees you generate concrete syntax that if parsed produces the original AST, not the same concrete syntax. If you knew the concrete syntax already there is no need to unparse in the first place ;)
@@ -114,6 +121,72 @@ | |||
it_behaves_like('literal interpolation', '%I[s1 s2]', '[\"s1\", \"s2\"]') | |||
it_behaves_like('literal interpolation', '%i[s1 s2]', '[\"s1\", \"s2\"]') | |||
it_behaves_like('literal interpolation', '%i[ s1 s2 ]', '[\"s1\", \"s2\"]') | |||
it_behaves_like('literal interpolation', '{"a" => "b"}', '{\"a\"=>\"b\"}') |
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 think it might be nice to have the hash-related tests in their own section. The tests for this cop are already quite messy and we should try to improve this.
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.
c9fdcdc
to
c6027b6
Compare
Fixes #11076.
This PR add autocorrect to
Lint/LiteralInInterpolation
for hash type.Replace this text with a summary of the changes in your PR.
The more detailed you are, the better.
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.