Skip to content

Commit

Permalink
Add Rails/RakeEnvironment cop
Browse files Browse the repository at this point in the history
  • Loading branch information
pocke committed Sep 19, 2019
1 parent ee0176b commit 869dd38
Show file tree
Hide file tree
Showing 7 changed files with 243 additions and 0 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### New features

* [#123](https://github.com/rubocop-hq/rubocop-rails/pull/123): Add new `Rails/ApplicationController` and `Rails/ApplicationMailer` cops. ([@eugeneius][])
* [#130](https://github.com/rubocop-hq/rubocop-rails/pull/130): Add new `Rails/RakeEnvironment` cop. ([@pocke][])

### Bug fixes

Expand Down Expand Up @@ -90,3 +91,4 @@
[@MaximeLaurenty]: https://github.com/MaximeLaurenty
[@eugeneius]: https://github.com/eugeneius
[@jas14]: https://github.com/jas14
[@pocke]: https://github.com/pocke
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,14 @@ Rails/Present:
# Convert usages of `unless blank?` to `if present?`
UnlessBlank: true

Rails/RakeEnvironment:
Description: 'Set :environment task as a dependency to all rake task.'
Enabled: true
VersionAdded: '2.4'
Include:
- '**/Rakefile'
- '**/*.rake'

Rails/ReadWriteAttribute:
Description: >-
Checks for read_attribute(:attr) and
Expand Down
104 changes: 104 additions & 0 deletions lib/rubocop/cop/rails/rake_environment.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop checks rake task definition without `environment` dependency.
# `environment` dependency is important because it loads application code
# for the rake task. The rake task cannot use application code, such as
# models, without `environment` dependency.
#
# You can ignore the offense if the task satisfies at least one of the
# following conditions:
#
# * The task does not need application code.
# * The task invokes :environment task.
# * The task's dependencies invoke :environment task.
#
# @example
# # bad
# task :foo do
# do_something
# end
#
# # good
# task foo: :environment do
# do_something
# end
#
class RakeEnvironment < Cop
MSG = 'Set :environment task as a dependency to all rake task.'

def_node_matcher :task_definition?, <<-PATTERN
(send nil? :task ...)
PATTERN

def on_send(node)
return unless task_definition?(node)
return if task_name(node) == :default

deps = dependencies(node)
return if environment_dependency_will_apply?(deps)

add_offense(node)
end

private

def task_name(node)
first_arg = node.arguments[0]
case first_arg&.type
when :sym, :str
return first_arg.value.to_sym
when :hash
return nil if first_arg.children.size != 1

pair = first_arg.children.first
key = pair.children.first
case key.type
when :sym, :str
key.value.to_sym
end
end
end

def environment_dependency_will_apply?(deps)
return false if deps.empty?

dep = deps.first
case dep.type
when :str, :sym
dep.value.to_sym == :environment
else
true
end
end

def dependencies(node)
first_arg = node.arguments[0]
return [] unless first_arg

return hash_style_dependencies(first_arg) if first_arg.hash_type?

task_args = node.arguments[1]
return [] unless task_args
return [] unless task_args.hash_type?

hash_style_dependencies(task_args)
end

def hash_style_dependencies(hash_node)
deps = hash_node.pairs.first&.value
return [] unless deps

case deps.type
when :array
deps.children
else
[deps]
end
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
require_relative 'rails/pluralization_grammar'
require_relative 'rails/presence'
require_relative 'rails/present'
require_relative 'rails/rake_environment'
require_relative 'rails/read_write_attribute'
require_relative 'rails/redundant_allow_nil'
require_relative 'rails/redundant_receiver_in_with_options'
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
* [Rails/PluralizationGrammar](cops_rails.md#railspluralizationgrammar)
* [Rails/Presence](cops_rails.md#railspresence)
* [Rails/Present](cops_rails.md#railspresent)
* [Rails/RakeEnvironment](cops_rails.md#railsrakeenvironment)
* [Rails/ReadWriteAttribute](cops_rails.md#railsreadwriteattribute)
* [Rails/RedundantAllowNil](cops_rails.md#railsredundantallownil)
* [Rails/RedundantReceiverInWithOptions](cops_rails.md#railsredundantreceiverinwithoptions)
Expand Down
38 changes: 38 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -1660,6 +1660,44 @@ NotNilAndNotEmpty | `true` | Boolean
NotBlank | `true` | Boolean
UnlessBlank | `true` | Boolean

## Rails/RakeEnvironment

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | No | 2.4 | -

This cop checks rake task definition without `environment` dependency.
`environment` dependency is important because it loads application code
for the rake task. The rake task cannot use application code, such as
models, without `environment` dependency.

You can ignore the offense if the task satisfies at least one of the
following conditions:

* The task does not need application code.
* The task invokes :environment task.
* The task's dependencies invoke :environment task.

### Examples

```ruby
# bad
task :foo do
do_something
end

# good
task foo: :environment do
do_something
end
```

### Configurable attributes

Name | Default value | Configurable values
--- | --- | ---
Include | `**/Rakefile`, `**/*.rake` | Array

## Rails/ReadWriteAttribute

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
89 changes: 89 additions & 0 deletions spec/rubocop/cop/rails/rake_environment_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::RakeEnvironment do
subject(:cop) { described_class.new(config) }

let(:config) { RuboCop::Config.new }

it 'registers an offense to task without :environment' do
expect_offense(<<~RUBY)
task :foo do
^^^^^^^^^ Set :environment task as a dependency to all rake task.
end
RUBY
end

it 'registers an offense to task with an dependency' do
expect_offense(<<~RUBY)
task foo: :bar do
^^^^^^^^^^^^^^ Set :environment task as a dependency to all rake task.
end
RUBY
end

it 'registers an offense to task with dependencies' do
expect_offense(<<~RUBY)
task foo: [:foo, :bar] do
^^^^^^^^^^^^^^^^^^^^^^ Set :environment task as a dependency to all rake task.
end
RUBY
end

it 'registers an offense to task with :environment ' \
'but it has other dependency before it' do
expect_offense(<<~RUBY)
task foo: [:bar, :environment] do
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Set :environment task as a dependency to all rake task.
end
RUBY
end

it 'registers an offense to task with a dependency as a method call' do
expect_offense(<<~RUBY)
task foo: [:bar, dep]
^^^^^^^^^^^^^^^^^^^^^ Set :environment task as a dependency to all rake task.
RUBY
end

it 'does not register an offense to task with :environment' do
expect_no_offenses(<<~RUBY)
task foo: :environment do
end
RUBY
end

it 'does not register an offense to task with :environment ' \
'and other dependencies' do
expect_no_offenses(<<~RUBY)
task foo: [:environment, :bar] do
end
RUBY
end

it 'does not register an offense to task with :environment and an argument' do
expect_no_offenses(<<~RUBY)
task :foo, [:arg] => :environment do
end
RUBY
end

it 'does not register an offense to task with a dependency ' \
'as a method call' do
expect_no_offenses(<<~RUBY)
task foo: dep
RUBY
end

it 'does not register an offense to task with dependencies ' \
'as a method call' do
expect_no_offenses(<<~RUBY)
task foo: [dep, :bar]
RUBY
end

it 'does not register an offense to the default task' do
expect_no_offenses(<<~RUBY)
task default: :spec
RUBY
end
end

0 comments on commit 869dd38

Please sign in to comment.