LSP icon indicating copy to clipboard operation
LSP copied to clipboard

refactor(types): Mark properties with NotRequired

Open rchl opened this issue 3 years ago • 0 comments

Make use of auto-generated types from https://github.com/sublimelsp/lsp-python-types that utilize NotRequired for properties that might not be present.

https://github.com/sublimelsp/lsp-python-types generates two separate files - lsp_types.py and lsp_types_sublime_text_33.py. Former uses new class syntax that is documentation-annotations-friendly but is not compatible with Python 3.3. Latter doesn't use the new class syntax. We have to use the latter which means that documentation for properties won't be picked up by pyright but otherwise there are no downsides.

Even though ST's 3.3 host doesn't support NotRequired (or even typing for that matter), that doesn't matter because for 3.3 the existing LSP code has a dummy type aliases that do nothing anyway. The important part is that type checkers like mypy and pyright type-check using Python version that supports those types.

Some notes about changes:

  • Our WorkspaceFolder class conflicted with same-named LSP type. Moved our class to workspace.py. Since it's a public export, plugins that imported it from LSP.plugin will be unaffected. The few that imported from LSP.core.protocol will be updated and I took care of that.
  • Mypy is not compatible with those generated types and errors with Cannot resolve name "X" (possible cyclic definition). This is seemingly already supported on master (as an opt-in) but not released yet. For now mypy is disabled due to that. I'm not sure but I don't think we can ignore type errors for specific file as if mypy can't process those types then I don't think it would be able to type check anyway.
  • Fixed various type issues with Promise.
  • Enabled type checking using pyright in tox.ini and CI and fixed ~~almost~~ all errors that it reported. ~~The remaining ones are the stupid cases like view = None # type: sublime.View that I think we shouldn't be doing as that looks and feels wrong. Haven't investigated how we could fix those yet. Potentially we can just ignore those.~~
  • I'm sure that there are places in the existing code that could be updated to use more of the generated types but that's something that can be done later, while making changes in relevant code.

rchl avatar Sep 20 '22 20:09 rchl