Wemake-python-styleguide: Forbid `except TypeError or ValueError`

[sobolevn]

This code should be invalid:

try:
    ...
except TypeError or ValueError:
    ...

[sobolevn]

Basically, all expressions except ast.Name and ast.Attribute, and ast.Tuple (with names and attributes) should be banned in except name.

[AlwxSin]

@sobolevn can you provide valid example, cause it's not clear which one should be accepted.

try:
    ...
except (TypeError, ValueError):
    ...

try:
    ...
except TypeError:
    ...
except ValueError:
    ...

[sobolevn]

Both! I have updated the description. I forgot about ast.Tuple

[orsinium]

can you provide valid example, cause it's not clear which one should be accepted

Both your examples are valid. The issue is about having an expression inside the except. Another example of a bad code:

def f():
    return ValueError

try:
    raise ValueError
except f():
    print('triggered')

I've never seen a dynamically defined except like in the example above and I don't have in mind when it could be useful. Exceptions that should be caught should be explicitly listed.

[loukikstudent]

Hi!
I am a student looking to contribute, i am sorry but i don't understand this completely, would you mind explaining a bit more on what needs to be achieved so i can send a PR.

[sobolevn]

@loukikstudent Hi! Awesome that you are interested in contributing to our project!

We need a new violation and a new check for except ast node.
You can start with looking at similar examples: https://github.com/wemake-services/wemake-python-styleguide/blob/master/wemake_python_styleguide/visitors/ast/exceptions.py

[loukikstudent]

@sobolevn Thank you so much for to revert back. I'll definitely look into it!

[lensvol]

Hello. Can I take this over?

[sobolevn]

🤗

[lensvol]

There is a problem with the case of ast.Tuple containing anything other than class name or ast.Attribute: source code that uses such constructs in except handler actually trips assertion checks inside flake8-bugbear visitors:

Test file:

try:
    pass
except (ValueError, "oops"):
    print(123)

Error output:

❯ flake8 1.py
Traceback (most recent call last):
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/bin/flake8", line 10, in <module>
    sys.exit(main())
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/main/cli.py", line 22, in main
    app.run(argv)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/main/application.py", line 360, in run
    self._run(argv)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/main/application.py", line 348, in _run
    self.run_checks()
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/main/application.py", line 262, in run_checks
    self.file_checker_manager.run()
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/checker.py", line 325, in run
    self.run_serial()
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/checker.py", line 309, in run_serial
    checker.run_checks()
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/checker.py", line 589, in run_checks
    self.run_ast_checks()
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/flake8/checker.py", line 496, in run_ast_checks
    for (line_number, offset, text, _) in runner:
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 36, in run
    visitor.visit(self.tree)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 156, in visit
    super().visit(node)
  File "/Users/lensvol/.pyenv/versions/3.7.4/lib/python3.7/ast.py", line 262, in visit
    return visitor(node)
  File "/Users/lensvol/.pyenv/versions/3.7.4/lib/python3.7/ast.py", line 270, in generic_visit
    self.visit(item)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 156, in visit
    super().visit(node)
  File "/Users/lensvol/.pyenv/versions/3.7.4/lib/python3.7/ast.py", line 262, in visit
    return visitor(node)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 292, in visit_Try
    self.generic_visit(node)
  File "/Users/lensvol/.pyenv/versions/3.7.4/lib/python3.7/ast.py", line 270, in generic_visit
    self.visit(item)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 156, in visit
    super().visit(node)
  File "/Users/lensvol/.pyenv/versions/3.7.4/lib/python3.7/ast.py", line 262, in visit
    return visitor(node)
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 165, in visit_ExceptHandler
    names = [_to_name_str(e) for e in node.type.elts]
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 165, in <listcomp>
    names = [_to_name_str(e) for e in node.type.elts]
  File "/Users/lensvol/Library/Caches/pypoetry/virtualenvs/wemake-python-styleguide-py3.7/lib/python3.7/site-packages/bugbear.py", line 130, in _to_name_str
    assert isinstance(node, ast.Attribute)
AssertionError

So it does not make sense to implement support for them right now, as our code will never be reached. Should I proceed without them?

[sobolevn]

Can you please report this to bugbear? And let's continue without them!