Skip to content

Commit 2c3d7b1

Browse files
petehamiltonkoic
authored andcommitted
In Rails >= 5, use optional: true, not required: false
The `required` option on belongs_to associations has been deprecated, instead the recommended (and default) approach is to set this to true by default and disable it using `optional: true`. The developer can override this default globally, however, so we can't say that things like `optional: false` are superfluous, or indeed that `required: true` isn't necessary. This change therefore converts any use of `required` to it's inverse `optional` equivalent. See: https://guides.rubyonrails.org/5_0_release_notes.html
1 parent 7cdce81 commit 2c3d7b1

File tree

5 files changed

+224
-0
lines changed

5 files changed

+224
-0
lines changed

config/enabled.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ Rails/AssertNot:
3030
Description: 'Use `assert_not` instead of `assert !`.'
3131
Enabled: true
3232

33+
Rails/BelongsTo:
34+
Description: >-
35+
Use `optional: true` instead of `required: false` for
36+
`belongs_to` relations'
37+
Enabled: true
38+
3339
Rails/Blank:
3440
Description: 'Enforces use of `blank?`.'
3541
Enabled: true

lib/rubocop/cop/rails/belongs_to.rb

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# This cop looks for belongs_to associations where we control whether the
7+
# association is required via the deprecated `required` option instead.
8+
#
9+
# Since Rails 5, belongs_to associations are required by default and this
10+
# can be controlled through the use of `optional: true`.
11+
#
12+
# From the release notes:
13+
#
14+
# belongs_to will now trigger a validation error by default if the
15+
# association is not present. You can turn this off on a
16+
# per-association basis with optional: true. Also deprecate required
17+
# option in favor of optional for belongs_to. (Pull Request)
18+
#
19+
# In the case that the developer is doing `required: false`, we
20+
# definitely want to autocorrect to `optional: true`.
21+
#
22+
# However, without knowing whether they've set overriden the default
23+
# value of `config.active_record.belongs_to_required_by_default`, we
24+
# can't say whether it's safe to remove `required: true` or whether we
25+
# should replace it with `optional: false` (or, similarly, remove a
26+
# superfluous `optional: false`). Therefore, in the cases we're using
27+
# `required: true`, we'll simply invert it to `optional: false` and the
28+
# user can remove depending on their defaults.
29+
#
30+
# @example
31+
# # bad
32+
# class Post < ApplicationRecord
33+
# belongs_to :blog, required: false
34+
# end
35+
#
36+
# # good
37+
# class Post < ApplicationRecord
38+
# belongs_to :blog, optional: true
39+
# end
40+
#
41+
# # bad
42+
# class Post < ApplicationRecord
43+
# belongs_to :blog, required: true
44+
# end
45+
#
46+
# # good
47+
# class Post < ApplicationRecord
48+
# belongs_to :blog, optional: false
49+
# end
50+
#
51+
# @see https://guides.rubyonrails.org/5_0_release_notes.html
52+
# @see https://github.com/rails/rails/pull/18937
53+
class BelongsTo < Cop
54+
extend TargetRailsVersion
55+
56+
minimum_target_rails_version 5.0
57+
58+
SUPERFLOUS_REQUIRE_FALSE_MSG =
59+
'You specified `required: false`, in Rails > 5.0 the required ' \
60+
'option is deprecated and you want to use `optional: true`.'.freeze
61+
62+
SUPERFLOUS_REQUIRE_TRUE_MSG =
63+
'You specified `required: true`, in Rails > 5.0 the required ' \
64+
'option is deprecated and you want to use `optional: false`. ' \
65+
'In most configurations, this is the default and you can omit ' \
66+
'this option altogether'.freeze
67+
68+
def_node_matcher :match_belongs_to_with_options, <<-PATTERN
69+
(send $_ :belongs_to _ (hash $...))
70+
PATTERN
71+
72+
def_node_matcher :match_required_false?, <<-PATTERN
73+
(pair (sym :required) false)
74+
PATTERN
75+
76+
def_node_matcher :match_required_true?, <<-PATTERN
77+
(pair (sym :required) true)
78+
PATTERN
79+
80+
def on_send(node)
81+
opt = extract_required_option(node)
82+
return unless opt
83+
return unless match_required_true?(opt) || match_required_false?(opt)
84+
85+
message =
86+
if match_required_true?(opt)
87+
SUPERFLOUS_REQUIRE_TRUE_MSG
88+
elsif match_required_false?(opt)
89+
SUPERFLOUS_REQUIRE_FALSE_MSG
90+
end
91+
92+
add_offense(node, message: message, location: :selector)
93+
end
94+
95+
def autocorrect(node)
96+
opt = extract_required_option(node)
97+
return unless opt
98+
99+
lambda do |corrector|
100+
if match_required_true?(opt)
101+
corrector.replace(opt.loc.expression, 'optional: false')
102+
elsif match_required_false?(opt)
103+
corrector.replace(opt.loc.expression, 'optional: true')
104+
end
105+
end
106+
end
107+
108+
def extract_required_option(node)
109+
_, opts = match_belongs_to_with_options(node)
110+
return unless opts
111+
112+
opts.find do |opt|
113+
match_required_true?(opt) ||
114+
match_required_false?(opt)
115+
end
116+
end
117+
end
118+
end
119+
end
120+
end

lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ module Cop
1414
require_relative 'rails/application_job'
1515
require_relative 'rails/application_record'
1616
require_relative 'rails/assert_not'
17+
require_relative 'rails/belongs_to'
1718
require_relative 'rails/blank'
1819
require_relative 'rails/bulk_change_table'
1920
require_relative 'rails/create_table_with_timestamps'

manual/cops_rails.md

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,60 @@ Name | Default value | Configurable values
162162
--- | --- | ---
163163
Include | `**/test/**/*` | Array
164164

165+
## Rails/BelongsTo
166+
167+
Enabled by default | Supports autocorrection
168+
--- | ---
169+
Enabled | Yes
170+
171+
This cop looks for belongs_to associations where we control whether the
172+
association is required via the deprecated `required` option instead.
173+
174+
Since Rails 5, belongs_to associations are required by default and this
175+
can be controlled through the use of `optional: true`.
176+
177+
From the release notes:
178+
179+
belongs_to will now trigger a validation error by default if the
180+
association is not present. You can turn this off on a
181+
per-association basis with optional: true. Also deprecate required
182+
option in favor of optional for belongs_to. (Pull Request)
183+
184+
In the case that the developer is doing `required: false`, we
185+
definitely want to autocorrect to `optional: true`.
186+
187+
However, without knowing whether they've set overriden the default
188+
value of `config.active_record.belongs_to_required_by_default`, we
189+
can't say whether it's safe to remove `required: true` or whether we
190+
should replace it with `optional: false` (or, similarly, remove a
191+
superfluous `optional: false`). Therefore, in the cases we're using
192+
`required: true`, we'll simply invert it to `optional: false` and the
193+
user can remove depending on their defaults.
194+
195+
### Examples
196+
197+
```ruby
198+
# bad
199+
class Post < ApplicationRecord
200+
belongs_to :blog, required: false
201+
end
202+
203+
# good
204+
class Post < ApplicationRecord
205+
belongs_to :blog, optional: true
206+
end
207+
208+
# bad
209+
class Post < ApplicationRecord
210+
belongs_to :blog, required: true
211+
end
212+
213+
# good
214+
class Post < ApplicationRecord
215+
belongs_to :blog, optional: false
216+
end
217+
```
218+
165219
## Rails/Blank
166220

167221
Enabled by default | Supports autocorrection
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::BelongsTo do
4+
subject(:cop) { described_class.new(config) }
5+
6+
let(:config) { RuboCop::Config.new }
7+
8+
it 'registers an offense when specifying `required: false`' do
9+
expect_offense(<<-RUBY.strip_indent)
10+
belongs_to :foo, required: false
11+
^^^^^^^^^^ You specified `required: false`, in Rails > 5.0 the required option is deprecated and you want to use `optional: true`.
12+
RUBY
13+
end
14+
15+
it 'registers an offense when specifying `required: true`' do
16+
expect_offense(<<-RUBY.strip_indent)
17+
belongs_to :foo, required: true
18+
^^^^^^^^^^ You specified `required: true`, in Rails > 5.0 the required option is deprecated and you want to use `optional: false`. In most configurations, this is the default and you can omit this option altogether
19+
RUBY
20+
end
21+
22+
it 'auto-corrects `required: false` to `optional: true`' do
23+
expect(autocorrect_source('belongs_to :foo, required: false'))
24+
.to eq('belongs_to :foo, optional: true')
25+
expect(cop.offenses.last.status).to eq(:corrected)
26+
end
27+
28+
it 'auto-corrects `required: true` to `optional: false`' do
29+
code = 'belongs_to :foo, required: true'
30+
expect(autocorrect_source(code))
31+
.to eq('belongs_to :foo, optional: false')
32+
expect(cop.offenses.last.status).to eq(:corrected)
33+
end
34+
35+
it 'registers no offense when setting `optional: true`' do
36+
expect_no_offenses('belongs_to :foo, optional: true')
37+
end
38+
39+
it 'registers no offense when requires: false is not set' do
40+
expect_no_offenses('belongs_to :foo')
41+
expect_no_offenses('belongs_to :foo, polymorphic: true')
42+
end
43+
end

0 commit comments

Comments
 (0)