False positive IncompatibleMethodOveride error

[russelldavis]

class Parent:
    def foo(self): ...

class Child(Parent):
    def foo(self, _arg=1): ...

Pyright reports "Parameter count mismatch", but since the extra parameter in Child has a default value, the method is still compatible with the number of parameters in Parent.

[erictraut]

This behavior is intended. Subclasses that override methods in a base class should have the same signature as the base class. This is a general principle in object-oriented design. If you disagree with this principle, you can choose to quiet this diagnostic rule.

[russelldavis]

This is a general principle in object-oriented design

It really isn't. What is definitely a general OO principle is Liskov substitution[1], and by that principle it's a false positive. Typescript agrees, BTW.

[1] https://en.wikipedia.org/wiki/SOLID

[russelldavis]

Another way to look at it: functions with default parameters are isomorphic with wrapper functions that forward on the defaults. So, let's say I rewrite the example this like this:

class Parent:
    def foo(self): ...

class Child(Parent):
    def foo(self):
        return self._foo(1)

    def _foo(self, _arg): ...

Now it's in compliance with your principle. What did that actually accomplish?

[erictraut]

Liskov substitution requires that objects in a hierarchy are substitutable. If you introduce a new (optional) parameter and someone provides an argument for that parameter, the target object can no longer be substituted for other objects in the hierarchy because they don't know about that argument.

The code you provided above is exactly how I would implement this because it doesn't result in this problem. As a small variation on your code, you could name "_foo" something like "foo_with_option" so callers who happen to know that it's a Child object can safely call that method.

Let me consult with a few more people on this topic before making a final decision. I'm of two minds on this one.

[russelldavis]

If you introduce a new (optional) parameter and someone provides an argument for that parameter, the target object can no longer be substituted for other objects in the hierarchy because they don't know about that argument.

I'm not seeing how the default arg version could break substitutability in a way that wouldn't also happen in the equivalent foo_with_option version. Mind posting a code snippet that shows it?

[erictraut]

I had a chance to discuss this today with a couple of my colleagues on the TypeScript team. They said this falls into a gray area, but they generally agreed with the argument you made above. They came to the same conclusion when they considered this point for TypeScript. Other typed languages, such as C# and C++, have come down differently on this topic.

I'll need to think about this more. Python's support for named parameters (including parameters that can be specified either positionally or named), *vargs, and **kwargs makes it tricky to come up with the right set of rules for when a subclass signature is truly a safe override. The logic that's currently implemented in Pyright checks for a match in arity (i.e. the parameter count), parameter names, and parameter types (although it allows for narrower types in the override). I'll need to think about a new set of (more complicated) rules that still guarantees safety.

[erictraut]

I've extended the logic for the "reportIncompatibleMethodOverride" rule. It now knows about postition-only parameters and allows a subclass to extend the signature as long as the extension includes only *vargs, **kwargs, and parameters with default values.

[erictraut]

This is addressed in Pyright version 1.1.57, which I just published. It will also be included in the next version of Pylance.

[alexkoay]

Closes #682 as well, thank you @russelldavis for getting this through.