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

Compatibility with flake8 --extend-select #75

Closed
JelleZijlstra opened this issue Jan 15, 2022 · 5 comments · Fixed by #101
Closed

Compatibility with flake8 --extend-select #75

JelleZijlstra opened this issue Jan 15, 2022 · 5 comments · Fixed by #101
Labels

Comments

@JelleZijlstra
Copy link
Collaborator

It doesn't work:

% python -m flake8 --select=Y091 tests/raise.pyi
tests/raise.pyi:4:9: Y091 Function body must not contain "raise"
tests/raise.pyi:7:9: Y091 Function body must not contain "raise"
% python -m flake8 --extend-select=Y091 tests/raise.pyi
% 
@JelleZijlstra
Copy link
Collaborator Author

https://flake8.pycqa.org/en/latest/plugin-development/index.html doesn't say anything about creating codes that are disabled by default, so probably there still isn't a native way to do this. In working on #86 I noticed that it's really easy to disable everything else while trying to turn on a disabled-by-default check, so I think it's going to be a better way forward to get rid of the disabled-by-default checks.

@Akuli
Copy link
Collaborator

Akuli commented Jan 16, 2022

There's a comment that says the functionality is "Adapted from flake8-bugbear". We could look at how flake8-bugbear does this, and whether it has the same problems.

@JelleZijlstra
Copy link
Collaborator Author

Their code is still basically the same: https://github.com/PyCQA/flake8-bugbear/blob/master/bugbear.py#L131.

@Akuli
Copy link
Collaborator

Akuli commented Jan 16, 2022

Here's another idea: You typically want to use the most modern syntax that a specific Python version supports. For example, if you target 3.9+ you can use explicit type aliases, but if you target 3.8+ you can't. (Edit: I'm no longer sure if this makes sense. Doesn't typing_extensions backport basically everything?)

Maybe a better approach would be configuring the python version to target, instead of enabling individual disabled-by-default error codes? Or if that isn't possible, enable everything by default (as if you targeted the latest Python version), and ask users to silence warnings if they support something older too?

JelleZijlstra added a commit that referenced this issue Jan 16, 2022
Disabled-by-default error codes are problematic; see #75.

Y092 finds a fairly harmless error and has significant false positives (python/typeshed#6930).
I'm removing it so we can get rid of disabled-by-default errors and simplify our
setup.
@JelleZijlstra
Copy link
Collaborator Author

Maybe a better approach would be configuring the python version to target

That doesn't make sense for stubs because stubs are mostly independent of Python version. TypeAlias for example should work in all Python versions in stubs (assuming you import it from typing_extensions); the problem is that pytype doesn't support it yet.

srittau pushed a commit that referenced this issue Jan 17, 2022
Disabled-by-default error codes are problematic; see #75.

Y092 finds a fairly harmless error and has significant false positives (python/typeshed#6930).
I'm removing it so we can get rid of disabled-by-default errors and simplify our
setup.
JelleZijlstra added a commit that referenced this issue Jan 19, 2022
And delete the machinery for disabling errors.

This made this check run on all tests, which helped me find a bug
(it triggered on __all__). I can split the test changes into a separate
PR if preferred.

I'll submit a separate PR to typeshed to disable this check for now.

Fixes #75. Fixes #86.
AlexWaygood pushed a commit that referenced this issue Jan 19, 2022
Enable TypeAlias check by default, and delete the machinery for disabling errors.

Fixes #75. Fixes #86.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants