pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.3k stars 1.14k forks source link

Have spellchecker ignore black/flake8/bandit directives #4320

Open eli88fine opened 3 years ago

eli88fine commented 3 years ago

Is your feature request related to a problem? Please describe

The spellchecker raises errors for things like # fmt: on (a black directive), # noqa (flake8 directive), and # nosec (bandit directive).

Describe the solution you'd like

These should be ignored. A potential implementation would be adding a filter like already exists for Sphinx directives.

Adding them to the "words to ignore" dictionary is suboptimal. A filter could be much more restrictive and require that it appear right next to the # comment symbol and not allow them to pass as false negatives of actual misspelled words in other portions of the comment.

Would you be open to a PR to address this?

Pierre-Sassoulas commented 3 years ago

I think it makes sense.

eli88fine commented 3 years ago

great! those are the tools I use that I've noticed causing problems. Are there any other directives you think would be useful for me to add while I'm adding these?

eli88fine commented 3 years ago

Also...I personally never use noqa or nosec without suppling a specific error code (i.e. "no blanket noqa statements"). Would you be ok with having the spellchecker only ignore these if they have a specified code to ignore (i.e. #noqa: E231). Or would you consider that too opinionated for pylint?

Pierre-Sassoulas commented 3 years ago

Pycharm use # noqa without specifier so I think this is fine to ignore. I'm also thinking of mypy (# mypy: ignore-errors) and isort (isort:skip).

eli88fine commented 3 years ago

sounds good. I'm familiar with mypy, but I'm a zimports user instead of isort.

could you specify the exact syntax you want added for isort? is that 'isort:skip' what appears directly after the # symbol, or is there something else (it wasn't clear from your note)?

Pierre-Sassoulas commented 3 years ago

Sorry. It's # isort:skip verbatim and isort:skip_file in a docstring, if I read the doc properly (Source: https://pycqa.github.io/isort/ search for "Skip processing of imports")

eli88fine commented 3 years ago

I'm struggling to figure out how to test a docstring for a module (for the isort module-level directive). It appears all the existing test cases are only testing docstrings in functions or classes (so no easy example to base the new code off of). If I do the following, I get an astroid error trying to run extract_node (ValueError: Empty tree, cannot extract from it). I'm no astroid expert...any suggestions? Is it OK to just leave this functionality out of the PR (I already included ignoring the isort directive in a comment)?

    def test_docstring_lines_containing_isort_directive(self):
        stmt = astroid.extract_node(
            '''""" my_module.py
                   isort by itself not as a directive still counts as a misspelling

                   isort:skip_file
    """'''
Pierre-Sassoulas commented 3 years ago

Yeah it's ok to leave it out and maybe add it later. I never used this isort feature myself.