python-trio / flake8-async

Highly opinionated linter for Trio code
https://flake8-async.readthedocs.io
MIT License
17 stars 2 forks source link

Support `# noqa` comments when calling directly #181

Closed Zac-HD closed 1 year ago

Zac-HD commented 1 year ago

When I tried calling flake8_trio directly from the commandline, it turns out that doesn't respect # noqa comments - which would unfortunately block direct use for now. If supporting them is viable, let's do that and add a --disable-noqa flag for the current behavior.

I'd also prefer to name the cli tool flake8-trio; underscores are pretty unusual in the terminal.

Once this is in, I think we're ready to release the big refactoring!

jakkdl commented 1 year ago

I'm making good progress on implementing this, but I just realized this will clash with flake-noqa. If you transition to running flake8-trio standalone, then you'll start to get NQA102 (no matching error) for all TRIOxxx codes.

I see two possible solutions:

  1. Use a different magic word than noqa (e.g. noqa-trio)
    • This means you need to replace all instances of # noqa: TRIOxxx in the codebase, and possibly replace some bare # noqa as well.
    • If this magic word is only parsed when run standalone, then you'd need to replace back all instances when running as non-standalone. So noqa-triod errors should be suppressed when running as a plugin as well, meaning you have a somewhat confusing state where you can suppress errors in two different ways.
    • I can build in the equivalent flake8-noqa NQA101,102&103 support into flake8-trio. Relatively simple (I've pretty much already done it just while debugging), but will require three extra error codes. Could duplicate even more options as well, but that starts feeling silly.
    • you'd maybe also want an error for finding # noqa: TRIOxxx when running standalone, or also parse them when running standalone unless you give a command-line flag.
  2. Contribute a config option to flake8-noqa where you can tell it to ignore certain codes in NQA102.
  3. Say that we're incompatible with NQA102
jakkdl commented 1 year ago

Actually yeah, after rubberducking with the comment-box - this seems like the best solution:

  1. parse both noqa and noqa-trio comments for suppressing errors when running standalone
  2. when run as plugin: ignore noqa markers (letting flake8 handle them so flake8-noqa works), but suppress errors marked noqa-trio.

Command-line flag to warn when finding # noqa: TRIOxxx is kind of already implemented by NQA102, but raising it when run as standalone will inform the user about the existence of noqa-trio. Although making it default-enabled means that a majority of people will disable it immediately (i.e. everybody not running flake8-nqa with NQA102), but making it default-disabled makes it kinda pointless. So maybe just make it a prominent header in the readme.

Zac-HD commented 1 year ago

For simplicity, let's just use # noqa and say that if you use flake8-noqa but without including flake8-trio in that run it's your problem to work out what's going on. Basically option (3), since we are still compatible if you're using everything as a flake8 plugin 🙂