mypy icon indicating copy to clipboard operation
mypy copied to clipboard

`get_dynamic_class_hook` does not handle `defer` properly

Open asottile-sentry opened this issue 1 year ago • 3 comments

Bug Report

typically, ctx.api.defer() can be called in a plugin when there is insufficient information to defer calling to a future pass. this "works" for get_dynamic_class_hook but causes the dynamic class assignment to be converted into a Var rendering it useless as a base class later.

I've adapted a test plugin from the mypy codebase to demonstrate this problem

To Reproduce

the plugin I've adapted responds to an EAGER=1 environment variable -- where it will not defer the class creation. EAGER=0 simulates when insufficient information is available so it must defer the class to a separate pass

# mypy.ini
[mypy]
plugins = _plugin
# _plugin.py
from __future__ import annotations

import os
from typing import Callable

from mypy.nodes import GDEF, Block, ClassDef, SymbolTable, SymbolTableNode, TypeInfo, Var
from mypy.plugin import ClassDefContext, DynamicClassDefContext, Plugin
from mypy.types import Instance, get_proper_type


_DEFERRED = os.environ.get('EAGER') == '1'


class DynPlugin(Plugin):
    def get_dynamic_class_hook(
        self, fullname: str
    ) -> Callable[[DynamicClassDefContext], None] | None:
        if fullname == "mod.declarative_base":
            return add_info_hook
        return None


def add_info_hook(ctx: DynamicClassDefContext) -> None:
    global _DEFERRED
    if not _DEFERRED:
        _DEFERRED = True
        ctx.api.defer()
        print('defering!')
        return

    class_def = ClassDef(ctx.name, Block([]))
    class_def.fullname = ctx.api.qualified_name(ctx.name)

    info = TypeInfo(SymbolTable(), class_def, ctx.api.cur_mod_id)
    class_def.info = info
    obj = ctx.api.named_type("builtins.object")
    info.mro = [info, obj.type]
    info.bases = [obj]
    ctx.api.add_symbol_table_node(ctx.name, SymbolTableNode(GDEF, info))
    print('creating!')


def plugin(version: str) -> type[DynPlugin]:
    return DynPlugin
# mod.py
def declarative_base(name: str) -> object:
    raise NotImplementedError

cls = declarative_base("wat")

class C(cls): pass

Expected Behavior

both of these should pass type checking

rm -rf .mypy_cache && EAGER=1 python -m mypy mod.py
rm -rf .mypy_cache && EAGER=0 python -m mypy mod.py

Actual Behavior

$ rm -rf .mypy_cache && EAGER=1 python -m mypy mod.py
creating!
Success: no issues found in 1 source file
$ rm -rf .mypy_cache && EAGER=0 python -m mypy mod.py
defering!
creating!
mod.py:6: error: Variable "mod.cls" is not valid as a type  [valid-type]
mod.py:6: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases
mod.py:6: error: Invalid base class "cls"  [misc]
Found 2 errors in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: current HEAD 10f18a82b612b6127659cd64aa60c10b9cc7a904
  • Mypy command-line flags: see above
  • Mypy configuration options from mypy.ini (and other config files): see above
  • Python version used: 3.12.2 (though it doesn't seem to matter)

asottile-sentry avatar Jun 18 '24 16:06 asottile-sentry

this is almost certainly wrong but this does make the example pass:

diff --git a/mypy/semanal.py b/mypy/semanal.py
index 03e6172bb..3cfd6a6b0 100644
--- a/mypy/semanal.py
+++ b/mypy/semanal.py
@@ -3126,7 +3126,16 @@ class SemanticAnalyzer(
         self.store_final_status(s)
         self.check_classvar(s)
         self.process_type_annotation(s)
+
+        orig, self.deferred = self.deferred, False
         self.apply_dynamic_class_hook(s)
+        new, self.deferred = self.deferred, orig
+        if new is True:
+            for expr in names_modified_by_assignment(s):
+                self.current_symbol_table().pop(expr.name)
+                self.mark_incomplete(expr.name, expr)
+            return
+
         if not s.type:
             self.process_module_assignment(s.lvalues, s.rvalue, s)
         self.process__all__(s)

may also give me enough information to make a bad workaround in the meantime 🤔

asottile-sentry avatar Jun 18 '24 16:06 asottile-sentry

scratch that, I think that only "worked" because subclassing Any is allowed

asottile-sentry avatar Jun 18 '24 17:06 asottile-sentry

this also makes the example pass and seems to properly preserve the dynamic class and may give me enough for a workaround:

         ctx.api.defer()
+        ph = PlaceholderNode(
+            ctx.api.qualified_name(ctx.name),
+            ctx.call,
+            ctx.call.line,
+            becomes_typeinfo=True
+        )
+        ctx.api.add_symbol_table_node(ctx.name, SymbolTableNode(GDEF, ph))
         print('defering!')

unclear how to generalize this to mypy though. I think the api of the get_dynamic_class_hook itself doesn't lend itself well to solving this "generically"

asottile-sentry avatar Jun 20 '24 20:06 asottile-sentry