Merge pull request #20 from DavidCEllis/methodmaker_super_fix

Fix for MethodMaker descriptors being called via super()

docs/approach_vs_tool.md

@@ -4,13 +4,27 @@ As this module's code generation is inspired by the workings of [David Beazley's
 I thought it was briefly worth discussing his note on learning an approach vs using a tool.
 
 I think that learning an approach is valuable, this module would not exist without the
-example given by `cluegen`. It also wouldn't exist if I hadn't needed to extend `cluegen`
-for some basic features (try using `Path` default values with `cluegen`).
+example given by `cluegen`.
+
+However, what I foundn was that in essentially every case where I wanted to use 
+these generating tools, I needed to modify them - often significantly. 
+It quickly became easier to just create my own tool and upload it as a package.
+
+For example, `cluegen` has a few subtle "exercises for the reader". It needs extending
+and fixing for some use-cases.
+   * Default values that are not builtins need to be passed as part of the globals 
+     dict to `exec`.
+   * No support for mutable defaults.
+   * Subclass methods will be overwritten if they call a cluegen method that has not been
+     generated via `super().methodname(...)`
+   * `inspect.signature(cls)` does not work if `cls.__init__` has not already been generated.
+     (I think this is actually a bug in inspect).
+   * Need an extra filter to support things like `ClassVar`.
 
 In the general spirit though, this module intends to provide some basic tools to help 
-build your own custom class generators.
+create your own customized boilerplate generators.
 The generator included in the base module is intended to be used to help 'bootstrap' a 
 modified generator with features that work how **you** want them to work.
 
-The `prefab` module is the more fully featured powertool *I* built with these tools. 
-However, much like a prefabricated building, it may not be what you desire.
+The `prefab` module is the more fully featured tool that handles the additional cases *I*
+needed. 

src/ducktools/classbuilder/__init__.py

@@ -160,19 +160,39 @@ def __repr__(self):
 
     def __get__(self, inst, cls):
         local_vars = {}
-        gen = self.code_generator(cls, self.funcname)
+
+        # This can be called via super().funcname(...) in which case the class
+        # may not be the correct one. If this is the correct class
+        # it should have this descriptor in the class dict under
+        # the correct funcname.
+        # Otherwise is should be found in the MRO of the class.
+        if cls.__dict__.get(self.funcname) is self:
+            gen_cls = cls
+        else:
+            for c in cls.__mro__[1:]:  # skip 'cls' as special cased
+                if c.__dict__.get(self.funcname) is self:
+                    gen_cls = c
+                    break
+            else:  # pragma: no cover
+                # This should only be reached if called with incorrect arguments
+                # manually
+                raise AttributeError(
+                    f"Could not find {self!r} in class {cls.__name__!r} MRO."
+                )
+
+        gen = self.code_generator(gen_cls, self.funcname)
         exec(gen.source_code, gen.globs, local_vars)
         method = local_vars.get(self.funcname)
 
         try:
-            method.__qualname__ = f"{cls.__qualname__}.{self.funcname}"
+            method.__qualname__ = f"{gen_cls.__qualname__}.{self.funcname}"
         except AttributeError:
             # This might be a property or some other special
             # descriptor. Don't try to rename.
             pass
 
         # Replace this descriptor on the class with the generated function
-        setattr(cls, self.funcname, method)
+        setattr(gen_cls, self.funcname, method)
 
         # Use 'get' to return the generated function as a bound method
         # instead of as a regular function for first usage.

tests/test_core.py

@@ -8,6 +8,7 @@
 
     builder,
     default_methods,
+    eq_maker,
     get_fields,
     get_flags,
     get_methods,
@@ -527,3 +528,33 @@ class SigClass:
         __slots__ = SlotFields(x=42)
 
     assert str(inspect.signature(SigClass)) == "(x=42)"
+
+
+def test_subclass_method_not_overwritten():
+    @slotclass
+    class X:
+        __slots__ = SlotFields(x=Field())
+
+    class Y(X):
+        def __init__(self, x, y):
+            self.y = y
+            super().__init__(x=x)
+
+    y_init_func = Y.__init__
+
+    assert X.__dict__["__eq__"] is eq_maker
+
+    y_inst = Y(0, 1)
+
+    # super().__init__ method generated correctly
+    assert y_init_func is Y.__init__
+    assert X.__dict__["__init__"] is not init_maker
+    assert (y_inst.x, y_inst.y) == (0, 1)
+
+    # Would fail previously as __init__ would be overwritten
+    y_inst_2 = Y(0, 2)
+
+    assert y_inst == y_inst_2
+
+    assert X.__dict__["__eq__"] is not eq_maker
+    assert "__eq__" not in Y.__dict__