flake8-bugbear icon indicating copy to clipboard operation
flake8-bugbear copied to clipboard

Assertion check fails on syntactically-valid Python code

Open lensvol opened this issue 5 years ago • 4 comments

Originally reported here: https://github.com/wemake-services/wemake-python-styleguide/issues/1599#issuecomment-703526278

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

lensvol avatar Oct 05 '20 14:10 lensvol

Sorry for my ignorance, but is this valid Python? If I make it raise ValueError, it complains about trying to catch an exception that does not inherit from BaseException, thus a TypeError:

Code:

#!/usr/bin/env python3

try:
    raise ValueError("Bla")
except (ValueError, "oops"):
    print(123)

Error:

cooper-mbp1:~ cooper$ python3 -V
Python 3.7.5
cooper-mbp1:~ cooper$ python3 /tmp/test.py
Traceback (most recent call last):
  File "/tmp/test.py", line 4, in <module>
    raise ValueError("Bla")
ValueError: Bla

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/tmp/test.py", line 5, in <module>
    except (ValueError, "oops"):
TypeError: catching classes that do not inherit from BaseException is not allowed

Can I please see an example that shows how this is valid and actually used please. Otherwise, I don't feel we need to fix this.

cooperlees avatar Oct 05 '20 15:10 cooperlees

Sorry, I should have been more clear in my report. What I meant is that this snippet is syntactically valid and can be parsed by Python, as shown by your own example.

While this code is semantically incorrect, a user may benefit from having it reported in a more human-friendly manner.

There is also an issue with failed assertion essentially terminating execution of flake8 process, thus preventing other plugins from doing useful work.

lensvol avatar Oct 05 '20 15:10 lensvol

Would you mind if I tried to solve that and did a pull request?

lensvol avatar Oct 08 '20 06:10 lensvol

If there is a clean way to know you're in an except and you find a str, to just report an error, sure.

cooperlees avatar Oct 08 '20 14:10 cooperlees