diff --git a/CHANGELOG.md b/CHANGELOG.md index f193878..10e922a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#65](https://github.com/rubocop/rubocop-rake/pull/65): Add `Rake/ConstantDefinitionInTask` cop. ([@corsonknowles][]) + ### Changes * [#66](https://github.com/rubocop/rubocop-rake/pull/66): Speed up loading rubocop-rake by lazily loading only the cops needed for a run. This requires RuboCop 1.89.0+. ([@koic][]) @@ -82,3 +86,4 @@ [@jaruuuu]: https://github.com/jaruuuu [@koic]: https://github.com/koic [@tejasbubane]: https://github.com/tejasbubane +[@corsonknowles]: https://github.com/corsonknowles diff --git a/config/default.yml b/config/default.yml index ac4b277..4a0d187 100644 --- a/config/default.yml +++ b/config/default.yml @@ -9,6 +9,11 @@ Rake/ClassDefinitionInTask: Enabled: true VersionAdded: '0.3.0' +Rake/ConstantDefinitionInTask: + Description: 'Do not assign a constant in rake task, because it will be defined to the top level.' + Enabled: pending + VersionAdded: '<>' + Rake/Desc: Description: 'Describe the task with `desc` method.' Enabled: true diff --git a/lib/rubocop/cop/rake.rb b/lib/rubocop/cop/rake.rb index ae1ec23..a34cf45 100644 --- a/lib/rubocop/cop/rake.rb +++ b/lib/rubocop/cop/rake.rb @@ -10,6 +10,7 @@ module Rake extend LazyLoader register_cop :ClassDefinitionInTask, "#{__dir__}/rake/class_definition_in_task" + register_cop :ConstantDefinitionInTask, "#{__dir__}/rake/constant_definition_in_task" register_cop :Desc, "#{__dir__}/rake/desc" register_cop :DuplicateTask, "#{__dir__}/rake/duplicate_task" register_cop :DuplicateNamespace, "#{__dir__}/rake/duplicate_namespace" diff --git a/lib/rubocop/cop/rake/constant_definition_in_task.rb b/lib/rubocop/cop/rake/constant_definition_in_task.rb new file mode 100644 index 0000000..92f8671 --- /dev/null +++ b/lib/rubocop/cop/rake/constant_definition_in_task.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rake + # Detects constant assignment in a task or namespace, + # because it is defined to the top level. + # It is confusing because the scope looks in the task or namespace, + # but actually it is defined to the top level. + # + # Class and module definitions are covered by `Rake/ClassDefinitionInTask`. + # + # @example + # # bad + # task :foo do + # CONST = 1 + # end + # + # # bad + # namespace :foo do + # CONST = 1 + # end + # + # # good - It is also defined to the top level, + # # but it looks expected behavior. + # CONST = 1 + # task :foo do + # end + # + class ConstantDefinitionInTask < Base + MSG = 'Do not assign a constant in rake task, because it will be defined to the top level.' + + def on_casgn(node) + return if Helper::ClassDefinition.in_class_definition?(node) + return unless Helper::TaskDefinition.in_task_or_namespace?(node) + + add_offense(node) + end + end + end + end +end diff --git a/spec/lazy_loading_spec.rb b/spec/lazy_loading_spec.rb index 48944ea..5c337e7 100644 --- a/spec/lazy_loading_spec.rb +++ b/spec/lazy_loading_spec.rb @@ -24,7 +24,7 @@ def run_script(source) puts "loaded_cop_files=\#{loaded.size}" RUBY - expect(output).to include('registered=5', 'loaded_cop_files=0') + expect(output).to include('registered=6', 'loaded_cop_files=0') end it 'resolves every helper file in `lib/rubocop/cop/rake/helper` through an autoload' do diff --git a/spec/rubocop/cop/rake/constant_definition_in_task_spec.rb b/spec/rubocop/cop/rake/constant_definition_in_task_spec.rb new file mode 100644 index 0000000..f6bc5eb --- /dev/null +++ b/spec/rubocop/cop/rake/constant_definition_in_task_spec.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rake::ConstantDefinitionInTask, :config do + it 'registers an offense to a constant assignment in a task' do + expect_offense(<<~RUBY) + task :foo do + CONST = 1 + ^^^^^^^^^ Do not assign a constant in rake task, because it will be defined to the top level. + end + RUBY + end + + it 'registers an offense to a constant assignment in a namespace' do + expect_offense(<<~RUBY) + namespace :foo do + CONST = 1 + ^^^^^^^^^ Do not assign a constant in rake task, because it will be defined to the top level. + + task :bar do + end + end + RUBY + end + + it 'does not register an offense to a constant assignment at the top level' do + expect_no_offenses(<<~RUBY) + CONST = 1 + + task :foo do + end + RUBY + end + + it 'does not register an offense to a constant assignment inside a class in a task' do + expect_no_offenses(<<~RUBY) + task :foo do + Class.new do + CONST = 1 + end + end + RUBY + end +end