pylint icon indicating copy to clipboard operation
pylint copied to clipboard

False-positive `no-member` in comprehension in unreachable code from platform check

Open Avasam opened this issue 2 years ago • 6 comments

Bug description

# pylint: disable=missing-module-docstring,invalid-name,unnecessary-comprehension

import sys

if sys.platform == "linux":
    import os
    print(os.getgroups())  # no error
    print([group for group in os.getgroups()])  # error
    print({group for group in os.getgroups()})  # error

Configuration

No response

Command used

pylint ./example.py

Pylint output

************* Module example
example.py:8:30: E1101: Module 'os' has no 'getgroups' member (no-member)
example.py:9:30: E1101: Module 'os' has no 'getgroups' member (no-member)

Expected behavior

Platforms and version checks to still work inside comprehensions

Pylint version

pylint 2.14.5
astroid 2.11.7
Python 3.9.13 (tags/v3.9.13:6de2ca5, May 17 2022, 16:36:42) [MSC v.1929 64 bit (AMD64)]

OS / Environment

Windows 10

Additional dependencies

No response

Avasam avatar Jul 28 '22 19:07 Avasam

I wasn't able to reproduce this on a couple of different versions, for example the latest:

pylint 2.15.0-dev0
astroid 2.12.2
Python 3.10.4 (v3.10.4:9d38120e33, Mar 23 2022, 17:29:05) [Clang 13.0.0 (clang-1300.0.29.30)]

I notice the line-numbers of the output don't match the example provided; are we looking at the right output @Avasam?

I see this:

************* Module example
example.py:6:10: R1721: Unnecessary use of a comprehension, use list(os.getgroups()) instead. (unnecessary-comprehension)
example.py:7:10: R1721: Unnecessary use of a comprehension, use set(os.getgroups()) instead. (unnecessary-comprehension)

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

mbyrnepr2 avatar Jul 29 '22 09:07 mbyrnepr2

Opened my eyes now :) This would require a new astroid brain, right Pierre? Many os.py functions are Unix-only: https://docs.python.org/3/library/os.html. Oh ignore me I see your control-flow label :)

mbyrnepr2 avatar Jul 29 '22 10:07 mbyrnepr2

We already have some brains that are os specific, but I think the problem here could be something else. pylint does not have the proper control flow to see that if sys.platform == "linux": means that on Linux the code is going to be ok. (Maybe it means we must put the attribute from all OSes in the astroid brain so we don't have false positives). Some investigations are required, imo.

Pierre-Sassoulas avatar Jul 29 '22 10:07 Pierre-Sassoulas

@mbyrnepr2 Line numbers are probably off because I forgot to paste the "disable" comments to remove pylint warnings that are not relevant to this issue. I've updated it. But it seems you've figured it out and been able to replicate :)

@Pierre-Sassoulas I am not familiar with Pylint's internal workings. Could you ignore anything that's in a simple, non-obfuscated version/platform check as defined by PEP-0484 ? (https://peps.python.org/pep-0484/#version-and-platform-checking). That's what Pyright does, without having to go as far as analyzing for literal types.

Also notice that in my example above, the error only happens in comprehensions. the simple os.getgroups() does not give me an error.

Avasam avatar Jul 29 '22 17:07 Avasam

We already do something similar for version check guard so it would be a step further but it makes sense. Especially considering that they are grouped together in pep 484 as you pointed out.

Pierre-Sassoulas avatar Jul 29 '22 19:07 Pierre-Sassoulas

Also seems to happen here, it would indeed be easier to ignore code not for the current platform and version:

# pylint: disable=missing-module-docstring,unused-import
import sys

if sys.platform != "win32":
    from signal import SIGCONT, SIGTSTP

nineteendo avatar May 08 '24 07:05 nineteendo