Skip to content

Commit 96821f8

Browse files
author
Anthony Robin
committed
[Fix rubocop#49] Introduce RescueFromExceptionsVariableName cop
Follows rubocop/rubocop#7122 Closes rubocop#49 Inspired from @pocke [refactoring][1] in the `Naming/RescuedExceptionsVariableName` cop, this PR handle the `rescue_from` Rails version. Obstructing points: - As discussed [in this comment][2], the `PreferredName` config should not be duplicated but read from the `Naming/RescuedExceptionsVariableName` cop. How can I read the config from another cop ? Is there already some example of cop working this way ? - I haven't been able to make the parser to not take the `||` of the block arguments which make the offending message being `|my_var|` instead of `my_var`. What is the way to avoid this ? [1]: rubocop/rubocop#7122 [2]: rubocop#49 (comment)
1 parent 2922567 commit 96821f8

File tree

7 files changed

+333
-0
lines changed

7 files changed

+333
-0
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### New features
66

77
* [#123](https://github.com/rubocop-hq/rubocop-rails/pull/123): Add new `Rails/ApplicationController` and `Rails/ApplicationMailer` cops. ([@eugeneius][])
8+
* [#79](https://github.com/rubocop-hq/rubocop-rails/issues/79): Add new `Rails/RescueFromExceptionsVariableName` cop. ([@anthony-robin][])
89

910
### Bug fixes
1011

‎config/default.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,13 @@ Rails/RequestReferer:
386386
- referer
387387
- referrer
388388

389+
Rails/RescueFromExceptionsVariableName:
390+
Description: 'Use consistent rescued exceptions variables naming.'
391+
Enabled: true
392+
VersionAdded: '0.67'
393+
VersionChanged: '0.68'
394+
PreferredName: e
395+
389396
Rails/ReversibleMigration:
390397
Description: 'Checks whether the change method of the migration file is reversible.'
391398
StyleGuide: 'https://rails.rubystyle.guide#reversible-migration'

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

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Cop
5+
module Rails
6+
# This cop makes sure that rescued exceptions variables are named as
7+
# expected.
8+
#
9+
# The `PreferredName` config option takes a `String`. It represents
10+
# the required name of the variable. Its default is `e` that is read
11+
# from `Naming/RescuedExceptionsVariableName` cop in the main Rubocop
12+
# repository.
13+
#
14+
# @example PreferredName: e (default)
15+
# # bad
16+
# rescue_from MyException do |exception|
17+
# # do something
18+
# end
19+
#
20+
# # good
21+
# rescue_from MyException do |e|
22+
# # do something
23+
# end
24+
#
25+
# # good
26+
# rescue_from MyException do |_e|
27+
# # do something
28+
# end
29+
#
30+
# @example PreferredName: exception
31+
# # bad
32+
# rescue_from MyException do |e|
33+
# # do something
34+
# end
35+
#
36+
# # good
37+
# rescue_from MyException do |exception|
38+
# # do something
39+
# end
40+
#
41+
# # good
42+
# rescue_from MyException do |_exception|
43+
# # do something
44+
# end
45+
#
46+
class RescueFromExceptionsVariableName < Cop
47+
include ConfigurableEnforcedStyle
48+
49+
MSG = 'Use `%<preferred>s` instead of `%<bad>s`.'
50+
51+
def on_block(node)
52+
name = variable_name(node)
53+
return unless name
54+
return if preferred_name(name).to_sym == name
55+
56+
add_offense(node, location: offense_range(node))
57+
end
58+
59+
def autocorrect(node)
60+
lambda do |corrector|
61+
offending_name = variable_name(node)
62+
preferred_name = preferred_name(offending_name)
63+
corrector.replace(offense_range(node), "|#{preferred_name}|")
64+
end
65+
end
66+
67+
private
68+
69+
def offense_range(resbody)
70+
variable = resbody.node_parts[1]
71+
variable.loc.expression
72+
end
73+
74+
def preferred_name(variable_name)
75+
preferred_name = cop_config.fetch('PreferredName', 'e')
76+
if variable_name.to_s.start_with?('_')
77+
"_#{preferred_name}"
78+
else
79+
preferred_name
80+
end
81+
end
82+
83+
def variable_name(node)
84+
asgn_node = node.node_parts[1]
85+
return unless asgn_node
86+
87+
asgn_node.children.last&.source&.to_sym
88+
end
89+
90+
def message(node)
91+
offending_name = variable_name(node)
92+
preferred_name = preferred_name(offending_name)
93+
format(MSG, preferred: preferred_name, bad: offending_name)
94+
end
95+
end
96+
end
97+
end
98+
end

‎lib/rubocop/cop/rails_cops.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
require_relative 'rails/refute_methods'
4949
require_relative 'rails/relative_date_constant'
5050
require_relative 'rails/request_referer'
51+
require_relative 'rails/rescue_from_exceptions_variable_name'
5152
require_relative 'rails/reversible_migration'
5253
require_relative 'rails/safe_navigation'
5354
require_relative 'rails/save_bang'

‎manual/cops.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
* [Rails/RefuteMethods](cops_rails.md#railsrefutemethods)
4848
* [Rails/RelativeDateConstant](cops_rails.md#railsrelativedateconstant)
4949
* [Rails/RequestReferer](cops_rails.md#railsrequestreferer)
50+
* [Rails/RescueFromExceptionsVariableName](cops_rails.md#railsrescuefromexceptionsvariablename)
5051
* [Rails/ReversibleMigration](cops_rails.md#railsreversiblemigration)
5152
* [Rails/SafeNavigation](cops_rails.md#railssafenavigation)
5253
* [Rails/SaveBang](cops_rails.md#railssavebang)

‎manual/cops_rails.md

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1913,6 +1913,65 @@ Name | Default value | Configurable values
19131913
--- | --- | ---
19141914
EnforcedStyle | `referer` | `referer`, `referrer`
19151915

1916+
## Rails/RescueFromExceptionsVariableName
1917+
1918+
Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
1919+
--- | --- | --- | --- | ---
1920+
Enabled | Yes | Yes | 0.67 | 0.68
1921+
1922+
This cop makes sure that rescued exceptions variables are named as
1923+
expected.
1924+
1925+
The `PreferredName` config option takes a `String`. It represents
1926+
the required name of the variable. Its default is `e` that is read
1927+
from `Naming/RescuedExceptionsVariableName` cop in the main Rubocop
1928+
repository.
1929+
1930+
### Examples
1931+
1932+
#### PreferredName: e (default)
1933+
1934+
```ruby
1935+
# bad
1936+
rescue_from MyException do |exception|
1937+
# do something
1938+
end
1939+
1940+
# good
1941+
rescue_from MyException do |e|
1942+
# do something
1943+
end
1944+
1945+
# good
1946+
rescue_from MyException do |_e|
1947+
# do something
1948+
end
1949+
```
1950+
#### PreferredName: exception
1951+
1952+
```ruby
1953+
# bad
1954+
rescue_from MyException do |e|
1955+
# do something
1956+
end
1957+
1958+
# good
1959+
rescue_from MyException do |exception|
1960+
# do something
1961+
end
1962+
1963+
# good
1964+
rescue_from MyException do |_exception|
1965+
# do something
1966+
end
1967+
```
1968+
1969+
### Configurable attributes
1970+
1971+
Name | Default value | Configurable values
1972+
--- | --- | ---
1973+
PreferredName | `e` | String
1974+
19161975
## Rails/ReversibleMigration
19171976

19181977
Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

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

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# frozen_string_literal: true
2+
3+
RSpec.describe RuboCop::Cop::Rails::RescueFromExceptionsVariableName, :config do
4+
subject(:cop) { described_class.new(config) }
5+
6+
it 'does not register an offense without variable' do
7+
expect_no_offenses(<<~RUBY)
8+
rescue_from MyException do
9+
# do something
10+
end
11+
RUBY
12+
end
13+
14+
context 'with default config' do
15+
context 'with default variable' do
16+
it 'does not register an offense with a single rescued exception' do
17+
expect_no_offenses(<<~RUBY)
18+
rescue_from MyException do |e|
19+
# do something
20+
end
21+
RUBY
22+
end
23+
24+
it 'does not register an offense with multiple rescued exceptions' do
25+
expect_no_offenses(<<~RUBY)
26+
rescue_from MyException, MyOtherException do |e|
27+
# do something
28+
end
29+
RUBY
30+
end
31+
32+
it 'does not register an offense with underscored prefix variable' do
33+
expect_no_offenses(<<~RUBY)
34+
rescue_from MyException do |_e|
35+
# do something
36+
end
37+
RUBY
38+
end
39+
40+
it 'does not register an offense using splat operator' do
41+
expect_no_offenses(<<~RUBY)
42+
rescue_from *handled do |e|
43+
# do something
44+
end
45+
RUBY
46+
end
47+
end
48+
49+
context 'when using another variable' do
50+
it 'registers an offense with a single rescued exception' do
51+
expect_offense(<<~RUBY)
52+
rescue_from MyException do |exception|
53+
^^^^^^^^^^^ Use `e` instead of `exception`.
54+
# do something
55+
end
56+
RUBY
57+
58+
expect_correction(<<~RUBY)
59+
rescue_from MyException do |e|
60+
# do something
61+
end
62+
RUBY
63+
end
64+
65+
it 'registers an offense with multiple rescued exceptions' do
66+
expect_offense(<<~RUBY)
67+
rescue_from MyException, MyOtherException do |exception|
68+
^^^^^^^^^^^ Use `e` instead of `exception`.
69+
# do something
70+
end
71+
RUBY
72+
73+
expect_correction(<<~RUBY)
74+
rescue_from MyException, MyOtherException do |e|
75+
# do something
76+
end
77+
RUBY
78+
end
79+
80+
it 'registers an offense with underscored prefix variable' do
81+
expect_offense(<<~RUBY)
82+
rescue_from MyException do |_exception|
83+
^^^^^^^^^^^^ Use `_e` instead of `_exception`.
84+
# do something
85+
end
86+
RUBY
87+
88+
expect_correction(<<~RUBY)
89+
rescue_from MyException do |_e|
90+
# do something
91+
end
92+
RUBY
93+
end
94+
95+
it 'registers an offense using splat operator' do
96+
expect_offense(<<~RUBY)
97+
rescue_from *handled do |exception|
98+
^^^^^^^^^^^ Use `e` instead of `exception`.
99+
# do something
100+
end
101+
RUBY
102+
103+
expect_correction(<<~RUBY)
104+
rescue_from *handled do |e|
105+
# do something
106+
end
107+
RUBY
108+
end
109+
end
110+
end
111+
112+
context 'with the `PreferredName` setup' do
113+
let(:cop_config) do
114+
{
115+
'PreferredName' => 'exception'
116+
}
117+
end
118+
119+
it 'does not register an offense when using the preferred name' do
120+
expect_no_offenses(<<~RUBY)
121+
rescue_from MyException do |exception|
122+
# do something
123+
end
124+
RUBY
125+
end
126+
127+
it 'does not register an offense when using the preferred name with' \
128+
'multiple rescued exceptions' do
129+
expect_no_offenses(<<~RUBY)
130+
rescue_from MyException, MyOtherException do |exception|
131+
# do something
132+
end
133+
RUBY
134+
end
135+
136+
it 'registers an offense when using another name' do
137+
expect_offense(<<~RUBY)
138+
rescue_from MyException do |e|
139+
^^^ Use `exception` instead of `e`.
140+
# do something
141+
end
142+
RUBY
143+
144+
expect_correction(<<~RUBY)
145+
rescue_from MyException do |exception|
146+
# do something
147+
end
148+
RUBY
149+
end
150+
151+
it 'registers an offense with underscored prefix variable' do
152+
expect_offense(<<~RUBY)
153+
rescue_from MyException do |_e|
154+
^^^^ Use `_exception` instead of `_e`.
155+
# do something
156+
end
157+
RUBY
158+
159+
expect_correction(<<~RUBY)
160+
rescue_from MyException do |_exception|
161+
# do something
162+
end
163+
RUBY
164+
end
165+
end
166+
end

0 commit comments

Comments
 (0)