flake8 icon indicating copy to clipboard operation
flake8 copied to clipboard

file-level noqa should raise an error if specific errors are provided (to avoid confusion)

Open bagerard opened this issue 2 years ago • 9 comments

how did you install flake8?

pip install flake8

unmodified output of flake8 --bug-report

{
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.10.7",
    "system": "Linux"
  },
  "plugins": [
    {
      "plugin": "mccabe",
      "version": "0.7.0"
    },
    {
      "plugin": "pycodestyle",
      "version": "2.10.0"
    },
    {
      "plugin": "pyflakes",
      "version": "3.0.1"
    }
  ],
  "version": "6.0.0"
}

describe the problem

what I expected to happen

I'm seeing developers using the file-level-noqa ignore wrongly from time to time, providing specific errors like # flake8: noqa: E712. What's confusing is that it gives the impression that it works as it doesn't throw that error anymore but the developers' understanding is incorrect (all errors are ignore). I understood that you don't want to support the file-level-specific-ignores but I think flake8 should do something to prevent this confusion.

See for instance https://til.codeinthehole.com/posts/filelevel-flake8-comments-ignore-all-errors/

My suggestion would be to detect that and throw an error? Somehow it is like providing a flake8 config with a wrong format

sample code

# flake8: noqa: E712

from os import *       # F401
assert True == True # E712

Let me know what you think, happy to work on this if you would welcome a PR

bagerard avatar Jan 29 '23 20:01 bagerard

given how much people were mad at 6.0 pointing out a common mistake I'm hesitant. I don't need another 2 weeks of needing to lock down the issue tracker while people scream at me in my email

asottile avatar Jan 29 '23 20:01 asottile

I understand the hesitation then :grimacing:. Thanks for the quick feedback and for your efforts in Python's ecosystem :rocket:

bagerard avatar Jan 29 '23 20:01 bagerard

the "simplest" fix for this would be to slap a $ on the file regex

asottile avatar Jan 29 '23 20:01 asottile

I almost feel like there needs to be a "mode" for linting people's use of flake8. Like flake8 --warn-on-misuse that detects stuff like this so people could use that to prevent nonsense like this from creeping in. It would be forever a "beta" feature that adds new things to it. Could be used for deprecation warnings of upcoming breaking releases too. Sadly it'll only draw more ire because we don't just accept every feature request to make the tool less useful to large teams.

sigmavirus24 avatar Jan 30 '23 14:01 sigmavirus24

@sigmavirus24 you can suggest it for https://github.com/PyCQA/flake8-bugbear

jakkdl avatar Apr 30 '23 19:04 jakkdl

My suggestion would be to detect that and throw an error

I'd argue the opposite: actually support the thing that users are trying to do.

This functionality is implemented already (as a CLI/config file option); it shouldn't cost that much effort to detect if the # flake8: noqa comment also specifies a list of errors (in exactly the same way as per-line # noqa comments do), and extend per-file-ignores list instead of the exclude one.

the "simplest" fix for this would be to slap a $ on the file regex

Well that sounds an awful lot like "quietly ignoring the directive" which is hardly a good behaviour for any kind of file processor… (Not to mention, the current behaviour of # noqa allowing text after the error list is actually useful – it allows to provide justification for using the directive within the same comment.)

LeXofLeviafan avatar Sep 30 '23 09:09 LeXofLeviafan

ChatGPT even suggests to use # flake8: noqa: E501 for file level error code silencing :)

But anyway what is the issue to actually allow this feature so that one can easily manage error suppressing per file in the file where one writes the code. This is arguably good feature?

At the moment I use following in flake config file: per-file-ignores = my_file_folder/my_file.py: E501

But not optimal imho.

And yes I agree to @LeXofLeviafan instead of throwing error let's just allow the feature

Kolumbs avatar Feb 12 '24 07:02 Kolumbs

chatgpt is hot garbage

asottile avatar Feb 12 '24 13:02 asottile

As a reminder to people saying "we should just do it" you are given software AS IS per the license. Unless you're maintaining the software, "just do it" isn't a justification. Please read https://blog.ian.stapletoncordas.co/2023/11/no-should-be-your-default before spewing that mentality around other projects

sigmavirus24 avatar Feb 12 '24 13:02 sigmavirus24