Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type annotation in sublime.py and sublime_plugin.py #5418

Closed
jfcherng opened this issue May 27, 2022 · 4 comments
Closed

Type annotation in sublime.py and sublime_plugin.py #5418

jfcherng opened this issue May 27, 2022 · 4 comments

Comments

@jfcherng
Copy link

jfcherng commented May 27, 2022

Description of the bug

Glad to see this in core but it just won't work at this moment.

image

  • To annotate type of list, use List[A] rather than [A].

  • To annotate type of tuple, use Tuple[A, B] rather than (A, B).

  • Union[A, B] can only be replaced by A | B as of Python 3.10 (this is fine in stubs but not in codes that actually run).

    class ListInputHandler(CommandInputHandler):
        """
        ListInputHandlers can be used to accept a choice input from a list items in
        the *Command Palette*. Return a subclass of this from `Command.input()`.
    
        *For an input handler to be shown to the user, the command returning the
        input handler MUST be made available in the Command Palette by adding the
        command to a :path:`Default.sublime-commands` file.*
        """
    
        def list_items(self) -> [str] | ([str], int) | [(str, Value)] | ([(str, Value)], int) | [ListInputItem] | ([ListInputItem], int):

    For example, the following codes won't run in py38:

    def foo(): str | sublime.Syntax: ...
    # SyntaxError: illegal target for annotation

Concerns

To be future-proof, mypy can be used to check type annotation issues. Just like how it's done in jfcherng-sublime/ST-API-stubs with configuration.

Any chance to put sublime.py and sublime_plugin.py on GitHub repo for the sake of suggesting/fixing type annotation issues? I can image that it would take a long time to make them working well by reporting issues and waiting for the next ST release if I want to deprecate jfcherng-sublime/ST-API-stubs, which is shipped in LSP-pyright.

Sublime Text build number

4134

Related Issues

@frou
Copy link

frou commented May 27, 2022

Yes, if SublimeHQ is going to start using tooling for this then please go with mypy as the source of truth for type checker results, since it's the quasi-official Python solution (Guido van Rossum is one of the developers). Not one of the off-brand tools like pyright, pytype, pyre, ...

EDIT: I retract my advocacy for Mypy over Pyright. Pyright is seriously impressive these days and seems to be maintained with a maniacally high level of quality.

@rchl
Copy link

rchl commented May 31, 2022

On the flip side, Guido van Rossum is currently working at Microsoft. Not sure where exactly but potentially close to Pyright/Pylance teams (saw some related activity from him).

That said, using mypy is probably just fine.

@FichteFoll
Copy link
Collaborator

Copying my comment from #2290 (comment):

There are a few obvious errors in the type annotations of sublime.py which is used to build the documentation now. I've made a few quick amendmends that reflect the actual behavior, but some annotations are still wrong or incomplete. Generally, the Value, Point, DIP, and CommandArgs types should be defined either as an alias or as a new type that is then explained in text (as it is currently). Some internal type aliases could proved to be useful regardless of whether they are documented as such.
Additionally, some __eq__ implementations don't check for the object's class where the initial rhs argument should be an object. A simple mypy run on the file should yield most of the remaining problems.

--- /opt/sublime_text/Lib/python38/sublime.py	Fri May 27 11:32:43 2022
+++ /opt/sublime_text/Lib/python38/sublime.py	Tue Jun  7 12:43:14 2022
@@ -10,7 +10,7 @@
 import sys
 import io
 import enum
-from typing import Callable, Optional, Any, Iterator, Union, Dict, List, Tuple
+from typing import Callable, Optional, Any, Iterator, Union, Dict, List, Tuple, Literal
 
 import sublime_api
 
@@ -777,7 +777,7 @@
 
 def open_dialog(
         callback: Callable[[Union[str, List[str], None]], None],
-        file_types: [(str, [str])] = [],
+        file_types: List[Tuple[str, List[str]]] = [],
         directory: Optional[str] = None,
         multi_select=False,
         allow_folders=False):
@@ -812,7 +812,7 @@
 
 def save_dialog(
         callback: Callable[[Union[str, None]], None],
-        file_types: [(str, [str])] = [],
+        file_types: List[Tuple[str, List[str]]] = [],
         directory: Optional[str] = None,
         name: Optional[str] = None,
         extension: Optional[str] = None):
@@ -1119,7 +1119,7 @@
     return bytes
 
 
-def find_resources(pattern: str) -> [str]:
+def find_resources(pattern: str) -> List[str]:
     """
     Finds resources whose file name matches the given glob pattern.
     """
@@ -1204,14 +1204,14 @@
     return Window(sublime_api.active_window())
 
 
-def windows() -> [Window]:
+def windows() -> List[Window]:
     """
     :returns: A list of all the open windows.
     """
     return [Window(id) for id in sublime_api.windows()]
 
 
-def get_macro() -> [dict]:
+def get_macro() -> List[Dict[str, Value]]:
     """
     :returns: A list of the commands and args that compromise the currently
               recorded macro. Each ``dict`` will contain the keys ``"command"``
@@ -1226,8 +1226,8 @@
 
     def __init__(self, id: int):
         self.window_id = id
-        self.settings_object = None
-        self.template_settings_object = None
+        self.settings_object: Optional[Settings] = None
+        self.template_settings_object: Optional[Settings] = None
 
     def __hash__(self) -> int:
         return self.window_id
@@ -1338,7 +1338,7 @@
         else:
             return View(view_id)
 
-    def file_history(self) -> [str]:
+    def file_history(self) -> List[str]:
         """
         Get the list of previously opened files. This is the same list
         as *File > Open Recent*.
@@ -1377,7 +1377,7 @@
         if view:
             sublime_api.window_focus_view(self.window_id, view.view_id)
 
-    def select_sheets(self, sheets: [Sheet]):
+    def select_sheets(self, sheets: List[Sheet]):
         """
         Change the selected sheets for the entire window.
 
@@ -1391,7 +1391,7 @@
         """
         sublime_api.window_bring_to_front(self.window_id)
 
-    def get_sheet_index(self, sheet: Sheet) -> (int, int):
+    def get_sheet_index(self, sheet: Sheet) -> Tuple[int, int]:
         """
         :returns: The a tuple of the group and index within the group of the
                   given `Sheet`.
@@ -1401,7 +1401,7 @@
         else:
             return (-1, -1)
 
-    def get_view_index(self, view: View) -> (int, int):
+    def get_view_index(self, view: View) -> Tuple[int, int]:
         """
         :returns: The a tuple of the group and index within the group of the
                   given `View`.
@@ -1423,7 +1423,7 @@
         """
         sublime_api.window_set_view_index(self.window_id, view.view_id, group, index)
 
-    def move_sheets_to_group(self, sheets: [Sheet], group: int, insertion_idx = -1, select = True):
+    def move_sheets_to_group(self, sheets: List[Sheet], group: int, insertion_idx = -1, select = True):
         """
         Moves all provided sheets to specified group at insertion index
         provided. If an index is not provided defaults to last index of the
@@ -1444,14 +1444,14 @@
             sheet_ids.append(sheet.id())
         sublime_api.window_move_sheets_to_group(self.window_id, sheet_ids, group, insertion_idx, select)
 
-    def sheets(self) -> [Sheet]:
+    def sheets(self) -> List[Sheet]:
         """
         :returns: All open sheets in the window.
         """
         sheet_ids = sublime_api.window_sheets(self.window_id)
         return [make_sheet(x) for x in sheet_ids]
 
-    def views(self, *, include_transient=False) -> [View]:
+    def views(self, *, include_transient=False) -> List[View]:
         """
         :param include_transient: Whether the transient sheet should be included.
 
@@ -1461,7 +1461,7 @@
         view_ids = sublime_api.window_views(self.window_id, include_transient)
         return [View(x) for x in view_ids]
 
-    def selected_sheets(self) -> [Sheet]:
+    def selected_sheets(self) -> List[Sheet]:
         """
         .. since:: 4083
 
@@ -1470,7 +1470,7 @@
         sheet_ids = sublime_api.window_selected_sheets(self.window_id)
         return [make_sheet(s) for s in sheet_ids]
 
-    def selected_sheets_in_group(self, group: int) -> [Sheet]:
+    def selected_sheets_in_group(self, group: int) -> List[Sheet]:
         """
         .. since:: 4083
 
@@ -1499,21 +1499,21 @@
         else:
             return View(view_id)
 
-    def sheets_in_group(self, group: int) -> [Sheet]:
+    def sheets_in_group(self, group: int) -> List[Sheet]:
         """
         :returns: A list of all sheets in the specified group.
         """
         sheet_ids = sublime_api.window_sheets_in_group(self.window_id, group)
         return [make_sheet(x) for x in sheet_ids]
 
-    def views_in_group(self, group: int) -> [View]:
+    def views_in_group(self, group: int) -> List[View]:
         """
         :returns: A list of all views in the specified group.
         """
         view_ids = sublime_api.window_views_in_group(self.window_id, group)
         return [View(x) for x in view_ids]
 
-    def transient_sheet_in_group(self, group: int) -> Sheet:
+    def transient_sheet_in_group(self, group: int) -> Optional[Sheet]:
         """
         :returns: The transient sheet in the specified group.
         """
@@ -1523,7 +1523,7 @@
         else:
             return None
 
-    def transient_view_in_group(self, group: int) -> View:
+    def transient_view_in_group(self, group: int) -> Optional[View]:
         """
         :returns: The transient view in the specified group.
         """
@@ -1533,7 +1533,7 @@
         else:
             return None
 
-    def layout(self) -> Dict[str, sublime.Value]:
+    def layout(self) -> Dict[str, Value]:
         """
         Get the group layout of the window.
         """
@@ -1545,7 +1545,7 @@
         """
         return sublime_api.window_get_layout(self.window_id)
 
-    def set_layout(self, layout: Dict[str, sublime.Value]):
+    def set_layout(self, layout: Dict[str, Value]):
         """
         Set the group layout of the window.
         """
@@ -1563,7 +1563,7 @@
         """
         return View(sublime_api.window_create_output_panel(self.window_id, name, unlisted))
 
-    def find_output_panel(self, name: str) -> View:
+    def find_output_panel(self, name: str) -> Optional[View]:
         """
         :returns:
             The `View` associated with the named output panel, or ``None`` if
@@ -1587,7 +1587,7 @@
         name = sublime_api.window_active_panel(self.window_id)
         return name or None
 
-    def panels(self) -> [str]:
+    def panels(self) -> List[str]:
         """
         Returns a list of the names of all panels that have not been marked as
         unlisted. Includes certain built-in panels in addition to output
@@ -1810,28 +1810,28 @@
 
         return sublime_api.window_symbol_locations(self.window_id, sym, source, type, kind_id, letter)
 
-    def lookup_symbol_in_index(self, symbol: str) -> [SymbolLocation]:
+    def lookup_symbol_in_index(self, symbol: str) -> List[SymbolLocation]:
         """
         :returns: All locations where the symbol is defined across files in the current project.
         :deprecated: Use `symbol_locations()` instead.
         """
         return sublime_api.window_lookup_symbol(self.window_id, symbol)
 
-    def lookup_symbol_in_open_files(self, symbol: str) -> [SymbolLocation]:
+    def lookup_symbol_in_open_files(self, symbol: str) -> List[SymbolLocation]:
         """
         :returns: All locations where the symbol is defined across open files.
         :deprecated: Use `symbol_locations()` instead.
         """
         return sublime_api.window_lookup_symbol_in_open_files(self.window_id, symbol)
 
-    def lookup_references_in_index(self, symbol: str) -> [SymbolLocation]:
+    def lookup_references_in_index(self, symbol: str) -> List[SymbolLocation]:
         """
         :returns: All locations where the symbol is referenced across files in the current project.
         :deprecated: Use `symbol_locations()` instead.
         """
         return sublime_api.window_lookup_references(self.window_id, symbol)
 
-    def lookup_references_in_open_files(self, symbol: str) -> [SymbolLocation]:
+    def lookup_references_in_open_files(self, symbol: str) -> List[SymbolLocation]:
         """
         :returns: All locations where the symbol is referenced across open files.
         :deprecated: Use `symbol_locations()` instead.
@@ -1932,7 +1932,7 @@
         """ :returns: The size of the region. """
         return self.size()
 
-    def __eq__(self, rhs: Region) -> bool:
+    def __eq__(self, rhs: object) -> bool:
         """
         :returns: Whether the two regions are identical. Ignores ``xpos``.
         """
@@ -1971,7 +1971,7 @@
                 "in <Region> requires int or Region as left operand"
                 f", not {fq_name}")
 
-    def to_tuple(self) -> (Point, Point):
+    def to_tuple(self) -> Tuple[Point, Point]:
         """
         .. since:: 4075
 
@@ -2228,7 +2228,7 @@
     def __hash__(self) -> int:
         return self.sheet_id
 
-    def __eq__(self, other: Sheet) -> bool:
+    def __eq__(self, other: object) -> bool:
         return isinstance(other, Sheet) and other.sheet_id == self.sheet_id
 
     def __repr__(self) -> str:
@@ -2402,7 +2402,7 @@
     def __hash__(self) -> int:
         return self.view_id
 
-    def __eq__(self, other: View) -> bool:
+    def __eq__(self, other: object) -> bool:
         return isinstance(other, View) and other.view_id == self.view_id
 
     def __bool__(self) -> bool:
@@ -2489,7 +2489,7 @@
         """
         return sublime_api.view_is_primary(self.view_id)
 
-    def window(self) -> Optional[None]:
+    def window(self) -> Optional[Window]:
         """
         :returns: A reference to the window containing the view, if any.
         """
@@ -2499,7 +2499,7 @@
         else:
             return Window(window_id)
 
-    def clones(self) -> [View]:
+    def clones(self) -> List[View]:
         """ :returns: All the other views into the same `Buffer`. See `View`. """
         return list(map(View, sublime_api.view_clones(self.view_id)))
 
@@ -2658,7 +2658,7 @@
         """
         return sublime_api.view_change_count(self.view_id)
 
-    def change_id(self) -> (int, int, int):
+    def change_id(self) -> Tuple[int, int, int]:
         """
         Get a 3-element tuple that can be passed to `transform_region_from()` to
         obtain a region equivalent to a region of the view in the past. This is
@@ -2668,7 +2668,7 @@
         """
         return sublime_api.view_change_id(self.view_id)
 
-    def transform_region_from(self, region: Region, change_id: (int, int, int)) -> Region:
+    def transform_region_from(self, region: Region, change_id: Tuple[int, int, int]) -> Region:
         """
         Transforms a region from a previous point in time to an equivalent
         region in the current state of the View. The ``change_id`` must have
@@ -2708,7 +2708,7 @@
         """
         return sublime_api.view_find(self.view_id, pattern, start_pt, flags)
 
-    def find_all(self, pattern: str, flags=FindFlags.NONE, fmt: Optional[str] = None, extractions: Optional[List[str]] = None) -> [Region]:
+    def find_all(self, pattern: str, flags=FindFlags.NONE, fmt: Optional[str] = None, extractions: Optional[List[str]] = None) -> List[Region]:
         """
         :param pattern: The regex or literal pattern to search by.
         :param flags: Controls various behaviors of find. See `FindFlags`.
@@ -2747,7 +2747,7 @@
         """
         return sublime_api.view_meta_info(self.view_id, key, pt)
 
-    def extract_tokens_with_scopes(self, region: Region) -> List[(Region, str)]:
+    def extract_tokens_with_scopes(self, region: Region) -> List[Tuple[Region, str]]:
         """
         :param region: The region from which to extract tokens and scopes.
         :returns: A list of pairs containing the `Region` and the scope of each token.
@@ -2865,7 +2865,7 @@
         """
         return sublime_api.view_lines(self.view_id, region)
 
-    def split_by_newlines(self, region: Region) -> [Region]:
+    def split_by_newlines(self, region: Region) -> List[Region]:
         """
         Splits the region up such that each `Region` returned exists on exactly
         one line.
@@ -2939,14 +2939,14 @@
         else:
             return sublime_api.view_expand_by_class(self.view_id, x, x, classes, separators, sub_word_separators)
 
-    def rowcol(self, tp: Point) -> (int, int):
+    def rowcol(self, tp: Point) -> Tuple[int, int]:
         """
         Calculates the 0-based line and column numbers of the point. Column
         numbers are returned as number of Unicode characters.
         """
         return sublime_api.view_row_col(self.view_id, tp)
 
-    def rowcol_utf8(self, tp: Point) -> (int, int):
+    def rowcol_utf8(self, tp: Point) -> Tuple[int, int]:
         """
         Calculates the 0-based line and column numbers of the point. Column
         numbers are returned as UTF-8 code units.
@@ -2955,7 +2955,7 @@
         """
         return sublime_api.view_row_col_utf8(self.view_id, tp)
 
-    def rowcol_utf16(self, tp: Point) -> (int, int):
+    def rowcol_utf16(self, tp: Point) -> Tuple[int, int]:
         """
         Calculates the 0-based line and column numbers of the point. Column
         numbers are returned as UTF-16 code units.
@@ -3118,8 +3118,8 @@
         else:
             return sublime_api.view_unfold_regions(self.view_id, x)
 
-    def add_regions(self, key: str, regions: [Region], scope="",
-                    icon="", flags=RegionFlags.NONE, annotations: [str] = [],
+    def add_regions(self, key: str, regions: List[Region], scope="",
+                    icon="", flags=RegionFlags.NONE, annotations: List[str] = [],
                     annotation_color="",
                     on_navigate: Optional[Callable[[str], None]] = None,
                     on_close: Optional[Callable[[], None]] = None):
@@ -3192,7 +3192,7 @@
         sublime_api.view_add_regions(
             self.view_id, key, regions, scope, icon, flags, annotations, annotation_color, on_navigate, on_close)
 
-    def get_regions(self, key: str) -> [Region]:
+    def get_regions(self, key: str) -> List[Region]:
         """
         :returns: The regions associated with the given ``key``, if any.
         """
@@ -3214,10 +3214,10 @@
     def erase_phantom_by_id(self, pid: int):
         sublime_api.view_erase_phantom(self.view_id, pid)
 
-    def query_phantom(self, pid: int) -> [Region]:
+    def query_phantom(self, pid: int) -> List[Region]:
         return sublime_api.view_query_phantoms(self.view_id, [pid])
 
-    def query_phantoms(self, pids: [int]) -> [Region]:
+    def query_phantoms(self, pids: List[int]) -> List[Region]:
         return sublime_api.view_query_phantoms(self.view_id, pids)
 
     def assign_syntax(self, syntax: Union[str, Syntax]):
@@ -3322,13 +3322,13 @@
         """
         return sublime_api.view_extract_completions(self.view_id, prefix, tp)
 
