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

auto-gen-config doesn't patch %r{/} warning correctly #847

Closed
tmorris-fiksu opened this issue Mar 1, 2014 · 2 comments · Fixed by #850
Closed

auto-gen-config doesn't patch %r{/} warning correctly #847

tmorris-fiksu opened this issue Mar 1, 2014 · 2 comments · Fixed by #850
Assignees
Labels

Comments

@tmorris-fiksu
Copy link
Contributor

To reproduce, create the file sample.rb included below. Run rubocop --auto-gen-config. rubocop --config rubocop-todo.yml gives the following warning:

...
sample.rb:2:9: C: Use %r only for regular expressions matching more than 2 '/' characters.
y.gsub!(%r{abc/xyz}, "A/Z")
        ^^^^^^^^^^^

1 file inspected, 1 offence detected
$ cat sample.rb
y = "123/abc/xyz/ijk"
y.gsub!(%r{abc/xyz}, "A/Z")
puts y
$ rubocop -V
0.18.1 (using Parser 2.1.5, running on ruby 1.9.3 x86_64-darwin12.5.0)

The generated rubocop-todo.yml includes:

# Offence count: 1
RegexpLiteral:
  MaxSlashes: 2

It should be MaxSlashes: 1 to avoid the warning.

@jonas054 jonas054 added the bug label Mar 1, 2014
@jonas054 jonas054 self-assigned this Mar 1, 2014
@jonas054
Copy link
Collaborator

jonas054 commented Mar 1, 2014

Thanks. I'll examine the problem.

@jonas054
Copy link
Collaborator

jonas054 commented Mar 2, 2014

Let me clarify what the problem is. MaxSlashes is the maximum number of (escaped) slashes that a slash-delimited regexp is allowed to have. If there are more slashes, it should be a %r regexp. This also means that %r literals must have more slashes than MaxSlashes.

So for sample.rb above, --auto-gen-config should generate

RegexpLiteral:
  MaxSlashes: 0

For code containing %r{abc} and %r{abc/123} it should generate MaxSlashes: -1, meaning that only %r literals should be used.

For code containing %r{abc} and /abc/ it should generate Enabled: false, because there's no MaxSlashes setting that would allow both of them.

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.

2 participants