-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
Issue
I want to propose to change how TRY003 works with some of the builtin exceptions. While I like how the rule applies to custom exceptions and "base" Exception, I think it's incorrect in the case of errors like TypeError and ValueError. It's a common pattern and is widely recommended to use those exceptions when type or value don't match the expectations.
Here you suggest to define a subclass of Exception, but not only defining a separate exception for every case is redundant and would bloat the code, I'm pretty sure using ValueError would more correct.
You already ignore NotImplementedError, which was a decision in the right direction! It doesn't make sense to subclass it every time. I think the same applies for at least ValueError and TypeError.
Solution
Currently you have such code to ignore NotImplementedError:
| // Ignore some built-in exceptions that don't make sense to subclass, like | |
| // `NotImplementedError`. | |
| if checker | |
| .semantic() | |
| .match_builtin_expr(func, "NotImplementedError") | |
| { | |
| return; | |
| } |
sadly it seems that
match_builtin_expr can only take a single symbol. While we could call it multiple times, I think it'd be slightly ineffective? A better approach might be changing this method (or defining a new one), so it can take an array of symbols to match against.
Info
Sample code:
def compute(a: int, b: int) -> int:
if a <= 0:
raise ValueError("a must be > 0") # Ruff: Avoid specifying long messages outside the exception class [TRY003]
if not b:
raise ValueError("b must be != 0") # Ruff: Avoid specifying long messages outside the exception class [TRY003]
return a + bruff command: ruff check --no-fix
ruff version: 0.7.4
ruff settings:
[tool.ruff.lint]
select = ["TRY003"]