Skip to content

Commit 3f0cdcc

Browse files
rrosenblumkoic
rrosenblum
authored and
koic
committed
Fix exception in RelativeDateConstant when auto-correcting anything other than direct constant assignment
1 parent 2c3d7b1 commit 3f0cdcc

File tree

2 files changed

+137
-107
lines changed

2 files changed

+137
-107
lines changed

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

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,15 @@ module Rails
1919
# end
2020
# end
2121
class RelativeDateConstant < Cop
22+
include RangeHelp
23+
2224
MSG = 'Do not assign %<method_name>s to constants as it ' \
2325
'will be evaluated only once.'.freeze
2426

25-
RELATIVE_DATE_METHODS = %i[ago from_now since until].freeze
26-
2727
def on_casgn(node)
28-
_scope, _constant, rhs = *node
29-
30-
# rhs would be nil in a or_asgn node
31-
return unless rhs
32-
33-
check_node(rhs)
28+
relative_date_assignment?(node) do |method_name|
29+
add_offense(node, message: format(MSG, method_name: method_name))
30+
end
3431
end
3532

3633
def on_masgn(node)
@@ -39,20 +36,29 @@ def on_masgn(node)
3936
return unless rhs && rhs.array_type?
4037

4138
lhs.children.zip(rhs.children).each do |(name, value)|
42-
check_node(value) if name.casgn_type?
39+
next unless name.casgn_type?
40+
41+
relative_date?(value) do |method_name|
42+
add_offense(node,
43+
location: range_between(name.loc.expression.begin_pos,
44+
value.loc.expression.end_pos),
45+
message: format(MSG, method_name: method_name))
46+
end
4347
end
4448
end
4549

4650
def on_or_asgn(node)
47-
lhs, rhs = *node
48-
49-
return unless lhs.casgn_type?
50-
51-
check_node(rhs)
51+
relative_date_or_assignment?(node) do |method_name|
52+
add_offense(node, message: format(MSG, method_name: method_name))
53+
end
5254
end
5355

