Skip to content

Commit

Permalink
Add cop Rails/AcceptsNestedAttributesForUpdateOnly
Browse files Browse the repository at this point in the history
  • Loading branch information
olivier-thatch committed Oct 9, 2023
1 parent ffa1464 commit 351499f
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 0 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

## master (unreleased)

* [#1035](https://github.com/rubocop/rubocop-rails/pull/1035): Add cop Rails/AcceptsNestedAttributesForUpdateOnly. ([@olivier-thatch][])

## 2.21.2 (2023-09-30)

### Bug fixes
Expand Down Expand Up @@ -952,3 +954,4 @@
[@nipe0324]: https://github.com/nipe0324
[@marocchino]: https://github.com/marocchino
[@jamiemccarthy]: https://github.com/jamiemccarthy
[@olivier-thatch]: https://github.com/olivier-thatch
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ Rails:
Enabled: true
DocumentationBaseURL: https://docs.rubocop.org/rubocop-rails

Rails/AcceptsNestedAttributesForUpdateOnly:
Description: 'Define the update_only option to the accepts_nested_attributes_for attributes writers.'
StyleGuide: 'https://rails.rubystyle.guide#accepts_nested_attributes_for-update_only-option'
Enabled: false
VersionAdded: '2.21'
Include:
- app/models/**/*.rb

Rails/ActionControllerFlashBeforeRender:
Description: 'Use `flash.now` instead of `flash` before `render`.'
Enabled: 'pending'
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ based on the https://rails.rubystyle.guide/[Rails Style Guide].

=== Department xref:cops_rails.adoc[Rails]

* xref:cops_rails.adoc#railsacceptsnestedattributesforupdateonly[Rails/AcceptsNestedAttributesForUpdateOnly]
* xref:cops_rails.adoc#railsactioncontrollerflashbeforerender[Rails/ActionControllerFlashBeforeRender]
* xref:cops_rails.adoc#railsactioncontrollertestcase[Rails/ActionControllerTestCase]
* xref:cops_rails.adoc#railsactionfilter[Rails/ActionFilter]
Expand Down
46 changes: 46 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
@@ -1,5 +1,51 @@
= Rails

== Rails/AcceptsNestedAttributesForUpdateOnly

|===
| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed

| Disabled
| Yes
| No
| 2.21
| -
|===

Looks for `accepts_nested_attributes_for` attributes writers that don't
specify an `:update_only` option.

=== Examples

[source,ruby]
----
# bad
class Member < ActiveRecord::Base
has_one :avatar
accepts_nested_attributes_for :avatar
end
# good
class Member < ActiveRecord::Base
has_one :avatar
accepts_nested_attributes_for :avatar, update_only: true
end
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| Include
| `+app/models/**/*.rb+`
| Array
|===

=== References

* https://rails.rubystyle.guide#accepts_nested_attributes_for-update_only-option

== Rails/ActionControllerFlashBeforeRender

|===
Expand Down
115 changes: 115 additions & 0 deletions lib/rubocop/cop/rails/accepts_nested_attributes_for_update_only.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# Looks for `accepts_nested_attributes_for` attributes writers that don't
# specify an `:update_only` option.
#
# @example
# # bad
# class Member < ActiveRecord::Base
# has_one :avatar
# accepts_nested_attributes_for :avatar
# end
#
# # good
# class Member < ActiveRecord::Base
# has_one :avatar
# accepts_nested_attributes_for :avatar, update_only: true
# end
class AcceptsNestedAttributesForUpdateOnly < Base
MSG = 'Specify a `:update_only` option.'
RESTRICT_ON_SEND = %i[accepts_nested_attributes_for].freeze

def_node_search :active_resource_class?, <<~PATTERN
(const (const {nil? cbase} :ActiveResource) :Base)
PATTERN

def_node_matcher :accepts_nested_attributes_for_without_options?, <<~PATTERN
(send _ {:accepts_nested_attributes_for} _)
PATTERN

def_node_matcher :accepts_nested_attributes_for_with_options?, <<~PATTERN
(send _ {:accepts_nested_attributes_for} ... (hash $...))
PATTERN

def_node_matcher :update_only_option?, <<~PATTERN
(pair (sym :update_only) {!nil (nil)})
PATTERN

def_node_matcher :with_options_block, <<~PATTERN
(block
(send nil? :with_options
(hash $...))
(args) ...)
PATTERN

def_node_matcher :accepts_nested_attributes_for_extension_block?, <<~PATTERN
(block
(send nil? :accepts_nested_attributes_for _)
(args) ...)
PATTERN

def on_send(node)
return if active_resource?(node.parent)
return if !accepts_nested_attributes_for_without_options?(node) && \
valid_options?(accepts_nested_attributes_for_with_options?(node))
return if valid_options_in_with_options_block?(node)

add_offense(node.loc.selector)
end

private

def valid_options_in_with_options_block?(node)
return true unless node.parent

n = node.parent.begin_type?
n ||= accepts_nested_attributes_for_extension_block?(node.parent) ? node.parent.parent : node.parent

contain_valid_options_in_with_options_block?(n)
end

def contain_valid_options_in_with_options_block?(node)
if (options = with_options_block(node))
return true if valid_options?(options)

return false unless node.parent

return true if contain_valid_options_in_with_options_block?(node.parent.parent)
end

false
end

def valid_options?(options)
return false if options.nil?

options = extract_option_if_kwsplat(options)

return true unless options
return true if options.any? do |o|
update_only_option?(o)
end

false
end

def extract_option_if_kwsplat(options)
if options.first.kwsplat_type? && options.first.children.first.hash_type?
return options.first.children.first.pairs
end

options
end

def active_resource?(node)
return false if node.nil?

active_resource_class?(node)
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 @@ -9,6 +9,7 @@
require_relative 'mixin/migrations_helper'
require_relative 'mixin/target_rails_version'

require_relative 'rails/accepts_nested_attributes_for_update_only'
require_relative 'rails/action_controller_flash_before_render'
require_relative 'rails/action_controller_test_case'
require_relative 'rails/action_filter'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::AcceptsNestedAttributesForUpdateOnly, :config do
context 'accepts_nested_attributes_for' do
it 'registers an offense when not specifying any options' do
expect_offense(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
accepts_nested_attributes_for :avatar
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Specify a `:update_only` option.
end
RUBY
end

it 'registers an offense when missing an explicit `:update_only` flag' do
expect_offense(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
accepts_nested_attributes_for :avatar, reject_if: :all_blank
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Specify a `:update_only` option.
end
RUBY
end

it 'does not register an offense when specifying `:update_only` flag' do
expect_no_offenses(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
accepts_nested_attributes_for :avatar, update_only: true
end
RUBY
end

it 'does not register an offense when specifying `:update_only` flag with double splat' do
expect_no_offenses(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
accepts_nested_attributes_for :avatar, **{update_only: true}
end
RUBY
end

it 'registers an offense when a variable passed with double splat' do
expect_offense(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
accepts_nested_attributes_for :avatar, **bar
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Specify a `:update_only` option.
end
RUBY
end

context 'with_options update_only: true' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
with_options update_only: true do
accepts_nested_attributes_for :avatar
end
end
RUBY
end

it 'does not register an offense for using `reject_if` option' do
expect_no_offenses(<<~RUBY)
class Member < ApplicationRecord
has_one :avatar
with_options update_only: true do
accepts_nested_attributes_for :avatar, reject_if: :all_blank
end
end
RUBY
end
end
end

context 'when an Active Record model does not have any associations' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class Member < ApplicationRecord
end
RUBY
end
end
end

0 comments on commit 351499f

Please sign in to comment.