Issue with noqa comment & commented out code w/ flake8-eradicate

[shestakovks]

Hello. I'm using flake8, flake8-noqa & flake8-eradicate which finds commented out code and gives "E800 | Found commented out code". Until flake8-noqa==1.2.5 it worked fine, but now I encounter weird behaviour.

Python version

$ python --version
Python 3.9.10

Dependencies:
flake8==4.0.1
flake8-eradicate==1.2.1
flake8-noqa==1.2.5

Current behaviour

code.py

# a = 5
a = 7

flake8 run

$ flake8 code.py
code.py:1:1: E800 Found commented out code

code.py

# a = 5  # noqa: E800
a = 7

flake8 run

$ flake8 code.py
code.py:1:1: NQA102 "# noqa: E800" has no matching violations

Expected behaviour

code.py

# a = 5
a = 7

flake8 run

$ flake8 code.py
code.py:1:1: E800 Found commented out code

code.py

# a = 5  # noqa: E800
a = 7

flake8 run -- no error

$ flake8 code.py

Can you please look into that?

[plinss]

The problem here is in flake8-eradicate, when it sees the noqa in the comment then it stops emitting the ‘E800’ error, so flake8-noqa really does not see any matching violations.

flake8-eradicate needs to return the error regardless of the noqa in the comment and simply let flake8’s noqa mechanism suppress the error. (Respecting the noqa is an anti-pattern that no plugin should do.)

Furthermore, looking at the underlying eradicate module that flake8-eradicate is using, it doesn’t seem to be properly parsing the noqa comment and ignores the code, e.g. I expect ‘# noqa: X101’ would still suppress the E800 error, when it shouldn’t (another reason to let flake8 handle the noqa.)

[plinss]

BTW, this only started happening recently because I fixed flake8-noqa’s comment parsing. Previous versions didn’t see the noqa when it wasn’t at the beginning of a comment, so it didn’t generate an error for the lack of a violation on the line.

[shestakovks]

Thank you for your time!

[njiles]

Hi - I just ran into this issue today. I appreciate that you have already said that this is an issue in flake8-eradicate, but the corresponding issue in that project has been open for two years without response. In your README you say

If the plugin is unmaintained, or the maintainers decline to address the issue for whatever reason, feel free to file an issue on this plugin to see if a work-around can be established.

I wondered if a workaround for E800 would be considered? Thank you!

[plinss]

Hi @njiles, I took another look at this and it's a bit weirder than I first thought. It's not that the flake8-eradicate respects the # noqa itself, so much as that the underlying eradicate module thinks any line with a noqa in it doesn't contain code, and is therefore a normal comment (and as I mentioned earlier, it doesn't care what code is used, if any).

A work-around that should work is to use flake8-eradicate's --eradicate-whitelist option to replace the default whitelist with one that doesn't include noqa. e.g. --eradicate-whitelist="pylint#pyright#nosec#type:\s*ignore#mypy:#fmt:\s*(on|off)#yapf:\s*(enable|disable)#isort:\s*(on|off|skip|skip_file|split|dont-add-imports(:\s*\[.*?\])?)#TODO#FIXME#XXX" would get you all the rest of the defaults.

Of course if you can simplify that list removing anything you dont't use, e.g. --eradicate-whitelist="pylint#pyright#nosec#type:\s*ignore#mypy:#TODO#FIXME#XXX" (and you can put that in the flake8 config rather than on the command line).

[njiles]

That seems to work perfectly, thank you for the fast response!

For any future readers: I have my configuration options stored in a .flake8 file rather than being passed via the command line; the syntax for the above suggestion is

eradicate-whitelist = "pylint#pyright#nosec#type:\s*ignore#mypy:#fmt:\s*(on|off)#yapf:\s*(enable|disable)#isort:\s*(on|off|skip|skip_file|split|dont-add-imports(:\s*\[.*?\])?)#TODO#FIXME#XXX"

(This is not documented in flake8-eradicate, see wemake-services/flake8-eradicate#281.)