wemake-services / flake8-eradicate

Flake8 plugin to find commented out or dead code
https://pypi.org/project/flake8-eradicate/
MIT License
310 stars 12 forks source link

`noqa` comments are detect as code #103

Closed tbrlpld closed 4 years ago

tbrlpld commented 4 years ago

This line raises an E800 warning.


# noqa: A100

These do not:


# noqa: 100
# noqa: something
# noqa: "Something"

If anything, the second block might be code (like a line of a multi-line dictionary definition). The first block is a valid noqa-comment and should not raise that warning.

sobolevn commented 4 years ago

@tbrlpld sorry, but this is not something we can work on our side. That's eradicate's issue. Can you please reopen it in the upstream repo?

tbrlpld commented 4 years ago

@sobolevn Just realized I was using it wrong anyways. The # noqa: should be an inline comment anyways. When it is used that way, it is not recognized as code.

I cam across this because of the DAR docstring violations when I want to ignore these violations on a multi-line docstring. This seems to a flake8 related though. I am not sure how can you define to ignore a rule on a multi-line docstring.

tbrlpld commented 4 years ago

Ok, so darglint ignores errors when the # noqa: is just embedded in the docstring like so ( https://github.com/terrencepreilly/darglint#ignoring-errors-in-a-docstring):

def myfunc():
    """
    Return true, always.

    Here is some more stuff.

    # noqa: DAR201
    """
    return True

This suppresses the specified error DAR201 as desired.

But, E800 is raised by this Flake8 plugin.

eradicate itself does not do anything here (not even with the aggressive -a setting enabled). I assume it just ignores everything in a docstring.

The same is true for multi-line strings. E800 is raised, but eradicate does not do anything here.

mystring = """Return true, always.

Here is some more stuff.

# noqa: DAR201
"""

@sobolevn do you still consider this an issue for eradicate or of flake8-eradicate?

sobolevn commented 4 years ago

Ok, this is our issue indeed. Why? Because we use physical_line https://github.com/sobolevn/flake8-eradicate/blob/master/flake8_eradicate.py#L35 instead of logical_line

Sorry! 🙂

Will you please fix it?

tbrlpld commented 4 years ago

I can try and give it a shot 😬 I will report back if I run into any issues.

On 10. Dec 2019, at 01:48, Nikita Sobolev notifications@github.com wrote:

Ok, this is our issue indeed. Why? Because we use physical_line https://github.com/sobolevn/flake8-eradicate/blob/master/flake8_eradicate.py#L35 https://github.com/sobolevn/flake8-eradicate/blob/master/flake8_eradicate.py#L35 instead of logical_line

Sorry! 🙂

Will you please fix it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sobolevn/flake8-eradicate/issues/103?email_source=notifications&email_token=AF5GCNJVBGSXRKFNH6KUSRTQX5QYDA5CNFSM4JXTAVMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGOT67I#issuecomment-563953533, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF5GCNPYQHH43HFE44QX2LDQX5QYDANCNFSM4JXTAVMA.

tbrlpld commented 4 years ago

After finally getting pyenv working with poetry I have an issue trying to run mypy on the master with not changes.

The contribution guidelines say to run mypy wemake_python_styleguide. This does not work, because there is no such file or directory.

$ mypy wemake_python_styleguide
mypy: can't read file 'wemake_python_styleguide': No such file or directory

I assume it should be

$ mypy flake8_eradicate.py
Success: no issues found in 1 source file

Is that correct? Should I open another issue to tackle that?

tbrlpld commented 4 years ago

@sobolevn logical_line does not work. It breaks the whole program, because logical lines do not consider commented lines.

sobolevn commented 4 years ago

Hm, any ideas how this can be solved?

tbrlpld commented 4 years ago

Not yet. I need to look more into how the interaction with flake8 works.

I would need some sort of information about the context of the physical line, e.g to determine if it is inside a docstring.

Or a way to tell the FileProcessor not to dump the comment lines.

Haven’t found any options on that yet. Also, there is not much documentation on what a logical line is or how that could be configured.

Anything you can point out to me would be helpful.

On Dec 12, 2019, at 21:06, Nikita Sobolev notifications@github.com wrote:

 Hm, any ideas how this can be solved?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

sobolevn commented 4 years ago

Source code is always helpful for me: https://github.com/PyCQA/flake8/blob/6cc0abbea2e2f44977c7ac5e75072e0ca20c0831/src/flake8/checker.py#L514 It is quite easy to follow and can be modified locally to test things.

tbrlpld commented 4 years ago

@sobolevn Thanks for pointing that out. That definitely helped.

I found a solution by utilizing the tokens that can be requested as input from flake8. See my pull request #106

Hope this is fine by you.