5456
def autocorrect(node)
55-
_scope, const_name, value = *node
57+
return unless node.casgn_type?
58+
59+
scope, const_name, value = *node
60+
return unless scope.nil?
61+
5662
indent = ' ' * node.loc.column
5763
new_code = ["def self.#{const_name.downcase}",
5864
"#{indent}#{value.source}",
@@ -62,27 +68,25 @@ def autocorrect(node)
6268

6369
private
6470

65-
def check_node(node)
66-
return unless node.irange_type? ||
67-
node.erange_type? ||
68-
node.send_type?
69-
70-
# for range nodes we need to check both their boundaries
71-
nodes = node.send_type? ? [node] : node.children
72-
73-
nodes.each do |n|
74-
if relative_date_method?(n)
75-
add_offense(node.parent,
76-
message: format(MSG, method_name: n.method_name))
77-
end
78-
end
79-
end
80-
81-
def relative_date_method?(node)
82-
node.send_type? &&
83-
RELATIVE_DATE_METHODS.include?(node.method_name) &&
84-
!node.arguments?
85-
end
71+
def_node_matcher :relative_date_assignment?, <<-PATTERN
72+
{
73+
(casgn _ _ (send _ ${:since :from_now :after :ago :until :before}))
74+
(casgn _ _ ({erange irange} _ (send _ ${:since :from_now :after :ago :until :before})))
75+
(casgn _ _ ({erange irange} (send _ ${:since :from_now :after :ago :until :before}) _))
76+
}
77+
PATTERN
78+
79+
def_node_matcher :relative_date_or_assignment?, <<-PATTERN
80+
(:or_asgn (casgn _ _) (send _ ${:since :from_now :after :ago :until :before}))
81+
PATTERN
82+
83+
def_node_matcher :relative_date?, <<-PATTERN
84+
{
85+
({erange irange} _ (send _ ${:since :from_now :after :ago :until :before}))
86+
({erange irange} (send _ ${:since :from_now :after :ago :until :before}) _)
87+
(send _ ${:since :from_now :after :ago :until :before})
88+
}
89+
PATTERN
8690
end
8791
end
8892
end

‎spec/rubocop/cop/rails/relative_date_constant_spec.rb

Lines changed: 97 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -3,87 +3,113 @@
33
RSpec.describe RuboCop::Cop::Rails::RelativeDateConstant do
44
subject(:cop) { described_class.new }
55

6-
it 'registers an offense for ActiveSupport::Duration.since' do
7-
expect_offense(<<-RUBY.strip_indent)
8-
class SomeClass
9-
EXPIRED_AT = 1.week.since
10-
^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
11-
end
12-
RUBY
13-
end
6+
context 'direct assignment' do
7+
it 'accepts a method with arguments' do
8+
expect_no_offenses(<<-RUBY.strip_indent)
9+
class SomeClass
10+
EXPIRED_AT = 1.week.since(base)
11+
end
12+
RUBY
13+
end
1414

15-
it 'accepts a method with arguments' do
16-
expect_no_offenses(<<-RUBY.strip_indent)
17-
class SomeClass
18-
EXPIRED_AT = 1.week.since(base)
19-
end
20-
RUBY
21-
end
15+
it 'accepts a lambda' do
16+
expect_no_offenses(<<-RUBY.strip_indent)
17+
class SomeClass
18+
EXPIRED_AT = -> { 1.year.ago }
19+
end
20+
RUBY
21+
end
2222

23-
it 'accepts a lambda' do
24-
expect_no_offenses(<<-RUBY.strip_indent)
25-
class SomeClass
26-
EXPIRED_AT = -> { 1.year.ago }
27-
end
28-
RUBY
29-
end
23+
it 'accepts a proc' do
24+
expect_no_offenses(<<-RUBY.strip_indent)
25+
class SomeClass
26+
EXPIRED_AT = Proc.new { 1.year.ago }
27+
end
28+
RUBY
29+
end
3030

31-
it 'accepts a proc' do
32-
expect_no_offenses(<<-RUBY.strip_indent)
33-
class SomeClass
34-
EXPIRED_AT = Proc.new { 1.year.ago }
35-
end
36-
RUBY
37-
end
31+
it 'registers an offense for ActiveSupport::Duration.since' do
32+
expect_offense(<<-RUBY.strip_indent)
33+
class SomeClass
34+
EXPIRED_AT = 1.week.since
35+
^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
36+
end
37+
RUBY
3838

39-
it 'registers an offense for relative date in ||=' do
40-
expect_offense(<<-RUBY.strip_indent)
41-
class SomeClass
42-
EXPIRED_AT ||= 1.week.since
43-
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
44-
end
45-
RUBY
46-
end
39+
expect_correction(<<-RUBY.strip_indent)
40+
class SomeClass
41+
def self.expired_at
42+
1.week.since
43+
end
44+
end
45+
RUBY
46+
end
4747

48-
it 'registers an offense for relative date in multiple assignment' do
49-
expect_offense(<<-RUBY.strip_indent)
50-
class SomeClass
51-
START, A, x = 2.weeks.ago, 1.week.since, 5
52-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign ago to constants as it will be evaluated only once.
53-
end
54-
RUBY
55-
end
48+
it 'registers an offense for exclusive end range' do
49+
expect_offense(<<-RUBY.strip_indent)
50+
class SomeClass
51+
TRIAL_PERIOD = DateTime.current..1.day.since
52+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
53+
end
54+
RUBY
55+
56+
expect_correction(<<-RUBY.strip_indent)
57+
class SomeClass
58+
def self.trial_period
59+
DateTime.current..1.day.since
60+
end
61+
end
62+
RUBY
63+
end
64+
65+
it 'registers an offense for inclusive end range' do
66+
expect_offense(<<-RUBY.strip_indent)
67+
class SomeClass
68+
TRIAL_PERIOD = DateTime.current...1.day.since
69+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
70+
end
71+
RUBY
72+
end
73+
74+
it 'registers an offense for exclusive begin range' do
75+
expect_offense(<<-RUBY.strip_indent)
76+
class SomeClass
77+
TRIAL_PERIOD = 1.day.ago..DateTime.current
78+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign ago to constants as it will be evaluated only once.
79+
end
80+
RUBY
81+
end
5682

57-
it 'registers an offense for exclusive end range' do
58-
expect_offense(<<-RUBY.strip_indent)
59-
class SomeClass
60-
TRIAL_PERIOD = DateTime.current..1.day.since
61-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
62-
end
63-
RUBY
83+
it 'registers an offense for inclusive begin range' do
84+
expect_offense(<<-RUBY.strip_indent)
85+
class SomeClass
86+
TRIAL_PERIOD = 1.day.ago...DateTime.current
87+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign ago to constants as it will be evaluated only once.
88+
end
89+
RUBY
90+
end
6491
end
6592

66-
it 'registers an offense for inclusive end range' do
67-
expect_offense(<<-RUBY.strip_indent)
68-
class SomeClass
69-
TRIAL_PERIOD = DateTime.current...1.day.since
70-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
71-
end
72-
RUBY
93+
context 'or assignment' do
94+
it 'registers an offense for relative date in ||=' do
95+
expect_offense(<<-RUBY.strip_indent)
96+
class SomeClass
97+
EXPIRED_AT ||= 1.week.since
98+
^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
99+
end
100+
RUBY
101+
end
73102
end
74103

75-
it 'autocorrects' do
76-
new_source = autocorrect_source(<<-RUBY.strip_indent)
77-
class SomeClass
78-
EXPIRED_AT = 1.week.since
79-
end
80-
RUBY
81-
expect(new_source).to eq(<<-RUBY.strip_indent)
82-
class SomeClass
83-
def self.expired_at
84-
1.week.since
104+
context 'mass assignment' do
105+
it 'registers an offense for relative date in multiple assignment' do
106+
expect_offense(<<-RUBY.strip_indent)
107+
class SomeClass
108+
START, A, x = 2.weeks.ago, 1.week.since, 5
109+
^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign ago to constants as it will be evaluated only once.
110+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not assign since to constants as it will be evaluated only once.
85111
end
86-
end
87-
RUBY
112+
RUBY
113+
end
88114
end
89115
end

0 commit comments

Comments
 (0)