Improve `id()` types

[kaste]

Problem description

Make the id's used within the ST API new proper types (e.g. NewType) instead of using the very unspecific int. E.g. instead of View.id() -> int make it View.id() -> ViewId. See below the trivial patch.

This is so that we, as a user, don't mix-n-match these ints. E.g. we may maintain a dict from BufferId to some value; then a type checker will guarantee that we don't map a ViewId accidentally.

Preferred solution

diff --git a/python38/sublime.py b/python38/sublime.py
index 7ca324e..3ca7af5 100644
--- a/python38/sublime.py
+++ b/python38/sublime.py
@@ -10,13 +10,17 @@ import json
 import sys
 import io
 import enum
-from typing import Callable, Optional, Any, Iterator, Iterable, Literal, TYPE_CHECKING
+from typing import Callable, Optional, Any, Iterator, Iterable, Literal, NewType, TYPE_CHECKING

 import sublime_api

 if TYPE_CHECKING:
     from sublime_types import DIP, Vector, Point, Value, CommandArgs, Kind, CompletionValue

+BufferId = NewType('BufferId', int)
+ViewId = NewType('ViewId', int)
+WindowId = NewType('WindowId', int)
+

 class _LogWriter(io.TextIOBase):
     def __init__(self):
@@ -1361,7 +1365,7 @@ class Window:
     def __repr__(self) -> str:
         return f'Window({self.window_id!r})'

-    def id(self) -> int:
+    def id(self) -> WindowId:
         """
         :returns: A number that uniquely identifies this window.
         """
@@ -2546,7 +2550,7 @@ class View:
     `View.clones()` or `Buffer.views()`.
     """

-    def __init__(self, id):
+    def __init__(self, id: int):
         self.view_id = id
         self.selection = Selection(id)
         self.settings_object = None
@@ -2566,11 +2570,11 @@ class View:
     def __repr__(self) -> str:
         return f'View({self.view_id!r})'

-    def id(self) -> int:
+    def id(self) -> ViewId:
         """ :returns: A number that uniquely identifies this view. """
         return self.view_id

-    def buffer_id(self) -> int:
+    def buffer_id(self) -> BufferId:
         """
         :returns: A number that uniquely identifies this view's `Buffer`.
         """
@@ -3687,7 +3691,7 @@ class Buffer:
     def __repr__(self) -> str:
         return f'Buffer({self.buffer_id!r})'

-    def id(self) -> int:
+    def id(self) -> BufferId:
         """
         Returns a number that uniquely identifies this buffer.

Additionally consider the usage in the constructors.

@@ -1344,7 +1348,7 @@ class Window:
     """
     """
 
-    def __init__(self, id: int):
+    def __init__(self, id: WindowId):
         self.window_id = id
         self.settings_object: Optional[Settings] = None
         self.template_settings_object: Optional[Settings] = None
@@ -3675,7 +3679,7 @@ class Buffer:
     .. since:: 4081
     """

-    def __init__(self, id):
+    def __init__(self, id: BufferId):
         self.buffer_id = id

     def __hash__(self) -> int:
@@ -2546,7 +2550,7 @@ class View:
     `View.clones()` or `Buffer.views()`.
     """

-    def __init__(self, id):
+    def __init__(self, id: ViewId):
         self.view_id = id
         self.selection = Selection(id)
         self.settings_object = None

[giampaolo]

Make the id's used within the ST API new proper types (e.g. NewType) instead of using the very unspecific int. E.g. instead of View.id() -> int make it View.id() -> ViewId. See below the trivial patch.

I don't see int as "unspecific". It's pretty clear it represents an identifier. Also it's immediately clear what the type is (int), whereas with this is not clear:

BufferId = NewType('BufferId', int)
ViewId = NewType('ViewId', int)
WindowId = NewType('WindowId', int)

It looks like unnecessary complication for little to no gain.

[jfcherng]

I am more of wondering what pitfall you accidentally hit so that you filed this PR.

[deathaxe]

A specific type would reveal accidental comparisons between view ids and buffer ids for instance, both of which are not related with each other.

[jfcherng]

A specific type would reveal accidental comparisons between view ids and buffer ids for instance, both of which are not related with each other.

I know that but I just think it won't be the case that someone suddenly comes up with this without hitting a pitfall by himself. I was just wondering what pitfall he hit. A real world case he encountered when writing a plugin.

I am not against his suggestion.

[kaste]

@giampaolo You're confused and probably refer int as e.g. some c-type, or some other "underlying" type, basically describing how it is handled by the cpu or the compiler etc.

But we talk about category theory types here. The simple explanation is usually that you can't do any counting and calculation with the id()s here. (E.g. you can't view.id() // 2 or time.time() + view.id() and get something meaningful.) So the question is usually what are the rules, what's the algebra for a type.

@jfcherng I have these types since the initial commit of my stubs here in Sublime land https://github.com/SublimeLinter/SublimeLinter/blob/42dea19d0646b8ada62b05b087fe3765c9a4d8e2/stubs/sublime.pyi#L114-L114. The PR's ultimate intention is to get rid of these stubs and just refer the real implementation at some time. (For that, we also need better run types for all the commands.)