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

style(type): stricter type checking #1908

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions charmcraft/application/commands/analyse.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,13 @@ def run(self, parsed_args: argparse.Namespace) -> int:
return self._run_formatted(parsed_args.filepath, ignore=ignore)
return self._run_streaming(parsed_args.filepath, ignore=ignore)

def _run_formatted(self, filepath: pathlib.Path, *, ignore=Container[str]) -> int:
def _run_formatted(self, filepath: pathlib.Path, *, ignore: Container[str]) -> int:
"""Run the command, formatting the output into JSON or similar at the end."""
results = list(self._services.analysis.lint_file(filepath))
emit.message(json.dumps(results, indent=4, default=pydantic_encoder))
return max(r.level for r in results).return_code

def _run_streaming(self, filepath: pathlib.Path, *, ignore=Container[str]) -> int:
def _run_streaming(self, filepath: pathlib.Path, *, ignore: Container[str]) -> int:
"""Run the command, printing linter results as we get them."""
max_level = lint.ResultLevel.OK
with emit.progress_bar(
Expand Down
2 changes: 1 addition & 1 deletion charmcraft/application/commands/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ def run(self, parsed_args: argparse.Namespace) -> None:
for package_type, title in [("charm", "charms"), ("bundle", "bundles")]:
if package_type in grouped:
human_msgs.append(f"{title}:")
pkg_info = []
pkg_info: list[dict[str, str]] = []
for item in grouped[package_type]:
if (name := item.get("name")) is not None:
human_msgs.append(f"- name: {name}")
Expand Down
4 changes: 1 addition & 3 deletions charmcraft/charm_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ def __init__(
builddir: pathlib.Path,
installdir: pathlib.Path,
entrypoint: pathlib.Path,
allow_pip_binary: bool = None,
binary_python_packages: list[str] | None = None,
python_packages: list[str] | None = None,
requirements: list[pathlib.Path] | None = None,
Expand All @@ -69,7 +68,6 @@ def __init__(
self.builddir = builddir
self.installdir = installdir
self.entrypoint = entrypoint
self.allow_pip_binary = allow_pip_binary
self.binary_python_packages = binary_python_packages or []
self.python_packages = python_packages or []
self.requirement_paths = requirements or []
Expand Down Expand Up @@ -481,4 +479,4 @@ def main():
if __name__ == "__main__":
with instrum.Timer("Full charm_builder.py main"):
main()
instrum.dump(get_charm_builder_metrics_path())
instrum.dump(get_charm_builder_metrics_path().as_posix())
2 changes: 1 addition & 1 deletion charmcraft/extensions/_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def _apply_extension(


def _apply_extension_property(
existing_property: dict | list, extension_property: dict | list
existing_property: dict | list | None, extension_property: dict | list
) -> dict | list:
if existing_property:
# If the property is not scalar, merge them
Expand Down
7 changes: 4 additions & 3 deletions charmcraft/instrum.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2022 Canonical Ltd.
# Copyright 2022,2024 Canonical Ltd.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
"""Provide utilities to measure performance in different parts of the app."""

import json
import pathlib
import uuid
from time import time
from typing import Any
Expand All @@ -34,7 +35,7 @@ def __init__(self):
# ancestors list when a measure starts (last item is direct parent); the
# first value is special, None, to reflect the "root", the rest are
# measurement ids
self.parents = [None] # start with a unique "root"
self.parents: list[str | None] = [None] # start with a unique "root"

# simple dict to hold measurements information; the key is the measurement
# id and each value holds all it info
Expand Down Expand Up @@ -74,7 +75,7 @@ def dump(self, filename: str) -> None:
with open(filename, "w") as fh:
json.dump(measurements, fh, indent=4)

def merge_from(self, filename: str) -> None:
def merge_from(self, filename: str | pathlib.Path) -> None:
"""Merge measurements from a file to the current ongoing structure."""
with open(filename) as fh:
to_merge = json.load(fh)
Expand Down
2 changes: 1 addition & 1 deletion charmcraft/jujuignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def __init__(
orig_rule: str,
invert: bool,
only_dirs: bool,
regex: typing.Pattern,
regex: re.Pattern[str] | str,
):
self.line_num = line_num
self.orig_rule = orig_rule
Expand Down
6 changes: 4 additions & 2 deletions charmcraft/models/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
# For further info, check https://github.com/canonical/charmcraft
"""Project-related models for Charmcraft."""

from __future__ import annotations

import abc
import datetime
import pathlib
Expand Down Expand Up @@ -433,7 +435,7 @@ def started_at(self) -> datetime.datetime:
return self._started_at

@classmethod
def unmarshal(cls, data: dict[str, Any]):
def unmarshal(cls, data: dict[str, Any]) -> CharmcraftProject:
"""Create a Charmcraft project from a dictionary of data."""
if cls is not CharmcraftProject:
return cls.model_validate(data)
Expand All @@ -447,7 +449,7 @@ def unmarshal(cls, data: dict[str, Any]):
raise ValueError(f"field type cannot be {project_type!r}")

@classmethod
def from_yaml_file(cls, path: pathlib.Path) -> Self:
def from_yaml_file(cls, path: pathlib.Path) -> CharmcraftProject:
"""Instantiate this model from a YAML file.

For use with craft-application.
Expand Down
4 changes: 3 additions & 1 deletion charmcraft/parts/plugins/_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from contextlib import suppress
from typing import Literal, cast

import craft_parts
import overrides
import pydantic
from craft_parts import Step, callbacks, plugins
Expand Down Expand Up @@ -346,9 +347,10 @@ def _get_legacy_dependencies_parameters(self) -> list[str]:

return parameters

def post_build_callback(self, step_info):
def post_build_callback(self, step_info: craft_parts.StepInfo) -> Literal[False]:
"""Collect metrics left by charm_builder.py."""
instrum.merge_from(env.get_charm_builder_metrics_path())
return False

def _get_os_special_priority_paths(self) -> str | None:
"""Return a str of PATH for special OS."""
Expand Down
4 changes: 2 additions & 2 deletions charmcraft/store/store.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def _build_errors(item):

def _build_revision(item: dict[str, Any]) -> Revision:
"""Build a Revision from a response item."""
bases = [(None if base is None else Base(**base)) for base in item["bases"]]
bases = [Base(**base) for base in item["bases"] if base is not None]
return Revision(
revision=item["revision"],
version=item["version"],
Expand Down Expand Up @@ -387,7 +387,7 @@ def list_releases(self, name: str) -> tuple[list[Release], list[Channel], list[R
channel=item["channel"],
expires_at=expires_at,
resources=resources,
base=base,
base=base, # pyright: ignore[reportArgumentType]
)
)

Expand Down
2 changes: 1 addition & 1 deletion charmcraft/utils/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def format_content(content: dict[str, str], fmt: Literal[OutputFormat.TABLE, "ta

@overload
def format_content(
content: str | (numbers.Real | (list | dict)), fmt: OutputFormat | (str | None)
content: str | (numbers.Real | (list | dict)) | None, fmt: OutputFormat | (str | None)
) -> str: ...


Expand Down
11 changes: 1 addition & 10 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,11 @@ include = [
"tests/unit",
"tests/integration",
]
analyzeUnannotatedFunctions = false
reportArgumentType = "warning"
analyzeUnannotatedFunctions = true
reportAttributeAccessIssue = "warning"
reportIncompatibleVariableOverride = "warning"
reportOptionalMemberAccess = "warning"


[tool.mypy]
python_version = "3.10"
packages = [
Expand All @@ -198,17 +196,10 @@ disallow_untyped_decorators = true
# Ignore typing errors in most legacy packages until we fix them.
module=[
"charmcraft.charm_builder",
"charmcraft.cmdbase",
"charmcraft.commands.extensions",
"charmcraft.commands.pack",
"charmcraft.commands.store",
"charmcraft.store.registry",
"charmcraft.store.store",
"charmcraft.extensions._utils",
"charmcraft.linters",
"charmcraft.models.charmcraft",
"charmcraft.package",
"charmcraft.providers",
]
ignore_errors = true

Expand Down
7 changes: 5 additions & 2 deletions tests/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@

import pathlib
import textwrap
from typing import cast

from charmcraft.utils import create_importable_name, get_lib_info


def create_lib_filepath(charm_name, lib_name, api=0, patch=1, lib_id="test-lib-id"):
def create_lib_filepath(
charm_name: str, lib_name: str, api: int = 0, patch: int = 1, lib_id: str = "test-lib-id"
) -> tuple[str, str]:
"""Helper to create the structures on disk for a given lib."""
charm_name = create_importable_name(charm_name)
base_dir = pathlib.Path("lib")
Expand All @@ -46,4 +49,4 @@ def create_lib_filepath(charm_name, lib_name, api=0, patch=1, lib_id="test-lib-i
# use get_lib_info to get the hash of the file, as the used hash is WITHOUT the metadata
# files (no point in duplicating that logic here)
libdata = get_lib_info(lib_path=lib_file)
return content, libdata.content_hash
return content, cast(str, libdata.content_hash)
14 changes: 7 additions & 7 deletions tests/integration/commands/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
# For further info, check https://github.com/canonical/charmcraft
"""Tests for init command."""
import argparse
import contextlib
import os
import pathlib
import re
Expand All @@ -28,13 +27,14 @@
import pytest
import pytest_check

import charmcraft
from charmcraft import errors
from charmcraft import application, errors
from charmcraft.application import commands
from charmcraft.utils import S_IXALL

with contextlib.suppress(ImportError):
try:
import pwd
except ImportError:
pwd = None

BASIC_INIT_FILES = frozenset(
pathlib.Path(p)
Expand Down Expand Up @@ -96,14 +96,14 @@


@pytest.fixture
def init_command():
return commands.InitCommand({"app": charmcraft.application.APP_METADATA, "services": None})
def init_command() -> commands.InitCommand:
return commands.InitCommand({"app": application.APP_METADATA, "services": None})


def create_namespace(
*,
name="my-charm",
author="J Doe",
author: str | None = "J Doe",
force=False,
profile=commands.init.DEFAULT_PROFILE,
project_dir: pathlib.Path | None = None,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/models/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def test_empty_config():
def test_correct_option_type(option, type_):
config = JujuConfig(options={"my-opt": option})

assert isinstance(config.options["my-opt"], type_)
assert isinstance(config.options["my-opt"], type_) # pyright: ignore[reportOptionalSubscript]


@pytest.mark.parametrize(
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/models/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
# For further info, check https://github.com/canonical/charmcraft
"""Tests for metadata models."""
import json
from typing import cast

import pytest

Expand Down Expand Up @@ -80,7 +81,7 @@
],
)
def test_charm_metadata_from_charm_success(charm_dict, expected):
charm = project.CharmcraftProject.unmarshal(charm_dict)
charm = cast(project.Charm, project.CharmcraftProject.unmarshal(charm_dict))

assert json.loads(json.dumps(metadata.CharmMetadata.from_charm(charm).marshal())) == expected

Expand All @@ -92,7 +93,7 @@ def test_charm_metadata_from_charm_success(charm_dict, expected):
],
)
def test_bundle_metadata_from_bundle(bundle_dict, expected):
bundle = project.Bundle.unmarshal(BASIC_BUNDLE_DICT)
bundle = cast(project.Bundle, project.Bundle.unmarshal(BASIC_BUNDLE_DICT))

assert (
json.loads(json.dumps(metadata.BundleMetadata.from_bundle(bundle).marshal())) == expected
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/services/test_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ def mock_zip_file(monkeypatch):

@pytest.fixture
def analysis_service():
return analysis.AnalysisService(app=application.APP_METADATA, services=None)
return analysis.AnalysisService(
app=application.APP_METADATA, services=None # pyright: ignore[reportArgumentType]
)


@pytest.mark.parametrize(
Expand Down
8 changes: 5 additions & 3 deletions tests/unit/services/test_package.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@
charmcraft_started_at="1970-01-01T00:00:00+00:00",
bases=[SIMPLE_BUILD_BASE],
)
MANIFEST_WITH_ATTRIBUTE = models.Manifest(
**SIMPLE_MANIFEST.marshal(),
analysis={"attributes": [models.Attribute(name="boop", result="success")]},
MANIFEST_WITH_ATTRIBUTE = models.Manifest.unmarshal(
{
**SIMPLE_MANIFEST.marshal(),
"analysis": {"attributes": [models.Attribute(name="boop", result="success")]},
},
)


Expand Down
4 changes: 3 additions & 1 deletion tests/unit/services/test_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ def store(service_factory) -> services.StoreService:

@pytest.fixture(scope="module")
def reusable_store():
store = services.StoreService(app=application.APP_METADATA, services=None)
store = services.StoreService(
app=application.APP_METADATA, services=None # pyright: ignore[reportArgumentType]
)
store.client = mock.Mock(spec_set=craft_store.StoreClient)
return store

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/utils/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,4 +279,4 @@ def test_format_content_table(content):
@pytest.mark.parametrize("fmt", ["yolo", 0])
def test_format_content_invalid(fmt):
with pytest.raises(ValueError, match="^Unknown output format "):
format_content(None, fmt)
format_content(None, fmt) # pyright: ignore[reportCallIssue]
Loading