From 49f5d1fa2bb602b6c79f2e4858e0a7af3ed5392d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20=C4=86ilimkovi=C4=87?= Date: Fri, 28 Aug 2020 17:45:32 +0200 Subject: [PATCH] Add new AttributeDefaultBlockValue cop --- CHANGELOG.md | 2 + config/default.yml | 7 + docs/modules/ROOT/pages/cops.adoc | 1 + docs/modules/ROOT/pages/cops_rails.adoc | 62 ++++++++ .../rails/attribute_default_block_value.rb | 68 +++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../attribute_default_block_value_spec.rb | 138 ++++++++++++++++++ 7 files changed, 279 insertions(+) create mode 100644 lib/rubocop/cop/rails/attribute_default_block_value.rb create mode 100644 spec/rubocop/cop/rails/attribute_default_block_value_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index b269f8b6c4..d72d55e8e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### New features * [#362](https://github.com/rubocop-hq/rubocop-rails/pull/362): Add new `Rails/WhereEquals` cop. ([@eugeneius][]) +* [#339](https://github.com/rubocop-hq/rubocop-rails/pull/339): Add new `Rails/AttributeDefaultBlockValue` cop. ([@cilim][]) ### Bug fixes @@ -298,3 +299,4 @@ [@bubaflub]: https://github.com/bubaflub [@dvandersluis]: https://github.com/dvandersluis [@Tietew]: https://github.com/Tietew +[@cilim]: https://github.com/cilim diff --git a/config/default.yml b/config/default.yml index b9d9d38571..239b20b2f8 100644 --- a/config/default.yml +++ b/config/default.yml @@ -105,6 +105,13 @@ Rails/AssertNot: Include: - '**/test/**/*' +Rails/AttributeDefaultBlockValue: + Description: 'Pass method call in block for attribute option `default`.' + Enabled: pending + VersionAdded: '2.9' + Include: + - 'models/**/*' + Rails/BelongsTo: Description: >- Use `optional: true` instead of `required: false` for diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 3f616650af..3232776cb4 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -27,6 +27,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide]. * xref:cops_rails.adoc#railsapplicationmailer[Rails/ApplicationMailer] * xref:cops_rails.adoc#railsapplicationrecord[Rails/ApplicationRecord] * xref:cops_rails.adoc#railsassertnot[Rails/AssertNot] +* xref:cops_rails.adoc#railsattributedefaultblockvalue[Rails/AttributeDefaultBlockValue] * xref:cops_rails.adoc#railsbelongsto[Rails/BelongsTo] * xref:cops_rails.adoc#railsblank[Rails/Blank] * xref:cops_rails.adoc#railsbulkchangetable[Rails/BulkChangeTable] diff --git a/docs/modules/ROOT/pages/cops_rails.adoc b/docs/modules/ROOT/pages/cops_rails.adoc index 1556dccf99..840b258451 100644 --- a/docs/modules/ROOT/pages/cops_rails.adoc +++ b/docs/modules/ROOT/pages/cops_rails.adoc @@ -409,6 +409,68 @@ assert_not x | Array |=== +== Rails/AttributeDefaultBlockValue + +|=== +| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged + +| Pending +| Yes +| Yes +| 2.9 +| - +|=== + +This cop looks for `attribute` class methods that specify a `:default` option +which value is a method call without a block. +It will accept all other values, such as literals and constants. + +=== Examples + +[source,ruby] +---- +# bad +class User < ApplicationRecord + attribute :confirmed_at, :datetime, default: Time.zone.now +end + +# good +class User < ActiveRecord::Base + attribute :confirmed_at, :datetime, default: -> { Time.zone.now } +end + +# good +class User < ActiveRecord::Base + attribute :role, :string, default: :customer +end + +# good +class User < ActiveRecord::Base + attribute :activated, :boolean, default: false +end + +# good +class User < ActiveRecord::Base + attribute :login_count, :integer, default: 0 +end + +# good +class User < ActiveRecord::Base + FOO = 123 + attribute :custom_attribute, :integer, default: FOO +end +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| Include +| `models/**/*` +| Array +|=== + == Rails/BelongsTo |=== diff --git a/lib/rubocop/cop/rails/attribute_default_block_value.rb b/lib/rubocop/cop/rails/attribute_default_block_value.rb new file mode 100644 index 0000000000..85d16caa26 --- /dev/null +++ b/lib/rubocop/cop/rails/attribute_default_block_value.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Rails + # This cop looks for `attribute` class methods that specify a `:default` option + # which value is a method call without a block. + # It will accept all other values, such as literals and constants. + # + # @example + # # bad + # class User < ApplicationRecord + # attribute :confirmed_at, :datetime, default: Time.zone.now + # end + # + # # good + # class User < ActiveRecord::Base + # attribute :confirmed_at, :datetime, default: -> { Time.zone.now } + # end + # + # # good + # class User < ActiveRecord::Base + # attribute :role, :string, default: :customer + # end + # + # # good + # class User < ActiveRecord::Base + # attribute :activated, :boolean, default: false + # end + # + # # good + # class User < ActiveRecord::Base + # attribute :login_count, :integer, default: 0 + # end + # + # # good + # class User < ActiveRecord::Base + # FOO = 123 + # attribute :custom_attribute, :integer, default: FOO + # end + class AttributeDefaultBlockValue < Cop + MSG = 'Pass method in a block to `:default` option.' + + def_node_matcher :default_attribute, <<~PATTERN + (send nil? :attribute _ _ (hash <$#attribute ...>)) + PATTERN + + def_node_matcher :attribute, '(pair (sym :default) $_)' + + def on_send(node) + default_attribute(node) do |attribute| + value = attribute.children.last + + add_offense(node, location: value) if value.send_type? + end + end + + def autocorrect(node) + expression = default_attribute(node).children.last + + lambda do |corrector| + corrector.replace(expression, "-> { #{expression.source} }") + end + end + end + end + end +end diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index ea05106e55..5acc3ba0b2 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -15,6 +15,7 @@ require_relative 'rails/application_mailer' require_relative 'rails/application_record' require_relative 'rails/assert_not' +require_relative 'rails/attribute_default_block_value' require_relative 'rails/belongs_to' require_relative 'rails/blank' require_relative 'rails/bulk_change_table' diff --git a/spec/rubocop/cop/rails/attribute_default_block_value_spec.rb b/spec/rubocop/cop/rails/attribute_default_block_value_spec.rb new file mode 100644 index 0000000000..a9ee49cadb --- /dev/null +++ b/spec/rubocop/cop/rails/attribute_default_block_value_spec.rb @@ -0,0 +1,138 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Rails::AttributeDefaultBlockValue do + subject(:cop) { described_class.new(config) } + + let(:config) { RuboCop::Config.new } + let(:message) { 'Pass method in a block to `:default` option.' } + + context 'when `:default` option is last' do + it 'disallows method' do + expect_offense(<<~RUBY) + def bar + end + + attribute :foo, :string, default: bar + ^^^ #{message} + RUBY + end + + it 'disallows method called from other instance object' do + expect_offense(<<~RUBY) + attribute :foo, :string, default: Foo.new.bar + ^^^^^^^^^^^ #{message} + RUBY + end + + it 'autocorrects `:default` method value in same row' do + expect_offense(<<~RUBY) + attribute :foo, :string, default: Foo.bar + ^^^^^^^ #{message} + RUBY + + expect_correction(<<~RUBY) + attribute :foo, :string, default: -> { Foo.bar } + RUBY + end + + it 'autocorrects `:default` method value in next row' do + expect_offense(<<~RUBY) + attribute :foo, :string, limit: 1, + default: Foo.bar + ^^^^^^^ #{message} + RUBY + + expect_correction(<<~RUBY) + attribute :foo, :string, limit: 1, + default: -> { Foo.bar } + RUBY + end + + it 'allows boolean false' do + expect_no_offenses(<<~RUBY) + attribute :foo, :boolean, default: false + RUBY + end + + it 'allows boolean true' do + expect_no_offenses(<<~RUBY) + attribute :foo, :boolean, default: true + RUBY + end + + it 'allows symbol' do + expect_no_offenses(<<~RUBY) + attribute :foo, :string, default: :bar + RUBY + end + + it 'allows int' do + expect_no_offenses(<<~RUBY) + attribute :foo, :integer, default: 1 + RUBY + end + + it 'allows decimal' do + expect_no_offenses(<<~RUBY) + attribute :foo, :decimal, default: 3.14 + RUBY + end + + it 'allows constant' do + expect_no_offenses(<<~RUBY) + CONSTANT = :foo + attribute :bar, :string, default: CONSTANT + RUBY + end + + it 'allows block' do + expect_no_offenses(<<~RUBY) + attribute :foo, :datetime, default: -> { Time.zone.now } + RUBY + end + + it 'allows without default value' do + expect_no_offenses(<<~RUBY) + attribute :foo, :datetime + RUBY + end + + it 'allows with array option' do + expect_no_offenses(<<~RUBY) + attribute :foo, :bar, array: FooBar.array + RUBY + end + + it 'allows with range option' do + expect_no_offenses(<<~RUBY) + attribute :foo, :bar, range: FooBar.range + RUBY + end + + it 'allows with limit option' do + expect_no_offenses(<<~RUBY) + attribute :foo, :bar, limit: FooBar.limit + RUBY + end + end + + context 'when `:default` option is next to last' do + it 'disallows `:default` method value' do + expect_offense(<<~RUBY) + attribute :foo, :string, range: true, default: Foo.bar, limit: 10 + ^^^^^^^ #{message} + RUBY + end + + it 'autocorrects `:default` method value' do + expect_offense(<<~RUBY) + attribute :foo, :string, default: Foo.bar, limit: 1 + ^^^^^^^ #{message} + RUBY + + expect_correction(<<~RUBY) + attribute :foo, :string, default: -> { Foo.bar }, limit: 1 + RUBY + end + end +end