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

Have spellchecker ignore black/flake8/bandit directives #4320

Open
eli88fine opened this issue Apr 8, 2021 · 8 comments
Open

Have spellchecker ignore black/flake8/bandit directives #4320

eli88fine opened this issue Apr 8, 2021 · 8 comments
Labels
Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@eli88fine
Copy link
Contributor

eli88fine commented Apr 8, 2021

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 Pierre-Sassoulas added the Enhancement ✨ Improvement to a component label Apr 8, 2021
@Pierre-Sassoulas
Copy link
Member

I think it makes sense.

@eli88fine
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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

@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jul 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

2 participants