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

Small Style cops perf tweaks #8359

Merged
merged 1 commit into from Aug 5, 2020
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
12 changes: 11 additions & 1 deletion lib/rubocop/cop/style/access_modifier_declarations.rb
Expand Up @@ -65,6 +65,8 @@ module Style
class AccessModifierDeclarations < Cop
include ConfigurableEnforcedStyle

ACCESS_MODIFIERS = %i[private protected public module_function].to_set.freeze

GROUP_STYLE_MESSAGE = [
'`%<access_modifier>s` should not be',
'inlined in method definitions.'
Expand All @@ -80,7 +82,7 @@ class AccessModifierDeclarations < Cop
PATTERN

def on_send(node)
return unless node.access_modifier?
return unless access_modifier?(node)
return if node.parent.pair_type?
return if cop_config['AllowModifiersOnSymbols'] &&
access_modifier_with_symbol?(node)
Expand All @@ -96,6 +98,14 @@ def on_send(node)

private

def access_modifier?(node)
maybe_access_modifier?(node) && node.access_modifier?
Copy link
Contributor

Choose a reason for hiding this comment

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

This is fine, but shouldn't it be Node#access_modifier? itself that should be sped up instead of just this cop? I think just changing the order of the conditions in rubocop-ast should give you most of the speed up, but I may be wrong

end

def maybe_access_modifier?(node)
!node.receiver && ACCESS_MODIFIERS.include?(node.method_name)
end

def offense?(node)
(group_style? && access_modifier_is_inlined?(node)) ||
(inline_style? && access_modifier_is_not_inlined?(node))
Expand Down
6 changes: 3 additions & 3 deletions lib/rubocop/cop/style/colon_method_call.rb
Expand Up @@ -30,12 +30,12 @@ def self.autocorrect_incompatible_with
end

def on_send(node)
# ignore Java interop code like Java::int
return if java_type_node?(node)

return unless node.receiver && node.double_colon?
return if node.camel_case_method?

# ignore Java interop code like Java::int
return if java_type_node?(node)

add_offense(node, location: :dot)
end

Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/style/eval_with_location.rb
Expand Up @@ -37,6 +37,8 @@ class EvalWithLocation < Cop
MSG_INCORRECT_LINE = 'Use `%<expected>s` instead of `%<actual>s`, ' \
'as they are used by backtraces.'

EVAL_METHODS = %i[eval class_eval module_eval instance_eval].to_set.freeze

def_node_matcher :eval_without_location?, <<~PATTERN
{
(send nil? :eval ${str dstr})
Expand All @@ -61,6 +63,8 @@ class EvalWithLocation < Cop
PATTERN

def on_send(node)
return unless EVAL_METHODS.include?(node.method_name)

eval_without_location?(node) do |code|
if with_lineno?(node)
on_with_lineno(node, code)
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/style/expand_path_arguments.rb
Expand Up @@ -73,7 +73,10 @@ class ExpandPathArguments < Cop
$_) :parent) :expand_path)
PATTERN

# rubocop:disable Metrics/PerceivedComplexity
def on_send(node)
return unless node.method?(:expand_path)

if (captured_values = file_expand_path(node))
current_path, default_dir = captured_values

Expand All @@ -88,6 +91,7 @@ def on_send(node)
add_offense(node, message: PATHNAME_NEW_MSG)
end
end
# rubocop:enable Metrics/PerceivedComplexity

def autocorrect(node)
lambda do |corrector|
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/style/format_string.rb
Expand Up @@ -40,6 +40,8 @@ class FormatString < Cop

MSG = 'Favor `%<prefer>s` over `%<current>s`.'

FORMAT_METHODS = %i[format sprintf %].freeze

def_node_matcher :formatter, <<~PATTERN
{
(send nil? ${:sprintf :format} _ _ ...)
Expand All @@ -53,6 +55,8 @@ class FormatString < Cop
PATTERN

def on_send(node)
return unless FORMAT_METHODS.include?(node.method_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a pattern that comes back very often for on_send, right? It wouldn't be too hard to tweak Commissionner so that a Cop could specify a list of method_name to be called for (and not be bothered for others); it would be quite fast. Useful?

Copy link
Contributor

Choose a reason for hiding this comment

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

@marcandre I'd find that incredibly useful for many cops I write.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool :-) Please check #8365 and let me know how well that fits what you'd like. It could be made more general but I think that on_send and node.method_name are by far the most important checks.


formatter(node) do |selector|
detected_style = selector == :% ? :percent : selector

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/style/format_string_token.rb
Expand Up @@ -41,6 +41,7 @@ class FormatStringToken < Cop
include ConfigurableEnforcedStyle

def on_str(node)
return unless node.value.include?('%')
return if node.each_ancestor(:xstr, :regexp).any?

tokens(node) do |detected_style, token_range|
Expand Down
4 changes: 3 additions & 1 deletion lib/rubocop/cop/style/hash_syntax.rb
Expand Up @@ -62,6 +62,7 @@ class HashSyntax < Cop
MSG_NO_MIXED_KEYS = "Don't mix styles in the same hash."
MSG_HASH_ROCKETS = 'Use hash rockets syntax.'

# rubocop:disable Metrics/PerceivedComplexity, Metrics/AbcSize
def on_hash(node)
pairs = node.pairs

Expand All @@ -73,10 +74,11 @@ def on_hash(node)
ruby19_no_mixed_keys_check(pairs)
elsif style == :no_mixed_keys
no_mixed_keys_check(pairs)
else
elsif node.source.include?('=>')
ruby19_check(pairs)
end
end
# rubocop:enable Metrics/PerceivedComplexity, Metrics/AbcSize

def ruby19_check(pairs)
check(pairs, '=>', MSG_19) if sym_indices?(pairs)
Expand Down
5 changes: 2 additions & 3 deletions lib/rubocop/cop/style/inverse_methods.rb
Expand Up @@ -64,12 +64,11 @@ def self.autocorrect_incompatible_with
PATTERN

def on_send(node)
return if part_of_ignored_node?(node)

inverse_candidate?(node) do |_method_call, lhs, method, rhs|
return unless inverse_methods.key?(method)
return if possible_class_hierarchy_check?(lhs, rhs, method)
return if negated?(node)
return if part_of_ignored_node?(node)
return if possible_class_hierarchy_check?(lhs, rhs, method)

add_offense(node,
message: format(MSG, method: method,
Expand Down
Expand Up @@ -18,8 +18,8 @@ class MethodCallWithoutArgsParentheses < Cop
'no arguments.'

def on_send(node)
return if ineligible_node?(node)
return unless !node.arguments? && node.parenthesized?
return if ineligible_node?(node)
return if ignored_method?(node.method_name)
return if same_name_assignment?(node)

Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/style/numeric_predicate.rb
Expand Up @@ -53,7 +53,11 @@ class NumericPredicate < Cop
'negative?' => '<'
}.freeze

COMPARISON_METHODS = %i[== > < positive? negative? zero?].to_set.freeze

def on_send(node)
return unless COMPARISON_METHODS.include?(node.method_name)

numeric, replacement = check(node)
return unless numeric

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/style/random_with_offset.rb
Expand Up @@ -56,6 +56,7 @@ class RandomWithOffset < Cop
PATTERN

def on_send(node)
return unless node.receiver
return unless integer_op_rand?(node) ||
rand_op_integer?(node) ||
rand_modified?(node)
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/style/redundant_exception.rb
Expand Up @@ -23,9 +23,13 @@ class RedundantException < Base
MSG_2 = 'Redundant `RuntimeError.new` call can be replaced with ' \
'just the message.'

RAISE_METHODS = %i[raise fail].freeze

# Switch `raise RuntimeError, 'message'` to `raise 'message'`, and
# `raise RuntimeError.new('message')` to `raise 'message'`.
def on_send(node)
return unless RAISE_METHODS.include?(node.method_name)

fix_exploded(node) || fix_compact(node)
end

Expand Down
35 changes: 25 additions & 10 deletions lib/rubocop/cop/style/redundant_sort.rb
Expand Up @@ -55,6 +55,8 @@ class RedundantSort < Cop
MSG = 'Use `%<suggestion>s` instead of '\
'`%<sorter>s...%<accessor_source>s`.'

SORT_METHODS = %i[sort sort_by].freeze

def_node_matcher :redundant_sort?, <<~MATCHER
{
(send $(send _ $:sort ...) ${:last :first})
Expand All @@ -71,20 +73,25 @@ class RedundantSort < Cop
}
MATCHER

# rubocop:disable Metrics/AbcSize
def on_send(node)
redundant_sort?(node) do |sort_node, sorter, accessor|
range = range_between(
sort_node.loc.selector.begin_pos,
node.loc.expression.end_pos
)
return unless sort_method?(node)

add_offense(node,
location: range,
message: message(node,
sorter,
accessor))
if (sort_node, sorter, accessor = redundant_sort?(node.parent))
ancestor = node.parent
elsif (sort_node, sorter, accessor = redundant_sort?(node.parent&.parent))
ancestor = node.parent.parent
else
return
end

add_offense(ancestor,
location: offense_range(sort_node, ancestor),
message: message(ancestor,
sorter,
accessor))
end
# rubocop:enable Metrics/AbcSize

def autocorrect(node)
sort_node, sorter, accessor = redundant_sort?(node)
Expand All @@ -108,6 +115,14 @@ def autocorrect(node)

private

def sort_method?(node)
SORT_METHODS.include?(node.method_name)
end

def offense_range(sort_node, ancestor)
range_between(sort_node.loc.selector.begin_pos, ancestor.loc.expression.end_pos)
end

def message(node, sorter, accessor)
accessor_source = range_between(
node.loc.selector.begin_pos,
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/style/signal_exception.rb
Expand Up @@ -191,6 +191,8 @@ def check_send(method_name, node)
end

def command_or_kernel_call?(name, node)
return unless node.method?(name)

node.command?(name) || kernel_call?(node, name)
end

Expand Down
16 changes: 10 additions & 6 deletions lib/rubocop/cop/style/zero_length_predicate.rb
Expand Up @@ -30,6 +30,8 @@ class ZeroLengthPredicate < Cop
NONZERO_MSG = 'Use `!empty?` instead of ' \
'`%<lhs>s %<opr>s %<rhs>s`.'

LENGTH_METHODS = %i[size length].freeze

def on_send(node)
check_zero_length_predicate(node)
check_nonzero_length_predicate(node)
Expand All @@ -44,31 +46,33 @@ def autocorrect(node)
private

def check_zero_length_predicate(node)
zero_length_predicate = zero_length_predicate(node)
return unless LENGTH_METHODS.include?(node.method_name)

zero_length_predicate = zero_length_predicate(node.parent)
return unless zero_length_predicate

lhs, opr, rhs = zero_length_predicate

return if non_polymorphic_collection?(node)
return if non_polymorphic_collection?(node.parent)

add_offense(
node,
node.parent,
message: format(ZERO_MSG, lhs: lhs, opr: opr, rhs: rhs)
)
end

def check_nonzero_length_predicate(node)
nonzero_length_predicate = nonzero_length_predicate(node)
return unless LENGTH_METHODS.include?(node.method_name)

nonzero_length_predicate = nonzero_length_predicate(node.parent)
return unless nonzero_length_predicate

lhs, opr, rhs = nonzero_length_predicate

return if non_polymorphic_collection?(node)
return if non_polymorphic_collection?(node.parent)

add_offense(
node,
node.parent,
message: format(NONZERO_MSG, lhs: lhs, opr: opr, rhs: rhs)
)
end
Expand Down