Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new rule - enforce that strings passed to exceptions have a variable in them #11979

Open
DetachHead opened this issue Jun 22, 2024 · 1 comment
Labels
rule Implementing or modifying a lint rule

Comments

@DetachHead
Copy link

DetachHead commented Jun 22, 2024

there have been countless times where i've encountered unhelpful error messages in software that don't provide enough information to the user. i think this is something that can be addressed with a lint rule. for example:

if not path.exists():
    raise Exception("file not found") # error: use a relevant variable in your error message to provide more information to the user

error messages like this are extremely common and are very frustrating when exposed to a user. what file was it looking for?

in this case, the fix would be to include the path variable in the message:

if not path.exists():
    raise Exception(f"file not found: {path}")

obviously there will be cases where this is intentional, so here are a couple ideas to help reduce the number of false positives:

  • the error could only be reported on certain exception types (eg. Exception, FileNotFoundError). ideally this would be user-configurable.
  • only report the error if the raised exception is being passed a literal string. ie. raise FooException() would not report the error but raise FooException("something failed") would
@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Jun 23, 2024
@denwong47
Copy link
Contributor

I can give this a go as it seems quite similar to the other PR (#11981) I just worked on, and I personally like long and detailed Exceptions!

I am proposing to only deal with Built-in Exceptions - as User defined exceptions can have particular contextual meaning already, and a variable is not necessarily required.

There are also 4 notable exceptions:

  • NotImplementedError - this, by definition, is not an error related to any runtime states.
  • UnicodeDecodeError, UnicodeEncodeError and UnicodeTranslateError - these by its __init__ already mandates contextual information.

I'll set this up to check against the remaining 52 types.

denwong47 added a commit to denwong47/ruff that referenced this issue Jun 24, 2024
…eption

messages without contextual information.
denwong47 added a commit to denwong47/ruff that referenced this issue Jun 25, 2024
…eption

messages without contextual information.
denwong47 added a commit to denwong47/ruff that referenced this issue Jun 25, 2024
…eption

messages without contextual information.
denwong47 added a commit to denwong47/ruff that referenced this issue Jun 25, 2024
…eption

messages without contextual information.
denwong47 added a commit to denwong47/ruff that referenced this issue Jun 25, 2024
…eption

messages without contextual information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

No branches or pull requests

3 participants