Skip to content
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

Style/SymbolArray autocorrect breaks :[] #7193

Closed
lamont-granquist opened this issue Jul 2, 2019 · 7 comments · Fixed by #7581
Closed

Style/SymbolArray autocorrect breaks :[] #7193

lamont-granquist opened this issue Jul 2, 2019 · 7 comments · Fixed by #7581
Labels

Comments

@lamont-granquist
Copy link

lamont-granquist commented Jul 2, 2019

here's the diff resulting from the autocorrect:

-      direct_access_methods = Enumerable.instance_methods + [ :[], :each, :each_index, :empty? ]
+      direct_access_methods = Enumerable.instance_methods + %i{\[\] each each_index empty?}

and those expressions are not equivalent:

% pry
[1] pry(main)> [ :[], :each, :each_index, :empty? ]
=> [:[], :each, :each_index, :empty?]
[2] pry(main)> %i{\[\] each each_index empty?}
=> [:"\\[\\]", :each, :each_index, :empty?]
% NOEXEC_DISABLE=1 rubocop --version
0.72.0
@lamont-granquist
Copy link
Author

not entirely unsurprisingly :[]= also gets broken.

@koic
Copy link
Member

koic commented Jul 3, 2019

I cannot reproduce by the default settings. Could you show a .rubocop.yml for this auto-correct?

@lamont-granquist
Copy link
Author

I'm using rubocop --only Style/SymbolArray so I don't think the contents of the .rubocop.yml matters?

The contents of the .rubocop.yml though is:

llCops:
  Exclude:
    - "spec/data/**/*"
    - "vendor/**/*"
    - "pkg/**/*"
    - "chef-config/pkg/**/*"
    - "habitat/**/*"
Security/Eval:
  Enabled: false
Lint/UselessAssignment:
  Enabled: false
Lint/DeprecatedClassMethods:
  Enabled: false
Lint/ParenthesesAsGroupedExpression:
  Enabled: false
Lint/AmbiguousRegexpLiteral:
  Enabled: false
Lint/AssignmentInCondition:
  Enabled: false
Lint/AmbiguousBlockAssociation:
  Enabled: false
Lint/UnneededSplatExpansion:
  Enabled: false
Lint/ShadowingOuterLocalVariable:
  Enabled: false
Lint/EmptyWhen:
  Enabled: false
Lint/IneffectiveAccessModifier:
  Enabled: false
Lint/ShadowedException:
  Enabled: false
Layout/AlignHash:
  Enabled: true
  EnforcedLastArgumentHashStyle: ignore_implicit

@pocke pocke added the bug label Jul 3, 2019
@pocke
Copy link
Collaborator

pocke commented Jul 3, 2019

I can reproduce it with a configuration of Style/PercentLiteralDelimiters cop.

Style/PercentLiteralDelimiters:
  PreferredDelimiters:
    default: '{}'

I guess you have the cop's setting in some .rubocop.yml.
Can you confirm your .rubocop.yml? You can use rubocop --debug to display configuration files that are used by rubocop. I guess it includes another .rubocop.yml.

@lamont-granquist
Copy link
Author

yeah, i just figured out the same thing.

i don't quite understand how it was picking it up but:

Style/PercentLiteralDelimiters:
  Enabled: true
  PreferredDelimiters:
    '%':  '{}'
    '%i': '{}'
    '%I': '{}'
    '%q': '{}'
    '%Q': '{}'
    '%r': '{}'
    '%s': '{}'
    '%w': '{}'
    '%W': '{}'
    '%x': '{}'

@lamont-granquist
Copy link
Author

Actually its possible I ran that in isolation with --only and then saw the %i[]s were added and then fixed the PercentLiteralDelimiters with a global run with autocorrect on against those rules. Although one way or another there's an autocorrect bug here, possibly two, since when the PercentLiteralDelimiters change any escaped old delimiter should become unescaped.

@buehmann
Copy link
Contributor

This is actually a problem in Style/PercentLiteralDelimiters and relates to #4363. The fix back then only covered the %w case correctly, but not %i.

I can reproduce the problem using the input

%i[\[\] each each_index empty?]

with this configuration:

Style/PercentLiteralDelimiters:
  Enabled: true
  PreferredDelimiters:
    '%i': '{}'

I am going a add a PR for fixing this in a minute.

buehmann added a commit to buehmann/rubocop that referenced this issue Dec 21, 2019
bbatsov pushed a commit that referenced this issue Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants