sublime_text icon indicating copy to clipboard operation
sublime_text copied to clipboard

[types] declare optional arguments in methods that are overridable

Open rchl opened this issue 2 months ago • 9 comments

Problem description

ST's internal python code that defines classes like Command, WindowCommand, TextCommand etc defines the run method like so:

    def run(self):
        """ :meta private: """

This is problematic for type checkers since implementations inheriting from those classes often specify additional arguments for run making type checkers report issues like:

   17:9   error   Method "run" overrides class "WindowCommand" in an incompatible manner 
                    Positional parameter count mismatch; base method has 1, but override has 2

Preferred solution

I'm not 100% sure if this wouldn't result in any runtime behavior changes but this would satisfy type checkers and it seems like a more correct signature for something that is optionally passed extra arguments.

    def run(self, *args, **kwargs):
        """ :meta private: """

Alternatives

type-ignoring those specific issues

Additional Information

No response

rchl avatar Oct 31 '25 07:10 rchl

I don't think it would break anything, but how does overridinig run(self, *args, **kwargs) with say run(self, characters) not fail type checkers too?

BenjaminSchaaf avatar Nov 04 '25 00:11 BenjaminSchaaf

My assumption is that it makes the base method types wide enough to encompass any number of arguments in the overridden method.

That said, I've just noticed that it makes typing less strict in case the base method specifies any arguments. For example with base:

    def run(self, edit: sublime.Edit):

The override can use a different type for edit or just omit it entirely and that won't be caught by type checker.

Not sure if there is a proper solution for this. ST's dynamic handling of run arguments might be incompatible with static type checking...

rchl avatar Nov 04 '25 08:11 rchl

class Command:
    def name(self): ...
    def is_enabled_(self, args: Args): ...
    def is_enabled(self) -> bool: ...
    def is_visible_(self, args: Args): ...
    def is_visible(self): ...
    def is_checked_(self, args: Args): ...
    def is_checked(self): ...
    def description_(self, args: Args): ...
    def description(self): ...
    def filter_args(self, args: Args) -> Args: ...
    def want_event(self): ...
    def run_(self, edit_token, args: Args): ...
    def run(self, *args, **kwargs) -> None: ...

class ApplicationCommand(Command): ...

class WindowCommand(Command):
    window = ...  # type: sublime.Window
    def __init__(self, window) -> None: ...

class TextCommand(Command):
    view = ...  # type: sublime.View
    def __init__(self, view) -> None: ...
    def run(self, edit: sublime.Edit, *args, **kwargs) -> None: ...

is used in GitSavvy and SublimeLinter

kaste avatar Nov 04 '25 11:11 kaste

So as I've suggested but that has the issue with making types too loose (per my previous comment).

rchl avatar Nov 04 '25 11:11 rchl

I probably didn't understand your comment, but I added def run(self, edit: sublime.Edit, *args, **kwargs) -> None: ...to TextCommand (=the base). And that works for the user defined text commands and requires them to have an Edit as their first arg.

kaste avatar Nov 04 '25 11:11 kaste

With those changes in an override like:

class TestTypesCommand(sublime_plugin.TextCommand):
    def run(self, edit) -> None:
        pass

The type of edit won't be known.

And if one assigns it a different type:

class TestTypesCommand(sublime_plugin.TextCommand):
    def run(self, edit: sublime.Buffer) -> None:
        pass

then this won't trigger an error.

Also not having edit argument won't trigger an error anymore:

class TestTypesCommand(sublime_plugin.TextCommand):
    def run(self) -> None:
        pass

All of those cases will work as expected without the changes.

rchl avatar Nov 04 '25 12:11 rchl

I have

class TestTypesCommand(sublime_plugin.TextCommand):
    def run(self, edit: sublime.Buffer) -> None:
        pass
Signature of "run" incompatible with supertype "TextCommand"                                                                                      ​override
                              Superclass:
                                  def run(self, edit: Edit, *args: Any, **kwargs: Any) -> None
                              Subclass:
                                  def run(self, edit: Buffer) -> None

And

class TestTypesCommand(sublime_plugin.TextCommand):
    def run(self) -> None:
        pass
Signature of "run" incompatible with supertype "TextCommand"                                                                                      ​override
                              Superclass:
                                  def run(self, edit: Edit, *args: Any, **kwargs: Any) -> None
                              Subclass:
                                  def run(self) -> None

on my computer.

kaste avatar Nov 04 '25 12:11 kaste

True for mypy. I've been checking with pyright. I guess I'll ask for the reason in its repo.

rchl avatar Nov 04 '25 13:11 rchl

It's currently considered a bug in pyright so the earlier suggested solution looks like the way to go.

https://github.com/microsoft/pyright/issues/11092

rchl avatar Nov 04 '25 17:11 rchl