Skip to content

Commit

Permalink
[rubocop#3666] Resolve uncommunicative name offenses
Browse files Browse the repository at this point in the history
  • Loading branch information
garettarrowood committed Dec 28, 2017
1 parent a559956 commit 76aad6c
Show file tree
Hide file tree
Showing 15 changed files with 68 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def no_surrounding_space?(arg, equals)
!arg.space_after? && !equals.space_after?
end

def message(_)
def message(_node)
format(MSG, type: style == :space ? 'missing' : 'detected')
end
end
Expand Down
27 changes: 14 additions & 13 deletions lib/rubocop/cop/layout/space_around_operators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,34 +110,35 @@ def operator_with_regular_syntax?(send_node)
!IRREGULAR_METHODS.include?(send_node.method_name)
end

def check_operator(op, right_operand)
with_space = range_with_surrounding_space(range: op)
def check_operator(operator, right_operand)
with_space = range_with_surrounding_space(range: operator)
return if with_space.source.start_with?("\n")

offense(op, with_space, right_operand) do |msg|
add_offense(with_space, location: op, message: msg)
offense(operator, with_space, right_operand) do |msg|
add_offense(with_space, location: operator, message: msg)
end
end

def offense(op, with_space, right_operand)
msg = offense_message(op, with_space, right_operand)
def offense(operator, with_space, right_operand)
msg = offense_message(operator, with_space, right_operand)
yield msg if msg
end

def offense_message(op, with_space, right_operand)
if op.is?('**')
def offense_message(operator, with_space, right_operand)
if operator.is?('**')
'Space around operator `**` detected.' unless with_space.is?('**')
elsif with_space.source !~ /^\s.*\s$/
"Surrounding space missing for operator `#{op.source}`."
elsif excess_leading_space?(op, with_space) ||
"Surrounding space missing for operator `#{operator.source}`."
elsif excess_leading_space?(operator, with_space) ||
excess_trailing_space?(right_operand, with_space)
"Operator `#{op.source}` should be surrounded by a single space."
"Operator `#{operator.source}` should be surrounded " \
'by a single space.'
end
end

def excess_leading_space?(op, with_space)
def excess_leading_space?(operator, with_space)
with_space.source =~ /^ / &&
(!allow_for_alignment? || !aligned_with_operator?(op))
(!allow_for_alignment? || !aligned_with_operator?(operator))
end

def excess_trailing_space?(right_operand, with_space)
Expand Down
22 changes: 11 additions & 11 deletions lib/rubocop/cop/layout/trailing_blank_lines.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@ class TrailingBlankLines < Cop
include ConfigurableEnforcedStyle

def investigate(processed_source)
sb = processed_source.buffer
return if sb.source.empty?
buffer = processed_source.buffer
return if buffer.source.empty?

# The extra text that comes after the last token could be __END__
# followed by some data to read. If so, we don't check it because
# there could be good reasons why it needs to end with a certain
# number of newlines.
return if ends_in_end?(processed_source)

whitespace_at_end = sb.source[/\s*\Z/]
whitespace_at_end = buffer.source[/\s*\Z/]
blank_lines = whitespace_at_end.count("\n") - 1
wanted_blank_lines = style == :final_newline ? 0 : 1

return unless blank_lines != wanted_blank_lines

offense_detected(sb, wanted_blank_lines, blank_lines,
offense_detected(buffer, wanted_blank_lines, blank_lines,
whitespace_at_end)
end

Expand All @@ -36,25 +36,25 @@ def autocorrect(range)

private

def offense_detected(sb, wanted_blank_lines, blank_lines,
def offense_detected(buffer, wanted_blank_lines, blank_lines,
whitespace_at_end)
begin_pos = sb.source.length - whitespace_at_end.length
autocorrect_range = range_between(begin_pos, sb.source.length)
begin_pos = buffer.source.length - whitespace_at_end.length
autocorrect_range = range_between(begin_pos, buffer.source.length)
begin_pos += 1 unless whitespace_at_end.empty?
report_range = range_between(begin_pos, sb.source.length)
report_range = range_between(begin_pos, buffer.source.length)

add_offense(autocorrect_range,
location: report_range,
message: message(wanted_blank_lines, blank_lines))
end

def ends_in_end?(processed_source)
sb = processed_source.buffer
buffer = processed_source.buffer

return true if sb.source.strip.start_with?('__END__')
return true if buffer.source.strip.start_with?('__END__')
return false if processed_source.tokens.empty?

extra = sb.source[processed_source.tokens.last.end_pos..-1]
extra = buffer.source[processed_source.tokens.last.end_pos..-1]
extra && extra.strip.start_with?('__END__')
end

Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/cop/mixin/hash_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def checkable_layout?(_node)
true
end

def deltas_for_first_pair(*)
def deltas_for_first_pair(*_nodes)
{}
end

Expand Down Expand Up @@ -93,7 +93,7 @@ def value_delta(first_pair, current_pair)
class SeparatorAlignment
include ValueAlignment

def deltas_for_first_pair(*)
def deltas_for_first_pair(*_nodes)
{}
end

Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/mixin/trailing_comma.rb
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ def no_elements_on_same_line?(node)
items.each_cons(2).none? { |a, b| on_same_line?(a, b) }
end

def on_same_line?(a, b)
a.last_line == b.line
def on_same_line?(range1, range2)
range1.last_line == range2.line
end

def avoid_comma(kind, comma_begin_pos, extra_info)
Expand Down Expand Up @@ -163,7 +163,7 @@ def autocorrect_range(item)
end

# By default, there's no reason to avoid auto-correct.
def avoid_autocorrect?(_)
def avoid_autocorrect?(_nodes)
false
end
end
Expand Down
9 changes: 5 additions & 4 deletions lib/rubocop/cop/style/comment_annotation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ class CommentAnnotation < Cop
'is missing a note.'.freeze

def investigate(processed_source)
processed_source.comments.each_with_index do |comment, ix|
next unless first_comment_line?(processed_source.comments, ix)
processed_source.comments.each_with_index do |comment, index|
next unless first_comment_line?(processed_source.comments, index)

margin, first_word, colon, space, note = split_comment(comment)
next unless annotation?(comment) &&
Expand All @@ -68,8 +68,9 @@ def autocorrect(comment)

private

def first_comment_line?(comments, ix)
ix.zero? || comments[ix - 1].loc.line < comments[ix].loc.line - 1
def first_comment_line?(comments, index)
index.zero? ||
comments[index - 1].loc.line < comments[index].loc.line - 1
end

def annotation_range(comment, margin, length)
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/string_literals.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def detect_quote_styles(node)
styles.map(&:source).uniq
end

def message(*)
def message(_node)
if style == :single_quotes
"Prefer single-quoted strings when you don't need string " \
'interpolation or special symbols.'
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/style/string_literals_in_interpolation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def autocorrect(node)

private

def message(*)
def message(_node)
# single_quotes -> single-quoted
kind = style.to_s.sub(/_(.*)s/, '-\1d')

Expand Down
10 changes: 5 additions & 5 deletions lib/rubocop/cop/team.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,21 +164,21 @@ def process_commissioner_errors(file, file_errors)
end
end

def handle_warning(e, location)
message = Rainbow("#{e.message} (from file: #{location})").yellow
def handle_warning(error, location)
message = Rainbow("#{error.message} (from file: #{location})").yellow

@warnings << message
warn message
puts e.backtrace if debug?
puts error.backtrace if debug?
end

def handle_error(e, location, cop)
def handle_error(error, location, cop)
message = Rainbow("An error occurred while #{cop.name}" \
" cop was inspecting #{location}.").red
@errors << message
warn message
if debug?
puts e.message, e.backtrace
puts error.message, error.backtrace
else
warn 'To see the complete backtrace run rubocop -d.'
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/formatter/fuubar_style_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ module Formatter
class FuubarStyleFormatter < ClangStyleFormatter
RESET_SEQUENCE = "\e[0m".freeze

def initialize(*)
def initialize(*output)
@severest_offense = nil

super
Expand Down
4 changes: 2 additions & 2 deletions lib/rubocop/formatter/html_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ def possible_ellipses(location)
location.first_line == location.last_line ? '' : " #{ELLIPSES}"
end

def escape(s)
CGI.escapeHTML(s)
def escape(string)
CGI.escapeHTML(string)
end

def base64_encoded_logo_image
Expand Down
17 changes: 9 additions & 8 deletions lib/rubocop/string_util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ def self.distance(*args)
new(*args).distance
end

def initialize(a, b)
if a.size < b.size
@shorter = a
@longer = b
def initialize(string_a, string_b)
if string_a.size < string_b.size
@shorter = string_a
@longer = string_b
else
@shorter = b
@longer = a
@shorter = string_b
@longer = string_a
end
end

Expand Down Expand Up @@ -116,8 +116,9 @@ class JaroWinkler < Jaro

attr_reader :boost_threshold, :scaling_factor

def initialize(a, b, boost_threshold = nil, scaling_factor = nil)
super(a, b)
def initialize(string_a, string_b,
boost_threshold = nil, scaling_factor = nil)
super(string_a, string_b)
@boost_threshold = boost_threshold || DEFAULT_BOOST_THRESHOLD
@scaling_factor = scaling_factor || DEFAULT_SCALING_FACTOR
end
Expand Down
2 changes: 2 additions & 0 deletions manual/cops_naming.md
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ baz { |age, height, gender| do_stuff(age, height, gender) }
Name | Default value | Configurable values
--- | --- | ---
MinParamNameLength | `1` | Integer
AllowedNames | `<none>` |
## Naming/UncommunicativeMethodArgName
Expand Down Expand Up @@ -446,6 +447,7 @@ end
Name | Default value | Configurable values
--- | --- | ---
MinArgNameLength | `3` | Integer
AllowedNames | `<none>` |

## Naming/VariableName

Expand Down
12 changes: 6 additions & 6 deletions spec/rubocop/node_pattern_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1027,20 +1027,20 @@
describe 'funcalls' do
module RuboCop
class NodePattern
def goodmatch(_arg1)
def goodmatch(_foo)
true
end

def badmatch(_arg1)
def badmatch(_foo)
false
end

def witharg(_arg1, arg2)
arg2
def witharg(_foo, bar)
bar
end

def withargs(_arg1, _arg2, arg3)
arg3
def withargs(_foo, _bar, qux)
qux
end
end
end
Expand Down
10 changes: 5 additions & 5 deletions tasks/cops_documentation.rake
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,17 @@ task generate_cops_documentation: :yard_for_generate_documentation do
table.join("\n") + "\n"
end

def format_table_value(v)
def format_table_value(val)
value =
case v
case val
when Array
if v.empty?
if val.empty?
'`[]`'
else
v.map { |config| format_table_value(config) }.join(', ')
val.map { |config| format_table_value(config) }.join(', ')
end
else
"`#{v.nil? ? '<none>' : v}`"
"`#{val.nil? ? '<none>' : val}`"
end
value.gsub("#{Dir.pwd}/", '').rstrip
end
Expand Down

0 comments on commit 76aad6c

Please sign in to comment.