TypeError: getattr() got an unexpected keyword argument 'class_context'

[MarcSkovMadsen]

Bug description

Linting the below script.py file raises an exception.

import pandas as pd
import holoviews as hv
import hvplot.pandas

data = pd.DataFrame({
    "x": [1,2,3],
    "y": [1,2,3]
})

plot1 = data.hvplot(x="x", y="y")
plot2 = data.hvplot(x="x", y="y")
selection_linker = (
    hv.selection.link_selections.instance()
)
selection_linker(plot1)
selection_linker(plot2)

Configuration

[tool.pylint.main]
py-version=3.9
output-format = "colorized"
max-attributes=12
max-args=10

[tool.pylint.format]
max-module-lines = 1000

Command used

pylint script.py

Pylint output

$ pylint 'script.py'
************* Module script
script.py:16:0: C0304: Final newline missing (missing-final-newline)
script.py:1:0: C0114: Missing module docstring (missing-module-docstring)
script.py:13:4: E1120: No value for argument 'self_or_cls' in unbound method call (no-value-for-parameter)
Exception on node <Call l.15 at 0x2880b0a7550> in file 'C:\repos\trading_analytics\mt-fumo-data-pipelines\script.py'
Traceback (most recent call last):
  File "C:\repos\trading_analytics\mt-fumo-data-pipelines\.venv\lib\site-packages\pylint\utils\ast_walker.py", line 90, in walk
    callback(astroid)
  File "C:\repos\trading_analytics\mt-fumo-data-pipelines\.venv\lib\site-packages\pylint\checkers\typecheck.py", line 1383, in visit_call
    self._check_not_callable(node, called)
  File "C:\repos\trading_analytics\mt-fumo-data-pipelines\.venv\lib\site-packages\pylint\checkers\typecheck.py", line 1696, in _check_not_callable
    if not inferred_call or inferred_call.callable():
  File "C:\repos\trading_analytics\mt-fumo-data-pipelines\.venv\lib\site-packages\astroid\bases.py", line 284, in callable
    self._proxied.getattr("__call__", class_context=False)
TypeError: getattr() got an unexpected keyword argument 'class_context'
script.py:1:0: F0002: script.py: Fatal error while checking 'script.py'. Please open an issue in our bug tracker so we address this. There is a pre-filled template that you can use in 'C:\Users\masma\AppData\Local\pylint\pylint\Cache\pylint-crash-2022-07-01-14.txt'. (astroid-error)

Expected behavior

No error would be raised

Pylint version

pylint 2.14.3
astroid 2.11.6
Python 3.9.7 (tags/v3.9.7:1016ef3, Aug 30 2021, 20:19:38) [MSC v.1929 64 bit (AMD64)]

OS / Environment

Windows

Additional dependencies

pip install pandas==1.4.2 holoviews==1.14.9 hvplot==0.8.0 pylint==2.14.3 astroid==2.11.6

[jacobtylerwalls]

Thanks for the report. I can reproduce with holoviews installed and with just this:

import holoviews as hv
selection_linker = (
    hv.selection.link_selections.instance()
)
selection_linker()

---

._proxied turns out to be another Instance, so that explains the crash.

(Pdb) self
<Instance of param.parameterized.ParameterizedFunction at 0x4534040464>
(Pdb) self._proxied
<Instance of param.parameterized.ParameterizedFunction at 0x4534039552>
(Pdb) self._proxied._proxied
<ClassDef.ParameterizedFunction l.3613 at 0x102dabd30>

---

This trivial diff fixes it, but I don't know whether it's papering over a deeper failure with chained Instances as _proxied. @DanielNoord do you know?

diff --git a/astroid/bases.py b/astroid/bases.py
index a3f2017f..644bb4ed 100644
--- a/astroid/bases.py
+++ b/astroid/bases.py
@@ -303,7 +303,10 @@ class Instance(BaseInstance):
 
     def callable(self):
         try:
-            self._proxied.getattr("__call__", class_context=False)
+            proxied = self._proxied
+            while isinstance(proxied, Instance):
+                proxied = proxied._proxied
+            proxied.getattr("__call__", class_context=False)
             return True
         except AttributeInferenceError:
             return False

---

Holoviews source: https://github.com/holoviz/holoviews/blob/5ce1e1d774a977f709670def5304bef7213ea6fe/holoviews/selection.py#L296

[DanielNoord]

Although that seems to fix the issue I don't think that is the actual fix.

class A:
    ...
A.__new__(A())

The above code will raise a TypeError. This is important as it shows that __new__ always needs to be called with a type instead of an instance.
The issue, I think, is that we are inferring the argument to __new__ as an instance instead of as a type.

hv.selection.link_selections.instance() calls param.ParameterizedFunction.instance through its inheritance. Source of which can be found here:
https://github.com/holoviz/param/blob/2958f6327ad3a5222eb773801e3e46f6130d9f65/param/parameterized.py#L3629

The following diff shows that for the call to inst=Parameterized.__new__(cls) we are inferring cls as an instance of param.ParameterizedFunction.

diff --git a/astroid/bases.py b/astroid/bases.py
index a3f2017f3..3d20ba832 100644
--- a/astroid/bases.py
+++ b/astroid/bases.py
@@ -434,6 +434,8 @@ class UnboundMethod(Proxy):
                 return
 
         node_context = context.extra_context.get(caller.args[0])
+        if caller.lineno == 3644:
+            print(list(caller.args[0].infer(context=node_context)))
         infer = caller.args[0].infer(context=node_context)
 
         yield from (Instance(x) if x is not Uninferable else x for x in infer)  # type: ignore[misc,arg-type]

