Skip to content

Conversation

amjith
Copy link
Member

@amjith amjith commented Aug 17, 2025

Description

Add mypy based type checking.

Thanks to @rolandwalker for adding type hints to mycli. I've used that as inspiration and reference to do the same for litecli.

Checklist

  • I've added this contribution to the CHANGELOG.md file.

amjith added 19 commits August 17, 2025 13:44
- Introduce packages/special/types.py with DBCursor Protocol
- Add/ tighten type hints across special/, sqlcompleter, sqlexecute, clibuffer/toolbar/style, config, filepaths, lexer, completion_refresher, key_bindings
- Remove remaining mypy ignore headers in special modules
- Replace Any with concrete types where safe; improve TextIO handling
- Fix utils.check_if_sqlitedotcommand to handle non-str inputs safely
- Remove mycli references in comments and update docstrings
- Minor refactors for clarity; ruff+mypy clean
…o implicit Optional, warn on Any). Note: run with --explicit-package-bases or via tox.

import logging
from typing import Dict, List, Tuple

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When using import annotations in Python 3.9, and for any later Python without the import, dict, list, and tuple can be used in their lowercased forms, without importing from typing.

@@ -1,3 +1,6 @@
# type: ignore

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this #type: ignore needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had added mypy: ignore-errors to all the files before I started. Looks like I left them in by mistake.

@@ -1,17 +1,19 @@
from __future__ import unicode_literals
# mypy: ignore-errors

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is mypy: ignore-errors doing for us?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had added mypy: ignore-errors to all the files before I started. Looks like I left them in by mistake.

else:
return expanduser("~/.config/litecli/")


def load_config(usr_cfg, def_cfg=None):
def load_config(usr_cfg: str, def_cfg: Optional[str] = None) -> ConfigObj:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str | None is preferred by most over Optional[str].

@@ -1,24 +1,31 @@
from __future__ import unicode_literals

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few changes here unrelated to type hinting.

@@ -16,7 +27,7 @@
}


def last_word(text, include="alphanum_underscore"):
def last_word(text: str, include: str = "alphanum_underscore") -> str:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type of include could even be a Literal of the keys of cleanup_regex.

@@ -187,7 +207,7 @@ def list_indexes(cur, arg=None, arg_type=PARSED_QUERY, verbose=False):
aliases=("\\s",),
case_sensitive=True,
)
def status(cur, **_):
def status(cur: DBCursor, **_: Any) -> List[Tuple]:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of style, there isn't much purpose in typing **_: Any. But it does no harm.


log = logging.getLogger(__name__)


@export
def set_favorite_queries(config):
def set_favorite_queries(config: Any) -> None:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Any the best we can do for config?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking at mycli for this and it wasn't typed so I just went to Any.

Copy link

@rolandwalker rolandwalker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would highly recommend replacing the capitalized versions of types with lowercased versions for builtins such as tuple. Python 3.9 will be EOL soon, and we even can remove from __future__ import annotations if support for 3.9 is removed.

@kracekumar
Copy link
Contributor

I will take a look at this pr over the weekend.

Copy link
Contributor

@kracekumar kracekumar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm as a first step to introduce the type hints. We can improve over time 👍🏾

from prompt_toolkit.application import get_app


def cli_is_multiline(cli):
def cli_is_multiline(cli: Any) -> Filter:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand sometimes types can be complicated to express so Any is preferred. Is it possible to add type here? If not documenting reason to use Any can be useful later to replace it with better type. I suspect cli by default is of the type, Any.

@@ -32,10 +41,11 @@ def refresh(self, executor, callbacks, completer_options=None):
self._restart_refresh.set()
return [(None, None, None, "Auto-completion refresh restarted.")]
else:
if executor.dbname == ":memory:":
if executor.dbname == ":memory":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one seems non-type change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use something like executor.dbname.startswith(":memory")?

* False if the query is destructive and the user doesn't want to proceed.
def convert(self, value: bool | str, param: click.Parameter | None, ctx: click.Context | None) -> bool:
if isinstance(value, bool):
return bool(value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just return value right like return value right?

@@ -1,3 +1,5 @@
# mypy: ignore-errors
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to ignore all the errors in the directory rather than at each file in test directory?

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

Successfully merging this pull request may close these issues.

3 participants