diff --git a/config/default.yml b/config/default.yml index ac4b277..a5f51dc 100644 --- a/config/default.yml +++ b/config/default.yml @@ -4,8 +4,8 @@ Rake: - 'Rakefile' - '**/*.rake' -Rake/ClassDefinitionInTask: - Description: 'Do not define a class or module in rake task, because it will be defined to the top level.' +Rake/ClassDefinitionInNamespace: + Description: 'Do not define a class or module in a rake namespace, because it will be defined to the top level.' Enabled: true VersionAdded: '0.3.0' @@ -24,7 +24,7 @@ Rake/DuplicateTask: Enabled: true VersionAdded: '0.4.0' -Rake/MethodDefinitionInTask: - Description: 'Do not define a method in rake task, because it will be defined to the top level.' +Rake/MethodDefinitionInNamespace: + Description: 'Do not define a method in a rake namespace, because it will be defined to the top level.' Enabled: true VersionAdded: '0.2.0' diff --git a/lib/rubocop/cop/rake/class_definition_in_task.rb b/lib/rubocop/cop/rake/class_definition_in_namespace.rb similarity index 64% rename from lib/rubocop/cop/rake/class_definition_in_task.rb rename to lib/rubocop/cop/rake/class_definition_in_namespace.rb index af80629..07a209e 100644 --- a/lib/rubocop/cop/rake/class_definition_in_task.rb +++ b/lib/rubocop/cop/rake/class_definition_in_namespace.rb @@ -3,13 +3,13 @@ module RuboCop module Cop module Rake - # Detects class or module definition in a task or namespace, + # Detects class or module definition in a namespace, # because it is defined to the top level. - # It is confusing because the scope looks in the task or namespace, + # It is confusing because the class appears to be defined in the namespace, # but actually it is defined to the top level. # # @example - # # bad + # # good # task :foo do # class C # end @@ -28,12 +28,14 @@ module Rake # task :foo do # end # - class ClassDefinitionInTask < Base - MSG = 'Do not define a %s in rake task, because it will be defined to the top level.' + class ClassDefinitionInNamespace < Base + MSG = 'Do not define a %s in a rake namespace, because it will be defined to the top level.' def on_class(node) return if Helper::ClassDefinition.in_class_definition?(node) - return unless Helper::TaskDefinition.in_task_or_namespace?(node) + return if Helper::TaskDefinition.in_task?(node) + + return unless Helper::TaskDefinition.in_namespace?(node) add_offense(node, message: format(MSG, type: node.type)) end diff --git a/lib/rubocop/cop/rake/helper/task_definition.rb b/lib/rubocop/cop/rake/helper/task_definition.rb index 9d6589e..5453e65 100644 --- a/lib/rubocop/cop/rake/helper/task_definition.rb +++ b/lib/rubocop/cop/rake/helper/task_definition.rb @@ -8,17 +8,31 @@ module TaskDefinition extend NodePattern::Macros extend self - def_node_matcher :task_or_namespace?, <<-PATTERN + def_node_matcher :namespace?, <<-PATTERN (block - (send _ {:task :namespace} ...) + (send _ {:namespace} ...) args _ ) PATTERN - def in_task_or_namespace?(node) + def_node_matcher :task?, <<-PATTERN + (block + (send _ {:task} ...) + args + _ + ) + PATTERN + + def in_namespace?(node) + node.each_ancestor(:block).any? do |a| + namespace?(a) + end + end + + def in_task?(node) node.each_ancestor(:block).any? do |a| - task_or_namespace?(a) + task?(a) end end end diff --git a/lib/rubocop/cop/rake/method_definition_in_task.rb b/lib/rubocop/cop/rake/method_definition_in_namespace.rb similarity index 66% rename from lib/rubocop/cop/rake/method_definition_in_task.rb rename to lib/rubocop/cop/rake/method_definition_in_namespace.rb index 864bfa9..867db0e 100644 --- a/lib/rubocop/cop/rake/method_definition_in_task.rb +++ b/lib/rubocop/cop/rake/method_definition_in_namespace.rb @@ -3,13 +3,13 @@ module RuboCop module Cop module Rake - # Detects method definition in a task or namespace, + # Detects method definition in a namespace, # because it is defined to the top level. - # It is confusing because the scope looks in the task or namespace, + # It is confusing because the method appears to be defined in the namespace, # but actually it is defined to the top level. # # @example - # # bad + # # good # task :foo do # def helper_method # do_something @@ -30,12 +30,14 @@ module Rake # task :foo do # end # - class MethodDefinitionInTask < Base - MSG = 'Do not define a method in rake task, because it will be defined to the top level.' + class MethodDefinitionInNamespace < Base + MSG = 'Do not define a method in a rake namespace, because it will be defined to the top level.' def on_def(node) return if Helper::ClassDefinition.in_class_definition?(node) - return unless Helper::TaskDefinition.in_task_or_namespace?(node) + return if Helper::TaskDefinition.in_task?(node) + + return unless Helper::TaskDefinition.in_namespace?(node) add_offense(node) end diff --git a/lib/rubocop/cop/rake_cops.rb b/lib/rubocop/cop/rake_cops.rb index 1075bc8..01f7bb6 100644 --- a/lib/rubocop/cop/rake_cops.rb +++ b/lib/rubocop/cop/rake_cops.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -require_relative 'rake/class_definition_in_task' +require_relative 'rake/class_definition_in_namespace' require_relative 'rake/desc' require_relative 'rake/duplicate_task' require_relative 'rake/duplicate_namespace' -require_relative 'rake/method_definition_in_task' +require_relative 'rake/method_definition_in_namespace' diff --git a/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb b/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb new file mode 100644 index 0000000..f416d22 --- /dev/null +++ b/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rake::ClassDefinitionInNamespace, :config do + it 'allows class definition in task' do + expect_no_offenses(<<~RUBY) + task :foo do + class C + end + end + RUBY + end + + it 'allows module definition in task' do + expect_no_offenses(<<~RUBY) + task :foo do + module M + end + end + RUBY + end + + it 'allows class definition in a task when wrapped in a namespace' do + expect_no_offenses(<<~RUBY) + namespace :bar do + task :foo do + class C + end + end + end + RUBY + end + + it 'registers an offense to a class definition in namespace' do + expect_offense(<<~RUBY) + namespace 'foo' do + class C + ^^^^^^^ Do not define a class in a rake namespace, because it will be defined to the top level. + end + end + RUBY + end + + it 'registers an offense to a module definition in namespace' do + expect_offense(<<~RUBY) + namespace 'foo' do + module M + ^^^^^^^^ Do not define a module in a rake namespace, because it will be defined to the top level. + end + end + RUBY + end + + it 'allows class definition in Class.new' do + expect_no_offenses(<<~RUBY) + task :foo do + Class.new do + class C + end + end + end + RUBY + end + + it 'allows class definition at the top level' do + expect_no_offenses(<<~RUBY) + class C + end + + task :foo do + end + RUBY + end +end diff --git a/spec/rubocop/cop/rake/class_definition_in_task_spec.rb b/spec/rubocop/cop/rake/class_definition_in_task_spec.rb deleted file mode 100644 index 1346917..0000000 --- a/spec/rubocop/cop/rake/class_definition_in_task_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe RuboCop::Cop::Rake::ClassDefinitionInTask, :config do - it 'registers an offense to a class definition in task' do - expect_offense(<<~RUBY) - task :foo do - class C - ^^^^^^^ Do not define a class in rake task, because it will be defined to the top level. - end - end - RUBY - end - - it 'registers an offense to a module definition in task' do - expect_offense(<<~RUBY) - task :foo do - module M - ^^^^^^^^ Do not define a module in rake task, because it will be defined to the top level. - end - end - RUBY - end - - it 'allows class definition in Class.new' do - expect_no_offenses(<<~RUBY) - task :foo do - Class.new do - class C - end - end - end - RUBY - end - - it 'allows class definition outside task' do - expect_no_offenses(<<~RUBY) - class C - end - - task :foo do - end - RUBY - end -end diff --git a/spec/rubocop/cop/rake/method_definition_in_task_spec.rb b/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb similarity index 70% rename from spec/rubocop/cop/rake/method_definition_in_task_spec.rb rename to spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb index 13a221e..b129255 100644 --- a/spec/rubocop/cop/rake/method_definition_in_task_spec.rb +++ b/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb @@ -1,11 +1,10 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Rake::MethodDefinitionInTask, :config do - it 'registers an offense to `def` in a task' do - expect_offense(<<~RUBY) +RSpec.describe RuboCop::Cop::Rake::MethodDefinitionInNamespace, :config do + it 'does not register an offense to `def` in a task' do + expect_no_offenses(<<~RUBY) task :foo do def helper_method - ^^^^^^^^^^^^^^^^^ Do not define a method in rake task, because it will be defined to the top level. do_something end end @@ -14,23 +13,33 @@ def helper_method do_something def helper_method - ^^^^^^^^^^^^^^^^^ Do not define a method in rake task, because it will be defined to the top level. do_something end def self.foo - ^^^^^^^^^^^^ Do not define a method in rake task, because it will be defined to the top level. do_something2 end end RUBY end + it 'does not register an offense to `def` in a task when wrapped in a namespace' do + expect_no_offenses(<<~RUBY) + namespace :bar do + task :foo do + def helper_method + do_something + end + end + end + RUBY + end + it 'registers an offense to `def` in a namespace' do expect_offense(<<~RUBY) namespace 'foo' do def helper_method - ^^^^^^^^^^^^^^^^^ Do not define a method in rake task, because it will be defined to the top level. + ^^^^^^^^^^^^^^^^^ Do not define a method in a rake namespace, because it will be defined to the top level. do_something end