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.

Sign up for GitHub

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

Jump to bottom

[Fix #7900] Fix Style/FormatStringToken false positive with formatted input and template style enforced, and add autocorrection #10160

Merged
merged 1 commit into from Jun 23, 2022
Merged

[Fix #7900] Fix Style/FormatStringToken false positive with formatted input and template style enforced, and add autocorrection #10160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions changelog/fix_fix_style_format_string_token_false_positive.md
View file
Open in desktop
@@ -0,0 +1 @@
* [#7900](https://github.com/rubocop/rubocop/issues/7900): Fix `Style/FormatStringToken` false positive with formatted input and `template` style enforced, and add autocorrection. ([@FnControlOption][])
65 changes: 48 additions & 17 deletions lib/rubocop/cop/style/format_string_token.rb
View file
Open in desktop
Expand Up @@ -69,6 +69,7 @@ module Style
class FormatStringToken < Base
include ConfigurableEnforcedStyle
include IgnoredMethods
extend AutoCorrector

def on_str(node)
return if format_string_token?(node) || use_ignored_method?(node)
Expand All @@ -77,13 +78,8 @@ def on_str(node)
return if detections.empty?
return if allowed_unannotated?(detections)

detections.each do |detected_style, token_range|
if detected_style == style
correct_style_detected
else
style_detected(detected_style)
add_offense(token_range, message: message(detected_style))
end
detections.each do |detected_sequence, token_range|
check_sequence(detected_sequence, token_range)
end
end

Expand All @@ -105,6 +101,38 @@ def use_ignored_method?(node)
(parent = node.parent) && parent.send_type? && ignored_method?(parent.method_name)
end

def check_sequence(detected_sequence, token_range)
if detected_sequence.style == style
correct_style_detected
elsif correctable_sequence?(detected_sequence.type)
style_detected(detected_sequence.style)
add_offense(token_range, message: message(detected_sequence.style)) do |corrector|
autocorrect_sequence(corrector, detected_sequence, token_range)
end
end
end

def correctable_sequence?(detected_type)
detected_type == 's' || style == :annotated || style == :unannotated
end

def autocorrect_sequence(corrector, detected_sequence, token_range)
return if style == :unannotated

name = detected_sequence.name
return if name.nil?

flags = detected_sequence.flags
width = detected_sequence.width
precision = detected_sequence.precision
type = detected_sequence.style == :template ? 's' : detected_sequence.type
Comment on lines +120 to +128
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that these lines can be simplified to:

return if style == :unannotated || detected_sequence.style == :unannotated

name = detected_sequence.name
flags = detected_sequence.flags
width = detected_sequence.width
precision = detected_sequence.precision
type = case detected_sequence.style
       when :annotated then detected_sequence.type
       when :template then 's'
       end

correction = case style
when :annotated then "%<#{name}>#{flags}#{width}#{precision}#{type}"
when :template then "%#{flags}#{width}#{precision}{#{name}}"
end
corrector.replace(token_range, correction)
end

def unannotated_format?(node, detected_style)
detected_style == :unannotated && !format_string_in_typical_context?(node)
end
Expand Down Expand Up @@ -142,30 +170,33 @@ def str_contents(source_map)
def token_ranges(contents)
format_string = RuboCop::Cop::Utils::FormatString.new(contents.source)

format_string.format_sequences.each do |seq|
next if seq.percent?
format_string.format_sequences.each do |detected_sequence|
next if detected_sequence.percent?

detected_style = seq.style
token = contents.begin.adjust(begin_pos: seq.begin_pos, end_pos: seq.end_pos)
token = contents.begin.adjust(begin_pos: detected_sequence.begin_pos,
end_pos: detected_sequence.end_pos)

yield(detected_style, token)
yield(detected_sequence, token)
end
end

def collect_detections(node)
detections = []
tokens(node) do |detected_style, token_range|
unless unannotated_format?(node, detected_style)
detections << [detected_style, token_range]
tokens(node) do |detected_sequence, token_range|
unless unannotated_format?(node, detected_sequence.style)
detections << [detected_sequence, token_range]
end
end
detections
end

def allowed_unannotated?(detections)
return false if detections.size > max_unannotated_placeholders_allowed
return false unless detections.all? do |detected_sequence,|
detected_sequence.style == :unannotated
end
return true if detections.size <= max_unannotated_placeholders_allowed

detections.all? { |detected_style,| detected_style == :unannotated }
detections.any? { |detected_sequence,| !correctable_sequence?(detected_sequence.type) }
end

def max_unannotated_placeholders_allowed
Expand Down