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

Disable F821 and F822 by default #177

Closed
wants to merge 12 commits into from

Conversation

AlexWaygood
Copy link
Collaborator

@AlexWaygood AlexWaygood commented Feb 6, 2022

This PR proposes disabling the troublesome F821 error code ("undefined name name") by default. It also fixes F822 ("undefined name name in __all__") so that variables that are annotated, but not assigned to, are recognised by flake8-pyi as being defined in the file. Implementing these changes will allow us to remove the remaining = ...s at the module level in typeshed.

I initially tried to implement this using the extend_default_ignore option that flake8 provides. However, this was not a good solution. If a user specifies --per-file-ignore options in their .flake8 config file, they will need to manually add F821 or F822 to each subdirectory, as specifying these options in a config file overrides the list of ignored-by-default error codes. Moreover... we're already monkeypatching pyflakes in this plugin, so a little more monkeypatching probably won't hurt too much. Lastly, this solution has the advantage that it only affects how .pyi files are parsed by flake8.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

This makes me worried that we'll suppress these errors in .py files too. I think that won't happen because we monkeypatch only for pyi files, but would it be possible to add a .py test case to make sure we don't break linting on .py files?

@AlexWaygood
Copy link
Collaborator Author

Okay, anybody know how to get the CI flake8 check to exclude the new .py file that deliberately has flake8 errors? 🙃

@AlexWaygood
Copy link
Collaborator Author

AlexWaygood commented Feb 12, 2022

I'm closing this since, as discussed in #183, we should be reducing the amount of monkeypatching we do, not adding to it.

This PR doesn't really add to the problem massively, but it's also not a long-term solution, so I don't think it's worth it for now.

@AlexWaygood AlexWaygood deleted the defaults branch February 12, 2022 10:21
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.

2 participants