Skip to content

Commit

Permalink
Apply repo-review MyPy suggestions
Browse files Browse the repository at this point in the history
MY103: MyPy warn unreachable

Must have `warn_unreachable` (true/false) to pass this check. There are
occasionally false positives (often due to platform or Python version
static checks), so it's okay to set it to false if you need to. But try
it first - it can catch real bugs too.

MY104: MyPy enables ignore-without-code

Must have `"ignore-without-code"` in `enable_error_code = [...]`. This
will force all skips in your project to include the error code, which
makes them more readable, and avoids skipping something unintended.

MY105: MyPy enables redundant-expr

Must have `"redundant-expr"` in `enable_error_code = [...]`. This helps
catch useless lines of code, like checking the same condition twice.

MY106: MyPy enables truthy-bool

Must have `"truthy-bool"` in `enable_error_code = []`. This catches
mistakes in using a value as truthy if it cannot be falsy.
  • Loading branch information
DimitriPapadopoulos committed Aug 24, 2024
1 parent 2b92ca6 commit 766f8c4
Show file tree
Hide file tree
Showing 10 changed files with 22 additions and 18 deletions.
4 changes: 4 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ title_format = "## [{version}](https://github.com/pypa/pipx/tree/{version}) - {p
issue_format = "[#{issue}](https://github.com/pypa/pipx/issues/{issue})"
package = "pipx"

[tool.mypy]
warn_unreachable = true
enable_error_code = [ "ignore-without-code", "redundant-expr", "truthy-bool" ]

