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

False positive not-an-iterable for typing.NewType #2296

Open
alien3211 opened this issue Jul 16, 2018 · 17 comments · May be fixed by #5542
Open

False positive not-an-iterable for typing.NewType #2296

alien3211 opened this issue Jul 16, 2018 · 17 comments · May be fixed by #5542
Labels

Comments

@alien3211
Copy link

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
  1. 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
Copy link
Contributor

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.

@PCManticore PCManticore added Bug 🪲 Astroid Related to astroid labels Jul 17, 2018
@brycepg
Copy link
Contributor

brycepg commented Jul 20, 2018

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
Copy link
Contributor

brycepg commented Jul 20, 2018

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

@PCManticore PCManticore changed the title False positive not-an-iterable False positive not-an-iterable for typing.NewType Dec 11, 2018
@Linekio
Copy link

Linekio commented May 6, 2019

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
Copy link

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
Copy link

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

@svenpanne
Copy link

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
Copy link
Contributor

@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
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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 :)

@CAM-Gerlach
Copy link
Contributor

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.

@Pierre-Sassoulas
Copy link
Member

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 :)

markafitzgerald1 added a commit to markafitzgerald1/simulate-cribbage-games that referenced this issue Sep 1, 2021
All remaining errors invalid and due to
pylint-dev/pylint#2296 - suppressing existing
reports in this codebase for now.
@cdce8p cdce8p added the typing label Oct 17, 2021
colatkinson added a commit to colatkinson/astroid that referenced this issue Dec 17, 2021
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
colatkinson added a commit to colatkinson/pylint that referenced this issue Dec 17, 2021
colatkinson added a commit to colatkinson/astroid that referenced this issue Dec 17, 2021
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
@colatkinson colatkinson linked a pull request Dec 17, 2021 that will close this issue
colatkinson added a commit to colatkinson/pylint that referenced this issue Dec 17, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.13.0 milestone Dec 17, 2021
colatkinson added a commit to colatkinson/astroid that referenced this issue Dec 18, 2021
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
colatkinson added a commit to colatkinson/pylint that referenced this issue Dec 23, 2021
colatkinson added a commit to colatkinson/astroid that referenced this issue Dec 23, 2021
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
colatkinson added a commit to colatkinson/astroid that referenced this issue Dec 24, 2021
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
colatkinson added a commit to colatkinson/astroid that referenced this issue Feb 28, 2022
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
colatkinson added a commit to colatkinson/astroid that referenced this issue Mar 7, 2022
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
@jacobtylerwalls jacobtylerwalls modified the milestones: 2.13.0, 2.14.0 Mar 14, 2022
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 2.14.0, 2.15.0 May 2, 2022
colatkinson added a commit to colatkinson/astroid that referenced this issue May 28, 2022
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
colatkinson added a commit to colatkinson/astroid that referenced this issue Jun 4, 2022
NewTypes are assumed not to inherit any members from their base classes.
This results in incorrect inference results. Avoid this by changing the
transformation for NewTypes to treat them like any other subclass.

pylint-dev/pylint#3162
pylint-dev/pylint#2296
@Pierre-Sassoulas Pierre-Sassoulas added Needs astroid update Needs an astroid update (probably a release too) before being mergable Needs PR This issue is accepted, sufficiently specified and now needs an implementation Work in progress and removed Needs astroid update Needs an astroid update (probably a release too) before being mergable Bug 🪲 Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Jul 18, 2022
@DanielNoord DanielNoord removed this from the 2.15.0 milestone Aug 23, 2022
@ssbarnea
Copy link
Contributor

ssbarnea commented Oct 1, 2022

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.

@jnussbaum
Copy link

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?

@Pierre-Sassoulas
Copy link
Member

Latest development are in pylint-dev/astroid#1301 (comment)

@couteau
Copy link

couteau commented May 21, 2024

I have a variant on this issue that is more generic than just understanding the return type of NewType. Here is the code that reproduces the bug:

from typing import TypeVar, Any, Type

T = TypeVar("T", bound=type)

def create_custom_list(name: str, base: T, attrs: dict[str, Any]) -> T:
    return type(name, (base,), attrs)

MyList = create_custom_list("MyList", list, {'extra': "extra"})

l = MyList(['foo'])

print(l[0])

So the function returns a custom type that is a subclass of the passed in type -- just like NewType -- and the type hint based on the TypeVar should let pylint know that, and that, in this case, the newly created class is a subclass of list. PyRight understands this, and will see instances of the class returned from the function as instances of the base class passed into the function.

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

Successfully merging a pull request may close this issue.