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

B001 misses some bare excepts #97

Closed
minusworld opened this issue Nov 19, 2019 · 16 comments
Closed

B001 misses some bare excepts #97

minusworld opened this issue Nov 19, 2019 · 16 comments

Comments

@minusworld
Copy link
Contributor

minusworld commented Nov 19, 2019

B001 and Flake8/pycodestyle E722 both check for bare_except, but B001 misses a lot of cases that E722 finds.

We noticed this when we tried running an internal tool that looks for overlaps in checks -- we are writing some of our own and don't want to overlap -- and found that every time B001 fires E722 also fires; but the reverse is not true.

Tool output:
B001 (6786) <-> E722 (34093): 13572 occurred at same line+path B001 implies E722: 100% E722 implies B001: 19%

I took a look at the implementations for B001 and E722 and found that bugbear uses the AST and E722 uses a regex:

Bugbear implementation uses AST.

def visit_ExceptHandler(self, node):
    if node.type is None:
        self.errors.append(B001(node.lineno, node.col_offset))
    self.generic_visit(node)

Flake8 implementation uses regex.

def bare_except(logical_line, noqa):
    if noqa:
        return

    regex = re.compile(r"except\s*:")
    match = regex.match(logical_line)
    if match:
        yield match.start(), "E722 do not use bare 'except'"

From the implementation, it looks like B001 and E722 should hit the same locations every time.

We have a platform that lets us run static analysis over a bunch of open source repositories at the same time, so I ran vanilla flake8 and bugbear at the same time to see if there was a pattern, but one wasn't immediately obvious.

I thought this might be related to forgetting to call visit or something like that (I've been bitten by that before!) but the reason for this disparity wasn't clear to me... so I'm making this issue. Feel free to reach out to me if you have any other questions!

Here are some examples:

image

image

image

@minusworld
Copy link
Contributor Author

Version

> flake8 --version
3.7.9 (flake8-bugbear: 19.8.0, mccabe: 0.6.1, pycodestyle: 2.5.0, pyflakes: 2.1.1) CPython 3.7.5 on Darwin

@ambv
Copy link
Member

ambv commented Nov 19, 2019

This is a very interesting find! Looks like we might not be visiting certain paths through the tree. If you could create a minimal example with this, that'd be helpful to test against and fix.

@joaoe
Copy link
Contributor

joaoe commented Jan 18, 2020

The problem is your code is in Python 2 syntax. For instance, you have print statements without parenthesis, old style except clauses, and tuple expansion in function arguments.
The file in the screenshot is
https://raw.githubusercontent.com/caktux/pytrader/b45b216dab3db78d6028d85e9a6f80419c22cea0/balancer.py
Bugbear is developed for Python 3, which uses the Python 3 ast module, which cannot parse Python 2. Flake8 skips bugbear entirely because it is registered as an ast_plugins plugin.
So, this is a won't fix.

@DylanYoung
Copy link

DylanYoung commented Jul 3, 2020

Then why does it have a whole section of Python 3 warnings? That's confusing. Can you elaborate on which parts don't work with Python 2? And shouldn't it just use whichever AST module is available in the running python interpreter? Or does each flake8 plugin run in its own interpreter?

@joaoe
Copy link
Contributor

joaoe commented Jul 3, 2020

Then why does it have a whole section of Python 3 warnings?

For when the syntax works in Python 3 but you have code using stuff that is from python 2, e.g., migrating or old habits.

@DylanYoung
Copy link

DylanYoung commented Jul 3, 2020

Many of the things those checks validate don't work in Python 3, so that doesn't really explain anything. In fact, none of them work in Python 3 except B303, right?

If those spurious checks can't be removed, it'd be nice if the README loudly proclaimed that it doesn't work in Python2 (I guess we just call it PyPy2 now since the EOL of CPython2, lol).

@cooperlees
Copy link
Collaborator

Maybe it's time to just drop our "limited" Python 2 support all together. It's past midpoint 2020.

@ambv
Copy link
Member

ambv commented Jul 6, 2020

+1

1 similar comment
@DylanYoung
Copy link

+1

@joaoe
Copy link
Contributor

joaoe commented Jul 7, 2020

Many of the things those checks validate don't work in Python 3, so that doesn't really explain anything. In fact, none of them work in Python 3 except B303, right?

All of them do not work in Python 3. That's the point, to warn about Python 2 code that does not work in Python 3, despite having the correct syntax.

@DylanYoung
Copy link

DylanYoung commented Jul 7, 2020

@joaoe And how long is any project in that state? It's an intermediate state that lasts maybe a day for most projects (if that) and there are existing tools that handle it better. That's precisely my point. I agree with @cooperlees.

@joaoe
Copy link
Contributor

joaoe commented Jul 7, 2020

And how long is any project in that state?

As long as they don't use a tool like bugbear to fix the issues :) And as long as developers don't make mistakes.

@Zac-HD
Copy link
Member

Zac-HD commented Aug 17, 2021

@cooperlees - time to close the issue, since Python 2 support has been dropped?

@cooperlees
Copy link
Collaborator

Nothing has been done to complete this. I think we need to remove these checks and all B30X too right?
#177

I guess we should make a plan and get this shipped.

@Zac-HD
Copy link
Member

Zac-HD commented Nov 30, 2022

Closed by #182 I think 🙂

@Zac-HD Zac-HD closed this as completed Nov 30, 2022
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

No branches or pull requests

6 participants