diff --git a/config/default.yml b/config/default.yml index 9b2b62de8d..a01259fd43 100644 --- a/config/default.yml +++ b/config/default.yml @@ -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 diff --git a/lib/rubocop/cop/rails/rake_environment.rb b/lib/rubocop/cop/rails/rake_environment.rb new file mode 100644 index 0000000000..5532fdfce4 --- /dev/null +++ b/lib/rubocop/cop/rails/rake_environment.rb @@ -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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 0f5d167883..8a2a548135 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/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' diff --git a/manual/cops.md b/manual/cops.md index 661364c625..a152ffc659 100644 --- a/manual/cops.md +++ b/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) diff --git a/manual/cops_rails.md b/manual/cops_rails.md index 6a1db5f5f4..93d1f8912d 100644 --- a/manual/cops_rails.md +++ b/manual/cops_rails.md @@ -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 diff --git a/spec/rubocop/cop/rails/rake_environment_spec.rb b/spec/rubocop/cop/rails/rake_environment_spec.rb new file mode 100644 index 0000000000..87525bbdae --- /dev/null +++ b/spec/rubocop/cop/rails/rake_environment_spec.rb @@ -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