Rubocop: Style/FormatStringToken is flagging non formatted strings

[doits]

Style/FormatStringToken is flagging simple strings which are not formatted as wrong. For example I want to output my %{str}:

# test.rb

puts 'my %{str}'

This gives me:

Style/FormatStringToken: Prefer annotated tokens (like `%<foo>s`) over template tokens (like `%{foo}`).

---

Expected behavior

Style/FormatStringToken should detect whether the string is used in formatting context or not. If not, it should not flag them as wrong.

Actual behavior

Looks like every string simply including %{...} is marked as invalid.

RuboCop version

$ rubocop -V
0.49.1 (using Parser 2.4.0.0, running on ruby 2.4.1 x86_64-darwin16)

[backus]

Maybe just add a disable? This is the formatting for a format string token. We could consider adding a "only flag this really conservatively" option but I don't think that should be the default behavior

[doits]

Disabling it is only a workaround. Afaik the linter in fact simply checks for a %{...} inside a string and marks this as bad, regardless of context. This begs for false positives all over the place.

I can disable the rule globally for me of course. But I don't think having it enabled is a good default for rubocop at all, if the checking is done like this (not aware of any context).

[backus]

Yeah it does do that but I'm not sure that people have tons of cases of writing "%{foo}" for reasons other than template strings. Can you share your use case?

[doits]

In my case Rails is using it for variable interpolation in I18n for example (https://guides.rubyonrails.org/i18n.html#passing-variables-to-translations). Mostly strings are stored inside a yml file, but if the translations are more dynamic (eg. based on user input, as it happens to be in my Rails app), strings are modified on the fly and passed directly to the translation backend.

Since %{...} is the standard way for Rails to specify placeholders, it happens for me that I have such placeholders in more places (not only in I18n), for example for interpolating a newsletter salutation. The body is dynamically entered by an editor, and I test in my specs that the %{salutation} placeholder is correctly replaced if it is used:

# rspec example

body            = 'Salutation: %{salutation}'
newsletter      = Newsletter.create(body: body)
newsletter_body = newsletter.body_for_user(user)

expect(newsletter_body).not_to include('%{salutation}')
expect(newsletter_body).to include("Salutation: Dear #{user.first_name}")

Same goes for unkown interpolations which should stay in my case:

# rspec example

body            = 'Unknown: %{unknown}'
newsletter      = Newsletter.create(body: body)
newsletter_body = newsletter.body_for_user(user)

expect(newsletter_body).to include('Unknown: %{unknown}')

Maybe I just had the bad luck that in my current project those strings are used in some places, if really nobody else uses such strings for other purposes than template strings.

[backus]

Then this fits the role of the cop. %{foo} and %<foo>s are alternate styles so you should either set your enforced style to templates or switch these to %<salutation>s and whatnot

[doits]

OK, I did not know I18n can handle those %<foo>s, too (I just confirmed it by testing and looking at the source).

Then I'll go with it and change the linter's style to the %{foo} way, since that is what I settled on long time ago in that project. Maybe I was the only one surprised by that linter in this way.

[pegasd]

In my Puppet rspec tests, I have the following piece of code that is flagged:

.with_content(%r{^    ChrootDirectory /var/chroot/%u$})

This is a valid piece of sshd_config contents. Do I have any options besides disabling the cop here?

[Drenmi]

This looks like a false positive @pegasd. Feel free to open a new issue. 🙂