If a generic class is subscripted, infer that class itself#771
If a generic class is subscripted, infer that class itself#771mthuurne wants to merge 3 commits intopylint-dev:masterfrom
Conversation
mthuurne
force-pushed
the
master
branch
2 times, most recently
from
a14df8c to
070dae4
Compare
|
I just got an |
|
Part of the problem seems to be that one How is this supposed to work? Should Or should there some kind of mechanism that ensures there is a single representative instance for each member of the |
In Python 3.6, 'typing.Generic' is redefined, so if we only check the first option, we're missing the actual definition.
When no module name is passed, it defaults to the empty string, which leads to an incorrect qname on the inferrence result.
|
might also close pylint-dev/pylint#3684 ? |
|
@PCManticore This pull request would be very helpful. Would you be able to provide some feedback about how you want it implemented? |
PCManticore
left a comment
PCManticore
left a comment
There was a problem hiding this comment.
Overall the change looks good, but I wonder the following:
- Can we define a
Genericobject in objects that acts as a future placeholder for a more thorough inference? This would be similar to the other objects that we have defined there, likeSuper,FrozenSet,DictKeysand whatnot. The way it could work would be that when we detect thetyping.Generic, we emit instead aGenericinstance after the inference. TheGenericinstance could contain the original type and for all intents and purposes could act as aInstanceorClassDef(I'm not sure whattyping.Genericis in Python) - You mentioned some possible errors that break pylint. I'd like to see those reflected in tests over here in
astroid, so it would be easier to detect what's broken and potentially fix the problems before shipping this change. - Same here with regards to the MRO issues, it would be great if those could be replicated in tests over here.
| """Infer a typing.X[...] subscript""" | ||
| try: | ||
| value = next(node.value.infer()) | ||
| for value in node.value.infer(): |
There was a problem hiding this comment.
Not sure why we were not passing the context parameter to value inference. Can you add it here and see if it has any unintended side effects? (e.g. if certain tests start to fail, let's keep it removed)
| yield util.Uninferable | ||
| return None | ||
| if isinstance(value, nodes.ClassDef): | ||
| if value.is_subtype_of("typing.Generic"): |
There was a problem hiding this comment.
Can you also pass the context to is_subtype_of? This should prevent potential circular inference that could cause RecursionErrors to pop up.
|
Probably outdated as it aim to fixes three issues that are closed. Seems like #927 is what fixed them. |
4 participants
Description
Ideally, when a generic class is subscripted, the value for the type variable would be remembered and used in later inference. However, just interring the subscripted class itself already solves multiple false positives in pylint, so this seems like a useful first step.
Type of Changes
Related Issue
Closes pylint-dev/pylint#3131
Closes pylint-dev/pylint#3505
It might also solve pylint-dev/pylint#3129, but I'm having trouble reproducing that issue on Python 3.8 even without this PR applied.