Ignore results annotated with '# noqa'

[RJ722]

Hey,

Description

This PR isn't production-ready by any means, but I just wanted to have an idea about the complexity it would add to Vulture to support # noqa comments. I'd be happy to add tests and documentation, once we all settle if it would be a good idea to carry this forward.

Another change with the introduction of # noqa comments would be the introduction of issue codes (it's possible without these as well, but would hurt specificity). For now, I've just added V001, V002, ... - the nomenclature for these issue codes is another thing to think about.

Related Issue

#160, #182, #190

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation in the README file.
• I have updated the documentation accordingly.
• I have added an entry in CHANGELOG.md.
• I have added tests to cover my changes.
• All new and existing tests passed.

[coveralls]

Pull Request Test Coverage Report for Build 727

• 23 of 23 (100.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.04%) to 98.962%

---

Totals
Change from base Build 726:	0.04%
Covered Lines:	572
Relevant Lines:	578

---

💛 - Coveralls

[jendrikseipp]

Nice work! I think the feature doesn't add too much complexity after all. Using issue codes is a good idea, I think. We should probably print them in the normal output as well.

[RJ722]

@jendrikseipp should ISSUE_CODE_MAP be a class variable for Vulture?

I'm asking because my guess is that most folks would only import Vulture, if using the API (from vulture import Vulture).

[jendrikseipp]

Almost there :-) Obviously, this also needs tests, preferably in a new file. When writing new tests, check if pytest.mark.parametrize could be useful. Maybe there's already an example for that in the codebase.

[RJ722]

Another thing I was pondering about. We don't start ignoring noqa until get_unused_code is called. What if the user bypasses it, e.g.:

from vulture import Vulture

v = Vulture()
v.scan('import this  # noqa: V004')
print(v.unused_imports)

Should we move the has_noqa logic to _define method?

[jendrikseipp]

Should we move the has_noqa logic to _define method?

Good idea! This aligns better with the existing code for ignoring things.

[jendrikseipp]

I think it would be good to separate testing the regex and testing ignoring unused code. For the former, I think we can use the test cases from the flake8 comment as test cases (plus some examples we don't want to recognize).

[RJ722]

TODO:

• One test is failing because Python3.8 onwards, the first line for property node is the same line at which the function starts.
• Write documentation for these changes -- @jendrikseipp can you take this on, please? 😅

[jendrikseipp]

Could you also add a test that verifies that we fixed the "or vs. union" bug?

[RJ722]

Could you also add a test that verifies that we fixed the "or vs. union" bug?

The way I've added the tests is basically ensuring that an example of the same issue code is covered in both categories - all and V00X (on different lines - both before and after). If there'd have been a problem with "or v/s union", we'd have seen it by now.

[RJ722]

But, now that I come think of it, I haven't added isolated cases - just V00X, or # noqa alone. Adding them.

[jendrikseipp]

Nice work! Here are some more nitpicks :-)

[jendrikseipp]

Thanks for the additional changes! I edited README.md slightly. Could you please resolve the merge conflicts? Then I can merge the PR.

[RJ722]

@jendrikseipp, thanks for your timely reviews, and help on this! :-)