Skip to content

Commit 5b230e5

Browse files
koickoic
authored
Merge pull request #594 from pirj/introduce-redundant-presence-validation-on-belongs_to
[Fix #587] Add Rails/RedundantPresenceValidationOnBelongsTo cop
2 parents 584deee + 301c7d4 commit 5b230e5

File tree

5 files changed

+401
-0
lines changed

5 files changed

+401
-0
lines changed

‎changelog/new_redundant_presence_validation_for_required_belongs_to_association_cop.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#594](https://github.com/rubocop/rubocop-rails/pull/594): Add `Rails/RedundantPresenceValidationOnBelongsTo` cop. ([@pirj][])

‎config/default.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,11 @@ Rails/RedundantForeignKey:
618618
Enabled: true
619619
VersionAdded: '2.6'
620620

621+
Rails/RedundantPresenceValidationOnBelongsTo:
622+
Description: 'Checks for redundant presence validation on belongs_to association.'
623+
Enabled: pending
624+
VersionAdded: '<<next>>'
625+
621626
Rails/RedundantReceiverInWithOptions:
622627
Description: 'Checks for redundant receiver in `with_options`.'
623628
Enabled: true

‎lib/rubocop/cop/rails/redundant_presence_validation_on_belongs_to.rb

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# Since Rails 5.0 the default for `belongs_to` is `optional: false`
7+
# unless `config.active_record.belongs_to_required_by_default` is
8+
# explicitly set to `false`. The presence validator is added
9+
# automatically, and explicit presence validation is redundant.
10+
#
11+
# @example
12+
# # bad
13+
# belongs_to :user
14+
# validates :user, presence: true
15+
#
16+
# # bad
17+
# belongs_to :user
18+
# validates :user_id, presence: true
19+
#
20+
# # bad
21+
# belongs_to :author, foreign_key: :user_id
22+
# validates :user_id, presence: true
23+
#
24+
# # good
25+
# belongs_to :user
26+
#
27+
# # good
28+
# belongs_to :author, foreign_key: :user_id
29+
#
30+
class RedundantPresenceValidationOnBelongsTo < Base
31+
include RangeHelp
32+
extend AutoCorrector
33+
extend TargetRailsVersion
34+
35+
MSG = 'Remove explicit presence validation for `%<association>s`.'
36+
RESTRICT_ON_SEND = %i[validates].freeze
37+
38+
minimum_target_rails_version 5.0
39+
40+
# @!method presence_validation?(node)
41+
# Match a `validates` statement with a presence check
42+
#
43+
# @example source that matches - by association
44+
# validates :user, presence: true
45+
#
46+
# @example source that matches - with presence options
47+
# validates :user, presence: { message: 'duplicate' }
48+
#
49+
# @example source that matches - by a foreign key
50+
# validates :user_id, presence: true
51+
def_node_matcher :presence_validation?, <<~PATTERN
52+
$(
53+
send nil? :validates
54+
(sym $_)
55+
...
56+
$(hash <$(pair (sym :presence) {true hash}) ...>)
57+
)
58+
PATTERN
59+
60+
# @!method optional_option?(node)
61+
# Match a `belongs_to` association with an optional option in a hash
62+
def_node_matcher :optional?, <<~PATTERN
63+
(send nil? :belongs_to _ ... #optional_option?)
64+
PATTERN
65+
66+
# @!method optional_option?(node)
67+
# Match an optional option in a hash
68+
def_node_matcher :optional_option?, <<~PATTERN
69+
{
70+
(hash <(pair (sym :optional) true) ...>) # optional: true
71+
(hash <(pair (sym :required) false) ...>) # required: false
72+
}
73+
PATTERN
74+
75+
# @!method any_belongs_to?(node, association:)
76+
# Match a class with `belongs_to` with no regard to `foreign_key` option
77+
#
78+
# @example source that matches
79+
# belongs_to :user
80+
#
81+
# @example source that matches - regardless of `foreign_key`
82+
# belongs_to :author, foreign_key: :user_id
83+
#
84+
# @param node [RuboCop::AST::Node]
85+
# @param association [Symbol]
86+
# @return [Array<RuboCop::AST::Node>, nil] matching node
87+
def_node_matcher :any_belongs_to?, <<~PATTERN
88+
(begin
89+
<
90+
$(send nil? :belongs_to (sym %association) ...)
91+
...
92+
>
93+
)
94+
PATTERN
95+
96+
# @!method belongs_to?(node, key:, fk:)
97+
# Match a class with a matching association, either by name or an explicit
98+
# `foreign_key` option
99+
#
100+
# @example source that matches - fk matches `foreign_key` option
101+
# belongs_to :author, foreign_key: :user_id
102+
#
103+
# @example source that matches - key matches association name
104+
# belongs_to :user
105+
#
106+
# @example source that does not match - explicit `foreign_key` does not match
107+
# belongs_to :user, foreign_key: :account_id
108+
#
109+
# @param node [RuboCop::AST::Node]
110+
# @param key [Symbol] e.g. `:user`
111+
# @param fk [Symbol] e.g. `:user_id`
112+
# @return [Array<RuboCop::AST::Node>] matching nodes
113+
def_node_matcher :belongs_to?, <<~PATTERN
114+
(begin
115+
<
116+
${
117+
#belongs_to_without_fk?(%key) # belongs_to :user
118+
#belongs_to_with_a_matching_fk?(%fk) # belongs_to :author, foreign_key: :user_id
119+
}
120+
...
121+
>
122+
)
123+
PATTERN
124+
125+
# @!method belongs_to_without_fk?(node, fk)
126+
# Match a matching `belongs_to` association, without an explicit `foreign_key` option
127+
#
128+
# @param node [RuboCop::AST::Node]
129+
# @param key [Symbol] e.g. `:user`
130+
# @return [Array<RuboCop::AST::Node>] matching nodes
131+
def_node_matcher :belongs_to_without_fk?, <<~PATTERN
132+
{
133+
(send nil? :belongs_to (sym %1)) # belongs_to :user
134+
(send nil? :belongs_to (sym %1) !hash) # belongs_to :user, -> { not_deleted }
135+
(send nil? :belongs_to (sym %1) !(hash <(pair (sym :foreign_key) _) ...>))
136+
}
137+
PATTERN
138+
139+
# @!method belongs_to_with_a_matching_fk?(node, fk)
140+
# Match a matching `belongs_to` association with a matching explicit `foreign_key` option
141+
#
142+
# @example source that matches
143+
# belongs_to :author, foreign_key: :user_id
144+
#
145+
# @param node [RuboCop::AST::Node]
146+
# @param fk [Symbol] e.g. `:user_id`
147+
# @return [Array<RuboCop::AST::Node>] matching nodes
148+
def_node_matcher :belongs_to_with_a_matching_fk?, <<~PATTERN
149+
(send nil? :belongs_to ... (hash <(pair (sym :foreign_key) (sym %1)) ...>))
150+
PATTERN
151+
152+
def on_send(node)
153+
validation, key, options, presence = presence_validation?(node)
154+
return unless validation
155+
156+
belongs_to = belongs_to_for(node.parent, key)
157+
return unless belongs_to
158+
return if optional?(belongs_to)
159+
160+
message = format(MSG, association: key.to_s)
161+
162+
add_offense(presence, message: message) do |corrector|
163+
remove_presence_validation(corrector, node, options, presence)
164+
end
165+
end
166+
167+
private
168+
169+
def belongs_to_for(model_class_node, key)
170+
if key.to_s.end_with?('_id')
171+
normalized_key = key.to_s.delete_suffix('_id').to_sym
172+
belongs_to?(model_class_node, key: normalized_key, fk: key)
173+
else
174+
any_belongs_to?(model_class_node, association: key)
175+
end
176+
end
177+
178+
def remove_presence_validation(corrector, node, options, presence)
179+
if options.children.one?
180+
corrector.remove(range_by_whole_lines(node.source_range, include_final_newline: true))
181+
else
182+
range = range_with_surrounding_comma(
183+
range_with_surrounding_space(range: presence.source_range, side: :left),
184+
:left
185+
)
186+
corrector.remove(range)
187+
end
188+
end
189+
end
190+
end
191+
end
192+
end

‎lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
require_relative 'rails/read_write_attribute'
7474
require_relative 'rails/redundant_allow_nil'
7575
require_relative 'rails/redundant_foreign_key'
76+
require_relative 'rails/redundant_presence_validation_on_belongs_to'
7677
require_relative 'rails/redundant_receiver_in_with_options'
7778
require_relative 'rails/redundant_travel_back'
7879
require_relative 'rails/reflection_class_name'

0 commit comments

Comments
 (0)