Incompatible function override in `InlineProcessor`

[MikeRomaa]

Hello! I was creating a custom extension that uses an InlineProcessor to implement custom escape logic. This is what the function and class signature look like:

class EscapeInlineProcessor(InlineProcessor):
    def handleMatch(self, m: Match[str], data: str) -> tuple[str | None, int, int]:
        ...

The type signature does not exactly match that of InlineProcessor.handleMatch, but it is compatible. However, when type checking with mypy, I was getting the following error:

error: Signature of "handleMatch" incompatible with supertype "Pattern"  [override]
note:      Superclass:
note:          def handleMatch(self, m: Match[str]) -> str | Element | None
note:      Subclass:
note:          def handleMatch(self, m: Match[str], data: str) -> tuple[str | None, int, int]

After digging around for a bit, I realized that the internal definition of InlineProcessor.handleMatch is not compatible with its superclass's definition of Pattern.handleMatch. The extra parameter is fine, but the return type is completely incompatible.

class Pattern:
    def handleMatch(self, m: Match[str]) -> Element | str:
        ...

class InlineProcessor(Pattern):
    def handleMatch(self, m: Match[str], data: str) -> tuple[Element | str | None, int | None, int | None]:
        ...

I have looked at some of the prior discussions regarding typings, and I understand that the general sentiment is that an entirely statically typed library is not desirable. That is not the issue here. The issue is one of ergonomics for developers using both mypy (and perhaps other static type checkers) and your library together. Having to use # type: ignore[override] just to silence a bug coming from a dependency is frustrating and should not be necessary.

I would also like to mention that #1399 did have a fix for this issue, but the substitute PR #1401 failed to address this.

[MikeRomaa]

I also just did a very quick search through the repository, and I don't think the implementation of handleMatch in the Pattern base class is ever used. My search was in no way exhaustive and I could be wrong, though.

[waylan]

Having to use # type: ignore[override] just to silence a bug coming from a dependency is frustrating and should not be necessary.

This is a matter of perspective. From my perspective there is no bug in our code as the code works just fine. The type definitions only exist as documentation, and are correct from that perspective. The issue is that the validator is enforcing a different set of rules than we have. Therefore, if there is any bug, it is with the validator not being flexible enough to match our intentions. That said, I understand that from your perspective that is irrelevant. So, without us changing our code's behavior, what can we do to ease the frustration?

After digging around for a bit, I realized that the internal definition of InlineProcessor.handleMatch is not compatible with its superclass's definition of Pattern.handleMatch. The extra parameter is fine, but the return type is completely incompatible.

Yes, those are two different classes which behave differently and the methods' return values are different by design. However, if I understand you correctly, the issue is not with the design, so much with the incompatibility with the type definitions. I'm not sure how to address this. We define different return types on each class and the actual values returned matches those types. So how is that a problem? Or can we not make one a subclass of the other and have different return types? That seems overly restrictive and not very DRY to me.

If we were to create a parent class which contained the shared code but defined no return type for the method and then each subclass defined its own return type, would that address the concern?