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

[Fix #11018] Add AllowedMethods and AllowedPatterns for Lint/NestedMethodDefinition #11019

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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#11018](https://github.com/rubocop/rubocop/issues/11018): Add `AllowedMethods` and `AllowedPatterns` for `Lint/NestedMethodDefinition`. ([@koic][])
2 changes: 2 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1957,6 +1957,8 @@ Lint/NestedMethodDefinition:
Description: 'Do not use nested method definitions.'
StyleGuide: '#no-nested-methods'
Enabled: true
AllowedMethods: []
AllowedPatterns: []
VersionAdded: '0.32'

Lint/NestedPercentLiteral:
Expand Down
51 changes: 50 additions & 1 deletion lib/rubocop/cop/lint/nested_method_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ module Lint
#
# # good
#
# # `class_eval`, `instance_eval`, `module_eval`, `class_exec`, `instance_exec`, and
# # `module_exec` blocks are allowed by default.
#
# def foo
# self.class.class_eval do
# def bar
Expand All @@ -54,7 +57,47 @@ module Lint
# end
# end
# end
#
# @example AllowedMethods: [] (default)
# # bad
# def do_something
# has_many :articles do
# def find_or_create_by_name(name)
# end
# end
# end
#
# @example AllowedMethods: ['has_many']
# # bad
# def do_something
# has_many :articles do
# def find_or_create_by_name(name)
# end
# end
# end
#
# @example AllowedPatterns: [] (default)
# # bad
# def foo(obj)
# obj.do_baz do
# def bar
# end
# end
# end
#
# @example AllowedPatterns: ['baz']
# # good
# def foo(obj)
# obj.do_baz do
# def bar
# end
# end
# end
#
class NestedMethodDefinition < Base
include AllowedMethods
include AllowedPattern

MSG = 'Method definitions must not be nested. Use `lambda` instead.'

def on_def(node)
Expand All @@ -77,7 +120,13 @@ def on_def(node)

def scoping_method_call?(child)
child.sclass_type? || eval_call?(child) || exec_call?(child) ||
class_or_module_or_struct_new_call?(child)
class_or_module_or_struct_new_call?(child) || allowed_method_name?(child)
end

def allowed_method_name?(node)
name = node.method_name

allowed_method?(name) || matches_allowed_pattern?(name)
end

# @!method eval_call?(node)
Expand Down
58 changes: 58 additions & 0 deletions spec/rubocop/cop/lint/nested_method_definition_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -266,4 +266,62 @@ def y
end
RUBY
end

context 'when `AllowedMethods: [has_many]`' do
let(:cop_config) do
{ 'AllowedMethods' => ['has_many'] }
end

it 'does not register offense for nested definition inside `has_many`' do
expect_no_offenses(<<~RUBY)
def do_something
has_many :articles do
def find_or_create_by_name(name)
end
end
end
RUBY
end

it 'registers offense for nested definition inside `denied_method`' do
expect_offense(<<~RUBY)
def do_something
denied_method :articles do
def find_or_create_by_name(name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Method definitions must not be nested. Use `lambda` instead.
end
end
end
RUBY
end
end

context 'when `AllowedPatterns: [baz]`' do
let(:cop_config) do
{ 'AllowedPatterns' => ['baz'] }
end

it 'does not register offense for nested definition inside `do_baz`' do
expect_no_offenses(<<~RUBY)
def foo(obj)
obj.do_baz do
def bar
end
end
end
RUBY
end

it 'registers offense for nested definition inside `do_qux`' do
expect_offense(<<~RUBY)
def foo(obj)
obj.do_qux do
def bar
^^^^^^^ Method definitions must not be nested. Use `lambda` instead.
end
end
end
RUBY
end
end
end