From 4ee5d212822d06a366b2095ede4bdf9d943f19a8 Mon Sep 17 00:00:00 2001 From: Manabu TERADA Date: Tue, 20 Oct 2026 10:16:52 +0900 Subject: [PATCH 1/2] Add type hints to all functions and enforce them with ruff Every function and method in src now has argument and return types. Zope and Plone objects use their concrete classes: SetupTool for the setup handlers, datastream in the transform, and Application, ConfigurationMachine and PloneSite in the test layer. Fixtures and helpers in tests are annotated too, with the classes pytest-plone uses (PloneSite, InstallerView). ruff now selects ANN. In tests ANN001 and ANN201 are ignored, because test functions take untyped fixtures and return nothing. No behaviour changes. Co-Authored-By: Claude Opus 5.5 --- pyproject.toml | 6 +++++- src/c2/transform/msoffice/setuphandlers.py | 7 ++++--- src/c2/transform/msoffice/testing.py | 9 +++++++-- src/c2/transform/msoffice/transform.py | 12 ++++++++---- tests/test_config.py | 2 +- tests/test_java.py | 12 +++++++----- tests/test_setup.py | 10 +++++++--- tests/test_transform.py | 18 +++++++++++++----- 8 files changed, 52 insertions(+), 24 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 7114952..88bcb18 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -122,6 +122,8 @@ fix = true lint.select = [ # flake8-2020 "YTT", + # flake8-annotations + "ANN", # flake8-bandit "S", # flake8-bugbear @@ -165,7 +167,9 @@ order-by-type = false preview = true [tool.ruff.lint.per-file-ignores] -"tests/*" = ["E501", "RUF001", "S101"] +# Test functions take pytest fixtures and return nothing; fixtures and +# helpers in tests are annotated by convention. +"tests/*" = ["ANN001", "ANN201", "E501", "RUF001", "S101"] [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/src/c2/transform/msoffice/setuphandlers.py b/src/c2/transform/msoffice/setuphandlers.py index 8bef927..e39f0bc 100644 --- a/src/c2/transform/msoffice/setuphandlers.py +++ b/src/c2/transform/msoffice/setuphandlers.py @@ -2,6 +2,7 @@ from c2.transform.msoffice import logger from plone.base.interfaces import INonInstallable from Products.CMFCore.utils import getToolByName +from Products.GenericSetup.tool import SetupTool from zope.interface import implementer @@ -10,12 +11,12 @@ @implementer(INonInstallable) class HiddenProfiles: - def getNonInstallableProfiles(self): + def getNonInstallableProfiles(self) -> list[str]: """Hide the uninstall profile from site creation and the add-ons list.""" return ["c2.transform.msoffice:uninstall"] -def post_install(context): +def post_install(context: SetupTool) -> None: """Register the Office MIME types and the transform. ``context`` is the setup tool. Running it again changes nothing. @@ -47,7 +48,7 @@ def post_install(context): transforms.manage_addTransform(config.TRANSFORM_NAME, TRANSFORM_MODULE) -def uninstall(context): +def uninstall(context: SetupTool) -> None: """Remove the transform. The MIME types stay; others may use them.""" transforms = getToolByName(context, "portal_transforms") if config.TRANSFORM_NAME not in transforms.objectIds(): diff --git a/src/c2/transform/msoffice/testing.py b/src/c2/transform/msoffice/testing.py index 4f2d037..476fd30 100644 --- a/src/c2/transform/msoffice/testing.py +++ b/src/c2/transform/msoffice/testing.py @@ -1,8 +1,11 @@ +from OFS.Application import Application from plone.app.contenttypes.testing import PLONE_APP_CONTENTTYPES_FIXTURE from plone.app.testing import applyProfile from plone.app.testing import FunctionalTesting from plone.app.testing import IntegrationTesting from plone.app.testing import PloneSandboxLayer +from Products.CMFPlone.Portal import PloneSite +from zope.configuration.config import ConfigurationMachine import c2.transform.msoffice @@ -10,11 +13,13 @@ class Layer(PloneSandboxLayer): defaultBases = (PLONE_APP_CONTENTTYPES_FIXTURE,) - def setUpZope(self, app, configurationContext): + def setUpZope( + self, app: Application, configurationContext: ConfigurationMachine + ) -> None: # z3c.autoinclude is disabled in the Plone fixture base layer. self.loadZCML(package=c2.transform.msoffice) - def setUpPloneSite(self, portal): + def setUpPloneSite(self, portal: PloneSite) -> None: applyProfile(portal, "c2.transform.msoffice:default") diff --git a/src/c2/transform/msoffice/transform.py b/src/c2/transform/msoffice/transform.py index 0c5ecd0..4eaf8ea 100644 --- a/src/c2/transform/msoffice/transform.py +++ b/src/c2/transform/msoffice/transform.py @@ -4,6 +4,7 @@ from c2.transform.msoffice.config import SITE_CHARSET from c2.transform.msoffice.config import TRANSFORM_NAME from pathlib import Path +from Products.PortalTransforms.data import datastream from Products.PortalTransforms.interfaces import ITransform from zope.interface import implementer @@ -116,16 +117,19 @@ class msoffice_to_text: output_encoding = SITE_CHARSET - def __init__(self, name=None): + def __init__(self, name: str | None = None) -> None: if name: self.__name__ = name - def name(self): + def name(self) -> str: return self.__name__ - def convert(self, orig, data, **kwargs): + def convert( + self, orig: bytes | str, data: datastream, **kwargs: object + ) -> datastream: """Set the text of ``orig`` (bytes) on ``data``. + ``data`` is the datastream that portal_transforms made for the result. On any failure the text is empty, so saving the file still works. """ if not isinstance(orig, bytes): @@ -140,5 +144,5 @@ def convert(self, orig, data, **kwargs): return data -def register(): +def register() -> msoffice_to_text: return msoffice_to_text() diff --git a/tests/test_config.py b/tests/test_config.py index 8233741..8cf0de4 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -6,7 +6,7 @@ @pytest.fixture -def stdlib_mimetypes_only(monkeypatch): +def stdlib_mimetypes_only(monkeypatch: pytest.MonkeyPatch) -> None: """Forget the host's mime.types (slim images do not have one). mimetypes.init(files=[]) keeps the current database, so it is replaced. diff --git a/tests/test_java.py b/tests/test_java.py index 6bd1a76..d8df1e5 100644 --- a/tests/test_java.py +++ b/tests/test_java.py @@ -6,8 +6,10 @@ from pathlib import Path from plone import api +from plone.app.contenttypes.content import File from plone.app.contenttypes.indexers import SearchableText_file from plone.namedfile.file import NamedBlobFile +from Products.CMFPlone.Portal import PloneSite import logging import os @@ -27,7 +29,7 @@ @pytest.fixture -def java(): +def java() -> None: if shutil.which("java") is None: if os.environ.get(REQUIRE_JAVA) == "1": pytest.fail(f"java is not on PATH and {REQUIRE_JAVA}=1") @@ -35,13 +37,13 @@ def java(): @pytest.fixture -def no_locale(monkeypatch): +def no_locale(monkeypatch: pytest.MonkeyPatch) -> None: """Run java without a locale, like a slim container image.""" monkeypatch.setenv("LC_ALL", "C") monkeypatch.delenv("LANG", raising=False) -def add_file(portal, name: str, content_type: str): +def add_file(portal: PloneSite, name: str, content_type: str) -> File: with api.env.adopt_roles(["Manager"]): return api.content.create( container=portal, @@ -56,11 +58,11 @@ def add_file(portal, name: str, content_type: str): ) -def text_of(portal, name: str, content_type: str) -> str: +def text_of(portal: PloneSite, name: str, content_type: str) -> str: return SearchableText_file(add_file(portal, name, content_type))() -def transformed(portal, name: str, content_type: str) -> str: +def transformed(portal: PloneSite, name: str, content_type: str) -> str: data = portal.portal_transforms.convertTo( "text/plain", (SAMPLES / name).read_bytes(), mimetype=content_type ) diff --git a/tests/test_setup.py b/tests/test_setup.py index f6394c7..397b324 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -1,6 +1,10 @@ from Acquisition import aq_base from c2.transform.msoffice import PACKAGE_NAME from plone.base.interfaces import INonInstallable +from Products.CMFPlone.controlpanel.browser.quickinstaller import InstallerView +from Products.CMFPlone.Portal import PloneSite +from Products.GenericSetup.tool import SetupTool +from Products.PortalTransforms.TransformEngine import TransformTool from zope.component import getUtility import logging @@ -14,12 +18,12 @@ @pytest.fixture -def transforms(portal): +def transforms(portal: PloneSite) -> TransformTool: return portal.portal_transforms @pytest.fixture -def setup_tool(portal): +def setup_tool(portal: PloneSite) -> SetupTool: return portal.portal_setup @@ -70,7 +74,7 @@ def test_uninstall_profile_is_hidden(portal): class TestUninstall: @pytest.fixture(autouse=True) - def uninstalled(self, installer): + def uninstalled(self, installer: InstallerView) -> None: installer.uninstall_product(PACKAGE_NAME) def test_addon_uninstalled(self, installer): diff --git a/tests/test_transform.py b/tests/test_transform.py index 885fafe..9d51ee3 100644 --- a/tests/test_transform.py +++ b/tests/test_transform.py @@ -17,14 +17,22 @@ class FakeRun: """Stands in for subprocess.run and records the calls.""" - def __init__(self, returncode=0, stdout=b"", stderr=b"", error=None): + def __init__( + self, + returncode: int = 0, + stdout: bytes = b"", + stderr: bytes = b"", + error: Exception | None = None, + ) -> None: self.returncode = returncode self.stdout = stdout self.stderr = stderr self.error = error self.calls = [] - def __call__(self, args, **kwargs): + def __call__( + self, args: list[str], **kwargs: object + ) -> subprocess.CompletedProcess[bytes]: self.calls.append((args, kwargs)) if self.error is not None: raise self.error @@ -34,18 +42,18 @@ def __call__(self, args, **kwargs): @pytest.fixture(autouse=True) -def clean_environment(monkeypatch): +def clean_environment(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("C2_TRANSFORM_MSOFFICE_TIMEOUT", raising=False) monkeypatch.delenv("C2_TRANSFORM_MSOFFICE_JAVA_OPTS", raising=False) -def fake_run(monkeypatch, **kwargs) -> FakeRun: +def fake_run(monkeypatch: pytest.MonkeyPatch, **kwargs: object) -> FakeRun: run = FakeRun(**kwargs) monkeypatch.setattr(transform.subprocess, "run", run) return run -def convert(orig) -> str: +def convert(orig: bytes | str) -> str: data = datastream("test") result = transform.msoffice_to_text().convert(orig, data) assert result is data From e77946b0ce5af9f5a53f1db677f5e48a0b0fbdc7 Mon Sep 17 00:00:00 2001 From: Manabu TERADA Date: Tue, 20 Oct 2026 10:31:53 +0900 Subject: [PATCH 2/2] Check the types with mypy in tox and in CI mypy 2.4.0 is pinned in the "dev" dependency group (PEP 735), so the pin does not go into the package metadata. The setup is the same as in c2.splitter.icu. - tox: a "mypy" environment with the Plone 6.2 dependencies on Python 3.14 runs "mypy src tests"; it is in the envlist. tox 4.22 or later is required for dependency_groups. - CI: a "mypy" job next to "lint". It needs no Java. - [tool.mypy]: disallow_untyped_defs, check_untyped_defs, warn_unused_ignores, warn_redundant_casts, warn_return_any and warn_unused_configs. tests.* may have untyped defs. mypy_path = "src" with explicit_package_bases, so that mypy reads the sources and not the installed package, which has no py.typed. The Zope and Plone modules without type information are listed one by one for ignore_missing_imports. Type errors found in the tests and fixed without changing what they test: FakeRun.calls had no element type; fake_run passed **kwargs typed as object to FakeRun, so it now takes the four keyword arguments; and three helpers returned Any from Plone as str, so the value is now assigned to a str variable first. src had no errors. Checked that mypy fails when msdoc2txt is declared to return str (5 errors) and when getNonInstallableProfiles returns list[int]. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/plone-package.yml | 14 +++++++++ AGENTS.md | 1 + pyproject.toml | 48 +++++++++++++++++++++++++++++ tests/test_java.py | 6 ++-- tests/test_transform.py | 16 +++++++--- tox.ini | 14 +++++++++ 6 files changed, 93 insertions(+), 6 deletions(-) diff --git a/.github/workflows/plone-package.yml b/.github/workflows/plone-package.yml index d8a0496..59f7823 100644 --- a/.github/workflows/plone-package.yml +++ b/.github/workflows/plone-package.yml @@ -33,6 +33,20 @@ jobs: - name: Ruff run: tox -e ruff-check + mypy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: '3.14' + - name: Install tox + run: python -m pip install tox + - name: Mypy + run: tox -e mypy + test: runs-on: ubuntu-latest needs: lint diff --git a/AGENTS.md b/AGENTS.md index 2bf8ced..932a0d2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,7 @@ Run the tests and the linters with tox: tox -e py314-Plone62 # one Python and Plone version tox -e ruff-check # lint and format check tox -e ruff-fix # fix and format +tox -e mypy # type check src and tests tox # the whole matrix ``` diff --git a/pyproject.toml b/pyproject.toml index 88bcb18..891d993 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,6 +59,11 @@ release = [ "zest.pocompile", ] +[dependency-groups] +dev = [ + "mypy==2.4.0", +] + [project.urls] Homepage = "https://github.com/cmscom/c2.transform.msoffice" PyPI = "https://pypi.org/project/c2.transform.msoffice" @@ -171,6 +176,49 @@ preview = true # helpers in tests are annotated by convention. "tests/*" = ["ANN001", "ANN201", "E501", "RUF001", "S101"] +[tool.mypy] +python_version = "3.10" +# Check the sources, not the installed package (src/c2 and src/c2/transform +# are namespace packages). +mypy_path = "src" +explicit_package_bases = true +disallow_untyped_defs = true +check_untyped_defs = true +warn_unused_ignores = true +warn_redundant_casts = true +warn_return_any = true +warn_unused_configs = true + +[[tool.mypy.overrides]] +module = ["tests.*"] +disallow_untyped_defs = false + +# Zope, Plone and their test tools have no type information. +[[tool.mypy.overrides]] +module = [ + "Acquisition", + "OFS.Application", + "plone", + "plone.app.contenttypes.content", + "plone.app.contenttypes.indexers", + "plone.app.contenttypes.testing", + "plone.app.testing", + "plone.base.interfaces", + "plone.namedfile.file", + "Products.CMFCore.utils", + "Products.CMFPlone.controlpanel.browser.quickinstaller", + "Products.CMFPlone.Portal", + "Products.GenericSetup.tool", + "Products.PortalTransforms.data", + "Products.PortalTransforms.interfaces", + "Products.PortalTransforms.TransformEngine", + "pytest_plone", + "zope.component", + "zope.configuration.config", + "zope.interface", +] +ignore_missing_imports = true + [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/tests/test_java.py b/tests/test_java.py index d8df1e5..6bb6a38 100644 --- a/tests/test_java.py +++ b/tests/test_java.py @@ -59,14 +59,16 @@ def add_file(portal: PloneSite, name: str, content_type: str) -> File: def text_of(portal: PloneSite, name: str, content_type: str) -> str: - return SearchableText_file(add_file(portal, name, content_type))() + text: str = SearchableText_file(add_file(portal, name, content_type))() + return text def transformed(portal: PloneSite, name: str, content_type: str) -> str: data = portal.portal_transforms.convertTo( "text/plain", (SAMPLES / name).read_bytes(), mimetype=content_type ) - return data.getData() + text: str = data.getData() + return text @pytest.mark.usefixtures("java", "no_locale") diff --git a/tests/test_transform.py b/tests/test_transform.py index 9d51ee3..bd21ab2 100644 --- a/tests/test_transform.py +++ b/tests/test_transform.py @@ -28,7 +28,7 @@ def __init__( self.stdout = stdout self.stderr = stderr self.error = error - self.calls = [] + self.calls: list[tuple[list[str], dict[str, object]]] = [] def __call__( self, args: list[str], **kwargs: object @@ -47,8 +47,15 @@ def clean_environment(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv("C2_TRANSFORM_MSOFFICE_JAVA_OPTS", raising=False) -def fake_run(monkeypatch: pytest.MonkeyPatch, **kwargs: object) -> FakeRun: - run = FakeRun(**kwargs) +def fake_run( + monkeypatch: pytest.MonkeyPatch, + *, + returncode: int = 0, + stdout: bytes = b"", + stderr: bytes = b"", + error: Exception | None = None, +) -> FakeRun: + run = FakeRun(returncode, stdout, stderr, error) monkeypatch.setattr(transform.subprocess, "run", run) return run @@ -57,7 +64,8 @@ def convert(orig: bytes | str) -> str: data = datastream("test") result = transform.msoffice_to_text().convert(orig, data) assert result is data - return data.getData() + text: str = data.getData() + return text def test_implements_itransform(): diff --git a/tox.ini b/tox.ini index 05d74b4..67d7aac 100644 --- a/tox.ini +++ b/tox.ini @@ -1,6 +1,9 @@ [tox] +# dependency_groups (the mypy environment) needs tox 4.22 +min_version = 4.22 envlist = ruff-check, + mypy, py{310,311,312,313}-Plone60, py{310,311,312,313}-Plone61, py{310,311,312,313,314}-Plone62, @@ -49,6 +52,17 @@ commands = ruff format --check --diff src tests +[testenv:mypy] +basepython = python3.14 +dependency_groups = dev +setenv = + constraints_file=constraints_plone62.txt +deps = + -rrequirements_plone62.txt +commands = + mypy src tests + + [testenv:ruff-fix] basepython = python3.12 skip_install = True