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

E721: pycodestyle v2.11.0 does not raise the error to int == type(obj) #1187

Open
ftnext opened this issue Aug 6, 2023 · 0 comments
Open

Comments

@ftnext
Copy link

ftnext commented Aug 6, 2023

Environment

% python -V
Python 3.11.4
% pycodestyle --version
2.11.0

% pip list
Package     Version
----------- -------
pip         23.2.1
pycodestyle 2.11.0
setuptools  68.0.0

Normal behavior

% cat example.py
if type(obj) == int:
    pass
% pycodestyle example.py
example.py:1:4: E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`

Great!

Strange behavior

Change type(obj) == int to int == type(obj) (swap the left and right sides).

Expected

Raise E721.
This code is still comparing the return value of type.

Actual

Do NOT raise E721.

% cat example.py
if int == type(obj):
    pass
% pycodestyle example.py
% echo $?
0

Investigation

>>> from pycodestyle import comparison_type
>>> list(comparison_type("if type(obj) == int:", noqa=False))
[(3, 'E721 do not compare types, for exact checks use `is` / `is not`, for instance checks use `isinstance()`')]
>>> list(comparison_type("if int == type(obj):", noqa=False))
[]

In v2.11.0 (21abd9b), COMPARE_TYPE_REGEX has 2 captures.

pycodestyle/pycodestyle.py

Lines 128 to 131 in 21abd9b

COMPARE_TYPE_REGEX = re.compile(
r'[=!]=\s+type(?:\s*\(\s*([^)]*[^ )])\s*\))'
r'|\btype(?:\s*\(\s*([^)]*[^ )])\s*\))\s+[=!]='
)

In comparison_type, the first captured substring is only referred (inst = match.group(1)).

pycodestyle/pycodestyle.py

Lines 1451 to 1452 in 21abd9b

@register_check
def comparison_type(logical_line, noqa):

pycodestyle/pycodestyle.py

Lines 1461 to 1470 in 21abd9b

match = COMPARE_TYPE_REGEX.search(logical_line)
if match and not noqa:
inst = match.group(1)
if inst and inst.isidentifier() and inst not in SINGLETONS:
return # Allow comparison for types which are not obvious
yield (
match.start(),
"E721 do not compare types, for exact checks use `is` / `is not`, "
"for instance checks use `isinstance()`",
)

>>> from pycodestyle import COMPARE_TYPE_REGEX
>>> from pycodestyle import SINGLETONS

>>> match = COMPARE_TYPE_REGEX.search("if type(obj) == int:")
>>> match
<re.Match object; span=(3, 15), match='type(obj) =='>
>>> match.group(1)  # None; Go yield (L1466)
>>> match.group(2)
'obj'

>>> match = COMPARE_TYPE_REGEX.search("if int == type(obj):")
>>> match
<re.Match object; span=(7, 19), match='== type(obj)'>
>>> match.group(1)
'obj'
>>> match.group(2)
>>> inst = match.group(1)
>>> inst and inst.isidentifier() and inst not in SINGLETONS  # Go return (L1465) not yield
True

If you need to use captured substrings, you might need to do something like inst = match.group(1) or match.group(2).
However, in that case, inst and inst.isidentifier() and inst not in SINGLETONS would return True, and E721 would no longer be raised.
Therefore, I believe that removing the return statement here would resolve the reported issue.

One concern I have is that it seems inst = match.group(1) has not been changed along with the last pull request.
https://github.com/PyCQA/pycodestyle/pull/1086/files
Depending on the reason for not changing this, we might need to consider a different solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant