Skip to content

Commit fc06921

Browse files
samuel-williams-shopifysamuel-williams-shopify
committed
Add warning for global exception variables.
1 parent 305d14b commit fc06921

File tree

4 files changed

+274
-0
lines changed

4 files changed

+274
-0
lines changed

‎lib/rubocop/socketry.rb‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
require_relative "socketry/plugin"
88
require_relative "socketry/layout/consistent_blank_line_indentation"
99
require_relative "socketry/layout/block_delimiter_spacing"
10+
require_relative "socketry/style/global_exception_variables"
1011

1112
# @namespace
1213
module RuboCop
@@ -15,5 +16,9 @@ module Socketry
1516
# @namespace
1617
module Layout
1718
end
19+
20+
# @namespace
21+
module Style
22+
end
1823
end
1924
end

‎lib/rubocop/socketry/config.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,8 @@ Layout/BlockDelimiterSpacing:
1212
Description: "Enforces consistent spacing before block delimiters."
1313
Enabled: true
1414
VersionAdded: "0.4.0"
15+
16+
Style/GlobalExceptionVariables:
17+
Description: "Warns against using global exception variables like $! and $@."
18+
Enabled: true
19+
VersionAdded: "0.5.0"

‎lib/rubocop/socketry/style/global_exception_variables.rb‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# frozen_string_literal: true
2+
3+
# Released under the MIT License.
4+
# Copyright, 2025, by Samuel Williams.
5+
6+
require "rubocop"
7+
8+
module RuboCop
9+
module Socketry
10+
module Style
11+
# A RuboCop cop that warns against using global exception variables.
12+
#
13+
# This cop discourages the use of:
14+
# - `$!` (last exception)
15+
# - `$@` (backtrace of last exception)
16+
# - `$ERROR_INFO` (English name for `$!`)
17+
# - `$ERROR_POSITION` (English name for `$@`)
18+
#
19+
# These global variables are implicit and can make code harder to understand.
20+
# Instead, use explicit exception handling with rescue blocks and local variables.
21+
#
22+
# @example
23+
# # bad
24+
# begin
25+
# risky_operation
26+
# rescue
27+
# puts $!.message
28+
# puts [email protected]
29+
# end
30+
#
31+
# # good
32+
# begin
33+
# risky_operation
34+
# rescue => error
35+
# puts error.message
36+
# puts error.backtrace.first
37+
# end
38+
class GlobalExceptionVariables < RuboCop::Cop::Base
39+
MSG = "Avoid using global exception variable `%<variable>s`. " \
40+
"Use explicit exception handling with `rescue => error` instead."
41+
42+
EXCEPTION_VARIABLES = %i[$! $@ $ERROR_INFO $ERROR_POSITION].freeze
43+
44+
def on_gvar(node)
45+
variable_name = node.children.first
46+
47+
return unless EXCEPTION_VARIABLES.include?(variable_name)
48+
49+
add_offense(
50+
node,
51+
message: format(MSG, variable: variable_name)
52+
)
53+
end
54+
end
55+
end
56+
end
57+
end
58+

