Create ``UninferableBase``

[DanielNoord]

Steps

• [x] For new features or bug fixes, add a ChangeLog entry describing what your PR does.
• [x] Write a good description on what the PR does.

Description

Closes #1680.

Type of Changes

	Type
✓	:sparkles: New feature
✓	:hammer: Refactoring

[coveralls]

Pull Request Test Coverage Report for Build 3503985614

• 17 of 17 (100.0%) changed or added relevant lines in 6 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.004%) to 92.292%

---

Totals
Change from base Build 3503977238:	0.004%
Covered Lines:	9878
Relevant Lines:	10703

---

💛 - Coveralls

[DanielNoord]

Yeah the issue I had with that is that that doesn't really make sense a year from now. UninferableBase is essentially just a normal class which gets instantiated. Similar to NodeNGType vs. NodeNG. I thought it would be better to just use a "typeless" name, but I'm fine with still adding it.

[Pierre-Sassoulas]

Similar to NodeNGType vs. NodeNG

We have something similar already ? Maybe we could use the same scheme for consistency ?

We could maybe bother Marc. This is a little naming brain teaser that will affect a looooot of typing later and should not take him too long to give his opinion on.

[DanielNoord]

We have something similar already ? Maybe we could use the same scheme for consistency ?

No 🤣 I meant, normal classes also don't have Type at the end of their name even though they could be considered the "type" of all their instances 😄

We could maybe bother Marc. This is a little naming brain teaser that will affect a looooot of typing later and should not take him too long to give his opinion on.

I tagged him in the original issue. I was hesitant to retag here.

[Pierre-Sassoulas]

I meant, normal classes also don't have Type at the end of their name even though they could be considered the "type" of all their instances

Ha, all right. Yeah that's not very satisfying (actually it's the reason why I don't like the name, but the issue is not the name itself it's the way it's implemented that make the typing not work like every other typing in python).

The astroid.typing.Uninferable vs astroid.util.Uninferable might be a little hacky but the result would be named almost as if the class was working "normally", what do you htink ?

[DanielNoord]

The astroid.typing.Uninferable vs astroid.util.Uninferable might be a little hacky but the result would be named almost as if the class was working "normally", what do you htink ?

That's bound to cause issues somewhere down the line. I'm not even sure you can do:

from typing import Uninferable
Uninferable = Uninferable()

That seems like a naming clash.

Anyway, this has a similar issue as UninferableType has: typing.Uninferable doesn't make a lot of sense in a year time. It's not a type or TypeVar, it's class which gets instantiated into a "global constant". Perhaps InstantiateMePleaseUninferable? 😅

[Pierre-Sassoulas]

Yeah, that would be:

from typing import Uninferable as InstantiateMePleaseUninferable
Uninferable = InstantiateMePleaseUninferable()

Then in code:

import astroid
from astroid.utils import Uninferable

def myfunc(node: astroid.typing.Uninferable | nodes.NodeNG) -> None:
    if node is Uninferable:
        print(...)

[DanielNoord]

I don't think we should use the same name for different things (which is what the current issue basically boils down to) and I don't think it makes sense to put the class definition in the typing module. This is not necessarily a typing issue. You could also see it as a refactoring to make the class of Uninferable available to those who want to create more than a singleton 😄

Anyway, imo in a years time I think it makes the most sense to have a name for the base class that shows that it is a base class rather than a name that shows where the refactoring effort originated from.

[Pierre-Sassoulas]

Yeah I don't have a good solution, maybe there isn't one;

[cdce8p]

Came across something similar recently, in the cpython/dataclasses.py module.

...
class _MISSING_TYPE:
    pass
MISSING = _MISSING_TYPE()
...

This is how they define sentinel values. That doesn't solve the typing issue though which got me thinking. Instead of writing UninferableBase or _Uninferable, it should work with type[Uninferable]. Which is the annotation we use already.

I.e. the change would just be to remove the @object.__new__ magic, rename Uninferable to _Uninferable and create a sentinel object Uninferable = _Uninferable() in the same file. Haven't tested it yet, but I think it should work.

[DanielNoord]

Yeah that should work but I was just thinking about two three years from now. What's the most logical name? Is that something with an underscore? Or something with type or base in it. I don't care too much but I don't want to make a choice that seems logical now but makes no sense in two years time

[cdce8p]

What's the most logical name? Is that something with an underscore? Or something with type or base in it. I don't care too much but I don't want to make a choice that seems logical now but makes no sense in two years time

The underscore has the advantage that it's (somewhat) private and we can change it if we want. Besides that, I think the beauty is that users will only ever deal with one -> Uninferable. So there isn't any confusions. Either you need the sentinel value or wrap it in type for typing.

[DanielNoord]

@cdce8p Note that both pyright and mypy still don't really like using type and the is Uninferable type guarding. See both:

from __future__ import annotations


class _MyClass:
    ...


MyClass = _MyClass()


def f(x: _MyClass | str) -> None:
    if x is MyClass:
        reveal_type(x)
    else:
        reveal_type(x)

❯ mypy test.py --strict
test.py:13: note: Revealed type is "Union[test._MyClass, builtins.str]"
test.py:15: note: Revealed type is "Union[test._MyClass, builtins.str]"
Success: no issues found in 1 source file

And:

from __future__ import annotations


class _MyClass:
    ...


MyClass = _MyClass()


def f(x: type[MyClass] | str) -> None:
    if x is MyClass:
        reveal_type(x)
    else:
        reveal_type(x)

❯ mypy test.py --strict
test.py:11: error: Variable "test.MyClass" is not valid as a type  [valid-type]
test.py:11: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
test.py:12: error: Non-overlapping identity check (left operand type: "Union[Type[MyClass?], str]", right operand type: "_MyClass")  [comparison-overlap]
test.py:13: note: Revealed type is "Union[Type[MyClass?], builtins.str]"
test.py:15: note: Revealed type is "Union[Type[MyClass?], builtins.str]"
Found 2 errors in 1 file (checked 1 source file)

In the last example pyright also complains about "Illegal type annotation: variable not allowed unless it is a type alias".

I might be missing something here but I think this resembles the potential situation with _Uninferable and Uninferable.

[DanielNoord]

I don't think there is a real good solution here, but it would be really nice to get this merged and start working on the migration to UninferableBase in pylint as well.

I'm open to other preferences as I don't really have a preference myself but I do think we should bite the bullet and choose one solution.

[DanielNoord]

I did the clean up of the code base in this PR as well. I must say, I saw a lot of places that could be made cleaner now that we use isinstance checks but I'll leave that for another time. For now the additional intellisense that pyright gives is a big plus!

[jacobtylerwalls]

I think Däniel's solution is the same as Marc's suggested here, just not using the underscore. (Which makes sense, it has to be public at least to pylint to let pylint use it as a type.)

There was some more discussion about typing that got us sidetracked, but what's being implemented in this PR is the easy to understand, classic way to type.

[codecov[bot]]

Codecov Report

Merging #1741 (f022df8) into main (bcaecce) will increase coverage by 0.01%. The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1741      +/-   ##
==========================================
+ Coverage   92.74%   92.76%   +0.01%     
==========================================
  Files          94       94              
  Lines       10962    10956       -6     
==========================================
- Hits        10167    10163       -4     
+ Misses        795      793       -2     

Flag	Coverage Δ
linux	92.52% <100.00%> (+0.01%)	:arrow_up:
pypy	88.46% <100.00%> (+0.01%)	:arrow_up:
windows	92.36% <100.00%> (+0.01%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
astroid/arguments.py	95.89% <100.00%> (ø)
astroid/bases.py	88.53% <100.00%> (ø)
astroid/brain/brain_builtin_inference.py	91.54% <100.00%> (ø)
astroid/brain/brain_dataclasses.py	94.28% <100.00%> (ø)
astroid/brain/brain_functools.py	98.66% <100.00%> (ø)
astroid/brain/brain_namedtuple_enum.py	92.98% <100.00%> (ø)
astroid/brain/brain_typing.py	88.43% <100.00%> (-0.08%)	:arrow_down:
astroid/builder.py	94.11% <100.00%> (ø)
astroid/constraint.py	100.00% <100.00%> (ø)
astroid/helpers.py	93.71% <100.00%> (ø)
... and 9 more