-    def find_all_results(self) -> [(str, int, int)]:
+    def find_all_results(self) -> List[Tuple[str, int, int]]:
         return sublime_api.view_find_all_results(self.view_id)
 
-    def find_all_results_with_text(self) -> [(str, int, int, str)]:
+    def find_all_results_with_text(self) -> List[Tuple[str, int, int, str]]:
         return sublime_api.view_find_all_results_with_text(self.view_id)
 
-    def command_history(self, index: int, modifying_only=False) -> (str, CommandArgs, int):
+    def command_history(self, index: int, modifying_only=False) -> Tuple[str, CommandArgs, int]:
         """
         Get info on previous run commands stored in the undo/redo stack.
 
@@ -3355,7 +3355,7 @@
         """ Set the overwrite status. See `overwrite_status()`. """
         sublime_api.view_set_overwrite_status(self.view_id, value)
 
-    def show_popup_menu(self, items: [str], on_done: Callable[[int], None], flags=0):
+    def show_popup_menu(self, items: List[str], on_done: Callable[[int], None], flags=0):
         """
         Show a popup menu at the caret, for selecting an item in a list.
 
@@ -3490,7 +3490,7 @@
     def __hash__(self) -> int:
         return self.buffer_id
 
-    def __eq__(self, other: Buffer) -> bool:
+    def __eq__(self, other: object) -> bool:
         return isinstance(other, Buffer) and self.buffer_id == other.buffer_id
 
     def __repr__(self) -> str:
@@ -3517,7 +3517,7 @@
         else:
             return name
 
-    def views(self) -> [View]:
+    def views(self) -> List[View]:
         """
         Returns a list of all views that are associated with this buffer.
         """
@@ -3695,7 +3695,7 @@
         return (f'Phantom({self.region!r}, {self.content!r}, '
                 f'{self.layout!r}, on_navigate={self.on_navigate!r})')
 
-    def to_tuple(self) -> ((Point, Point), str, PhantomLayout, Optional[Callable[[str], None]]):
+    def to_tuple(self) -> Tuple[Tuple[Point, Point], str, PhantomLayout, Optional[Callable[[str], None]]]:
         """
         Returns a tuple of this phantom containing the region, content, layout
         and callback.
@@ -3941,7 +3941,7 @@
             details)
 
 
-def list_syntaxes() -> [Syntax]:
+def list_syntaxes() -> List[Syntax]:
     """ List all known syntaxes.
 
     Returns a list of Syntax.
@@ -3957,7 +3957,7 @@
     return sublime_api.get_syntax(path)
 
 
-def find_syntax_by_name(name: str) -> [Syntax]:
+def find_syntax_by_name(name: str) -> List[Syntax]:
     """ Find syntaxes with the specified name.
 
     Name must match exactly. Return a list of Syntax.
@@ -3965,7 +3965,7 @@
     return [syntax for syntax in list_syntaxes() if syntax.name == name]
 
 
-def find_syntax_by_scope(scope: str) -> [Syntax]:
+def find_syntax_by_scope(scope: str) -> List[Syntax]:
     """ Find syntaxes with the specified scope.
 
     Scope must match exactly. Return a list of Syntax.
@@ -4009,7 +4009,7 @@
         self.scope: str = scope
         """ The base scope name of the syntax. """
 
-    def __eq__(self, other: Syntax) -> bool:
+    def __eq__(self, other: object) -> bool:
         return isinstance(other, Syntax) and self.path == other.path
 
     def __hash__(self) -> int:

@deathaxe
Copy link
Collaborator

deathaxe commented Nov 8, 2022

Typing has been fixed in ST 4135/4137

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants