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

Enhance Style/FormatString's autocorrection #12234

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12234](https://github.com/rubocop/rubocop/pull/12234): Enhance `Style/FormatString`'s autocorrection when using known conversion methods whose return value is not an array. ([@koic][])
27 changes: 24 additions & 3 deletions lib/rubocop/cop/style/format_string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,25 @@ module RuboCop
module Cop
module Style
# Enforces the use of a single string formatting utility.
# Valid options include Kernel#format, Kernel#sprintf and String#%.
# Valid options include `Kernel#format`, `Kernel#sprintf`, and `String#%`.
#
# The detection of String#% cannot be implemented in a reliable
# The detection of `String#%` cannot be implemented in a reliable
# manner for all cases, so only two scenarios are considered -
# if the first argument is a string literal and if the second
# argument is an array literal.
#
# Autocorrection will be applied when using argument is a literal or known built-in conversion
# methods such as `to_d`, `to_f`, `to_h`, `to_i`, `to_r`, `to_s`, and `to_sym` on variables,
# provided that their return value is not an array. For example, when using `to_s`,
# `'%s' % [1, 2, 3].to_s` can be autocorrected without any incompatibility:
#
# [source,ruby]
# ----
# '%s' % [1, 2, 3] #=> '1'
# format('%s', [1, 2, 3]) #=> '[1, 2, 3]'
# '%s' % [1, 2, 3].to_s #=> '[1, 2, 3]'
# ----
#
# @example EnforcedStyle: format (default)
# # bad
# puts sprintf('%10s', 'hoge')
Expand Down Expand Up @@ -42,6 +54,9 @@ class FormatString < Base
MSG = 'Favor `%<prefer>s` over `%<current>s`.'
RESTRICT_ON_SEND = %i[format sprintf %].freeze

# Known conversion methods whose return value is not an array.
AUTOCORRECTABLE_METHODS = %i[to_d to_f to_h to_i to_r to_s to_sym].freeze

# @!method formatter(node)
def_node_matcher :formatter, <<~PATTERN
{
Expand All @@ -53,7 +68,7 @@ class FormatString < Base

# @!method variable_argument?(node)
def_node_matcher :variable_argument?, <<~PATTERN
(send {str dstr} :% {send_type? lvar_type?})
(send {str dstr} :% #autocorrectable?)
PATTERN

def on_send(node)
Expand All @@ -70,6 +85,12 @@ def on_send(node)

private

def autocorrectable?(node)
return true if node.lvar_type?

node.send_type? && !AUTOCORRECTABLE_METHODS.include?(node.method_name)
end

def message(detected_style)
format(MSG, prefer: method_name(style), current: method_name(detected_style))
end
Expand Down
158 changes: 158 additions & 0 deletions spec/rubocop/cop/style/format_string_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,85 @@
RUBY
end

context 'when using a known autocorrectable method that does not convert to an array' do
it 'registers an offense for `to_s`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_s
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_s)
RUBY
end

it 'registers an offense for `to_h`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_h
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_h)
RUBY
end

it 'registers an offense for `to_i`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_i
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_i)
RUBY
end

it 'registers an offense for `to_f`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_f
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_f)
RUBY
end

it 'registers an offense for `to_r`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_r
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_r)
RUBY
end

it 'registers an offense for `to_d`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_d
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_d)
RUBY
end

it 'registers an offense for `to_sym`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_sym
^ Favor `sprintf` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts sprintf("%s", a.to_sym)
RUBY
end
end

it 'registers an offense for variable argument but does not autocorrect' do
expect_offense(<<~RUBY)
puts "%f" % a
Expand Down Expand Up @@ -134,6 +213,85 @@
RUBY
end

context 'when using a known conversion method that does not convert to an array' do
it 'registers an offense for `to_s`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_s
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_s)
RUBY
end

it 'registers an offense for `to_h`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_h
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_h)
RUBY
end

it 'registers an offense for `to_i`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_i
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_i)
RUBY
end

it 'registers an offense for `to_f`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_f
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_f)
RUBY
end

it 'registers an offense for `to_r`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_r
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_r)
RUBY
end

it 'registers an offense for `to_d`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_d
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_d)
RUBY
end

it 'registers an offense for `to_sym`' do
expect_offense(<<~RUBY)
puts "%s" % a.to_sym
^ Favor `format` over `String#%`.
RUBY

expect_correction(<<~RUBY)
puts format("%s", a.to_sym)
RUBY
end
end

it 'registers an offense for variable argument but does not autocorrect' do
expect_offense(<<~RUBY)
puts "%f" % a
Expand Down