Skip to content

Commit

Permalink
Add on_after_<type> callbacks
Browse files Browse the repository at this point in the history
I doubled check that this has basically no performance impact (less than ~0.1% overall) after #8882
  • Loading branch information
marcandre authored and mergify[bot] committed Oct 20, 2020
1 parent a6dd18c commit def3600
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@
* [#8892](https://github.com/rubocop-hq/rubocop/issues/8892): Fix an error for `Style/StringConcatenation` when correcting nested concatenable parts. ([@fatkodima][])
* [#8781](https://github.com/rubocop-hq/rubocop/issues/8781): Fix handling of comments in `Style/SafeNavigation` autocorrection. ([@dvandersluis][])
* [#8907](https://github.com/rubocop-hq/rubocop/pull/8907): Fix an incorrect auto-correct for `Layout/ClassStructure` when heredoc constant is defined after public method. ([@koic][])
* [#8889](https://github.com/rubocop-hq/rubocop/pull/8889): Cops can use new `after_<type>` callbacks (only for nodes that may have children nodes, like `:send` and unlike `:sym`). ([@marcandre][])

### Changes

Expand Down
2 changes: 2 additions & 0 deletions docs/modules/ROOT/pages/development.adoc
Expand Up @@ -248,6 +248,8 @@ module RuboCop
end
----

Note that `on_send` will be called on a given `node` before the callbacks `on_<some type>` for its children are called. There's also a callback `after_send` that is called after the children are processed. There's a similar `after_<some type>` callback for all types, except those that never have children.

Update the spec to cover the expected syntax:

[source,ruby]
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/base.rb
Expand Up @@ -271,7 +271,7 @@ def callbacks_needed
# @api private
def self.callbacks_needed
@callbacks_needed ||= public_instance_methods.select do |m|
m.match?(/^on_/) &&
m.match?(/^on_|^after_/) &&
!Base.method_defined?(m) # exclude standard "callbacks" like 'on_begin_investigation'
end
end
Expand Down
20 changes: 11 additions & 9 deletions lib/rubocop/cop/commissioner.rb
Expand Up @@ -7,7 +7,7 @@ module Cop
class Commissioner
include RuboCop::AST::Traversal

RESTRICTED_CALLBACKS = %i[on_send on_csend].freeze
RESTRICTED_CALLBACKS = %i[on_send on_csend after_send after_csend].freeze
private_constant :RESTRICTED_CALLBACKS

# How a Commissioner returns the results of the investigation
Expand Down Expand Up @@ -60,16 +60,18 @@ def initialize(cops, forces = [], options = {})
method_name = :"on_#{node_type}"
next unless method_defined?(method_name)

if RESTRICTED_CALLBACKS.include?(method_name)
trigger_restricted = "trigger_restricted_cops(:on_#{node_type}, node)"
end
# Hacky: Comment-out code as needed
r = '#' unless RESTRICTED_CALLBACKS.include?(method_name) # has Restricted?
c = '#' if NO_CHILD_NODES.include?(node_type) # has Children?

class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
def on_#{node_type}(node)
trigger_responding_cops(:on_#{node_type}, node)
#{trigger_restricted}
#{'super(node)' unless NO_CHILD_NODES.include?(node_type)}
end
def on_#{node_type}(node)
trigger_responding_cops(:on_#{node_type}, node)
#{r} trigger_restricted_cops(:on_#{node_type}, node)
#{c} super(node)
#{c} trigger_responding_cops(:after_#{node_type}, node)
#{c}#{r} trigger_restricted_cops(:after_#{node_type}, node)
end
RUBY
end

Expand Down
11 changes: 11 additions & 0 deletions spec/rubocop/cop/commissioner_spec.rb
Expand Up @@ -13,6 +13,10 @@ def on_int(node); end
alias_method :on_def, :on_int
alias_method :on_send, :on_int
alias_method :on_csend, :on_int
alias_method :after_int, :on_int
alias_method :after_def, :on_int
alias_method :after_send, :on_int
alias_method :after_csend, :on_int
end)
end
let(:cop) do
Expand Down Expand Up @@ -51,6 +55,9 @@ def method

it 'traverses the AST and invoke cops specific callbacks' do
expect(cop).to receive(:on_def).once
expect(cop).to receive(:on_int).once
expect(cop).not_to receive(:after_int)
expect(cop).to receive(:after_def).once
offenses
end

Expand All @@ -72,7 +79,9 @@ def method

it 'calls on_send for the right method calls' do
expect(cop).to receive(:on_send).once
expect(cop).to receive(:after_send).once
expect(cop).not_to receive(:on_csend)
expect(cop).not_to receive(:after_csend)
offenses
end

Expand All @@ -82,6 +91,8 @@ def method
it 'calls on_send for the right method calls' do
expect(cop).to receive(:on_send).once
expect(cop).to receive(:on_csend).once
expect(cop).to receive(:after_send).once
expect(cop).to receive(:after_csend).once
offenses
end
end
Expand Down

0 comments on commit def3600

Please sign in to comment.