Check for call context in PartialFunction.infer_call_result

[nickdrozd]

Type of Changes

	Type
✓	🐛 Bug fix

|

Description

Check a context's call context. Fixes four Mypy errors.

[codecov]

Codecov Report

Merging #2098 (8faeb47) into main (b4bb2d9) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2098      +/-   ##
==========================================
- Coverage   92.76%   92.76%   -0.01%     
==========================================
  Files          94       94              
  Lines       11016    11011       -5     
==========================================
- Hits        10219    10214       -5     
  Misses        797      797              

Flag	Coverage Δ
linux	92.52% <100.00%> (-0.01%)	⬇️
pypy	88.05% <100.00%> (-0.01%)	⬇️
windows	92.31% <100.00%> (-0.01%)	⬇️

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

Impacted Files	Coverage Δ
astroid/objects.py	94.04% <100.00%> (+0.03%)	⬆️

... and 1 file with indirect coverage changes

[DanielNoord]

Shouldn't this be an assert? We're inferring a call so callcontext should probably be set

[nickdrozd]

If that's the case then maybe that information should be encoded at the type level. Maybe CallContext could be reworked to be a special type of InferenceContext? I don't personally have any confidence that the call context will always be defined here, but I don't understand the inference apparatus very well.

[DanielNoord]

I don't know, but it makes no sense to change the logic if we are inadvertently introducing bugs with it. This changes the behaviour of the function and feels a bit like moving Chesterton's Fence.
I think a deepdive into infer_call_result would be really worthwhile but it should probably start from the top rather than doing individual changes in single definitions of it.

[DanielNoord]

I have just cleaned up the signature of infer_call_result. However, if not had any insight into whether callcontext is always set.

[nickdrozd]

I have just cleaned up the signature of infer_call_result. However, if not had any insight into whether callcontext is always set.

So revert back to checking, rather than asserting?

[DanielNoord]

I don't know. Asserting might very well be correct! Or we might want a subclass where the attribute has always been set?

[jacobtylerwalls]

I think this change makes sense for now. Whether CallContext must be part of the context object depends on whether or not the call is to a property. Here, this PR is only altering PartialFunction.infer_call_result, in a clear case where the code would have crashed if the assumption CallContext was always set was violated.

Here is where the CallContext is usually set if you are going through infer_call:

astroid/astroid/inference.py

Lines 291 to 295 in 92fe829

	if hasattr(callee, "infer_call_result"):
	callcontext.callcontext = CallContext(
	args=self.args, keywords=self.keywords, callee=callee
	)
	yield from callee.infer_call_result(caller=self, context=callcontext)

Let's just get the mypy errors fixed 👍

[DanielNoord]

Let's just get the mypy errors fixed +1

Although I heavily agree with this I'm also wary of adding technical debt by adding asserts and code that we don't fully understand. In this case it might be fine but I do think we should be re-evaluating on a case by case basis and not just add them everywhere.

[jacobtylerwalls]

Agree! (I walked myself through the infer_call_result patterns before approving 📚 .)