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.31k stars 1.14k forks source link

Warn when code makes use of field named with "unused_" or "ignored_" prefix. #3418

Open nathanielmanistaatgoogle opened 4 years ago

nathanielmanistaatgoogle commented 4 years ago

ENVIRONMENT: pylint 2.4.4 astroid 2.3.3 Python 3.5.3 (default, Sep 27 2018, 17:25:39) [GCC 6.3.0 20170516]

STEPS TO REPRODUCE: 1) Lint

"""Lol."""

def lol():
    """Lollollol!"""
    unused_integer = 3
    return 5

. Observe that it lints cleanly. 2) Add print(unused_integer) to the code, making it

"""Lol."""

def lol():
    """Lollollol!"""
    unused_integer = 3
    print(unused_integer)
    return 5

. 3) Lint the code again.

EXPECTED RESULTS: Pylint will object that a field with a name that starts with unused_ is in fact used.

OBSERVED RESULTS: Pylint makes no objection to the code that uses the intended-to-be-unused field.

PCManticore commented 4 years ago

Thanks for the report. I believe this makes sense. For context, the variable is marked as unused because the default pattern for dummy-variables-rgx includes unused as well as ignored patterns, but there is no check currently to verify the opposite case which is the one you indicate.

Pierre-Sassoulas commented 2 years ago

Closing as I don't see a use case for this. Why would you want to keep a variable is it's unused ?

nathanielmanistaatgoogle commented 2 years ago

Closing as I don't see a use case for this.

I think the circumstance here is that alerting on uses of these fields will lead maintainers to reflect "oh, when this was originally written the field was unused, but now I'm changing the code in such a way to use the field; I will rename it by at least dropping the no-longer-true unused_ prefix".

Why would you want to keep a variable is it's unused?

... it sounds like you may not be familiar with the longstanding general practice of preferring unused_<helpfully descriptive name> over _ in circumstances that require a field? Like for citizen, unused_tax_records in population: rather than for citizen, _ in population: - have you seen this before? Pylint has for years handled code that does this, and it's an encouraged best practice in all the codebases in which I currently write and review code. 🙂

Pierre-Sassoulas commented 2 years ago

Hmm yeah, I did not know about that. Does pylint enforce this convention (no unused-variable raised when they are named like this) ? (I can't test right now as I'm on mobile, sorry)

jacobtylerwalls commented 2 years ago

Does pylint enforce this convention (no unused-variable raised when they are named like this) ?

Yes, I regularly use this pattern.

Original request makes sense to me.