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

Error codes too long for flake8>=6.0.0 #230

Closed
rtb-zla-karma opened this issue Apr 8, 2024 · 6 comments · Fixed by #234
Closed

Error codes too long for flake8>=6.0.0 #230

rtb-zla-karma opened this issue Apr 8, 2024 · 6 comments · Fixed by #234

Comments

@rtb-zla-karma
Copy link

rtb-zla-karma commented Apr 8, 2024

flake8 stopped accepting error codes which don't match pattern ^[A-Z]{1,3}[0-9]{0,3}$ in .flake8 config file under ignore= option. Change introduced in this commit PyCQA/flake8@1346dde .

It does work however for --ignore command line option (but that basically ignores my .flake8 config file):
PyCQA/flake8#1802 (comment)

flake8 --ignore=ABCD123 appears to work perfectly

It also works for inline # noqa ignores.

Your codes use 5 characters instead of 3. I wanted to

ignore =
    #  I have other errors in this file too
    ASYNC900, ASYNC910, ASYNC911

But it returns

Traceback (most recent call last):
  File "/venv/path/bin/flake8", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/venv/path/lib/python3.12/site-packages/flake8/main/cli.py", line 23, in main
    app.run(argv)
  File "/venv/path/lib/python3.12/site-packages/flake8/main/application.py", line 198, in run
    self._run(argv)
  File "/venv/path/lib/python3.12/site-packages/flake8/main/application.py", line 186, in _run
    self.initialize(argv)
  File "/venv/path/lib/python3.12/site-packages/flake8/main/application.py", line 165, in initialize
    self.plugins, self.options = parse_args(argv)
                                 ^^^^^^^^^^^^^^^^
  File "/venv/path/lib/python3.12/site-packages/flake8/options/parse_args.py", line 53, in parse_args
    opts = aggregator.aggregate_options(option_manager, cfg, cfg_dir, rest)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/path/lib/python3.12/site-packages/flake8/options/aggregator.py", line 30, in aggregate_options
    parsed_config = config.parse_config(manager, cfg, cfg_dir)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/path/lib/python3.12/site-packages/flake8/options/config.py", line 131, in parse_config
    raise ValueError(
ValueError: Error code 'ASYNC900' supplied to 'ignore' option does not match '^[A-Z]{1,3}[0-9]{0,3}$'

Env info

Ubuntu 20.04.2 LTS
Python 3.12.2
flake8==7.0.0
flake8-async==24.3.6

In flake8 repo, some Anthony guy who commited the change, behaves like jerk saying basically "it works like it supposed to and stop talking about it" without any consideration for maybe adding an option to configure this pattern so that plugin like yours can work:

I don't know if:

  1. I'm dumb enough to not be able to find some config option to ignore or change this pattern;
  2. I have to use noqa in bunch of places;
  3. you have to change your error codes;
  4. flake8 must stop enforcing it so rigidly.

Help.

@jakkdl
Copy link
Member

jakkdl commented Apr 9, 2024

oh rip. Yeah long error codes has been "officially disallowed" for quite a while, see

flake8-async/setup.py

Lines 53 to 59 in d29a33d

entry_points={
# You're not allowed to register error codes longer than 3 characters. But flake8
# doesn't enforce anything about the characters trailing the code, so we can say
# the code is ASY and then just always happen to print NCxxx directly after it.
"flake8.extension": ["ASY = flake8_async:Plugin"],
"console_scripts": ["flake8-async=flake8_async:main"],
},

but it's never actually impacted anything before. Given past interactions with them I'm not surprised they don't really care about the impact on plugins, and almost surely won't change back even if it breaks tons of them.

You can work around it currently in a few ways:

  1. By using --disable instead of --ignore, since that's a flag that we control. When running through flake8 it will also parse it from the config.
  2. running it as a standalone, but that currently doesn't support reading from config files.
  3. Run it through pre-commit, in which case you can configure it with parameters in the .pre-commit-config.yaml file.
  4. Use the ruff implementation, though they only have some checks implemented and the stable version splits flake8-async and flake8-trio. See Re-code flake8-trio and flake8-async rules to match upstream astral-sh/ruff#10303

We could try to work around it by also accepting ASYxxx codes in --disable=, and do some post-processing to copy over disables to our actual internal codes. (Surely that won't break in the future...) Though at that point it's maybe just easier to ask people to use disable.

Actually renaming all error codes... sounds like a huge pain, would break lots of stuff downstream, create a discrepancy with the ruff implementation, and be very ugly. So maybe we just put a big red warning in the documentation, and consider moving further away from using flake8 as a backend at all. See #124

In flake8 repo, some Anthony guy who commited the change

Anthony Sottile is the primary maintainer of flake8 and several other tools. But yeah he can often be quite rude when dealing with other people.

@jakkdl
Copy link
Member

jakkdl commented Apr 9, 2024

It does work however for --ignore command line option (but that basically ignores my .flake8 config file)

Oh and another workaround here would be to use --extend-ignore on the command line, which wouldn't override ignore = ... from the config file.

@rtb-zla-karma
Copy link
Author

@jakkdl Thank you for suggestions and explaining some nuances in projects!

Actually renaming all error codes... sounds like a huge pain

It is and I understand. https://github.com/guilatrova/tryceratops devs have changed their error codes (from TCnnn to TRYnnn) quite recently and they are also a part of ruff. I think their experience can make it easier for you.

@jakkdl
Copy link
Member

jakkdl commented Apr 9, 2024

I don't think the tryceratops rename is particularly relevant. They renamed because of a conflict with flake8-type-checking and as such didn't really have much of a choice in the matter. I don't want to rename at all, and even if we did change to ASYxxx or w/e it might make more sense to have ruff stick to using ASYNCxxx since it's so much more readable.
We did very recently rename all error codes from TRIOxxx to ASYNCxxx, so it's not entirely unfamiliar territory.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 10, 2024

Yeah, I think we should probably just document the issue and move on with our lives.

As it happens I do also have PyQCA commit bits, but I really don't have the time and energy to work out what's happened and build a consensus around not-breaking codes which aren't exactly three letters 😥

@jakkdl
Copy link
Member

jakkdl commented Apr 10, 2024

Wow the solution in flake8-pylint is... simultaneously hilarious and horrifying: orsinium-labs/flake8-pylint#5

The PR changes the error codes for all violations by removing the first digit from all codes. So, PLC0114 is replaced with PLC114. It causes overlaps for some error codes, but not many. I wish I had a better solution.

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 a pull request may close this issue.

3 participants