Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IndexError in property incompatible with supertype #16896

Closed
andrey-popov opened this issue Feb 9, 2024 · 2 comments · Fixed by #17352
Closed

IndexError in property incompatible with supertype #16896

andrey-popov opened this issue Feb 9, 2024 · 2 comments · Fixed by #17352
Labels
crash topic-descriptors Properties, class vs. instance attributes

Comments

@andrey-popov
Copy link

Crash Report

Checking the following minimal example with mypy causes it to crash with an IndexError:

from typing import Callable


class Message:
    pass


Handler = Callable[[Message], None]
VecHandler = Callable[[list[Message]], None]


class Foo:
    @property
    def method(self) -> Callable[[int, Handler | VecHandler], None]:
        raise NotImplementedError


class Bar(Foo):
    @property
    def method(self) -> Callable[[int, Handler], None]:
        raise NotImplementedError

Traceback

tests/mypy.py:19: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.10.0+dev
Traceback (most recent call last):
  File "/home/docker-user/.local/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/__main__.py", line 15, in console_entry
    main()
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/main.py", line 100, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/main.py", line 182, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/build.py", line 192, in build
    result = _build(
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/build.py", line 266, in _build
    graph = dispatch(sources, manager, stdout)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/build.py", line 2942, in dispatch
    process_graph(graph, manager)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/build.py", line 3340, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/build.py", line 3441, in process_stale_scc
    graph[id].type_check_first_pass()
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/build.py", line 2310, in type_check_first_pass
    self.type_checker().check_first_pass()
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 481, in check_first_pass
    self.accept(d)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 595, in accept
    stmt.accept(self)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/nodes.py", line 1142, in accept
    return visitor.visit_class_def(self)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 2313, in visit_class_def
    self.accept(defn.defs)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 595, in accept
    stmt.accept(self)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/nodes.py", line 1223, in accept
    return visitor.visit_block(self)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 2775, in visit_block
    self.accept(s)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 595, in accept
    stmt.accept(self)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/nodes.py", line 897, in accept
    return visitor.visit_decorator(self)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 4891, in visit_decorator
    self.visit_decorator_inner(e)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 4934, in visit_decorator_inner
    found_method_base_classes = self.check_method_override(e)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 1906, in check_method_override
    result = self.check_method_or_accessor_override_for_base(defn, base)
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 1941, in check_method_or_accessor_override_for_base
    if self.check_method_override_for_base_with_name(defn, name, base):
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 2050, in check_method_override_for_base_with_name
    self.check_override(
  File "/home/docker-user/.local/lib/python3.10/site-packages/mypy/checker.py", line 2224, in check_override
    context: Context = node.arguments[i + len(override.bound_args)]
IndexError: list index out of range
tests/mypy.py:19: : note: use --pdb to drop into pdb

To Reproduce

Simply run

mypy tests/mypy.py

where the content of the file is included above. The crash is reported at the line of the second @property.

The expected behaviour is that mypy finishes without a crash and reports an incompatible return type in the subclass.

mypy works as expected if I remove both @property or remove the first parameter (int) from the signatures.

Your Environment

  • Mypy version used: master@517f5aee2
  • Mypy command-line flags: none
  • Mypy configuration options from mypy.ini (and other config files): none
  • Python version used: 3.10
  • Operating system and version: Debian GNU/Linux 11 (bullseye)
@AlexWaygood AlexWaygood added the topic-descriptors Properties, class vs. instance attributes label Feb 9, 2024
@nautics889
Copy link
Contributor

nautics889 commented Feb 11, 2024

The problem arises from here, check_override(), as can be seen from a traceback.
node.arguments in that case has only one item 'self', of course.
Meanwhile override.arg_types collection (the list being iterated at that moment) somehow has two elements: int and Handler (i. e. it considers the arguments of the callable of the return type as the function's (.method()'s in our case) arguments):

>>> override.arg_types
[builtins.int, def (b.Message)]

Once mypy's checker trying to define context like this, it fails with IndexError:

                        if isinstance(node, FuncDef):
                            # here i == 1, and len(node.arguments) == 2, so node.arguments[1] fails
                            context: Context = node.arguments[i + len(override.bound_args)]
                        else:
                            context = node

I believe in this case when client's Bar.method() is supposed to be a property, the context variable should be node itself, like in 'else' block.


So far, I have found several ways to bypass it; however, mypy still considers the return type as a function's argument in both cases.


  1. Check if defn is a property somewhere in check_method_override_for_base_with_name(), add passing this to check_override().
diff 1 (adding 'defined_as_property')
diff --git a/mypy/checker.py b/mypy/checker.py
index 391f28e93..47ee18ab5 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -2026,7 +2026,8 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                 if original_node and is_property(original_node):
                     original_type = get_property_type(original_type)

-            if isinstance(typ, FunctionLike) and is_property(defn):
+            defined_as_property = is_property(defn)
+            if isinstance(typ, FunctionLike) and defined_as_property:
                 typ = get_property_type(typ)
                 if (
                     isinstance(original_node, Var)
@@ -2051,6 +2052,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                     typ,
                     original_type,
                     defn.name,
+                    defined_as_property,
                     name,
                     base.name,
                     original_class_or_static,
@@ -2139,6 +2141,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
         override: FunctionLike,
         original: FunctionLike,
         name: str,
+        defined_as_property: bool,
         name_in_super: str,
         supertype: str,
         original_class_or_static: bool,
@@ -2220,7 +2223,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                     ):
                         arg_type_in_super = original.arg_types[i]

-                        if isinstance(node, FuncDef):
+                        if isinstance(node, FuncDef) and not defined_as_property:
                             context: Context = node.arguments[i + len(override.bound_args)]
                         else:
                             context = node

Result
sandbox\b.py:20: error: Argument 2 of "my_custom_method" is incompatible with supertype "Foo"; supertype defines the argument type as "Callable[[Message], None] | Callable[[list[Message]], None]"  [override]
sandbox\b.py:20: note: This violates the Liskov substitution principle
sandbox\b.py:20: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Found 1 error in 1 file (checked 1 source file)

Process finished with exit code 1

So, it turns out, Mypy reports an incompatible type but at the same considers return type as Argument 2 of "my_custom_method". Log message should be "Return type of "my_custom_method" is incompatible with supertype "Foo"".


  1. Simply adding length check before assigning of context.
diff 2 (checking length of node.arguments before)
diff --git a/mypy/checker.py b/mypy/checker.py
index 391f28e93..9cb460a0f 100644
--- a/mypy/checker.py
+++ b/mypy/checker.py
@@ -2220,7 +2220,7 @@ class TypeChecker(NodeVisitor[None], CheckerPluginInterface):
                     ):
                         arg_type_in_super = original.arg_types[i]

-                        if isinstance(node, FuncDef):
+                        if isinstance(node, FuncDef) and i + len(override.bound_args) < len(node.arguments):
                             context: Context = node.arguments[i + len(override.bound_args)]
                         else:
                             context = node
Result
sandbox\b.py:20: error: Argument 2 of "my_custom_method" is incompatible with supertype "Foo"; supertype defines the argument type as "Callable[[Message], None] | Callable[[list[Message]], None]"  [override]
sandbox\b.py:20: note: This violates the Liskov substitution principle
sandbox\b.py:20: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Found 1 error in 1 file (checked 1 source file)

Process finished with exit code 1

Again, same effect, reports an incompatible Argument 2 in the subclass.


I suspect the problem is in how override.arg_types gets defined.

@nautics889
Copy link
Contributor

nautics889 commented Feb 11, 2024

Pay attention to checker.py, line 2030. It looks like this:

            ...
            
            if isinstance(typ, FunctionLike) and is_property(defn):
                typ = get_property_type(typ)  # here
                if (
                    isinstance(original_node, Var)
                    and not original_node.is_final
                    
                    ...
                    

Before re-assigning typ (before typ = get_property_type(typ), line 2030):

>>> typ.ret_type
def (builtins.int, def (b.Message))
>>> typ.arg_types
[]

...it means typ is overriding method itself.

After re-assigning typ (after line 2030)

>>> typ.ret_type
None
>>> typ.arg_types
[builtins.int, def (b.Message)]

...it means typ is overriding method's return type (i. e. property's return type).

Then that typ variable is passed to check_override() as an argument named override.
At this moment, I guess, typ is supposed to represent the whole overriding method (def method(self) -> Callable[[int, Handler], None]), while in fact it represents it's return type instead (Callable[[int, Handler], None]).

ilevkivskyi added a commit that referenced this issue Jun 9, 2024
Fixes #16896

Fix is simple, do not assume that an error context given by the caller
of the override check for callable type is a method defining such type,
because it may be a property.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crash topic-descriptors Properties, class vs. instance attributes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants