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/FormatStringToken false positive #5245

Closed
pegasd opened this issue Dec 13, 2017 · 20 comments
Closed

Style/FormatStringToken false positive #5245

pegasd opened this issue Dec 13, 2017 · 20 comments
Labels

Comments

@pegasd
Copy link

pegasd commented Dec 13, 2017

I came across the following false positive while doing rspec-puppet testing on an sshd module.
Sometimes contents of a file can include valid string tokens that look like unannotated Ruby tokens.
These are usually wrapped inside a RegEx for testing.

For example, sshd_config can include %u which is a valid username reference.

Expected behavior

The following code shouldn't be flagged as offensive:

it {
  is_expected.to contain_file('/etc/ssh/sshd_config')
    .with_content(%r{^    ChrootDirectory /var/chroot/%u$})
}

Actual behavior

RuboCop triggers Style/FormatStringToken error:

spec/classes/sshd_spec.rb:3:55: C: Style/FormatStringToken: Prefer annotated tokens (like %<foo>s) over unannotated tokens (like %s).
    .with_content(%r{^    ChrootDirectory /var/chroot/%u$})
                                                      ^^

Steps to reproduce the problem

The snippet above triggers the described offense.

RuboCop version

$ rubocop -V
0.52.0
@jeremywadsack
Copy link

Another instance of this is the use of printf style format strings which I think are pretty common — in my experience much more common than format strings (outside of Rails I18n).

Style/FormatStringToken: Prefer annotated tokens (like %<foo>s) over unannotated tokens (like %s).
      @partition ||= dataset.table("#{table.table_id}$#{@date.strftime('%Y%m%d')}")

[Tagging #4539 for x-ref.]

@envygeeks
Copy link

We ran into this with strftime too.

@pocke
Copy link
Collaborator

pocke commented Dec 14, 2017

@pegasd Thanks for your bug report! I'll fix it.

@jeremywadsack @envygeeks
The strftime's issue will be fixed by #5230
See #5223 #5230

@davidhrbac
Copy link

We ran into this with select like

 "date_format(IF(dateEfficiency>NOW(),dateEfficiency,dateExpiration),'%Y%m%d%H%i%s')" \
                                                                              ^^

@pegasd
Copy link
Author

pegasd commented Dec 14, 2017

@davidhrbac Yeah, I think that particular case is discussed in #5230

pocke added a commit to pocke/rubocop that referenced this issue Dec 14, 2017
@ctm
Copy link

ctm commented Dec 14, 2017

scanf also takes a string that uses formatting characters:

irb(main):001:0> s = '12'
=> "12"
irb(main):002:0> require 'scanf'
=> true
irb(main):003:0> s.scanf('%f').first
=> 12.0
irb(main):004:0> s.scanf('%<ignored>f').first
=> nil

@jeremywadsack
Copy link

@pocke Thanks for linking that fix for strftime — sorry I missed that when looking for previous reports.

@envygeeks
Copy link

Thanks for the quick fix!

@agross
Copy link

agross commented Dec 18, 2017

I see this as a false positive for "foo/bar.rb".pathmap('%d/baz/%f') (pathmap is defined by Rake).

@agross
Copy link

agross commented Dec 18, 2017

Also in this spec:

context 'something' do
  let(:content) { template % address }
                           ^^
  let(:expected) { template % something(address) }

  let(:address) { '[email protected]' }

  context 'text' do
    let(:template) { 'prefix %s suffix' }

    it 'yields encoded content' do
      expect(subject.run(content)).to eq(expected)
    end
  end
end

wfleming added a commit to codeclimate/styleguide that referenced this issue Dec 18, 2017
This appeared when we recently upgraded RuboCop: I don't think it's a
style we should enforce.

The named tokens in format strings can be useful in very complicated
cases, but those cases are not the norm, and the check's suggested style
makes simple cases *less* readable.

In 0.52.0 it also has bugs leading to many false positives where
changing the code as suggested would result in incorrect behavior:
rubocop/rubocop#5245.
wfleming added a commit to codeclimate/styleguide that referenced this issue Dec 18, 2017
This appeared when we recently upgraded RuboCop: I don't think it's a
style we should enforce.

The named tokens in format strings can be useful in very complicated
cases, but those cases are not the norm, and the check's suggested style
makes simple cases *less* readable.

In 0.52.0 it also has bugs leading to many false positives where
changing the code as suggested would result in incorrect behavior:
rubocop/rubocop#5245.
@lionel218
Copy link

Hi guys, I using the rubocop version rubocop-0.52.1 and rails 5.1.1 ruby version is 2.4.2p198
But I just got rubocop error for strftime.
C: Style/FormatStringToken: Prefer annotated tokens (like %s) over unannotated tokens (like %s).
format('%s %s-%s', starts_at.strftime('%a'), starts_at.strftime('%l%P'), ends_at.strftime('%l%P').strip)

How can I solve this issue?

@mikegee
Copy link
Contributor

mikegee commented Mar 3, 2018

@lionel218 the cop wants you to use something like:

format('%<day>s %<start>s-%<end>s', day: starts_at.strftime('%a'), start: starts_at.strftime('%l%P'), end: ends_at.strftime('%l%P').strip)

@lionel218
Copy link

Hi @mikegee , Just solved this issue.

Thanks for your kindness.

@lionel218
Copy link

lionel218 commented Mar 3, 2018

Hi @mikegee , How can I solve this issue?
Style/FormatStringToken: Prefer annotated tokens (like %s) over unannotated tokens (like %s).
starts_at.strftime('%A')

screen shot 2018-03-03 at 11 40 14 pm
Style/FormatStringToken: Prefer annotated tokens (like %s) over unannotated tokens (like %s).
cutoff = 3.years.ago.to_date.strftime('%Y/%m/%d')

@lionel218
Copy link

Please help me for this issue.

@mikegee
Copy link
Contributor

mikegee commented Mar 5, 2018

@lionel218 I failed to duplicate the issue with the example you provided. Please open a new issue with all the details if you think this bug is still in Rubocop. Thanks!

@lionel218
Copy link

lionel218 commented Mar 5, 2018

@mikegee , Thanks for your kindness.
But I can't create new issue because the submit issue button is disabled now.

Please check for my code and rubocop issue.

def event_date(open_house)
format('%s %s-%s', day: open_house.starts_at.strftime('%a'),
start: open_house.starts_at.strftime('%l'),
end: open_house.ends_at.strftime('%l %p').strip)
end
This is my helper action, but just got this issue on rubocop-0.53.0, ruby-2.4.2, rails-5.1.1

app/helpers/open_houses_helper.rb:3:77: C: Style/FormatStringToken: Prefer annotated tokens (like %s) over unannotated tokens (like %s).
format('%s %s-%s', day: open_house.starts_at.strftime
('%a'),
^^
app/helpers/open_houses_helper.rb:5:78: C: Style/FormatStringToken: Prefer annotated tokens (like %s) over unannotated tokens (like %s).
end: open_house.ends_at.strftime('%l
%p').strip)
^^

@mikegee
Copy link
Contributor

mikegee commented Mar 5, 2018

@lionel218 I confirmed with that code and opened another issue for you: #5630

@lionel218
Copy link

Thanks so much @mikegee

@Seenox
Copy link

Seenox commented Aug 20, 2021

Sorry for bumping an old issue, but since it shows up in first page of Google if you search for:
Prefer annotated tokens (like `%<foo>s`) over template tokens (like `%{foo}`)
I thought I'd share what has helped me to work my way around that.

My code:

[...]
    when /RedHat|CentOS|Fedora|SLES/                                                                                  
      installed_packages = Facter::Core::Execution.exec('rpm -qa --qf "%{NAME} %{VERSION}\n"').split('\n')
    when /Debian|Ubuntu/
      ...
[...]

Was throwing an error:
rubocop: Prefer annotated tokens (like `%<foo>s`) over template tokens (like `%{foo}`).

Disabling Style/FormatStringToken right before and re-enabling it right after have helped my pipeline to succeed:

[...]
    when /RedHat|CentOS|Fedora|SLES/                                                                                  
      # rubocop:disable Style/FormatStringToken
      installed_packages = Facter::Core::Execution.exec('rpm -qa --qf "%{NAME} %{VERSION}\n"').split('\n')
      # rubocop:enable Style/FormatStringToken
    when /Debian|Ubuntu/
[...]

It may not be the correct/best way to solve it, but it was the quickest for someone like me who has little knowledge about robocup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants