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 after_<type> callbacks #8889

Merged
merged 1 commit into from Oct 20, 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
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