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.25k stars 1.12k forks source link

False positve with consider-iterating-dictionary #9360

Open EmmanuelMess opened 8 months ago

EmmanuelMess commented 8 months ago

Bug description

Similar to #5478

/tmp/test.py:

import logging
from enum import Enum

class ConstraintType(Enum):  # pylint: disable=missing-class-docstring,too-few-public-methods
    DISTANCE = 0
    FORCE = 1

constraintType = "none"

switch = {
    "distance": ConstraintType.DISTANCE,
    "force": ConstraintType.FORCE
}

if constraintType not in switch.keys():
    logging.error(f"Forgot to add a constraint type \"{constraintType}\" to the AST")
    result = None

result = switch[constraintType]

Generates consider-iterating-dictionary:

/tmp/test.py:17:25: C0201: Consider iterating the dictionary directly instead of calling .keys() (consider-iterating-dictionary)

Configuration

No response

Command used

pylint /tmp/test.py

Pylint output

************* Module test
/tmp/test.py:17:25: C0201: Consider iterating the dictionary directly instead of calling .keys() (consider-iterating-dictionary)

------------------------------------------------------------------
Your code has been rated at 9.09/10 (previous run: 4.55/10, +4.55)

Expected behavior

The error should not appear, that is an if, not a loop.

Pylint version

pylint 3.0.3
astroid 3.0.2
Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0]

OS / Environment

Ubuntu 22.04

Additional dependencies

astroid==3.0.2
dill==0.3.7
exceptiongroup==1.2.0
flake8==6.1.0
iniconfig==2.0.0
isort==5.13.2
lark==1.1.8
mccabe==0.7.0
mypy==1.8.0
mypy-extensions==1.0.0
numpy==1.26.2
packaging==23.2
platformdirs==4.1.0
pluggy==1.3.0
pycodestyle==2.11.1
pyflakes==3.1.0
pylint==3.0.3
PySide6==6.6.1
PySide6-Addons==6.6.1
PySide6-Essentials==6.6.1
pytest==7.4.4
shiboken6==6.6.1
tomli==2.0.1
tomlkit==0.12.3
typing_extensions==4.9.0
mbyrnepr2 commented 8 months ago

I think this is expected behaviour and is documented as such: https://pylint.pycqa.org/en/stable/user_guide/messages/convention/consider-iterating-dictionary.html

Emitted when the keys of a dictionary are iterated through the .keys() method or when .keys() is used for a membership check..

Perhaps the content of the message could be updated to not necessarily mention iteration; what do you think @EmmanuelMess ? Edit: Perhaps something like: "Use of .keys() is not required here"?

EmmanuelMess commented 8 months ago

Perhaps the content of the message could be updated to not necessarily mention iteration; what do you think @EmmanuelMess ? Edit: Perhaps something like: "Use of .keys() is not required here"?

That would make it more clear, yes. As it is worded now makes little sense in this context.

mbyrnepr2 commented 8 months ago

Thanks! The name ‘consider-iterating-dict’ could also be updated so I’ll remove the ‘needs pr’ label until there is interest in that also from others.