[#78] Autocorrect EnumHash Cop #95
Conversation
santib
force-pushed
the
autocorrect-hashenum-cop
branch
from
69cb551
to
f5a09e7
Compare
|
#94 has been merged. Can you rebase this PR with latest master branch? After rebasing I will review this PR. |
santib
force-pushed
the
autocorrect-hashenum-cop
branch
from
f5a09e7
to
801ddb9
Compare
Thanks! I just pushed the branch after rebasing. |
santib
force-pushed
the
autocorrect-hashenum-cop
branch
2 times, most recently
from
9dc926b
to
c5141f1
Compare
lib/rubocop/cop/rails/enum_hash.rb
Outdated
| value = child.children.first | ||
| case value | ||
| when String | ||
| "'#{value}' => #{index}" |
There was a problem hiding this comment.
Use String#dup instead of "'#{value}'".
https://docs.ruby-lang.org/en/2.3.0/String.html#method-i-dump
eval(any_string.dump) always eauqlas any_string.
There was a problem hiding this comment.
I did it this way to prevent escaped keys, for example:
got: "enum status: {:old=>0, :\"very active\"=>1, \"is archived\"=>2, 42=>3}\n"
if that's not a problem would it be fine to just build a real hash and stringify it? It'd solve the problem being discussed here #95 (comment)
lib/rubocop/cop/rails/enum_hash.rb
Outdated
| when String | ||
| "'#{value}' => #{index}" | ||
| when Symbol | ||
| value = "'#{value}'" if value =~ /\s/ |
There was a problem hiding this comment.
I'd like to use a stricter way to quote the symbol. Comparing /\s/ is not match :'foo-bar' and so on.
@koic Do you have any idea to make it strict? I guess we have a method to detect the necessity of quoting.
There was a problem hiding this comment.
@pocke I agree with it is better to be stricter, but unfortunately I have not got a good idea yet...
There was a problem hiding this comment.
I found a check that foo: style is acceptable.
But it is not portable. We need to extend the method to a node extension or mixin if we'd like to use it for the cop.
So I think the current implementation is acceptable for now. I think it works on 99% cases because enum key name does not have special symbols in most cases.
|
@koic Perfect, will fix it later today |
santib
force-pushed
the
autocorrect-hashenum-cop
branch
from
c5141f1
to
9c535da
Compare
santib
force-pushed
the
autocorrect-hashenum-cop
branch
from
9c535da
to
9d54e36
Compare
|
@koic @pocke Just rebased and built a new version of the autocorrect. I did a separate commit just for now but I'll squash once everything looks good. I'm not sure if it's acceptable to do it this way because in other cops I saw mostly changes to strings rather than stringifying objects such as I just did. If it's not acceptable, just let me know and I'll work on another version. I assumed that other cops (such as prefer new hash syntax) will autocorrect the hash built in this solution, is that correct? |
I think it is good as a minimal implementation because this approach has achieved its purpose to prevent unintended reordering of database values. @pocke Do you have any thoughts? |
|
If there is no problem, it will merge soon. |
|
I agree with @koic . The implementation looks good. |
|
Thanks! |
|
There seems to be an issue with autocorrection of nested constants: #114 Any ideas on the right way to approach fixing this? |
Resolves #78.
Adds autocorrect feature for #78.
Depends on #94.