From eb38421ee2a35de651132f5f0558e05bcc6e91c8 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 17 Feb 2023 20:51:08 +1300 Subject: [PATCH 1/7] Fix MethodDefinitionInTask to allow methods defined in task As methods defined in task do not pollute top level For example, this rakefile below will show that `do_something` does not appear in top level methods ``` task :a do def do_something puts 'a' end do_something end task :b do def do_something puts 'b' end do_something end puts methods.include?(:do_something) ``` --- lib/rubocop/cop/rake/helper/task_definition.rb | 14 ++++++++++++++ lib/rubocop/cop/rake/method_definition_in_task.rb | 10 +++++----- .../cop/rake/method_definition_in_task_spec.rb | 9 +++------ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/lib/rubocop/cop/rake/helper/task_definition.rb b/lib/rubocop/cop/rake/helper/task_definition.rb index 9d6589e..add419c 100644 --- a/lib/rubocop/cop/rake/helper/task_definition.rb +++ b/lib/rubocop/cop/rake/helper/task_definition.rb @@ -8,6 +8,14 @@ module TaskDefinition extend NodePattern::Macros extend self + def_node_matcher :namespace?, <<-PATTERN + (block + (send _ {:namespace} ...) + args + _ + ) + PATTERN + def_node_matcher :task_or_namespace?, <<-PATTERN (block (send _ {:task :namespace} ...) @@ -16,6 +24,12 @@ module TaskDefinition ) PATTERN + def in_namespace?(node) + node.each_ancestor(:block).any? do |a| + namespace?(a) + end + end + def in_task_or_namespace?(node) node.each_ancestor(:block).any? do |a| task_or_namespace?(a) diff --git a/lib/rubocop/cop/rake/method_definition_in_task.rb b/lib/rubocop/cop/rake/method_definition_in_task.rb index 864bfa9..548b7c4 100644 --- a/lib/rubocop/cop/rake/method_definition_in_task.rb +++ b/lib/rubocop/cop/rake/method_definition_in_task.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 scope looks in the namespace, # but actually it is defined to the top level. # # @example - # # bad + # # good # task :foo do # def helper_method # do_something @@ -31,11 +31,11 @@ module Rake # end # class MethodDefinitionInTask < Base - MSG = 'Do not define a method in rake task, because it will be defined to the top level.' + 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 unless Helper::TaskDefinition.in_namespace?(node) add_offense(node) end diff --git a/spec/rubocop/cop/rake/method_definition_in_task_spec.rb b/spec/rubocop/cop/rake/method_definition_in_task_spec.rb index 13a221e..e7d70ef 100644 --- a/spec/rubocop/cop/rake/method_definition_in_task_spec.rb +++ b/spec/rubocop/cop/rake/method_definition_in_task_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) + 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,12 +13,10 @@ 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 @@ -30,7 +27,7 @@ def self.foo 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 From 2b0dcdea40d5ff5a44791903e66754d372889b55 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 17 Feb 2023 21:32:54 +1300 Subject: [PATCH 2/7] Fix ClassDefinitionInTask to allow class defined in task As classes defined in task do not pollute top level. For example, this Rakefile shows class C in task does not pollute top level ``` task :a do class C def do_something puts 'a' end end C.new.do_something end task :b do class C def do_something puts 'b' end end C.new.do_something end puts Object.const_defined?('C') ``` --- .../cop/rake/class_definition_in_task.rb | 10 +++---- .../cop/rake/class_definition_in_task_spec.rb | 30 +++++++++++++++---- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/lib/rubocop/cop/rake/class_definition_in_task.rb b/lib/rubocop/cop/rake/class_definition_in_task.rb index af80629..a83490c 100644 --- a/lib/rubocop/cop/rake/class_definition_in_task.rb +++ b/lib/rubocop/cop/rake/class_definition_in_task.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 scope looks in the namespace, # but actually it is defined to the top level. # # @example - # # bad + # # good # task :foo do # class C # end @@ -29,11 +29,11 @@ module Rake # end # class ClassDefinitionInTask < Base - MSG = 'Do not define a %s in rake task, because it will be defined to the top level.' + 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 unless Helper::TaskDefinition.in_namespace?(node) add_offense(node, message: format(MSG, type: node.type)) 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 index 1346917..b8664eb 100644 --- a/spec/rubocop/cop/rake/class_definition_in_task_spec.rb +++ b/spec/rubocop/cop/rake/class_definition_in_task_spec.rb @@ -1,21 +1,39 @@ # 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) + it 'allows class definition in task' do + expect_no_offenses(<<~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) + it 'allows module definition in task' do + expect_no_offenses(<<~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 '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 From b82abd610351de66e96b3862d739ba7ef639b329 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Fri, 17 Feb 2023 21:37:31 +1300 Subject: [PATCH 3/7] Remove un-used helper method --- lib/rubocop/cop/rake/helper/task_definition.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/lib/rubocop/cop/rake/helper/task_definition.rb b/lib/rubocop/cop/rake/helper/task_definition.rb index add419c..69c5a76 100644 --- a/lib/rubocop/cop/rake/helper/task_definition.rb +++ b/lib/rubocop/cop/rake/helper/task_definition.rb @@ -16,25 +16,11 @@ module TaskDefinition ) PATTERN - def_node_matcher :task_or_namespace?, <<-PATTERN - (block - (send _ {:task :namespace} ...) - args - _ - ) - PATTERN - def in_namespace?(node) node.each_ancestor(:block).any? do |a| namespace?(a) end end - - def in_task_or_namespace?(node) - node.each_ancestor(:block).any? do |a| - task_or_namespace?(a) - end - end end end end From dcb54ac414ce356f9c6ee6ad8296715dec07dc8c Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Sat, 11 Nov 2023 21:55:52 +1300 Subject: [PATCH 4/7] Rename class definition cop to reflect fixed behaviour Since the cop is now fixed to warn against class in namespaces, update the cop name accordingly. --- config/default.yml | 4 ++-- ...definition_in_task.rb => class_definition_in_namespace.rb} | 2 +- lib/rubocop/cop/rake_cops.rb | 2 +- ..._in_task_spec.rb => class_definition_in_namespace_spec.rb} | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) rename lib/rubocop/cop/rake/{class_definition_in_task.rb => class_definition_in_namespace.rb} (96%) rename spec/rubocop/cop/rake/{class_definition_in_task_spec.rb => class_definition_in_namespace_spec.rb} (90%) diff --git a/config/default.yml b/config/default.yml index ac4b277..4a8c67b 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' diff --git a/lib/rubocop/cop/rake/class_definition_in_task.rb b/lib/rubocop/cop/rake/class_definition_in_namespace.rb similarity index 96% rename from lib/rubocop/cop/rake/class_definition_in_task.rb rename to lib/rubocop/cop/rake/class_definition_in_namespace.rb index a83490c..90a7788 100644 --- a/lib/rubocop/cop/rake/class_definition_in_task.rb +++ b/lib/rubocop/cop/rake/class_definition_in_namespace.rb @@ -28,7 +28,7 @@ module Rake # task :foo do # end # - class ClassDefinitionInTask < Base + 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) diff --git a/lib/rubocop/cop/rake_cops.rb b/lib/rubocop/cop/rake_cops.rb index 1075bc8..32bc46e 100644 --- a/lib/rubocop/cop/rake_cops.rb +++ b/lib/rubocop/cop/rake_cops.rb @@ -1,6 +1,6 @@ # 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' diff --git a/spec/rubocop/cop/rake/class_definition_in_task_spec.rb b/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb similarity index 90% rename from spec/rubocop/cop/rake/class_definition_in_task_spec.rb rename to spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb index b8664eb..04a42da 100644 --- a/spec/rubocop/cop/rake/class_definition_in_task_spec.rb +++ b/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Rake::ClassDefinitionInTask, :config do +RSpec.describe RuboCop::Cop::Rake::ClassDefinitionInNamespace, :config do it 'allows class definition in task' do expect_no_offenses(<<~RUBY) task :foo do @@ -50,7 +50,7 @@ class C RUBY end - it 'allows class definition outside task' do + it 'allows class definition at the top level' do expect_no_offenses(<<~RUBY) class C end From 973be1dca2c391e239c0f282ddc17b7ae3dd5a12 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Sat, 11 Nov 2023 21:56:17 +1300 Subject: [PATCH 5/7] Rename method definition cop to reflect fixed behaviour Since the cop is now fixed to warn against method in namespaces, update the cop name accordingly. --- config/default.yml | 4 ++-- ...efinition_in_task.rb => method_definition_in_namespace.rb} | 2 +- lib/rubocop/cop/rake_cops.rb | 2 +- ...in_task_spec.rb => method_definition_in_namespace_spec.rb} | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename lib/rubocop/cop/rake/{method_definition_in_task.rb => method_definition_in_namespace.rb} (96%) rename spec/rubocop/cop/rake/{method_definition_in_task_spec.rb => method_definition_in_namespace_spec.rb} (95%) diff --git a/config/default.yml b/config/default.yml index 4a8c67b..a5f51dc 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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/method_definition_in_task.rb b/lib/rubocop/cop/rake/method_definition_in_namespace.rb similarity index 96% rename from lib/rubocop/cop/rake/method_definition_in_task.rb rename to lib/rubocop/cop/rake/method_definition_in_namespace.rb index 548b7c4..b560ce8 100644 --- a/lib/rubocop/cop/rake/method_definition_in_task.rb +++ b/lib/rubocop/cop/rake/method_definition_in_namespace.rb @@ -30,7 +30,7 @@ module Rake # task :foo do # end # - class MethodDefinitionInTask < Base + 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) diff --git a/lib/rubocop/cop/rake_cops.rb b/lib/rubocop/cop/rake_cops.rb index 32bc46e..01f7bb6 100644 --- a/lib/rubocop/cop/rake_cops.rb +++ b/lib/rubocop/cop/rake_cops.rb @@ -4,4 +4,4 @@ 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/method_definition_in_task_spec.rb b/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb similarity index 95% 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 e7d70ef..92dca59 100644 --- a/spec/rubocop/cop/rake/method_definition_in_task_spec.rb +++ b/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Rake::MethodDefinitionInTask, :config do +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 From fc00ab12eb41172cf8e4cfd2cbabc6c6c165f038 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Tue, 23 Jan 2024 16:22:57 +1300 Subject: [PATCH 6/7] Allow definitions within a task that is within a namespace. This is similar to definitions within a task (without a namespace). --- .../cop/rake/class_definition_in_namespace.rb | 2 ++ lib/rubocop/cop/rake/helper/task_definition.rb | 14 ++++++++++++++ .../cop/rake/method_definition_in_namespace.rb | 2 ++ .../cop/rake/class_definition_in_namespace_spec.rb | 11 +++++++++++ .../rake/method_definition_in_namespace_spec.rb | 12 ++++++++++++ 5 files changed, 41 insertions(+) diff --git a/lib/rubocop/cop/rake/class_definition_in_namespace.rb b/lib/rubocop/cop/rake/class_definition_in_namespace.rb index 90a7788..98f45e3 100644 --- a/lib/rubocop/cop/rake/class_definition_in_namespace.rb +++ b/lib/rubocop/cop/rake/class_definition_in_namespace.rb @@ -33,6 +33,8 @@ class ClassDefinitionInNamespace < Base def on_class(node) return if Helper::ClassDefinition.in_class_definition?(node) + return if Helper::TaskDefinition.in_task?(node) + return unless Helper::TaskDefinition.in_namespace?(node) add_offense(node, message: format(MSG, type: node.type)) diff --git a/lib/rubocop/cop/rake/helper/task_definition.rb b/lib/rubocop/cop/rake/helper/task_definition.rb index 69c5a76..5453e65 100644 --- a/lib/rubocop/cop/rake/helper/task_definition.rb +++ b/lib/rubocop/cop/rake/helper/task_definition.rb @@ -16,11 +16,25 @@ module TaskDefinition ) PATTERN + 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?(a) + end + end end end end diff --git a/lib/rubocop/cop/rake/method_definition_in_namespace.rb b/lib/rubocop/cop/rake/method_definition_in_namespace.rb index b560ce8..f27f99b 100644 --- a/lib/rubocop/cop/rake/method_definition_in_namespace.rb +++ b/lib/rubocop/cop/rake/method_definition_in_namespace.rb @@ -35,6 +35,8 @@ class MethodDefinitionInNamespace < Base def on_def(node) return if Helper::ClassDefinition.in_class_definition?(node) + return if Helper::TaskDefinition.in_task?(node) + return unless Helper::TaskDefinition.in_namespace?(node) add_offense(node) diff --git a/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb b/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb index 04a42da..f416d22 100644 --- a/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb +++ b/spec/rubocop/cop/rake/class_definition_in_namespace_spec.rb @@ -19,6 +19,17 @@ module M 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 diff --git a/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb b/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb index 92dca59..b129255 100644 --- a/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb +++ b/spec/rubocop/cop/rake/method_definition_in_namespace_spec.rb @@ -23,6 +23,18 @@ def self.foo 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 From 7b9f68fba470d64e5c01ad6869cc730424eeaa80 Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Sat, 5 Apr 2025 18:21:49 +1300 Subject: [PATCH 7/7] Clarify code comment --- lib/rubocop/cop/rake/class_definition_in_namespace.rb | 2 +- lib/rubocop/cop/rake/method_definition_in_namespace.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rubocop/cop/rake/class_definition_in_namespace.rb b/lib/rubocop/cop/rake/class_definition_in_namespace.rb index 98f45e3..07a209e 100644 --- a/lib/rubocop/cop/rake/class_definition_in_namespace.rb +++ b/lib/rubocop/cop/rake/class_definition_in_namespace.rb @@ -5,7 +5,7 @@ module Cop module Rake # 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 namespace, + # It is confusing because the class appears to be defined in the namespace, # but actually it is defined to the top level. # # @example diff --git a/lib/rubocop/cop/rake/method_definition_in_namespace.rb b/lib/rubocop/cop/rake/method_definition_in_namespace.rb index f27f99b..867db0e 100644 --- a/lib/rubocop/cop/rake/method_definition_in_namespace.rb +++ b/lib/rubocop/cop/rake/method_definition_in_namespace.rb @@ -5,7 +5,7 @@ module Cop module Rake # Detects method definition in a namespace, # because it is defined to the top level. - # It is confusing because the scope looks in the namespace, + # It is confusing because the method appears to be defined in the namespace, # but actually it is defined to the top level. # # @example