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

Add new checks for except (...): clauses #105

Merged
merged 1 commit into from Jan 4, 2020
Merged

Add new checks for except (...): clauses #105

merged 1 commit into from Jan 4, 2020

Conversation

Zac-HD
Copy link
Member

@Zac-HD Zac-HD commented Jan 3, 2020

When using a tuple of exception types in an except clause, it's easy for inexperienced Pythonistas to include elements that don't have any function at all. For example:

try: ...
except (Exception, TypeError): ...	 # TypeError is a subclass of Exception (builtins only)
except (MyError, MyError): ...  	 # Duplicate exception types are useless
except (MyError,): ...  			 # Should use bare name, not tuple
except (MyError, BaseException): ... # Even non-builtins subclass BaseException

This PR adds checks for all these cases, recommends the improved version (with as-clause if applicable), and thus closes #98.

CC @cooperlees

Copy link
Member

@ambv ambv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic.

@@ -539,6 +578,14 @@ def visit(self, node):
"to be silenced. Exceptions should be silenced in except blocks. Control "
"statements can be moved outside the finally block."
)
B013 = Error(
message="B013 A length-one tuple literal is redundant. "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually just say a "one-tuple".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, but I've left it as "length-one tuple" to minimize the terminology that new programmers need to learn (right now to use the linter).

For example I have a tox config for formatters+linters that I encourage all my students to use when they're learning Python - in my experience automated feedback on what "good code" looks like can be really helpful, and the other bugbear messages are great at explaining why as well as what.

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, nothing to fault here. Thanks!

I can understand both statements about the one element tuple, so up to you two there :)

README.rst Show resolved Hide resolved
bugbear.py Outdated Show resolved Hide resolved
@Zac-HD
Copy link
Member Author

Zac-HD commented Jan 4, 2020

Turns out that this is useful for well-linted projects too:

hypothesis\internal\validation.py:85:5: B014 Redundant exception types in except (TypeError, OverflowError, ValueError, ArithmeticError):. Write except (TypeError, ValueError, ArithmeticError):, which catches exactly the same exceptions.

@cooperlees
Copy link
Collaborator

Will merge and release tonight (US Pacific time)

@cooperlees cooperlees merged commit e35343c into PyCQA:master Jan 4, 2020
@Zac-HD Zac-HD deleted the except-checks branch January 5, 2020 00:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn on useless entries in multi-type except clauses
3 participants