Improve inference for generic classes (PEP 695)

[cdce8p]

Type of Changes

	Type
✓	🐛 Bug fix

Description

Generic classes with PEP 695 should be considered subscriptable, i.e. have a __class_getitem__ method.

class Foo[T]: ...

Refs pylint-dev/pylint#9406
Will close the issue after adding some more tests in pylint.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.79%. Comparing base (d7a2761) to head (7f71bde).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2433      +/-   ##
==========================================
- Coverage   92.79%   92.79%   -0.01%     
==========================================
  Files          94       94              
  Lines       11109    11118       +9     
==========================================
+ Hits        10309    10317       +8     
- Misses        800      801       +1     

Flag	Coverage Δ
linux	92.59% <100.00%> (-0.01%)	⬇️
pypy	92.79% <100.00%> (-0.01%)	⬇️
windows	92.69% <100.00%> (-0.01%)	⬇️

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

Files	Coverage Δ
astroid/brain/brain_typing.py	87.73% <100.00%> (+0.55%)	⬆️
astroid/nodes/scoped_nodes/scoped_nodes.py	92.52% <ø> (ø)
astroid/protocols.py	90.28% <100.00%> (ø)

... and 2 files with indirect coverage changes

[Pierre-Sassoulas]

LGTM

[jacobtylerwalls]

Thanks for tackling this. One question for my own information: why do we need to use inference? Can we just check node.type_params?

[cdce8p]

Tbh I'm not quite sure either. All other methods do this as well. My best guess is that with the brain_** we overwrite the default inference. However, since we still want that, we need to call node.infer() ourselves.

[jacobtylerwalls]

Interestingly, if I add assert node is value, it passes.

Can we use this opportunity to explore avoiding inference here?

[cdce8p]

This really isn't the "correct" solution but it works as the result is ignored anyway when inferring the __class_getitem__ call. It just can't raise an Exception - that would lead to Uninferable.

The typing brain solves that by inferring the T = TypeVar(...) calls as new classes which is better but probably also not fully right.

If it was actually uses, I believe we would've seen issue reports for it by now.

astroid/astroid/brain/brain_typing.py

Lines 139 to 144 in 16da308

	typename = node.args[0].as_string().strip("'")
	try:
	node = extract_node(TYPING_TYPE_TEMPLATE.format(typename))
	except AstroidSyntaxError as exc:
	raise InferenceError from exc
	return node.infer(context=context_itton)

astroid/astroid/brain/brain_typing.py

Lines 47 to 58 in 16da308

	TYPING_TYPE_TEMPLATE = """
	class Meta(type):
	def __getitem__(self, item):
	return self
	@property
	def __args__(self):
	return ()
	class {0}(metaclass=Meta):
	pass
	"""

[jacobtylerwalls]

Open to different ideas here, but I think this would be a nice opportunity to verify things still work if we don't use inference.

[jacobtylerwalls]

Should we guard this under if node.type_params: ?

[cdce8p]

Should we guard this under if node.type_params: ?

That is done by the predicate. No need to duplicate it I think

def _looks_like_generic_class_pep695(node: ClassDef) -> bool:
    """Check if class is using type parameter. Python 3.12+."""
    return len(node.type_params) > 0

[cdce8p]

Can we use this opportunity to explore avoiding inference here?

Investigated that a bit more. With the previous setup, we would end up calling the inference_tip twice as the infer call was on node itself. All other function is brain_typing infer an attribute like func or args[0]. As the second call happened during the first inference pass, it would just get filtered out by the inference cache. So it's fine to remove it here. Also tested it with Home Assistant were everything continues to work as expected.

astroid/astroid/inference_tip.py

Lines 45 to 49 in e43e045

	if partial_cache_key in _CURRENTLY_INFERRING:
	# If through recursion we end up trying to infer the same
	# func + node we raise here.
	_CURRENTLY_INFERRING.remove(partial_cache_key)
	raise UseInferenceDefault