diff --git a/changelog/new_style_top_level_method_definition_cop.md b/changelog/new_style_top_level_method_definition_cop.md new file mode 100644 index 00000000000..d62e8bdc934 --- /dev/null +++ b/changelog/new_style_top_level_method_definition_cop.md @@ -0,0 +1 @@ +* [#9734](https://github.com/rubocop/rubocop/pull/9734): Add `Style/TopLevelMethodDefinition` cop. ([@tejasbubane][]) diff --git a/config/default.yml b/config/default.yml index 77ffac4c734..6e6940d1420 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4651,6 +4651,11 @@ Style/TernaryParentheses: - require_parentheses_when_complex AllowSafeAssignment: true +Style/TopLevelMethodDefinition: + Description: 'This cop looks for top-level method definitions.' + Enabled: false + VersionAdded: '<>' + Style/TrailingBodyOnClass: Description: 'Class body goes below class statement.' Enabled: true diff --git a/lib/rubocop.rb b/lib/rubocop.rb index 1e1deb9121d..9e3235121d8 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -598,6 +598,7 @@ require_relative 'rubocop/cop/style/symbol_literal' require_relative 'rubocop/cop/style/symbol_proc' require_relative 'rubocop/cop/style/ternary_parentheses' +require_relative 'rubocop/cop/style/top_level_method_definition' require_relative 'rubocop/cop/style/trailing_body_on_class' require_relative 'rubocop/cop/style/trailing_body_on_method_definition' require_relative 'rubocop/cop/style/trailing_body_on_module' diff --git a/lib/rubocop/cop/style/top_level_method_definition.rb b/lib/rubocop/cop/style/top_level_method_definition.rb new file mode 100644 index 00000000000..d868dbf2780 --- /dev/null +++ b/lib/rubocop/cop/style/top_level_method_definition.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Style + # Newcomers to ruby applications may write top-level methods, + # when ideally they should be organized in appropriate classes or modules. + # This cop looks for definitions of top-level methods and warns about them. + # + # However for ruby scripts it is perfectly fine to use top-level methods. + # Hence this cop is disabled by default. + # + # @example + # # bad + # def some_method + # end + # + # # bad + # def self.some_method + # end + # + # # bad + # define_method(:foo) { puts 1 } + # + # # good + # module Foo + # def some_method + # end + # end + # + # # good + # class Foo + # def self.some_method + # end + # end + # + # # good + # Struct.new do + # def some_method + # end + # end + # + # # good + # class Foo + # define_method(:foo) { puts 1 } + # end + class TopLevelMethodDefinition < Base + MSG = 'Do not define methods at the top-level.' + + RESTRICT_ON_SEND = %i[define_method].freeze + + def on_def(node) + return unless node.root? + + add_offense(node) + end + alias on_defs on_def + alias on_send on_def + + def on_block(node) + return unless define_method_block?(node) && node.root? + + add_offense(node) + end + + private + + # @!method define_method_block?(node) + def_node_matcher :define_method_block?, <<~PATTERN + (block (send _ {:define_method} _) ...) + PATTERN + end + end + end +end diff --git a/spec/rubocop/cop/style/top_level_method_definition_spec.rb b/spec/rubocop/cop/style/top_level_method_definition_spec.rb new file mode 100644 index 00000000000..3d08bb58596 --- /dev/null +++ b/spec/rubocop/cop/style/top_level_method_definition_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Style::TopLevelMethodDefinition, :config do + it 'registers an offense top-level methods' do + expect_offense(<<~RUBY) + def foo; end + ^^^^^^^^^^^^ Do not define methods at the top-level. + RUBY + end + + it 'registers an offense top-level class methods' do + expect_offense(<<~RUBY) + def self.foo; end + ^^^^^^^^^^^^^^^^^ Do not define methods at the top-level. + RUBY + end + + context 'top-level define_method' do + it 'registers offense with inline block' do + expect_offense(<<~RUBY) + define_method(:foo) { puts 1 } + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define methods at the top-level. + RUBY + end + + it 'registers offense for multi-line block' do + expect_offense(<<~RUBY) + define_method(:foo) do |x| + ^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define methods at the top-level. + puts 1 + end + RUBY + end + + it 'registers offense for proc argument' do + expect_offense(<<~RUBY) + define_method(:foo, instance_method(:bar)) + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not define methods at the top-level. + RUBY + end + end + + it 'does not register an offense when using module' do + expect_no_offenses(<<~RUBY) + module Foo + def foo; end + end + RUBY + end + + it 'does not register an offense when using class' do + expect_no_offenses(<<~RUBY) + class Foo + def self.foo; end + end + RUBY + end + + it 'does not register an offense when using Struct' do + expect_no_offenses(<<~RUBY) + Foo = Struct.new do + def foo; end + end + RUBY + end + + it 'does not register an offense when defined within arbitrary block' do + expect_no_offenses(<<~RUBY) + Foo = types.each do |type| + def foo(type) + puts type + end + end + RUBY + end + + it 'does not register an offense when define_method is not top-level' do + expect_no_offenses(<<~RUBY) + class Foo + define_method(:a) { puts 1 } + + define_method(:b) do |x| + puts x + end + + define_method(:c, instance_method(:d)) + end + RUBY + end +end