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

False positive for Style/FormatStringToken in Rails validates format #7436

Open
DMA57361 opened this issue Oct 16, 2019 · 5 comments
Open

False positive for Style/FormatStringToken in Rails validates format #7436

DMA57361 opened this issue Oct 16, 2019 · 5 comments
Labels
bug library specific This issue is related to a particular library, like Rails or RSpec, and not Ruby itself

Comments

@DMA57361
Copy link

In one of our Rails apps we've a validation that uses the "format" helper to constrain a value to a matching regex, it seems the Style/FormatStringToken cop has incorrectly matched this a use of the format method, which it is not.


Expected behavior

Given an example.rb file with the following content:

class SomeModel < ApplicationRecord
  validates :colour, format: { with: /\A#[0-9a-fA-F]{6}\Z/, message: '%{colour} is not a valid hex colour' }
end

and running the command

rubocop example.rb --only Style/FormatStringToken

I expect to see no offenses.

Actual behavior

A single Style/FormatStringToken offense is reported

$ rubocop example.rb --only Style/FormatStringToken
Inspecting 1 file
C

Offenses:

example.rb:2:71: C: Style/FormatStringToken: Prefer annotated tokens (like %<foo>s) over template tokens (like %{foo}).
  validates :colour, format: { with: /\A#[0-9a-fA-F]{6}\Z/, message: '%{colour} is not a valid hex colour' }
                                                                      ^^^^^^^^^

1 file inspected, 1 offense detected

As an aside, the use of %<colour> in this context is not valid and would produce this error when attempting to save an invalid record:

I18n::MissingInterpolationArgument: missing interpolation argument :colour in "%<colour> is not a valid hex colour"

Steps to reproduce the problem

Covered above in expected behaviour

RuboCop version

$ rubocop -V
0.75.1 (using Parser 2.6.5.0, running on ruby 2.6.5 x86_64-darwin18)
@buehmann
Copy link
Contributor

Thanks! This is caused by 1c20018.

@DMA57361
Copy link
Author

Adding support for looking inside hashes roughly matches the above and what I've just come across in a different application, where we've now spotted a slew of additional false positives of this type. All of them involve using a percent-escaped style string within a hash (some rails validations similar to the above, other is code working with Rails' I18n translations).

As such what I'd originally assumed was a problem being caused by the use of a "format" validation seems to just be a coincidence. A simple { a: '%{test}' } is producing an offense under this cop for me.

@Drenmi Drenmi added the library specific This issue is related to a particular library, like Rails or RSpec, and not Ruby itself label Oct 22, 2019
@AlexWayfer
Copy link
Contributor

Named variables of R18n are also affected: https://github.com/r18n/r18n/tree/038223d/r18n-core#named-variables

@bbatsov bbatsov added the bug label Apr 9, 2020
@dvandersluis
Copy link
Member

I was taking a look at this to try to fix it but I'm not sure if we can without changing the behaviour of cop. The cop explicitly has tests checking for bare %{foo} or %<foo>x strings (depending on enforced style), without format, etc., and registering offenses for them.

@bbatsov should we fix this up so that the cop only applies to arguments to a format, sprintf or printf send node, or should we close the issue?

@dvandersluis
Copy link
Member

@bbatsov ping!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug library specific This issue is related to a particular library, like Rails or RSpec, and not Ruby itself
Projects
None yet
Development

No branches or pull requests

6 participants