‎test/rubocop/socketry/style/global_exception_variables.rb‎

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
# frozen_string_literal: true
2+
3+
# Released under the MIT License.
4+
# Copyright, 2025, by Samuel Williams.
5+
6+
require "rubocop/socketry/style/global_exception_variables"
7+
8+
describe RuboCop::Socketry::Style::GlobalExceptionVariables do
9+
let(:config) {RuboCop::Config.new}
10+
let(:cop) {subject.new(config)}
11+
12+
# Test $! variable
13+
with "code using $!" do
14+
let(:source) do
15+
<<~RUBY
16+
begin
17+
risky_operation
18+
rescue
19+
puts $!.message
20+
end
21+
RUBY
22+
end
23+
24+
it "registers an offense" do
25+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
26+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
27+
report = investigator.investigate(processed_source)
28+
offenses = report.offenses
29+
expect(offenses).not.to be(:empty?)
30+
expect(offenses.first.message).to be(:include?, "$!")
31+
end
32+
end
33+
34+
# Test $@ variable
35+
with "code using $@" do
36+
let(:source) do
37+
<<~RUBY
38+
begin
39+
risky_operation
40+
rescue
41+
puts [email protected]
42+
end
43+
RUBY
44+
end
45+
46+
it "registers an offense" do
47+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
48+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
49+
report = investigator.investigate(processed_source)
50+
offenses = report.offenses
51+
expect(offenses).not.to be(:empty?)
52+
expect(offenses.first.message).to be(:include?, "$@")
53+
end
54+
end
55+
56+
# Test $ERROR_INFO variable
57+
with "code using $ERROR_INFO" do
58+
let(:source) do
59+
<<~RUBY
60+
require 'English'
61+
begin
62+
risky_operation
63+
rescue
64+
puts $ERROR_INFO.message
65+
end
66+
RUBY
67+
end
68+
69+
it "registers an offense" do
70+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
71+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
72+
report = investigator.investigate(processed_source)
73+
offenses = report.offenses
74+
expect(offenses).not.to be(:empty?)
75+
expect(offenses.first.message).to be(:include?, "$ERROR_INFO")
76+
end
77+
end
78+
79+
# Test $ERROR_POSITION variable
80+
with "code using $ERROR_POSITION" do
81+
let(:source) do
82+
<<~RUBY
83+
require 'English'
84+
begin
85+
risky_operation
86+
rescue
87+
puts $ERROR_POSITION.first
88+
end
89+
RUBY
90+
end
91+
92+
it "registers an offense" do
93+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
94+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
95+
report = investigator.investigate(processed_source)
96+
offenses = report.offenses
97+
expect(offenses).not.to be(:empty?)
98+
expect(offenses.first.message).to be(:include?, "$ERROR_POSITION")
99+
end
100+
end
101+
102+
# Test proper exception handling (should not register offense)
103+
with "code using explicit exception variable" do
104+
let(:source) do
105+
<<~RUBY
106+
begin
107+
risky_operation
108+
rescue => error
109+
puts error.message
110+
puts error.backtrace.first
111+
end
112+
RUBY
113+
end
114+
115+
it "does not register an offense" do
116+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
117+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
118+
report = investigator.investigate(processed_source)
119+
offenses = report.offenses
120+
expect(offenses).to be(:empty?)
121+
end
122+
end
123+
124+
# Test with StandardError captured explicitly
125+
with "code using named exception variable" do
126+
let(:source) do
127+
<<~RUBY
128+
begin
129+
risky_operation
130+
rescue StandardError => e
131+
puts e.message
132+
puts e.backtrace.first
133+
end
134+
RUBY
135+
end
136+
137+
it "does not register an offense" do
138+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
139+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
140+
report = investigator.investigate(processed_source)
141+
offenses = report.offenses
142+
expect(offenses).to be(:empty?)
143+
end
144+
end
145+
146+
# Test multiple uses in same block
147+
with "code using multiple global exception variables" do
148+
let(:source) do
149+
<<~RUBY
150+
begin
151+
risky_operation
152+
rescue
153+
puts $!.message
154+
puts [email protected]("\\n")
155+
end
156+
RUBY
157+
end
158+
159+
it "registers multiple offenses" do
160+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
161+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
162+
report = investigator.investigate(processed_source)
163+
offenses = report.offenses
164+
expect(offenses.size).to be == 2
165+
end
166+
end
167+
168+
# Test usage outside rescue block (still should be flagged)
169+
with "code using $! outside rescue block" do
170+
let(:source) do
171+
<<~RUBY
172+
def log_last_error
173+
puts $!.message if $!
174+
end
175+
RUBY
176+
end
177+
178+
it "registers offenses" do
179+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
180+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
181+
report = investigator.investigate(processed_source)
182+
offenses = report.offenses
183+
expect(offenses.size).to be == 2
184+
end
185+
end
186+
187+
# Test that other global variables are not flagged
188+
with "code using other global variables" do
189+
let(:source) do
190+
<<~RUBY
191+
puts $stdout
192+
puts $stderr
193+
puts $LOAD_PATH
194+
RUBY
195+
end
196+
197+
it "does not register an offense" do
198+
processed_source = RuboCop::ProcessedSource.new(source, RUBY_VERSION.to_f)
199+
investigator = RuboCop::Cop::Commissioner.new([cop], [], raise_error: true)
200+
report = investigator.investigate(processed_source)
201+
offenses = report.offenses
202+
expect(offenses).to be(:empty?)
203+
end
204+
end
205+
end
206+

0 commit comments

Comments
 (0)