Cop idea: Enforce preferred style for format string sequences

[backus]

You should use one of these styles consistently:

# Positional format sequences with field type characters
"Hi, my name is %s and I am %d years old. %0.1f to be exact." % ["John", 22, 22.45]

# Named format sequences without field type characters
"Hi, my name is %{name} and I am %{age} years old. %{precise_age} to be exact." % { name: "John", age: 22, precise_age: 22.45 }

# Named format sequences with field type characters
"Hi, my name is %<name>s and I am %<age>d years old. %<age>0.1f to be exact." % { name: "John", age:  22.45 }

I prefer the third option because you get to name your variables and also get some type hinting and coercion.

[bbatsov]

Yeah, that'd be nice.

[cdenneen]

Anyone currently using existing code with %{variable} (especially when using rubocop tests with puppet code) just got bit with errors because of this commit. Setting default back to template fixes it.

[backus]

Yeah it was a new rule that came out somewhat recently. Not sure what you mean by "bit with errors." It is just recommending one of two styles

[cdenneen]

@backus

Rakefile:52:63: C: Style/FormatStringToken: Prefer annotated tokens (like %<foo>s) over template tokens (like %{foo}).
  config.log_format = '%{path}:%{linenumber}:%{check}:%{KIND}:%{message}'
                                                              ^^^^^^^^^^
spec/spec_helper_acceptance.rb:23:15: C: Style/FormatStringToken: Prefer annotated tokens (like %<foo>s) over template tokens (like %{foo}).
        'role/%{role}',
              ^^^^^^^

But changing either of these to %<role> will break puppet-lint rake task

rake aborted!
ArgumentError: malformed format string - %:
~/puppet/.bundle/gems/fast_gettext-1.1.0/lib/fast_gettext/vendor/string.rb:70:in `%'
~/puppet/.bundle/gems/fast_gettext-1.1.0/lib/fast_gettext/vendor/string.rb:70:in `%'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint.rb:103:in `format_message'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint.rb:152:in `block in report'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint.rb:142:in `each'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint.rb:142:in `report'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint.rb:201:in `print_problems'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint/tasks/puppet-lint.rb:84:in `block (3 levels) in define'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint/tasks/puppet-lint.rb:81:in `each'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint/tasks/puppet-lint.rb:81:in `block (2 levels) in define'
~/puppet/.bundle/gems/puppet-lint-2.3.0/lib/puppet-lint/tasks/puppet-lint.rb:75:in `block in define'
~/puppet/.bundle/gems/rake-12.0.0/exe/rake:27:in `<top (required)>'
/opt/boxen/rbenv/versions/2.2.4/bin/bundle:23:in `load'
/opt/boxen/rbenv/versions/2.2.4/bin/bundle:23:in `<main>'
Tasks: TOP => lint
(See full trace by running task with --trace)

So then to fix you have to update the .rubocop.yml

Style/FormatStringToken:
  EnforcedStyle: template

[backus]

It should be %<role>s not %<role> if you want to use the default style @cdenneen

[cdenneen]

@backus thanks I think that will fix the issues with puppet-lint failing (wish the stacktrace was clearer).

[backus]

Yeah the stacktrace from your fast_gettext dependency isn't that helpful but rubocop does try to nudge you in the right direction with its message. The reason this is recommended by default is because you can encode a tiny bit of type information into the tokens. For example, you might be able to write %<linenumber>d if that value is expected to be an integer.

[cdenneen]

Thanks I thought i was for integer and f for float... good to know it's d for digit?​

[backus]

I think so yeah.

[rhymes]

I found one issue with this cop. It flags things like this:

Time.current.strftime('%Y-%m-%d.%H.%M.%S.%N')