Merge pull request #130 from pocke/rake-env

Add Rails/RakeEnvironment cop

CHANGELOG.md

@@ -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
 
@@ -95,3 +96,4 @@
 [@jas14]: https://github.com/jas14
 [@forresty]: https://github.com/forresty
 [@sinsoku]: https://github.com/sinsoku
+[@pocke]: https://github.com/pocke

config/default.yml

@@ -333,6 +333,15 @@ 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
+  Safe: false
+  VersionAdded: '2.4'
+  Include:
+    - '**/Rakefile'
+    - '**/*.rake'
+
 Rails/ReadWriteAttribute:
   Description: >-
                  Checks for read_attribute(:attr) and

lib/rubocop/cop/rails/rake_environment.rb

@@ -0,0 +1,91 @@
+# 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.
+      #
+      # @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
+          return if with_dependencies?(node)
+
+          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 with_dependencies?(node)
+          first_arg = node.arguments[0]
+          return false unless first_arg
+
+          if first_arg.hash_type?
+            with_hash_style_dependencies?(first_arg)
+          else
+            task_args = node.arguments[1]
+            return false unless task_args
+            return false unless task_args.hash_type?
+
+            with_hash_style_dependencies?(task_args)
+          end
+        end
+
+        def with_hash_style_dependencies?(hash_node)
+          deps = hash_node.pairs.first&.value
+          return false unless deps
+
+          case deps.type
+          when :array
+            !deps.values.empty?
+          else
+            true
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -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'

manual/cops.md

@@ -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)

manual/cops_rails.md

@@ -1660,6 +1660,43 @@ NotNilAndNotEmpty | `true` | Boolean
 NotBlank | `true` | Boolean
 UnlessBlank | `true` | Boolean
 
+## Rails/RakeEnvironment
+
+Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+--- | --- | --- | --- | ---
+Enabled | No | 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.
+
+### 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

spec/rubocop/cop/rails/rake_environment_spec.rb

@@ -0,0 +1,86 @@
+# 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 'does not register an offense to task with :environment ' \
+     'but it has other dependency before it' do
+    expect_no_offenses(<<~RUBY)
+      task foo: [:bar, `:environment`] do
+      end
+    RUBY
+  end
+
+  it 'does not register an offense to task with an dependency' do
+    expect_no_offenses(<<~RUBY)
+      task foo: :bar do
+      end
+    RUBY
+  end
+
+  it 'does not register an offense to task with dependencies' do
+    expect_no_offenses(<<~RUBY)
+      task foo: [:foo, :bar] 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: [:bar, dep]
+    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