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

don't autofix noqa'd errors, add --disable-noqa, add non-passing test for noqa reliability #190

Merged
merged 2 commits into from
May 5, 2023

Conversation

jakkdl
Copy link
Member

@jakkdl jakkdl commented May 2, 2023

big refactor for noqa functionality fixing most of the remaining ones in #185, but as I added the test for all errors being properly noqa'd and was greeted with a wall of errors I realized it might be good to commit and push before tackling them. If it's a minor fix I'll just push/amend it to this PR, otherwise open a separate one.

  • Moves # noqa logic into Flake8TrioVisitor_cst, other than for ast visitors where some logic still is in __init__.py
    • utility visitor NoqaHandler removed
  • add --disable-noqa, and use that instead of whether we're standalone to decide whether to parse noqa or not.
  • the list of noqas is no longer parsed once by a utility visitor, but instead parsed by each visitor. This is a performance hit (though probably not very large, if any) but is sorta necessary as autofixing messes up line numbers. autofixing does not mess up line numbers in fact.
  • should_autofix now takes a CSTNode parameter so it can determine the line number of it, and check if it's noqa'd
  • visitor91x now doesn't autofix simple noqa scenarios, but doesn't handle loops and the like visitor91x handles all cases
  • tests and shit

@jakkdl
Copy link
Member Author

jakkdl commented May 3, 2023

turns out fixing the errors made the PR simpler, as I had mistaken assumptions about how lineno-parsing was handled in CSTTransformers.

@jakkdl
Copy link
Member Author

jakkdl commented May 3, 2023

uh, is it an update to trio or pyright that has it throwing new errors? It now seems to be analyzing trio properly for some reason and seeing missing objects and whether functions are coroutines.

@Zac-HD
Copy link
Member

Zac-HD commented May 4, 2023

I don't know why pyright has changed, but short-term I'd just ignore tests/ and longer term #187.

@jakkdl
Copy link
Member Author

jakkdl commented May 5, 2023

I don't know why pyright has changed, but short-term I'd just ignore tests/ and longer term #187.

added commit which excludes tests/eval_files and tests/autofix_files

@jakkdl
Copy link
Member Author

jakkdl commented May 5, 2023

I don't know why pyright has changed, but short-term I'd just ignore tests/ and longer term #187.

also related to #160

@Zac-HD Zac-HD merged commit 5009792 into python-trio:main May 5, 2023
6 checks passed
@jakkdl jakkdl deleted the noqa_autofix branch May 6, 2023 12:22
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.

None yet

2 participants