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

noqa does only work as intended with single letter error codes #1101

Closed
asottile opened this issue Apr 3, 2021 · 9 comments
Closed

noqa does only work as intended with single letter error codes #1101

asottile opened this issue Apr 3, 2021 · 9 comments
Milestone

Comments

@asottile
Copy link
Member

asottile commented Apr 3, 2021

In GitLab by @MarkusPiotrowski on Jun 15, 2019, 17:04

Please describe the problem or feature

If this is a bug report, please explain with examples (and example code) what you expected to happen and what actually happened.

During the development and testing of a flake8 plugin I become aware that noqa's with my specific error codes, which consist of three letters and three digits (like 'XXX100') - as recommended in the recent flake8 manual:

Please Note: Your entry point does not need to be exactly 4 characters as of Flake8 3.0. Consider using an entry point with 3 letters followed by 3 numbers (i.e. ABC123 ).

are not handled correctly. They are just interpreted as simple # noqa without error code.

The reason may be this regular expression:

NOQA_INLINE_REGEXP = re.compile(
    # comments ...
    r"# noqa(?::[\s]?(?P<codes>([A-Z][0-9]+(?:[,\s]+)?)+))?",
    re.IGNORECASE,
)

This regex expects exactly one letter and at least one digit for the error code. So with the string # noqa: ABC123 the group codes will be None.

I guess the part representing the error code should be (at least) ...([A-Z]+[0-9]+... to allow error codes with more than one letter?

Note that also a single letter without number like # noqa: D (to turn off all pydocstyle warnings) will not work as expected with this regex.

How to test: Add # noqa: ABC123 or # noqa: D to a code line and add a trailing whitespace. The trailing whitespace (W291) will not be detected by flake8.

Please describe how you installed Flake8

> pip install flake8

Note: Some *nix distributions patch Flake8 arbitrarily to accommodate incompatible software versions. If you're on one of those distributions, your issue may be closed and you will be asked to open an issue with your distribution package maintainers instead.

Please provide the exact, unmodified output of flake8 --bug-report

D:\flake8-assertAlmostEqual>flake8 --bug-report
{
  "dependencies": [
    {
      "dependency": "entrypoints",
      "version": "0.3"
    }
  ],
  "platform": {
    "python_implementation": "CPython",
    "python_version": "2.7.14",
    "system": "Windows"
  },
  "plugins": [
    {
      "is_local": false,
      "plugin": "flake8-assertAlmostEqual",
      "version": "0.2.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-docstrings",
      "version": "1.3.0, pydocstyle: 3.0.0"
    },
    {
      "is_local": false,
      "plugin": "mccabe",
      "version": "0.6.1"
    },
    {
      "is_local": false,
      "plugin": "pycodestyle",
      "version": "2.5.0"
    },
    {
      "is_local": false,
      "plugin": "pyflakes",
      "version": "2.1.1"
    }
  ],
  "version": "3.7.7"
}
@asottile asottile added this to the 3.7.8 milestone Apr 3, 2021
@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @sigmavirus24 on Jun 15, 2019, 17:11

To be clear,

Note that also a single letter without number like # noqa: D (to turn off all pydocstyle warnings) will not work as expected with this regex.

It was never intended for folks to be able to turn off an entire class of errors.

That said, the inability to turn off something like ABC123 is definitely a bug.

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @asottile on Jun 15, 2019, 17:57

@MarkusPiotrowski should be an easy fix to add a + after [A-Z] -- want to take a stab at the PR?

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @MarkusPiotrowski on Jun 16, 2019, 04:41

@sigmavirus24, sorry I had the naive idea that error codes in noqas work similarly as in ignore or select. E.g. something like # noqa: W2 does work to silence a W291 issue. Maybe the manual can be a little bit more elobarate about the allowed formats of error codes for noqas?

@asottile I'll give it a try. I just wonder if the regex should be more specific, e.g. fix the number of letters to 1 - 3?

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @asottile on Jun 16, 2019, 07:28

as far as I can tell, there's no limit to the code name length:

$ git -C flake8-typing-imports/ diff
diff --git a/flake8_typing_imports.py b/flake8_typing_imports.py
index 7d96989..055c0b4 100644
--- a/flake8_typing_imports.py
+++ b/flake8_typing_imports.py
@@ -466,7 +466,7 @@ class Plugin:
             guard = '`if False:  # TYPE_CHECKING`'
         else:
             guard = '`if TYPE_CHECKING:`'
-        msg = f'TYP001 guard import by {guard}: {{}} (not in {{}})'
+        msg = f'TYPING001 guard import by {guard}: {{}} (not in {{}})'
 
         error_versions: Dict[Tuple[int, int, str], List[Version]]
         error_versions = collections.defaultdict(list)
diff --git a/setup.cfg b/setup.cfg
index ea978ae..0f529b7 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -25,7 +25,7 @@ python_requires = >=3.6
 
 [options.entry_points]
 flake8.extension =
-    TYP=flake8_typing_imports:Plugin
+    TYPING=flake8_typing_imports:Plugin
 
 [bdist_wheel]
 universal = True
$ cat t.py
from typing import TYPE_CHECKING
$ flake8 --format='%(code)s' t.py
F401
TYPING001

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @MarkusPiotrowski on Jun 16, 2019, 09:00

mentioned in merge request !326

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @asottile on Jun 16, 2019, 10:17

closed via merge request !326

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @MarkusPiotrowski on Jun 16, 2019, 10:17

closed via commit 0ac3376

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @asottile on Jun 16, 2019, 10:17

mentioned in commit 076dfee

@asottile
Copy link
Member Author

asottile commented Apr 3, 2021

In GitLab by @asottile on Jun 16, 2019, 10:18

Thanks for the report and fix 🎉

@asottile asottile closed this as completed Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant