From f7fcaf091e3665e47b45c1c62a4b58b1a033730a Mon Sep 17 00:00:00 2001 From: Masataka Pocke Kuwabara Date: Wed, 18 Sep 2019 22:20:28 +0900 Subject: [PATCH] Add Rake/ClassDefinitionInTask cop --- .rubocop.yml | 7 ++- CHANGELOG.md | 4 ++ config/default.yml | 5 ++ lib/rubocop-rake.rb | 2 + .../cop/rake/class_definition_in_task.rb | 49 +++++++++++++++++++ .../cop/rake/helper/class_definition.rb | 31 ++++++++++++ .../cop/rake/helper/task_definition.rb | 28 +++++++++++ .../cop/rake/method_definition_in_task.rb | 39 +-------------- lib/rubocop/cop/rake_cops.rb | 1 + .../cop/rake/class_definition_in_task_spec.rb | 48 ++++++++++++++++++ 10 files changed, 176 insertions(+), 38 deletions(-) create mode 100644 lib/rubocop/cop/rake/class_definition_in_task.rb create mode 100644 lib/rubocop/cop/rake/helper/class_definition.rb create mode 100644 lib/rubocop/cop/rake/helper/task_definition.rb create mode 100644 spec/rubocop/cop/rake/class_definition_in_task_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 6a13fb2..d7b32a6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -23,6 +23,12 @@ Style/UnneededPercentQ: Exclude: - '*.gemspec' +Style/ModuleFunction: + Enabled: false + +Style/Documentation: + Enabled: false + Style/PercentLiteralDelimiters: Exclude: - '*.gemspec' @@ -33,4 +39,3 @@ RSpec/ExampleLength: Naming/FileName: Exclude: - 'lib/rubocop-rake.rb' - diff --git a/CHANGELOG.md b/CHANGELOG.md index f4db290..7a2d49f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## master (unreleased) +### New features + +* [#6](https://github.com/pocke/rubocop-rake/issues/6): Add `Rake/ClassDefinitionInTask` cop. ([@pocke][]) + ### Bug fixes * [#8](https://github.com/pocke/rubocop-rake/issues/8): Make Rake/Desc to not require description for the default task. ([@pocke][]) diff --git a/config/default.yml b/config/default.yml index 515a137..9d7fd42 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1,3 +1,8 @@ +Rake/ClassDefinitionInTask: + Description: 'Do not define a class or module in rake task, because it will be defined to the top level.' + Enabled: true + VersionAdded: '0.3.0' + Rake/Desc: Description: 'Describe the task with `desc` method.' Enabled: true diff --git a/lib/rubocop-rake.rb b/lib/rubocop-rake.rb index 441e856..9c5689b 100644 --- a/lib/rubocop-rake.rb +++ b/lib/rubocop-rake.rb @@ -8,4 +8,6 @@ RuboCop::Rake::Inject.defaults! +require_relative 'rubocop/cop/rake/helper/class_definition' +require_relative 'rubocop/cop/rake/helper/task_definition' require_relative 'rubocop/cop/rake_cops' diff --git a/lib/rubocop/cop/rake/class_definition_in_task.rb b/lib/rubocop/cop/rake/class_definition_in_task.rb new file mode 100644 index 0000000..835b22a --- /dev/null +++ b/lib/rubocop/cop/rake/class_definition_in_task.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rake + # This cop detects class or module definition 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. + # + # @example + # # bad + # task :foo do + # class C + # end + # end + # + # # bad + # namespace :foo do + # module M + # end + # end + # + # # good - It is also defined to the top level, + # # but it looks expected behavior. + # class C + # end + # task :foo do + # end + # + class ClassDefinitionInTask < Cop + MSG = 'Do not define a %s in rake task, 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) + + add_offense(node) + end + + def message(node) + format(MSG, type: node.type) + end + + alias on_module on_class + end + end + end +end diff --git a/lib/rubocop/cop/rake/helper/class_definition.rb b/lib/rubocop/cop/rake/helper/class_definition.rb new file mode 100644 index 0000000..670c81c --- /dev/null +++ b/lib/rubocop/cop/rake/helper/class_definition.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rake + module Helper + module ClassDefinition + extend NodePattern::Macros + extend self + + def_node_matcher :class_definition?, <<~PATTERN + { + class module sclass + (block + (send (const {nil? cbase} {:Class :Module}) :new) + args + _ + ) + } + PATTERN + + def in_class_definition?(node) + node.each_ancestor(:class, :module, :sclass, :block).any? do |a| + class_definition?(a) + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rake/helper/task_definition.rb b/lib/rubocop/cop/rake/helper/task_definition.rb new file mode 100644 index 0000000..9d6589e --- /dev/null +++ b/lib/rubocop/cop/rake/helper/task_definition.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rake + module Helper + module TaskDefinition + extend NodePattern::Macros + extend self + + def_node_matcher :task_or_namespace?, <<-PATTERN + (block + (send _ {:task :namespace} ...) + args + _ + ) + PATTERN + + def in_task_or_namespace?(node) + node.each_ancestor(:block).any? do |a| + task_or_namespace?(a) + end + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rake/method_definition_in_task.rb b/lib/rubocop/cop/rake/method_definition_in_task.rb index 811e3ab..03a74d1 100644 --- a/lib/rubocop/cop/rake/method_definition_in_task.rb +++ b/lib/rubocop/cop/rake/method_definition_in_task.rb @@ -33,49 +33,14 @@ module Rake class MethodDefinitionInTask < Cop MSG = 'Do not define a method in rake task, because it will be defined to the top level.' - def_node_matcher :bad_method?, <<~PATTERN - (send nil? :bad_method ...) - PATTERN - - def_node_matcher :class_definition?, <<~PATTERN - { - class module sclass - (block - (send (const {nil? cbase} {:Class :Module}) :new) - args - _ - ) - } - PATTERN - - def_node_matcher :task_or_namespace?, <<-PATTERN - (block - (send _ {:task :namespace} ...) - args - _ - ) - PATTERN - def on_def(node) - return if in_class_definition?(node) - return unless in_task_or_namespace?(node) + return if Helper::ClassDefinition.in_class_definition?(node) + return unless Helper::TaskDefinition.in_task_or_namespace?(node) add_offense(node) end alias on_defs on_def - - private def in_class_definition?(node) - node.each_ancestor(:class, :module, :sclass, :block).any? do |a| - class_definition?(a) - end - end - - private def in_task_or_namespace?(node) - node.each_ancestor(:block).any? do |a| - task_or_namespace?(a) - end - end end end end diff --git a/lib/rubocop/cop/rake_cops.rb b/lib/rubocop/cop/rake_cops.rb index 5be07ce..b661205 100644 --- a/lib/rubocop/cop/rake_cops.rb +++ b/lib/rubocop/cop/rake_cops.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +require_relative 'rake/class_definition_in_task' require_relative 'rake/desc' require_relative 'rake/method_definition_in_task' diff --git a/spec/rubocop/cop/rake/class_definition_in_task_spec.rb b/spec/rubocop/cop/rake/class_definition_in_task_spec.rb new file mode 100644 index 0000000..7b2e960 --- /dev/null +++ b/spec/rubocop/cop/rake/class_definition_in_task_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rake::ClassDefinitionInTask do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + + 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