Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cop Rails/AcceptsNestedAttributesForUpdateOnly #1035

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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