Skip to content

Commit

Permalink
Add Lint/TrailingRescueException (#512)
Browse files Browse the repository at this point in the history
* Add Lint/TrailingRescueException

This prevents the use of Paths as the default value for a trailing exception,
and recommends the user use the block rescue instead to be able to filter by exception type

* Update src/ameba/rule/lint/trailing_rescue_exception.cr

Co-authored-by: Sijawusz Pur Rahnama <[email protected]>

* Update spec/ameba/rule/lint/trailing_rescue_exception_spec.cr

Co-authored-by: Sijawusz Pur Rahnama <[email protected]>

* Update src/ameba/rule/lint/trailing_rescue_exception.cr

Co-authored-by: Sijawusz Pur Rahnama <[email protected]>

* Update src/ameba/rule/lint/trailing_rescue_exception.cr

Co-authored-by: Sijawusz Pur Rahnama <[email protected]>

* Update src/ameba/rule/lint/trailing_rescue_exception.cr

Co-authored-by: Sijawusz Pur Rahnama <[email protected]>

* Update docs for `Lint/TrailingRescueException`

---------

Co-authored-by: Sijawusz Pur Rahnama <[email protected]>
  • Loading branch information
nobodywasishere and Sija authored Nov 30, 2024
1 parent 5841fca commit 780b2c6
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 0 deletions.
25 changes: 25 additions & 0 deletions spec/ameba/rule/lint/trailing_rescue_exception_spec.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
require "../../../spec_helper"

module Ameba::Rule::Lint
subject = TrailingRescueException.new

it "passes for trailing rescue with literal values" do
expect_no_issues subject, <<-CRYSTAL
puts "hello" rescue "world"
puts :meow rescue 1234
CRYSTAL
end

it "passes for trailing rescue with class initialization" do
expect_no_issues subject, <<-CRYSTAL
puts "hello" rescue MyClass.new
CRYSTAL
end

it "fails if trailing rescue has exception name" do
expect_issue subject, <<-CRYSTAL
puts "hello" rescue MyException
# ^^^^^^^^^^^ error: Trailing rescues with a path aren't allowed, use a block rescue instead to filter by exception type
CRYSTAL
end
end
53 changes: 53 additions & 0 deletions src/ameba/rule/lint/trailing_rescue_exception.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
module Ameba::Rule::Lint
# A rule that prohibits the common misconception about how trailing rescue statements work,
# preventing Paths (exception class names or otherwise) from being
# used as the trailing value. The value after the trailing rescue statement is the
# value to use if an exception occurs, not the exception for the rescue to capture.
#
# For example, this is considered invalid - if an exception occurs in `method.call`,
# `value` will be assigned the value of `MyException`:
#
# ```
# value = method.call("param") rescue MyException
# ```
#
# And should instead be written as this in order to capture only `MyException` exceptions:
#
# ```
# value = begin
# method.call("param")
# rescue MyException
# "default value"
# end
# ```
#
# Or to rescue all exceptions (instead of just `MyException`):
#
# ```
# value = method.call("param") rescue "default value"
# ```
#
# YAML configuration example:
#
# ```
# Lint/TrailingRescueException:
# Enabled: true
# ```
class TrailingRescueException < Base
properties do
since_version "1.7.0"
description "Disallows trailing rescue with a path"
end

MSG = "Trailing rescues with a path aren't allowed, use a block rescue instead to filter by exception type"

def test(source, node : Crystal::ExceptionHandler)
return unless node.suffix &&
(rescues = node.rescues) &&
(resc = rescues.first?) &&
resc.body.is_a?(Crystal::Path)

issue_for resc.body, MSG, prefer_name_location: true
end
end
end

0 comments on commit 780b2c6

Please sign in to comment.