Cleanup & preparation for further changes (#5)

* Deal with ast module deprecations

* Add some type hints

* ci: Update action versions

.github/workflows/ci.yaml

@@ -16,9 +16,9 @@ jobs:
         python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v4
+        uses: actions/setup-python@v5
         with:
           python-version: ${{ matrix.python-version }}
       - name: Install dependencies

fluent/migrate/_context.py

@@ -1,20 +1,23 @@
+from __future__ import annotations
+from typing import Dict, Optional, Set, Tuple, cast
+
 import os
 import codecs
 from functools import partial
 import logging
 from itertools import zip_longest
 
+from compare_locales.parser import getParser
+from compare_locales.plurals import get_plural
 import fluent.syntax.ast as FTL
 from fluent.syntax.parser import FluentParser
 from fluent.syntax.serializer import FluentSerializer
-from compare_locales.parser import getParser
-from compare_locales.plurals import get_plural
 
+from .changesets import Changes
+from .errors import UnreadableReferenceError
 from .evaluator import Evaluator
 from .merge import merge_resource
-from .errors import (
-    UnreadableReferenceError,
-)
+from .transforms import Source
 
 
 class InternalContext:
@@ -23,7 +26,11 @@ class InternalContext:
     For the public interface, see `context.MigrationContext`.
     """
 
-    def __init__(self, lang, reference_dir, localization_dir, enforce_translated=False):
+    dependencies: Dict[Tuple[str, str], Set[Tuple[str, Source]]] = {}
+    localization_dir: str
+    reference_dir: str
+
+    def __init__(self, lang, enforce_translated=False):
         self.fluent_parser = FluentParser(with_spans=False)
         self.fluent_serializer = FluentSerializer()
 
@@ -33,7 +40,7 @@ def __init__(self, lang, reference_dir, localization_dir, enforce_translated=Fal
         if self.plural_categories is None:
             logger = logging.getLogger("migrate")
             logger.warning(
-                'Plural rule for "{}" is not defined in ' "compare-locales".format(lang)
+                f'Plural rule for "{lang}" is not defined in "compare-locales"'
             )
             self.plural_categories = ("one", "other")
 
@@ -51,7 +58,7 @@ def __init__(self, lang, reference_dir, localization_dir, enforce_translated=Fal
         # AST hierarchy and evaluating nodes which are migration Transforms.
         self.evaluator = Evaluator(self)
 
-    def read_ftl_resource(self, path):
+    def read_ftl_resource(self, path: str):
         """Read an FTL resource and parse it into an AST."""
         f = codecs.open(path, "r", "utf8")
         try:
@@ -80,7 +87,7 @@ def read_ftl_resource(self, path):
 
         return ast
 
-    def read_legacy_resource(self, path):
+    def read_legacy_resource(self, path: str):
         """Read a legacy resource and parse it into a dict."""
         parser = getParser(path)
         parser.readFile(path)
@@ -91,7 +98,7 @@ def read_legacy_resource(self, path):
             if entity.localized or self.enforce_translated
         }
 
-    def read_reference_ftl(self, path):
+    def read_reference_ftl(self, path: str):
         """Read and parse a reference FTL file.
 
         A missing resource file is a fatal error and will raise an
@@ -109,7 +116,7 @@ def read_reference_ftl(self, path):
             logging.getLogger("migrate").error(error_message)
             raise UnreadableReferenceError(error_message)
 
-    def read_localization_ftl(self, path):
+    def read_localization_ftl(self, path: str):
         """Read and parse an existing localization FTL file.
 
         Create a new FTL.Resource if the file doesn't exist or can't be
@@ -134,7 +141,7 @@ def read_localization_ftl(self, path):
             )
             return FTL.Resource()
 
-    def maybe_add_localization(self, path):
+    def maybe_add_localization(self, path: str):
         """Add a localization resource to migrate translations from.
 
         Uses a compare-locales parser to create a dict of (key, string value)
@@ -153,15 +160,15 @@ def maybe_add_localization(self, path):
         else:
             self.localization_resources[path] = collection
 
-    def get_legacy_source(self, path, key):
+    def get_legacy_source(self, path: str, key: str):
         """Get an entity value from a localized legacy source.
 
         Used by the `Source` transform.
         """
         resource = self.localization_resources[path]
         return resource.get(key, None)
 
-    def get_fluent_source_pattern(self, path, key):
+    def get_fluent_source_pattern(self, path: str, key: str):
         """Get a pattern from a localized Fluent source.
 
         If the key contains a `.`, does an attribute lookup.
@@ -219,7 +226,11 @@ def message_id(message):
                 return False
         return True
 
-    def merge_changeset(self, changeset=None, known_translations=None):
+    def merge_changeset(
+        self,
+        changeset: Optional[Changes] = None,
+        known_translations: Optional[Changes] = None,
+    ):
         """Return a generator of FTL ASTs for the changeset.
 
         The input data must be configured earlier using the `add_*` methods.
@@ -278,7 +289,9 @@ def merge_changeset(self, changeset=None, known_translations=None):
             # The result for this path is a complete `FTL.Resource`.
             yield path, snapshot
 
-    def in_changeset(self, changeset, known_translations, path, ident):
+    def in_changeset(
+        self, changeset: Changes, known_translations: Changes, path: str, ident
+    ) -> bool:
         """Check if a message should be migrated in this changeset.
 
         The message is identified by path and ident.
@@ -313,11 +326,13 @@ def in_changeset(self, changeset, known_translations, path, ident):
         # See https://bugzilla.mozilla.org/show_bug.cgi?id=1321271
         # We only return True if our current changeset touches
         # the transform, and we have all of the dependencies.
-        active_deps = message_deps & changeset
+        active_deps = cast(bool, message_deps & changeset)
         available_deps = message_deps & known_translations
         return active_deps and message_deps == available_deps
 
-    def serialize_changeset(self, changeset, known_translations=None):
+    def serialize_changeset(
+        self, changeset: Changes, known_translations: Optional[Changes] = None
+    ):
         """Return a dict of serialized FTLs for the changeset.
 
         Given `changeset`, return a dict whose keys are resource paths and

fluent/migrate/blame.py

@@ -1,21 +1,43 @@
+from __future__ import annotations
+from typing import Dict, Iterable, Tuple, TypedDict, cast
+
 import argparse
 import json
 import os
 
-from compare_locales.parser import getParser, Junk
+from compare_locales.parser import Junk, getParser
 from compare_locales.parser.fluent import FluentEntity
 from compare_locales import mozpath
 import hglib
 from hglib.util import b, cmdbuilder
 
+BlameData = Dict[str, Dict[str, Tuple[int, float]]]
+"File path -> message key -> [userid, timestamp]"
+
+
+class BlameResult(TypedDict):
+    authors: list[str]
+    blame: BlameData
+
+
+class LineBlame(TypedDict):
+    date: tuple[float, float]
+    line: str
+    user: str
+
+
+class FileBlame(TypedDict):
+    lines: list[LineBlame]
+    path: str
+
 
 class Blame:
-    def __init__(self, client):
+    def __init__(self, client: hglib.client.hgclient):
         self.client = client
-        self.users = []
-        self.blame = {}
+        self.users: list[str] = []
+        self.blame: BlameData = {}
 
-    def attribution(self, file_paths):
+    def attribution(self, file_paths: Iterable[str]) -> BlameResult:
         args = cmdbuilder(
             b"annotate",
             *[b(p) for p in file_paths],
@@ -32,7 +54,7 @@ def attribution(self, file_paths):
 
         return {"authors": self.users, "blame": self.blame}
 
-    def handleFile(self, file_blame):
+    def handleFile(self, file_blame: FileBlame):
         path = mozpath.normsep(file_blame["path"])
 
         try:
@@ -48,12 +70,13 @@ def handleFile(self, file_blame):
             if isinstance(e, Junk):
                 continue
             if e.val_span:
-                key_vals = [(e.key, e.val_span)]
+                key_vals: list[tuple[str, str]] = [(e.key, e.val_span)]
             else:
                 key_vals = []
             if isinstance(e, FluentEntity):
                 key_vals += [
-                    (f"{e.key}.{attr.key}", attr.val_span) for attr in e.attributes
+                    (f"{e.key}.{attr.key}", cast(str, attr.val_span))
+                    for attr in e.attributes
                 ]
             for key, (val_start, val_end) in key_vals:
                 entity_lines = file_blame["lines"][
@@ -67,9 +90,9 @@ def handleFile(self, file_blame):
                 if user not in self.users:
                     self.users.append(user)
                 userid = self.users.index(user)
-                self.blame[path][key] = [userid, timestamp]
+                self.blame[path][key] = cast(Tuple[int, float], [userid, timestamp])
 
-    def readFile(self, parser, path):
+    def readFile(self, parser, path: str):
         parser.readFile(os.path.join(self.client.root().decode("utf-8"), path))
 
 

fluent/migrate/changesets.py

@@ -1,12 +1,25 @@
+from __future__ import annotations
+from typing import Set, Tuple, TypedDict
+
 import time
 
+from .blame import BlameResult
+
+Changes = Set[Tuple[str, str]]
+
+
+class Changeset(TypedDict):
+    author: str
+    first_commit: float
+    changes: Changes
+
 
-def by_first_commit(item):
+def by_first_commit(item: Changeset):
     """Order two changesets by their first commit date."""
     return item["first_commit"]
 
 
-def convert_blame_to_changesets(blame_json):
+def convert_blame_to_changesets(blame_json: BlameResult) -> list[Changeset]:
     """Convert a blame dict into a list of changesets.
 
     The blame information in `blame_json` should be a dict of the following
@@ -38,7 +51,7 @@ def convert_blame_to_changesets(blame_json):
 
     """
     now = time.time()
-    changesets = [
+    changesets: list[Changeset] = [
         {"author": author, "first_commit": now, "changes": set()}
         for author in blame_json["authors"]
     ]

fluent/migrate/context.py

@@ -1,3 +1,6 @@
+from __future__ import annotations
+from typing import List, Set, Tuple, cast
+
 import logging
 
 import fluent.syntax.ast as FTL
@@ -48,25 +51,31 @@ class MigrationContext(InternalContext):
     """
 
     def __init__(
-        self, locale, reference_dir, localization_dir, enforce_translated=False
+        self,
+        locale: str,
+        reference_dir: str,
+        localization_dir: str,
+        enforce_translated=False,
     ):
         super().__init__(
             locale,
-            reference_dir,
-            localization_dir,
             enforce_translated=enforce_translated,
         )
         self.locale = locale
         # Paths to directories with input data, relative to CWD.
         self.reference_dir = reference_dir
         self.localization_dir = localization_dir
 
-        # A dict whose keys are `(path, key)` tuples corresponding to target
-        # FTL translations, and values are sets of `(path, key)` tuples
-        # corresponding to localized entities which will be migrated.
         self.dependencies = {}
+        """
+        A dict whose keys are `(path, key)` tuples corresponding to target
+        FTL translations, and values are sets of `(path, key)` tuples
+        corresponding to localized entities which will be migrated.
+        """
 
-    def add_transforms(self, target, reference, transforms):
+    def add_transforms(
+        self, target: str, reference: str, transforms: List[FTL.Message | FTL.Term]
+    ):
         """Define transforms for target using reference as template.
 
         `target` is a path of the destination FTL file relative to the
@@ -102,10 +111,10 @@ def get_sources(acc, cur):
         self.reference_resources[target] = reference_ast
 
         for node in transforms:
-            ident = node.id.name
+            ident = cast(str, node.id.name)
             # Scan `node` for `Source` nodes and collect the information they
             # store into a set of dependencies.
-            dependencies = fold(get_sources, node, set())
+            dependencies = cast(Set[Tuple[str, Source]], fold(get_sources, node, set()))
             # Set these sources as dependencies for the current transform.
             self.dependencies[(target, ident)] = dependencies
 

fluent/migrate/helpers.py

@@ -7,6 +7,9 @@
 (As opposed to Transforms which are AST nodes on their own and only return the
 migrated AST nodes when they are evaluated by a MigrationContext.) """
 
+from __future__ import annotations
+from typing import List
+
 from fluent.syntax import FluentParser, ast as FTL
 from fluent.syntax.visitor import Transformer
 from .transforms import Transform, CONCAT, COPY, COPY_PATTERN
@@ -123,7 +126,7 @@ def into_argument(self, node):
             )
 
 
-def transforms_from(ftl, **substitutions):
+def transforms_from(ftl, **substitutions) -> List[FTL.Message | FTL.Term]:
     """Parse FTL code into a list of Message nodes with Transforms.
 
     The FTL may use a fabricated COPY function inside of placeables which