From f1bfe38dbc8497bc8e75e929c56ab0ce3c9f0dfd Mon Sep 17 00:00:00 2001 From: Masataka Pocke Kuwabara Date: Thu, 19 Sep 2019 21:16:28 +0900 Subject: [PATCH] Add Rails/RakeEnvironment cop --- CHANGELOG.md | 2 + config/default.yml | 9 ++ lib/rubocop/cop/rails/rake_environment.rb | 91 +++++++++++++++++++ lib/rubocop/cop/rails_cops.rb | 1 + manual/cops.md | 1 + manual/cops_rails.md | 37 ++++++++ .../cop/rails/rake_environment_spec.rb | 86 ++++++++++++++++++ 7 files changed, 227 insertions(+) create mode 100644 lib/rubocop/cop/rails/rake_environment.rb create mode 100644 spec/rubocop/cop/rails/rake_environment_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index fcee2a44ee..445378d682 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/config/default.yml b/config/default.yml index 9b2b62de8d..2c37d8beea 100644 --- a/config/default.yml +++ b/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 diff --git a/lib/rubocop/cop/rails/rake_environment.rb b/lib/rubocop/cop/rails/rake_environment.rb new file mode 100644 index 0000000000..8f6ac2cb13 --- /dev/null +++ b/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 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 60c061078c..e1c1b09491 100644 --- a/manual/cops_rails.md +++ b/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 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..f92d564ffa --- /dev/null +++ b/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