Skip to content

Commit

Permalink
Add Rake/ClassDefinitionInTask cop
Browse files Browse the repository at this point in the history
  • Loading branch information
pocke committed Sep 18, 2019
1 parent 0a4f009 commit f7fcaf0
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 38 deletions.
7 changes: 6 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ Style/UnneededPercentQ:
Exclude:
- '*.gemspec'

Style/ModuleFunction:
Enabled: false

Style/Documentation:
Enabled: false

Style/PercentLiteralDelimiters:
Exclude:
- '*.gemspec'
Expand All @@ -33,4 +39,3 @@ RSpec/ExampleLength:
Naming/FileName:
Exclude:
- 'lib/rubocop-rake.rb'

4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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][])
Expand Down
5 changes: 5 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
@@ -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'

This comment has been minimized.

Copy link
@bbatsov

bbatsov Oct 4, 2019

Seems you forgot to scope this to Rake files.

This comment has been minimized.

Copy link
@pocke

pocke Oct 4, 2019

Author Collaborator

Good catch! I'll add it. Thank you!


Rake/Desc:
Description: 'Describe the task with `desc` method.'
Enabled: true
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop-rake.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
49 changes: 49 additions & 0 deletions lib/rubocop/cop/rake/class_definition_in_task.rb
Original file line number Diff line number Diff line change
@@ -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 %<type>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
31 changes: 31 additions & 0 deletions lib/rubocop/cop/rake/helper/class_definition.rb
Original file line number Diff line number Diff line change
@@ -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
28 changes: 28 additions & 0 deletions lib/rubocop/cop/rake/helper/task_definition.rb
Original file line number Diff line number Diff line change
@@ -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
39 changes: 2 additions & 37 deletions lib/rubocop/cop/rake/method_definition_in_task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rake_cops.rb
Original file line number Diff line number Diff line change
@@ -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'
48 changes: 48 additions & 0 deletions spec/rubocop/cop/rake/class_definition_in_task_spec.rb
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f7fcaf0

Please sign in to comment.