[[tool.mypy.overrides]]
module = [
"pycowsay.*",
Expand Down
2 changes: 1 addition & 1 deletion src/pipx/animate.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def print_animation(
def win_cursor(visible: bool) -> None:
if sys.platform != "win32": # hello mypy
return
ci = _CursorInfo()
ci = _CursorInfo() # type: ignore[unreachable]
handle = ctypes.windll.kernel32.GetStdHandle(-11)
ctypes.windll.kernel32.GetConsoleCursorInfo(handle, ctypes.byref(ci))
ci.visible = visible
Expand Down
2 changes: 1 addition & 1 deletion src/pipx/commands/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def environment(value: str) -> ExitCode:
"PIPX_HOME_ALLOW_SPACE": str(paths.ctx.allow_spaces_in_home_path).lower(),
}
if value is None:
print("Environment variables (set by user):")
print("Environment variables (set by user):") # type: ignore[unreachable]
print("")
for env_variable in ENVIRONMENT_VARIABLES:
env_value = os.getenv(env_variable, "")
Expand Down
2 changes: 1 addition & 1 deletion src/pipx/commands/inject.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def inject_dep(
) -> bool:
logger.debug("Injecting package %s", package_spec)

if not venv_dir.exists() or not next(venv_dir.iterdir()):
if not venv_dir.exists() or next(venv_dir.iterdir(), None) is None:
raise PipxError(
f"""
Can't inject {package_spec!r} into nonexistent Virtual Environment
Expand Down
4 changes: 2 additions & 2 deletions src/pipx/commands/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,11 @@ def run(
# we can't parse this as a package
package_name = app

content = None if spec is not None else maybe_script_content(app, is_path)
content = None if spec is not None else maybe_script_content(app, is_path) # type: ignore[redundant-expr]
if content is not None:
run_script(content, app_args, python, pip_args, venv_args, verbose, use_cache)
else:
package_or_url = spec if spec is not None else app
package_or_url = spec if spec is not None else app # type: ignore[redundant-expr]
run_package(
package_name,
package_or_url,
Expand Down
2 changes: 1 addition & 1 deletion src/pipx/commands/uninject.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def uninject(
) -> ExitCode:
"""Returns pipx exit code"""

if not venv_dir.exists() or not next(venv_dir.iterdir()):
if not venv_dir.exists() or next(venv_dir.iterdir(), None) is None:
raise PipxError(f"Virtual environment {venv_dir.name} does not exist.")

venv = Venv(venv_dir, verbose=verbose)
Expand Down
4 changes: 2 additions & 2 deletions src/pipx/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
not args.no_cache,
)
# We should never reach here because run() is NoReturn.
return ExitCode(1)
return ExitCode(1) # type: ignore[unreachable]
elif args.command == "install":
return commands.install(
None,
Expand Down Expand Up @@ -409,7 +409,7 @@ def run_pipx_command(args: argparse.Namespace, subparsers: Dict[str, argparse.Ar
python_flag_passed=python_flag_passed,
)
elif args.command == "runpip":
if not venv_dir:
if not venv_dir: # type: ignore[truthy-bool]
raise PipxError("Developer error: venv_dir is not defined.")
return commands.run_pip(package, venv_dir, args.pipargs, args.verbose)
elif args.command == "ensurepath":
Expand Down
14 changes: 7 additions & 7 deletions src/pipx/paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,25 +107,25 @@ def shared_libs(self) -> Path:
return (self._base_shared_libs or self.home / "shared").resolve()

def make_local(self) -> None:
self._base_home = OVERRIDE_PIPX_HOME or get_expanded_environ("PIPX_HOME")
self._base_home = OVERRIDE_PIPX_HOME or get_expanded_environ("PIPX_HOME") # type: ignore[redundant-expr]
self._default_home = DEFAULT_PIPX_HOME
self._base_bin = OVERRIDE_PIPX_BIN_DIR or get_expanded_environ("PIPX_BIN_DIR")
self._base_bin = OVERRIDE_PIPX_BIN_DIR or get_expanded_environ("PIPX_BIN_DIR") # type: ignore[redundant-expr]
self._default_bin = DEFAULT_PIPX_BIN_DIR
self._base_man = OVERRIDE_PIPX_MAN_DIR or get_expanded_environ("PIPX_MAN_DIR")
self._base_man = OVERRIDE_PIPX_MAN_DIR or get_expanded_environ("PIPX_MAN_DIR") # type: ignore[redundant-expr]
self._default_man = DEFAULT_PIPX_MAN_DIR
self._base_shared_libs = OVERRIDE_PIPX_SHARED_LIBS or get_expanded_environ("PIPX_SHARED_LIBS")
self._base_shared_libs = OVERRIDE_PIPX_SHARED_LIBS or get_expanded_environ("PIPX_SHARED_LIBS") # type: ignore[redundant-expr]
self._default_log = Path(user_log_path("pipx"))
self._default_cache = Path(user_cache_path("pipx"))
self._default_trash = self._default_home / "trash"
self._fallback_home = next(iter([fallback for fallback in FALLBACK_PIPX_HOMES if fallback.exists()]), None)
self._home_exists = self._base_home is not None or any(fallback.exists() for fallback in FALLBACK_PIPX_HOMES)

def make_global(self) -> None:
self._base_home = OVERRIDE_PIPX_GLOBAL_HOME or get_expanded_environ("PIPX_GLOBAL_HOME")
self._base_home = OVERRIDE_PIPX_GLOBAL_HOME or get_expanded_environ("PIPX_GLOBAL_HOME") # type: ignore[redundant-expr]
self._default_home = DEFAULT_PIPX_GLOBAL_HOME
self._base_bin = OVERRIDE_PIPX_GLOBAL_BIN_DIR or get_expanded_environ("PIPX_GLOBAL_BIN_DIR")
self._base_bin = OVERRIDE_PIPX_GLOBAL_BIN_DIR or get_expanded_environ("PIPX_GLOBAL_BIN_DIR") # type: ignore[redundant-expr]
self._default_bin = DEFAULT_PIPX_GLOBAL_BIN_DIR
self._base_man = OVERRIDE_PIPX_GLOBAL_MAN_DIR or get_expanded_environ("PIPX_GLOBAL_MAN_DIR")
self._base_man = OVERRIDE_PIPX_GLOBAL_MAN_DIR or get_expanded_environ("PIPX_GLOBAL_MAN_DIR") # type: ignore[redundant-expr]
self._default_man = DEFAULT_PIPX_GLOBAL_MAN_DIR
self._default_log = self._default_home / "logs"
self._default_cache = self._default_home / ".cache"
Expand Down
2 changes: 1 addition & 1 deletion src/pipx/shared_libs.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ def upgrade(self, *, pip_args: List[str], verbose: bool = False, raises: bool =
return

if pip_args is None:
pip_args = []
pip_args = [] # type: ignore[unreachable]

logger.info(f"Upgrading shared libraries in {self.root}")

Expand Down
4 changes: 2 additions & 2 deletions tests/test_upgrade_shared.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def test_upgrade_shared(shared_libs, capsys, caplog):
assert "Upgrading shared libraries in" in caplog.text
assert "Already upgraded libraries in" not in caplog.text
assert shared_libs.has_been_updated_this_run is True
assert shared_libs.is_valid is True
assert shared_libs.is_valid is True # type: ignore[unreachable]
shared_libs.has_been_updated_this_run = False
assert run_pipx_cli(["upgrade-shared", "-v"]) == 0
captured = capsys.readouterr()
Expand Down Expand Up @@ -59,7 +59,7 @@ def pip_version():
assert shared_libs.is_valid is False
assert run_pipx_cli(["upgrade-shared", "-v", "--pip-args=pip==24.0"]) == 0
assert shared_libs.is_valid is True
assert pip_version() == "24.0"
assert pip_version() == "24.0" # type: ignore[unreachable]
shared_libs.has_been_updated_this_run = False # reset for next run
assert run_pipx_cli(["upgrade-shared", "-v", "--pip-args=pip==23.3.2"]) == 0
assert shared_libs.is_valid is True
Expand Down

0 comments on commit 766f8c4

Please sign in to comment.