False positive not-an-iterable for typing.NewType
Steps to reproduce
- 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
- 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]
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.
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
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
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)
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?
Looks like the problem was introduced with https://github.com/PyCQA/astroid/commit/283befa92865faad7847b2d40a569e03ae00f7cc#diff-82f7976cc16d2319c09d26ba124fc46b
What exactly did https://github.com/PyCQA/astroid/commit/283befa92865faad7847b2d40a569e03ae00f7cc#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"?
@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!
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.
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.
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 :)
Yep, thanks; that makes perfect sense. What are our next steps here? I'd like to offer to help, but I am completely unfamiliar with pylint's internals (I only contributed some minor docs updates in my role as Spyder/Docs maintainer), and don't want to overbook myself.
PCManticore won't answer this though, he's retired from pylint :)
Ah sorry, didn't know that. Thanks for the update.
change the brain to make the pseudo class inherit from the given type (e.g. List[int])
I think @brycepg hint is the right one, so the change should be made in brain_typing.py. I'm no astroid expert but If you want to experiment, open à MR, I'll try to review and help :)
Is there any workaround for this 4 years old issue? I kinda need to use a NewType for aliasing Dict[str, Any] and I don't know how to make pylint happy.
I give an upvote to this. In my codebase, there are some usages of NewType, and pylint barks at them. Is there any chance that pylint will soon support NewType?
Latest development are in https://github.com/pylint-dev/astroid/pull/1301#issuecomment-1162485313