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

Add new InternalAffairs/NodeMatcherDirective cop. #9506

Merged
merged 2 commits into from
Feb 27, 2021
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
1 change: 1 addition & 0 deletions lib/rubocop/cop/bundler/duplicated_gem.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def on_new_investigation

private

# @!method gem_declarations(node)
def_node_search :gem_declarations, '(send nil? :gem str ...)'

def duplicated_gem_nodes
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/bundler/gem_comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class GemComment < Base
VERSION_SPECIFIERS_OPTION = 'version_specifiers'
RESTRICT_ON_SEND = %i[gem].freeze

# @!method gem_declaration?(node)
def_node_matcher :gem_declaration?, '(send nil? :gem str ...)'

def on_send(node)
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/bundler/insecure_protocol_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class InsecureProtocolSource < Base

RESTRICT_ON_SEND = %i[source].freeze

# @!method insecure_protocol_source?(node)
def_node_matcher :insecure_protocol_source?, <<~PATTERN
(send nil? :source
$(sym ${:gemcutter :rubygems :rubyforge}))
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/bundler/ordered_gems.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ def previous_declaration(node)
declarations.to_a[node_index - 1]
end

# @!method gem_declarations(node)
def_node_search :gem_declarations, <<~PATTERN
(:send nil? :gem str ...)
PATTERN
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/gemspec/date_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class DateAssignment < Base

MSG = 'Do not use `date =` in gemspec, it is set automatically when the gem is packaged.'

# @!method gem_specification(node)
def_node_matcher :gem_specification, <<~PATTERN
(block
(send
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/gemspec/duplicated_assignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class DuplicatedAssignment < Base
MSG = '`%<assignment>s` method calls already given on line '\
'%<line_of_first_occurrence>d of the gemspec.'

# @!method gem_specification(node)
def_node_search :gem_specification, <<~PATTERN
(block
(send
Expand All @@ -49,6 +50,7 @@ class DuplicatedAssignment < Base
(arg $_)) ...)
PATTERN

# @!method assignment_method_declarations(node)
def_node_search :assignment_method_declarations, <<~PATTERN
(send
(lvar #match_block_variable_name?) #assignment_method? ...)
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/gemspec/ordered_dependencies.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ def get_dependency_name(node)
node.method_name
end

# @!method dependency_declarations(node)
def_node_search :dependency_declarations, <<~PATTERN
(send (lvar _) {:add_dependency :add_runtime_dependency :add_development_dependency} (str _) ...)
PATTERN
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/gemspec/required_ruby_version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,12 @@ class RequiredRubyVersion < Base
'.rubocop.yml) should be equal.'
MISSING_MSG = '`required_ruby_version` should be specified.'

# @!method required_ruby_version(node)
def_node_search :required_ruby_version, <<~PATTERN
(send _ :required_ruby_version= $_)
PATTERN

# @!method defined_ruby_version(node)
def_node_matcher :defined_ruby_version, <<~PATTERN
{$(str _) $(array (str _) (str _))
(send (const (const nil? :Gem) :Requirement) :new $(str _))}
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/gemspec/ruby_version_globals_usage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ module Gemspec
class RubyVersionGlobalsUsage < Base
MSG = 'Do not use `RUBY_VERSION` in gemspec file.'

# @!method ruby_version?(node)
def_node_matcher :ruby_version?, '(const {cbase nil?} :RUBY_VERSION)'

# @!method gem_specification?(node)
def_node_search :gem_specification?, <<~PATTERN
(block
(send
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require_relative 'internal_affairs/example_description'
require_relative 'internal_affairs/method_name_equal'
require_relative 'internal_affairs/node_destructuring'
require_relative 'internal_affairs/node_matcher_directive'
require_relative 'internal_affairs/node_type_predicate'
require_relative 'internal_affairs/offense_location_keyword'
require_relative 'internal_affairs/redundant_described_class_as_subject'
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs/example_description.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class << self
/\b(does not|doesn't) (auto[- ]?)?correct/
].freeze

# @!method offense_example?(node)
def_node_matcher :offense_example?, <<~PATTERN
(block
(send _ {:it :specify} $_description)
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs/method_name_equal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class MethodNameEqual < Base
'`method_name == %<method_name>s`.'
RESTRICT_ON_SEND = %i[==].freeze

# @!method method_name?(node)
def_node_matcher :method_name?, <<~PATTERN
(send
$(send
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/internal_affairs/node_destructuring.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ class NodeDestructuring < Base
MSG = 'Use the methods provided with the node extensions instead ' \
'of manually destructuring nodes.'

# @!method node_variable?(node)
def_node_matcher :node_variable?, <<~PATTERN
{(lvar [#node_suffix? _]) (send nil? [#node_suffix? _])}
PATTERN

# @!method node_destructuring?(node)
def_node_matcher :node_destructuring?, <<~PATTERN
{(masgn (mlhs ...) {(send #node_variable? :children) (array (splat #node_variable?))})}
PATTERN
Expand Down
151 changes: 151 additions & 0 deletions lib/rubocop/cop/internal_affairs/node_matcher_directive.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
# frozen_string_literal: true

module RuboCop
module Cop
module InternalAffairs
# Checks that node matcher definitions are tagged with a YARD `@!method`
# directive so that editors are able to find the dynamically defined
# method.
#
# @example
# # bad
# def_node_matcher :foo?, <<~PATTERN
# ...
# PATTERN
#
# # good
# # @!method foo?(node)
# def_node_matcher :foo?, <<~PATTERN
# ...
# PATTERN
#
class NodeMatcherDirective < Base
extend AutoCorrector
include RangeHelp

MSG = 'Preceed `%<method>s` with a `@!method` YARD directive.'
MSG_WRONG_NAME = '`@!method` YARD directive has invalid method name, ' \
'use `%<expected>s` instead of `%<actual>s`.'
MSG_TOO_MANY = 'Multiple `@!method` YARD directives found for this matcher.'

RESTRICT_ON_SEND = %i[def_node_matcher def_node_search].to_set.freeze
REGEXP = /^\s*#\s*@!method\s+(?<method_name>[a-z0-9_]+[?!]?)(?:\((?<args>.*)\))?/.freeze

# @!method pattern_matcher?(node)
def_node_matcher :pattern_matcher?, <<~PATTERN
(send _ %RESTRICT_ON_SEND {str sym} {str dstr})
PATTERN

def on_send(node)
return if node.arguments.none?
return unless valid_method_name?(node)

actual_name = node.arguments.first.value
directives = method_directives(node)
return too_many_directives(node) if directives.size > 1

directive = directives.first
return if directive_correct?(directive, actual_name)

register_offense(node, directive, actual_name)
end

private

def valid_method_name?(node)
node.arguments.first.str_type? || node.arguments.first.sym_type?
end

def method_directives(node)
comments = processed_source.ast_with_comments[node]

comments.map do |comment|
match = comment.text.match(REGEXP)
next unless match

{ node: comment, method_name: match[:method_name], args: match[:args] }
end.compact
end

def too_many_directives(node)
add_offense(node, message: MSG_TOO_MANY)
end

def directive_correct?(directive, actual_name)
directive && directive[:method_name] == actual_name.to_s
end

def register_offense(node, directive, actual_name)
message = formatted_message(directive, actual_name, node.method_name)

add_offense(node, message: message) do |corrector|
if directive
correct_directive(corrector, directive, actual_name)
else
insert_directive(corrector, node, actual_name)
end
end
end

def formatted_message(directive, actual_name, method_name)
if directive
format(MSG_WRONG_NAME, expected: actual_name, actual: directive[:method_name])
else
format(MSG, method: method_name)
end
end

def insert_directive(corrector, node, actual_name)
# If the pattern matcher uses arguments (`%1`, `%2`, etc.), include them in the directive
arguments = pattern_arguments(node.arguments[1].value)

range = range_with_surrounding_space(
range: node.loc.expression,
side: :left,
newlines: false
)
indentation = range.source.match(/^\s*/)[0]
directive = "#{indentation}# @!method #{actual_name}(#{arguments.join(', ')})\n"
directive = "\n#{directive}" if add_newline?(node)

corrector.insert_before(range, directive)
end

def pattern_arguments(pattern)
arguments = %w[node]
max_pattern_var = pattern.scan(/(?<=%)\d+/).map(&:to_i).max
max_pattern_var&.times { |i| arguments << "arg#{i + 1}" }
arguments
end

def add_newline?(node)
# Determine if a blank line should be inserted before the new directive
# in order to spread out pattern matchers
return if node.sibling_index&.zero?
return unless node.parent

prev_sibling = node.parent.child_nodes[node.sibling_index - 1]
return unless prev_sibling && pattern_matcher?(prev_sibling)

node.loc.line == last_line(prev_sibling) + 1
end

def last_line(node)
if node.last_argument.heredoc?
node.last_argument.loc.heredoc_end.line
else
node.loc.last_line
end
end

def correct_directive(corrector, directive, actual_name)
correct = "@!method #{actual_name}"
regexp = /@!method\s+#{Regexp.escape(directive[:method_name])}/

replacement = directive[:node].text.gsub(regexp, correct)
corrector.replace(directive[:node], replacement)
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/internal_affairs/node_type_predicate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class NodeTypePredicate < Base
MSG = 'Use `#%<type>s_type?` to check node type.'
RESTRICT_ON_SEND = %i[==].freeze

# @!method node_type_check(node)
def_node_matcher :node_type_check, <<~PATTERN
(send (send $_ :type) :== (sym $_))
PATTERN
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/internal_affairs/offense_location_keyword.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ def on_send(node)

private

# @!method node_type_check(node)
def_node_matcher :node_type_check, <<~PATTERN
(send nil? :add_offense $_node $hash)
PATTERN

# @!method offending_location_argument(node)
def_node_matcher :offending_location_argument, <<~PATTERN
(pair (sym :location) $(send (send $_node :loc) $_keyword))
PATTERN
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class RedundantDescribedClassAsSubject < Base

MSG = 'Remove the redundant `subject`%<additional_message>s.'

# @!method described_class_subject?(node)
def_node_matcher :described_class_subject?, <<~PATTERN
(block
(send nil? :subject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class RedundantLetRuboCopConfigNew < Base

MSG = 'Remove `let` that is `RuboCop::Config.new` with no arguments%<additional_message>s.'

# @!method let_rubocop_config_new?(node)
def_node_matcher :let_rubocop_config_new?, <<~PATTERN
(block
(send nil? :let
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class RedundantLocationArgument < Base
MSG = 'Redundant location argument to `#add_offense`.'
RESTRICT_ON_SEND = %i[add_offense].freeze

# @!method redundant_location_argument(node)
def_node_matcher :redundant_location_argument, <<~PATTERN
(send nil? :add_offense _
(hash <$(pair (sym :location) (sym :expression)) ...>)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,19 @@ class RedundantMessageArgument < Base
MSG = 'Redundant message argument to `#add_offense`.'
RESTRICT_ON_SEND = %i[add_offense].freeze

# @!method node_type_check(node)
def_node_matcher :node_type_check, <<~PATTERN
(send nil? :add_offense $_node $hash)
PATTERN

# @!method redundant_message_argument(node)
def_node_matcher :redundant_message_argument, <<~PATTERN
(pair
(sym :message)
${(const nil? :MSG) (send nil? :message) (send nil? :message _)})
PATTERN

# @!method message_method_call(node)
def_node_matcher :message_method_call, '(send nil? :message $_node)'

def on_send(node)
Expand Down
4 changes: 4 additions & 0 deletions lib/rubocop/cop/internal_affairs/style_detected_api_use.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,18 +67,22 @@ class StyleDetectedApiUse < Base
no_acceptable_style! style_detected
].freeze

# @!method correct_style_detected_check(node)
def_node_matcher :correct_style_detected_check, <<~PATTERN
(send nil? :correct_style_detected)
PATTERN

# @!method negative_style_detected_method_check(node)
def_node_matcher :negative_style_detected_method_check, <<~PATTERN
(send nil? /(?:opposite|unexpected|ambiguous|unrecognized)_style_detected|conflicting_styles_detected/ ...)
PATTERN

# @!method no_acceptable_style_check(node)
def_node_matcher :no_acceptable_style_check, <<~PATTERN
(send nil? :no_acceptable_style!)
PATTERN

# @!method style_detected_check(node)
def_node_matcher :style_detected_check, <<~PATTERN
(send nil? :style_detected ...)
PATTERN
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/internal_affairs/useless_message_assertion.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ module InternalAffairs
class UselessMessageAssertion < Base
MSG = 'Do not specify cop behavior using `described_class::MSG`.'

# @!method described_class_msg(node)
def_node_search :described_class_msg, <<~PATTERN
(const (send nil? :described_class) :MSG)
PATTERN

# @!method rspec_expectation_on_msg?(node)
def_node_matcher :rspec_expectation_on_msg?, <<~PATTERN
(send (send nil? :expect #contains_described_class_msg?) :to ...)
PATTERN
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/layout/block_alignment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ class BlockAlignment < Base

MSG = '%<current>s is not aligned with %<prefer>s%<alt_prefer>s.'

# @!method block_end_align_target?(node, child)
def_node_matcher :block_end_align_target?, <<~PATTERN
{assignment?
splat
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/layout/class_structure.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ class ClassStructure < Base
MSG = '`%<category>s` is supposed to appear before ' \
'`%<previous>s`.'

# @!method dynamic_constant?(node)
def_node_matcher :dynamic_constant?, <<~PATTERN
(casgn nil? _ (send ...))
PATTERN
Expand Down