False positive not-an-iterable for typing.NewType

[alien3211]

Steps to reproduce

1. Put this code in test.py:

from typing import List, NewType

a = NewType("a", List[int])

def fun() -> a:
    """Some function."""
    data = [1, 2, 3]

    return a(data)

def fun1():
    """some function."""
    for x in fun():
        pass

2. Run pylint test.py

Current behavior

test.py:14:13: E1133: Non-iterable value fun() is used in an iterating context (not-an-iterable)

Expected behavior

pylint --version output

pylint 2.0.0
astroid 2.0.0.dev4
Python 3.6.5 (default, May 3 2018, 10:08:28)
[GCC 5.4.0 20160609]

[PCManticore]

Hi, thanks for the report! What happens here is that pylint or better to say astroid does not know that NewList acts as a transparent constructor for its arguments, and most likely considers it a type in itself, which is wrong since it should just be a transparent layer so that the type checkers could pick that up.

[brycepg]

the typing astroid brain is trying to create a class for this function. I'm not too sure why the commiter wanted that to happen. If I copy the relevant code out of typing and rename the function, the type is fine

[brycepg]

I think if I change the brain to make the psuedo class inherit from the given type (e.g. List[int]) it would fix this issue and provide a platform for future annotation checking

[Linekio]

Same with sympy :

from sympy import Array

az = Array(range(10))

for a in az[::2]:
    pass

Result:

me@me-HP ~ $ pylint --version
pylint 2.3.1
astroid 2.2.2
Python 3.6.7 (default, Oct 22 2018, 11:32:17) 
[GCC 8.2.0]
me@me-HP ~ $ pylint ze.py 
************* Module ze
ze.py:5:9: E1133: Non-iterable value az[::2] is used in an iterating context (not-an-iterable)

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

[LarsMichelsen]

To me this looks like a regression.

I am in progress of moving a Python 2 code base which was checked with pylint 1.9.5 and astroid 1.6.6 to Python3 which is now checked with pylint 2.4.4 and astroid 2.3.3.

In the pylint 2 code base this is detected correctly and works as expected:

# > pylint --version
# Using config file .pylintrc
# pylint 1.9.5,
# astroid 1.6.6
# Python 2.7.17 (default, Nov  7 2019, 10:07:09)
# [GCC 7.4.0]

#!/usr/bin/env python2

from typing import NewType, Text

UserId = NewType("UserId", Text)

# OK
UserId("x").upper()
# [pylint:no-member] Instance of 'UserId' has no 'xupper' member
UserId("x").xupper()
# OK
"".upper()

And the now broken result:

# > pylint --version
# pylint 2.4.4
# astroid 2.3.3
# Python 3.7.3 (default, Apr  3 2019, 19:16:38)
# [GCC 8.0.1 20180414 (experimental) [trunk revision 259383]]

#!/usr/bin/env python3
from typing import NewType, Text

UserId = NewType("UserId", Text)

# [pylint:no-member] Instance of 'UserId' has no 'upper' member
UserId("x").upper()
# [pylint:no-member] Instance of 'UserId' has no 'xupper' member
UserId("x").xupper()
# OK
"".upper()

In the astroid change log of version 2.0 I can find this:

* typing.X[...] and typing.NewType are inferred as classes instead of instances.

Which seems to be related with the file

https://github.com/PyCQA/astroid/commits/b7bdbf1d378bdc5d3f3f0375381242a4a61d0bb0/astroid/brain/brain_typing.py

Is this the relevant part?

This is kind of a blocker at the moment. I even don't have an idea for a good workaround.
Any ideas or pointers?

[LarsMichelsen]

Looks like the problem was introduced with pylint-dev/astroid@283befa#diff-82f7976cc16d2319c09d26ba124fc46b

[svenpanne]

What exactly did pylint-dev/astroid@283befa#diff-82f7976cc16d2319c09d26ba124fc46b fix? The previous behavior looked more correct to me, at least it didn't break the (very valuable) NewType. Can't we simply revert that "fix"?

[CAM-Gerlach]

@rogalski @PCManticore Could you answer this so we can move forward on this issue? If this commit can simply be reverted, or the problem otherwise addressed, this will resolve a major blocker that breaks most non-trivial use of NewType with pylint. Thanks!

[Pierre-Sassoulas]

Hey :) We can't revert the commit directly because there are other changes mixed in it. (Lot of use cases in def test_typing_types(self): were added for example). But we need to address the problem that it's still possible to use the attribute of the old type when using NewType so the no-member on upper() is a false positive.

[CAM-Gerlach]

Hey :) We can't revert the commit directly

Thanks, understood—I wasn't necessarily thinking a literal git revert, rather a more abstract reversion of the atomic change (vis a vis the at least nominally greater difficulty of developing some new/modified code to handle this). Sorry that wasn't clear! Of course, before we think about that, I imagine it would be best to hear from the author, committer and maintainer, to understand what should best be done to address this while retaining the other presumed benefits of the change.

[Pierre-Sassoulas]

I agree with you I think a partial minimal revert (changing test case for NewType that aren't accurate for example) is the way to go. I'm under the impression that NewType should be behave as if it was a class inheritance and right now it's simply a class def without inheritance. Maybe the fix is simple. PCManticore won't answer this though, he's retired from pylint :)