[<Instance of param.parameterized.ParameterizedFunction at 0x4409978064>]

However, by adding print(cls) on L3643 and running the code example we get:

<class 'holoviews.core.util.Config'>
<class 'holoviews.core.util.sanitize_identifier_fn'>
<class 'holoviews.core.util.sanitize_identifier_fn'>
<class 'holoviews.core.util.sanitize_identifier_fn'>
<class 'holoviews.core.util.sanitize_identifier_fn'>
<class 'holoviews.selection.link_selections'>

This follows the normal assumption that __new__ would crash with an instance instead of a type.

TLDR: Chained Instances in _proxied can't really occur normally and are only the result of incorrect inference within astroid. That's a good thing as it confirms the typing I added 3 days ago to Instance._proxied: pylint-dev/astroid@ffc368f 😄

As for the fix itself, I have no idea yet and it will likely need to happen somewhere within caller.args[0].infer(context=node_context).

[jacobtylerwalls]

Thank you for digging into the source! 🚀

TLDR: Chained Instances in _proxied can't really occur normally and are only the result of incorrect inference within astroid. That's a good thing as it confirms the typing I added 3 days ago to Instance._proxied: pylint-dev/astroid@ffc368f 😄

Sounds like the situation will be improved when we land pylint-dev/astroid#1189, and we can start filtering out inferred values based on constraints (here, the isinstance guard).

That said, there's still a problem here, right? Because pylint shouldn't crash on user code, even if malformed.

Extending your example:

class A:
    ...
invalid = A.__new__(A())
invalid()

Exception on node <Call l.4 at 0x10633bb20> in file '/Users/.../pylint/a.py'
Traceback (most recent call last):
  File "/Users/.../pylint/pylint/utils/ast_walker.py", line 90, in walk
    callback(astroid)
  File "/Users/.../pylint/pylint/checkers/typecheck.py", line 1397, in visit_call
    self._check_not_callable(node, called)
  File "/Users/.../pylint/pylint/checkers/typecheck.py", line 1710, in _check_not_callable
    if not inferred_call or inferred_call.callable():
  File "/Users/.../astroid/astroid/bases.py", line 306, in callable
    self._proxied.getattr("__call__", class_context=False)
TypeError: getattr() got an unexpected keyword argument 'class_context'

Given that, do you have thoughts about alternatives?

[DanielNoord]

diff --git a/astroid/bases.py b/astroid/bases.py
index a3f2017f3..2d3d04559 100644
--- a/astroid/bases.py
+++ b/astroid/bases.py
@@ -436,7 +436,7 @@ class UnboundMethod(Proxy):
         node_context = context.extra_context.get(caller.args[0])
         infer = caller.args[0].infer(context=node_context)
 
-        yield from (Instance(x) if x is not Uninferable else x for x in infer)  # type: ignore[misc,arg-type]
+        yield from (Instance(x) if isinstance(x, nodes.ClassDef) else x for x in infer)  # type: ignore[misc,arg-type]
 
     def bool_value(self, context=None):
         return True

This should be a good first start. Not sure if we should also allow FunctionDef or other types (I haven't even run the test suite), but this no longer crashes 😄

[jacobtylerwalls]

Yeah, I just came to that conclusion but with not isinstance(x, Instance) to be a bit more conservative.

[jacobtylerwalls]

I'll give it a go.

[DanielNoord]

Btw, wouldn't it be better to iterate over infer and yield Uninferable if we shouldn't pass it to Instance.

In both our solutions, if for some reason we end up with x being UnboundMethod or nodes.Const we would yield that x. That doesn't seem right as it would raise a TypeError so the node should be Uninferable.

[jacobtylerwalls]

That doesn't seem right as it would raise a TypeError so the node should be Uninferable.

But correct inference is important -- in my example, the user wrote invalid code, and there's no reason astroid can't infer that correctly.

[DanielNoord]

Yeah, but shouldn't invalid be an Uninferable? A.__new__(A()) doesn't return anything as it creates a TypeError so its return value is Uninferable.

[jacobtylerwalls]

Duh, I forgot we were inside _infer_builtin_new. Maybe it could raise something stronger like InferenceError.

[DanielNoord]

I think other paths in the function to return Uninferable, but you'll probably need to investigate a little. We don't crash so invalid becomes something in our representation currently. It is perfectly possible that some decorator 10 calls higher up makes it into an Uniferable but that we could make that more explicit earlier.

[jacobtylerwalls]

I think failing locally and letting callers decide what to do is what we do elsewhere:

https://github.com/PyCQA/astroid/blob/3621e2e7d68653d66cbf770e6dcb61ba541117f1/astroid/arguments.py#L176-L187

I'm looking at this diff, and it passes the astroid suite (will check pylint next)

diff --git a/astroid/bases.py b/astroid/bases.py
index a3f2017f..91762ef7 100644
--- a/astroid/bases.py
+++ b/astroid/bases.py
@@ -436,7 +436,12 @@ class UnboundMethod(Proxy):
         node_context = context.extra_context.get(caller.args[0])
         infer = caller.args[0].infer(context=node_context)
 
-        yield from (Instance(x) if x is not Uninferable else x for x in infer)  # type: ignore[misc,arg-type]
+        for inferred in infer:
+            if isinstance(inferred, type(Uninferable)):  # isinstance helps mypy
+                yield inferred
+            if isinstance(inferred, (nodes.ClassDef, nodes.Lambda, Proxy)):
+                yield Instance(inferred)
+            raise InferenceError
 
     def bool_value(self, context=None):
         return True