feat/strict-pyright (#5451)

- enable pyright strict mode
- fix the issues raised

This should make the build process cleaner and more resilient.
It should make it much easier to catch design flaws.

Co-authored-by: Darragh Elliott <me@delliott.net>
Reviewed-on: #5451
Reviewed-by: tobiasd <tobiasd@fsfe.org>
Co-authored-by: delliott <delliott@fsfe.org>
Co-committed-by: delliott <delliott@fsfe.org>

build/fsfe_website_build/lib/misc.py

@@ -14,14 +14,9 @@ from lxml import etree
 logger = logging.getLogger(__name__)
 
 
-def keys_exists(element: dict, *keys: str) -> bool:
+def keys_exists(element: dict[Any, Any], *keys: str) -> bool:
     """Check if *keys (nested) exists in `element` (dict)."""
-    if not isinstance(element, dict):
-        message = "keys_exists() expects dict as first argument."
-        logger.error(message)
-        raise TypeError(message)
-
-    _element = element
+    _element = element  # make a copy to prevent manipulating up the scope
     for key in keys:
         try:
             _element = _element[key]
@@ -83,7 +78,7 @@ def lang_from_filename(file: Path) -> str:
     return lang
 
 
-def run_command(commands: list) -> str:
+def run_command(commands: list[str]) -> str:
     """Run the passed command.
 
     Useful to standardise how we manage output,

build/fsfe_website_build/lib/process_file.py

@@ -8,6 +8,7 @@ import logging
 import re
 from datetime import datetime
 from pathlib import Path
+from typing import Any
 
 from lxml import etree
 
@@ -16,14 +17,14 @@ from fsfe_website_build.lib.misc import get_basename, get_version, lang_from_fil
 logger = logging.getLogger(__name__)
 
 
-def _get_xmls(file: Path, parser: etree.XMLParser) -> list:
+def _get_xmls(file: Path, parser: etree.XMLParser) -> list[etree.Element]:
     """Include second level elements of a given XML file.
 
     This emulates the behaviour of the original
     build script which wasn't able to load top
     level elements from any file
     """
-    elements = []
+    elements: list[etree.Element] = []
     if file.exists():
         tree = etree.parse(file, parser)
         root = tree.getroot()
@@ -38,7 +39,7 @@ def _get_xmls(file: Path, parser: etree.XMLParser) -> list:
     return elements
 
 
-def _get_attributes(file: Path) -> dict:
+def _get_attributes(file: Path) -> dict[str, Any]:
     """Get attributes of top level element in a given XHTML file."""
     tree = etree.parse(file)
     root = tree.getroot()
@@ -194,9 +195,7 @@ def _build_xmlstream(
     return page
 
 
-def process_file(
-    source: Path, infile: Path, transform: etree.XSLT
-) -> etree._XSLTResultTree:
+def process_file(source: Path, infile: Path, transform: etree.XSLT) -> str:
     """Process a given file using the correct xsl sheet."""
     logger.debug("Processing %s", infile)
     lang = lang_from_filename(infile)
@@ -237,4 +236,4 @@ def process_file(
             )
     except AssertionError:
         logger.debug("Output generated for file %s is not valid xml", infile)
-    return result
+    return str(result)

build/fsfe_website_build/phase0/prepare_early_subdirectories.py

@@ -26,7 +26,7 @@ def prepare_early_subdirectories(
         # here for out subdir scripts
         import early_subdir  # noqa: PLC0415 # pyright: ignore [reportMissingImports]
 
-        early_subdir.run(source, processes, subdir_path)
+        early_subdir.run(source, processes, subdir_path)  # pyright: ignore [reportUnknownMemberType]
         # Remove its path from where things can be imported
         sys.path.remove(str(subdir_path.resolve()))
         # Remove it from loaded modules

build/fsfe_website_build/phase1/prepare_subdirectories.py

@@ -30,7 +30,7 @@ def prepare_subdirectories(
         # here for out subdir scripts
         import subdir  # noqa: PLC0415 # pyright: ignore [reportMissingImports]
 
-        subdir.run(source, languages, processes, subdir_path)
+        subdir.run(source, languages, processes, subdir_path)  # pyright: ignore [reportUnknownMemberType]
         # Remove its path from where things can be imported
         sys.path.remove(str(subdir_path.resolve()))
         # Remove it from loaded modules

build/fsfe_website_build/phase1/update_css.py

@@ -11,7 +11,7 @@ distributed to the web server.
 import logging
 from pathlib import Path
 
-import minify
+import minify  # pyright: ignore [reportMissingTypeStubs]
 
 from fsfe_website_build.lib.misc import run_command, update_if_changed
 

build/fsfe_website_build/phase1/update_xmllists.py

@@ -40,7 +40,7 @@ def _update_for_base(  # noqa: PLR0913
     lastyear: str,
 ) -> None:
     """Update the xmllist for a given base file."""
-    matching_files = set()
+    matching_files: set[str] = set()
     # If sources exist
     if base.with_suffix(".sources").exists():
         # Load every file that matches the pattern
@@ -94,7 +94,7 @@ def _update_for_base(  # noqa: PLR0913
             matching_files.add(
                 f"{source}/global/data/modules/{module.get('id').strip()}"
             )
-    matching_files = sorted(matching_files)
+    matching_files = set(sorted(matching_files))  # noqa: C414
     update_if_changed(
         Path(f"{base.parent}/.{base.name}.xmllist"),
         ("\n".join(matching_files) + "\n") if matching_files else "",
@@ -139,7 +139,7 @@ def _update_module_xmllists(
 
 def _check_xmllist_deps(source: Path, file: Path) -> None:
     """If any of the sources in an xmllist are newer than it, touch the xmllist."""
-    xmls = set()
+    xmls: set[Path] = set()
     with file.open(mode="r") as fileobj:
         for line in fileobj:
             path_line = source.joinpath(line.strip())

build/fsfe_website_build/phase2/process_files.py

@@ -73,7 +73,7 @@ def _process_set(  # noqa: PLR0913
             logger.debug("Building %s", target_file)
             result = process_file(source, source_file, transform)
             target_file.parent.mkdir(parents=True, exist_ok=True)
-            result.write_output(target_file)
+            target_file.write_text(result)
 
 
 def process_files(

build/fsfe_website_build/phase3/serve_websites.py

@@ -19,14 +19,14 @@ logger = logging.getLogger(__name__)
 def _run_webserver(path: str, port: int) -> None:
     """Given a path and a port it will serve that dir on that localhost:port."""
     os.chdir(path)
-    handler = http.server.CGIHTTPRequestHandler
+    handler = http.server.SimpleHTTPRequestHandler
 
     with socketserver.TCPServer(("", port), handler) as httpd:
         httpd.serve_forever()
 
 
 def serve_websites(
-    serve_dir: Path, sites: list, base_port: int, increment_number: int
+    serve_dir: Path, sites: list[Path], base_port: int, increment_number: int
 ) -> None:
     """Serve the sites in serve_dir from base port.
 
@@ -40,7 +40,7 @@ def serve_websites(
         for path in Path(serve_dir).iterdir()
         if (path.is_dir() and (path.name in site_names))
     )
-    serves = []
+    serves: list[tuple[str, int]] = []
     for index, directory in enumerate(dirs):
         port = base_port + (increment_number * index)
         url = f"http://127.0.0.1:{port}"

build/fsfe_website_build_tests/lib/process_file_test.py

@@ -70,8 +70,8 @@ def process_file_link_rewrites_test(
     assert isinstance(sample_xsl_transformer, etree.XSLT)
 
     result_doc = process_file(Path(), xml_path, sample_xsl_transformer)
-    assert isinstance(result_doc, etree._XSLTResultTree)
+    assert isinstance(result_doc, str)
     # We get a list, but as we have only one link in the above sample
     # we only need to care about the first one
-    link_node = result_doc.xpath("//a[@href and @test_url]")[0]
+    link_node = etree.fromstring(result_doc).xpath("//a[@href and @test_url]")[0]
     assert link_node.get("href") == link_out

fsfe.org/search/subdir.py

@@ -5,15 +5,20 @@
 
 import json
 import logging
-import multiprocessing.pool
+import multiprocessing
 from pathlib import Path
+from typing import TYPE_CHECKING
 
 import iso639
-import nltk
+import nltk  # pyright: ignore[reportMissingTypeStubs]
 from fsfe_website_build.lib.misc import update_if_changed
 from lxml import etree
-from nltk.corpus import stopwords as nltk_stopwords
+from nltk.corpus import (  # pyright: ignore[reportMissingTypeStubs]
+    stopwords as nltk_stopwords,  # pyright: ignore[reportMissingTypeStubs]
+)
 
+if TYPE_CHECKING:
+    from collections.abc import Generator
 logger = logging.getLogger(__name__)
 
 
@@ -33,14 +38,13 @@ def _find_teaser(document: etree.ElementTree) -> str:
     return ""
 
 
-def _process_file(file: Path, stopwords: set[str]) -> dict:
+def _process_file(file: Path, stopwords: set[str]) -> dict[str, str | None]:
     """Generate the search index entry for a given file and set of stopwords."""
     xslt_root = etree.parse(file)
     tags = (
-        tag.get("key")
-        for tag in filter(
-            lambda tag: tag.get("key") != "front-page", xslt_root.xpath("//tag")
-        )
+        str(tag.get("key"))
+        for tag in xslt_root.xpath("//tag")
+        if tag.get("key") != "front-page"
     )
     return {
         "url": f"/{file.with_suffix('.html').relative_to(file.parents[-2])}",
@@ -75,8 +79,8 @@ def run(source: Path, languages: list[str], processes: int, working_dir: Path) -
     # Download all stopwords
     nltkdir = "./.nltk_data"
     source_dir = working_dir.parent
-    nltk.data.path = [nltkdir, *nltk.data.path]
-    nltk.download("stopwords", download_dir=nltkdir, quiet=True)
+    nltk.data.path = [nltkdir, *nltk.data.path]  # pyright: ignore [(reportUnknownMemberType)]
+    nltk.download("stopwords", download_dir=nltkdir, quiet=True)  # pyright: ignore [(reportUnknownMemberType)]
     with multiprocessing.Pool(processes) as pool:
         logger.debug("Indexing %s", source_dir)
 
@@ -87,12 +91,12 @@ def run(source: Path, languages: list[str], processes: int, working_dir: Path) -
         # Use iso639 to get the english name of the language
         # from the two letter iso639-1 code we use to mark files.
         # Then if that language has stopwords from nltk, use those stopwords.
-        files_with_stopwords = (
+        files_with_stopwords: Generator[tuple[Path, set[str]]] = (
             (
                 file,
                 (
                     set(
-                        nltk_stopwords.words(
+                        nltk_stopwords.words(  # pyright: ignore [(reportUnknownMemberType)]
                             iso639.Language.from_part1(
                                 file.suffixes[0].removeprefix("."),
                             ).name.lower(),
@@ -101,7 +105,7 @@ def run(source: Path, languages: list[str], processes: int, working_dir: Path) -
                     if iso639.Language.from_part1(
                         file.suffixes[0].removeprefix("."),
                     ).name.lower()
-                    in nltk_stopwords.fileids()
+                    in nltk_stopwords.fileids()  # pyright: ignore [(reportUnknownMemberType)]
                     else set()
                 ),
             )

fsfe.org/tags/subdir.py

@@ -5,9 +5,8 @@
 """Generate tag pages for all tags used on the site."""
 
 import logging
-import multiprocessing.pool
+import multiprocessing
 from collections import defaultdict
-from functools import partial
 from pathlib import Path
 
 from fsfe_website_build.lib.misc import (
@@ -88,9 +87,7 @@ def run(source: Path, languages: list[str], processes: int, working_dir: Path) -
         logger.debug("Updating tags for %s", working_dir)
         # Create a complete and current map of which tag is used in which files
         files_by_tag: dict[str, set[Path]] = defaultdict(set)
-        tags_by_lang: dict[str, dict[str, str | None]] = defaultdict(
-            partial(defaultdict, None)
-        )
+        tags_by_lang: defaultdict[str, dict[str, str | None]] = defaultdict(dict)
         # Fill out files_by_tag and tags_by_lang
         for file in filter(
             lambda file:
@@ -143,7 +140,7 @@ def run(source: Path, languages: list[str], processes: int, working_dir: Path) -
 
         logger.debug("Updating tag sets")
         # Get count of files with each tag in each section
-        filecount: dict[str, dict[str, int]] = defaultdict(partial(defaultdict, int))
+        filecount: dict[str, dict[str, int]] = defaultdict(dict)
         for section in ["news", "events"]:
             for tag in files_by_tag:
                 filecount[section][tag] = len(

pyproject.toml

@@ -77,6 +77,8 @@ ignore = [
 "build/fsfe_website_build_tests/*" = [
   "D",
 ] # We do not need to document the tests.
+[tool.pyright]
+typeCheckingMode = "strict"
 [tool.pytest.ini_options]
 testpaths = ["build/fsfe_website_build_tests"]
 python_files = ["*_test.py"]

status.fsfe.org/translations/subdir.py

@@ -26,7 +26,7 @@ def _generate_translation_data(lang: str, file: Path) -> dict[str, str] | None:
     ext = file.suffix.removeprefix(".")
     working_file = file.with_suffix("").with_suffix(f".{lang}.{ext}")
 
-    result = run_command(["git", "log", '--pretty="%ct"', "-1", file]).strip('"')
+    result = run_command(["git", "log", '--pretty="%ct"', "-1", str(file)]).strip('"')
     time = int(result)
     original_date = datetime.datetime.fromtimestamp(time)
 
@@ -117,7 +117,7 @@ def _create_overview(
 def _create_translation_file(
     target_dir: Path,
     lang: str,
-    data: dict[int, list[dict]],
+    data: dict[int, list[dict[str, str]]],
 ) -> None:
     work_file = target_dir.joinpath(f"translations.{lang}.xml")
     page = etree.Element("translation-status")
@@ -126,7 +126,7 @@ def _create_translation_file(
     for priority, file_data_list in data.items():
         prio = etree.SubElement(page, "priority", value=str(priority))
         for file_data in file_data_list:
-            etree.SubElement(prio, "file", **file_data)
+            etree.SubElement(prio, "file", attrib=file_data)
 
     en_texts_file = Path("global/data/texts/texts.en.xml")
     lang_texts_file = Path(f"global/data/texts/texts.{lang}.xml")