Merge pull request #429 from twisted/config-class

Make configuration a proper class, push settings & check coverage to 100%

pyproject.toml

@@ -64,7 +64,7 @@ line_length = 88
 strict = true
 # 2022-09-04: Trial's API isn't annotated yet, which limits the usefulness of type-checking
 #             the unit tests. Therefore they have not been annotated yet.
-exclude = '^src/towncrier/test/.*\.py$'
+exclude = '^src/towncrier/test/test_.*\.py$'
 
 [[tool.mypy.overrides]]
 module = 'click_default_group'
@@ -91,6 +91,10 @@ source = ["src", ".tox/*/site-packages"]
 [tool.coverage.report]
 show_missing = true
 skip_covered = true
+exclude_lines = [
+    "pragma: no cover",
+    "if TYPE_CHECKING:",
+]
 omit = [
     "src/towncrier/__main__.py",
     "src/towncrier/test/*",

src/towncrier/_builder.py

@@ -9,7 +9,7 @@
 import traceback
 
 from collections import OrderedDict, defaultdict
-from typing import Any, DefaultDict, Iterator, Mapping, Sequence
+from typing import Any, DefaultDict, Iterable, Iterator, Mapping, Sequence
 
 from jinja2 import Template
 
@@ -28,7 +28,7 @@ def strip_if_integer_string(s: str) -> str:
 # Returns ticket, category and counter or (None, None, None) if the basename
 # could not be parsed or doesn't contain a valid category.
 def parse_newfragment_basename(
-    basename: str, definitions: Sequence[str]
+    basename: str, frag_type_names: Iterable[str]
 ) -> tuple[str, str, int] | tuple[None, None, None]:
     invalid = (None, None, None)
     parts = basename.split(".")
@@ -38,14 +38,14 @@ def parse_newfragment_basename(
     if len(parts) == 2:
         ticket, category = parts
         ticket = strip_if_integer_string(ticket)
-        return (ticket, category, 0) if category in definitions else invalid
+        return (ticket, category, 0) if category in frag_type_names else invalid
 
     # There are at least 3 parts. Search for a valid category from the second
     # part onwards.
     # The category is used as the reference point in the parts list to later
     # infer the issue number and counter value.
     for i in range(1, len(parts)):
-        if parts[i] in definitions:
+        if parts[i] in frag_type_names:
             # Current part is a valid category according to given definitions.
             category = parts[i]
             # Use the previous part as the ticket number.
@@ -83,7 +83,7 @@ def find_fragments(
     base_directory: str,
     sections: Mapping[str, str],
     fragment_directory: str | None,
-    definitions: Sequence[str],
+    frag_type_names: Iterable[str],
     orphan_prefix: str | None = None,
 ) -> tuple[Mapping[str, Mapping[tuple[str, str, int], str]], list[str]]:
     """
@@ -115,7 +115,7 @@ def find_fragments(
         for basename in files:
 
             ticket, category, counter = parse_newfragment_basename(
-                basename, definitions
+                basename, frag_type_names
             )
             if category is None:
                 continue
@@ -244,7 +244,7 @@ def render_fragments(
     template: str,
     issue_format: str | None,
     fragments: Mapping[str, Mapping[str, Mapping[str, Sequence[str]]]],
-    definitions: Sequence[str],
+    definitions: Mapping[str, Mapping[str, Any]],
     underlines: Sequence[str],
     wrap: bool,
     versiondata: Mapping[str, str],

src/towncrier/_settings/load.py

@@ -7,19 +7,49 @@
 import sys
 
 from collections import OrderedDict
-from typing import Any, Mapping
+from dataclasses import dataclass
+from typing import TYPE_CHECKING, Any, Mapping
 
 import pkg_resources
 
 from .._settings import fragment_types as ft
 
 
+if TYPE_CHECKING:
+    # We only use Literal for type-checking and Mypy always brings its own
+    # typing_extensions so this is safe without further dependencies.
+    if sys.version_info < (3, 8):
+        from typing_extensions import Literal
+    else:
+        from typing import Literal
+
 if sys.version_info < (3, 11):
     import tomli as tomllib
 else:
     import tomllib
 
 
+@dataclass
+class Config:
+    package: str
+    package_dir: str
+    single_file: bool
+    filename: str
+    directory: str | None
+    version: str | None
+    name: str | None
+    sections: Mapping[str, str]
+    types: Mapping[str, Mapping[str, Any]]
+    template: str
+    start_string: str
+    title_format: str | Literal[False]
+    issue_format: str | None
+    underlines: list[str]
+    wrap: bool
+    all_bullets: bool
+    orphan_prefix: str
+
+
 class ConfigError(Exception):
     def __init__(self, *args: str, **kwargs: str):
         self.failing_option = kwargs.get("failing_option")
@@ -34,7 +64,7 @@ def __init__(self, *args: str, **kwargs: str):
 
 def load_config_from_options(
     directory: str | None, config_path: str | None
-) -> tuple[str, Mapping[str, Any]]:
+) -> tuple[str, Config]:
     if config_path is None:
         if directory is None:
             directory = os.getcwd()
@@ -43,10 +73,10 @@ def load_config_from_options(
         config = load_config(base_directory)
     else:
         config_path = os.path.abspath(config_path)
-        if directory:
-            base_directory = os.path.abspath(directory)
-        else:
+        if directory is None:
             base_directory = os.path.dirname(config_path)
+        else:
+            base_directory = os.path.abspath(directory)
         config = load_config_from_file(os.path.dirname(config_path), config_path)
 
     if config is None:
@@ -55,7 +85,7 @@ def load_config_from_options(
     return base_directory, config
 
 
-def load_config(directory: str) -> Mapping[str, Any] | None:
+def load_config(directory: str) -> Config | None:
 
     towncrier_toml = os.path.join(directory, "towncrier.toml")
     pyproject_toml = os.path.join(directory, "pyproject.toml")
@@ -70,14 +100,14 @@ def load_config(directory: str) -> Mapping[str, Any] | None:
     return load_config_from_file(directory, config_file)
 
 
-def load_config_from_file(directory: str, config_file: str) -> Mapping[str, Any]:
+def load_config_from_file(directory: str, config_file: str) -> Config:
     with open(config_file, "rb") as conffile:
         config = tomllib.load(conffile)
 
     return parse_toml(directory, config)
 
 
-def parse_toml(base_path: str, config: Mapping[str, Any]) -> Mapping[str, Any]:
+def parse_toml(base_path: str, config: Mapping[str, Any]) -> Config:
     if "tool" not in config:
         raise ConfigError("No [tool.towncrier] section.", failing_option="all")
 
@@ -134,22 +164,22 @@ def parse_toml(base_path: str, config: Mapping[str, Any]) -> Mapping[str, Any]:
             failing_option="template",
         )
 
-    return {
-        "package": config.get("package", ""),
-        "package_dir": config.get("package_dir", "."),
-        "single_file": single_file,
-        "filename": config.get("filename", "NEWS.rst"),
-        "directory": config.get("directory"),
-        "version": config.get("version"),
-        "name": config.get("name"),
-        "sections": sections,
-        "types": types,
-        "template": template,
-        "start_string": config.get("start_string", _start_string),
-        "title_format": config.get("title_format", _title_format),
-        "issue_format": config.get("issue_format"),
-        "underlines": config.get("underlines", _underlines),
-        "wrap": wrap,
-        "all_bullets": all_bullets,
-        "orphan_prefix": config.get("orphan_prefix", "+"),
-    }
+    return Config(
+        package=config.get("package", ""),
+        package_dir=config.get("package_dir", "."),
+        single_file=single_file,
+        filename=config.get("filename", "NEWS.rst"),
+        directory=config.get("directory"),
+        version=config.get("version"),
+        name=config.get("name"),
+        sections=sections,
+        types=types,
+        template=template,
+        start_string=config.get("start_string", _start_string),
+        title_format=config.get("title_format", _title_format),
+        issue_format=config.get("issue_format"),
+        underlines=config.get("underlines", _underlines),
+        wrap=wrap,
+        all_bullets=all_bullets,
+        orphan_prefix=config.get("orphan_prefix", "+"),
+    )

src/towncrier/build.py

@@ -114,51 +114,47 @@ def __main(
     to_err = draft
 
     click.echo("Loading template...", err=to_err)
-    with open(config["template"], "rb") as tmpl:
+    with open(config.template, "rb") as tmpl:
         template = tmpl.read().decode("utf8")
 
     click.echo("Finding news fragments...", err=to_err)
 
-    definitions = config["types"]
-
-    if config.get("directory"):
-        fragment_base_directory = os.path.abspath(config["directory"])
+    if config.directory is not None:
+        fragment_base_directory = os.path.abspath(config.directory)
         fragment_directory = None
     else:
         fragment_base_directory = os.path.abspath(
-            os.path.join(base_directory, config["package_dir"], config["package"])
+            os.path.join(base_directory, config.package_dir, config.package)
         )
         fragment_directory = "newsfragments"
 
     fragment_contents, fragment_filenames = find_fragments(
         fragment_base_directory,
-        config["sections"],
+        config.sections,
         fragment_directory,
-        definitions,
-        config["orphan_prefix"],
+        config.types,
+        config.orphan_prefix,
     )
 
     click.echo("Rendering news fragments...", err=to_err)
     fragments = split_fragments(
-        fragment_contents, definitions, all_bullets=config["all_bullets"]
+        fragment_contents, config.types, all_bullets=config.all_bullets
     )
 
     if project_version is None:
-        project_version = config.get("version")
+        project_version = config.version
         if project_version is None:
             project_version = get_version(
-                os.path.join(base_directory, config["package_dir"]), config["package"]
+                os.path.join(base_directory, config.package_dir), config.package
             ).strip()
 
     if project_name is None:
-        project_name = config.get("name")
+        project_name = config.name
         if not project_name:
-            package = config.get("package")
+            package = config.package
             if package:
                 project_name = get_project_name(
-                    os.path.abspath(
-                        os.path.join(base_directory, config["package_dir"])
-                    ),
+                    os.path.abspath(os.path.join(base_directory, config.package_dir)),
                     package,
                 )
             else:
@@ -168,13 +164,13 @@ def __main(
     if project_date is None:
         project_date = _get_date().strip()
 
-    if config["title_format"]:
-        top_line = config["title_format"].format(
+    if config.title_format:
+        top_line = config.title_format.format(
             name=project_name, version=project_version, project_date=project_date
         )
         render_title_with_fragments = False
         render_title_separately = True
-    elif config["title_format"] is False:
+    elif config.title_format is False:
         # This is an odd check but since we support both "" and False with
         # different effects we have to do something a bit abnormal here.
         top_line = ""
@@ -188,22 +184,22 @@ def __main(
     rendered = render_fragments(
         # The 0th underline is used for the top line
         template,
-        config["issue_format"],
+        config.issue_format,
         fragments,
-        definitions,
-        config["underlines"][1:],
-        config["wrap"],
+        config.types,
+        config.underlines[1:],
+        config.wrap,
         {"name": project_name, "version": project_version, "date": project_date},
-        top_underline=config["underlines"][0],
-        all_bullets=config["all_bullets"],
+        top_underline=config.underlines[0],
+        all_bullets=config.all_bullets,
         render_title=render_title_with_fragments,
     )
 
     if render_title_separately:
         content = "\n".join(
             [
                 top_line,
-                config["underlines"][0] * len(top_line),
+                config.underlines[0] * len(top_line),
                 rendered,
             ]
         )
@@ -219,10 +215,9 @@ def __main(
         click.echo(content)
     else:
         click.echo("Writing to newsfile...", err=to_err)
-        start_string = config["start_string"]
-        news_file = config["filename"]
+        news_file = config.filename
 
-        if config["single_file"] is False:
+        if config.single_file is False:
             # The release notes for each version are stored in a separate file.
             # The name of that file is generated based on the current version and project.
             news_file = news_file.format(
@@ -232,10 +227,10 @@ def __main(
         append_to_newsfile(
             base_directory,
             news_file,
-            start_string,
+            config.start_string,
             top_line,
             content,
-            single_file=config["single_file"],
+            single_file=config.single_file,
         )
 
         click.echo("Staging newsfile...", err=to_err)

src/towncrier/check.py

@@ -104,27 +104,27 @@ def __main(
         click.echo(f"{n}. {change}")
     click.echo("----")
 
-    news_file = os.path.normpath(os.path.join(base_directory, config["filename"]))
+    news_file = os.path.normpath(os.path.join(base_directory, config.filename))
     if news_file in files:
         click.echo("Checks SKIPPED: news file changes detected.")
         sys.exit(0)
 
-    if config.get("directory"):
-        fragment_base_directory = os.path.abspath(config["directory"])
+    if config.directory:
+        fragment_base_directory = os.path.abspath(config.directory)
         fragment_directory = None
     else:
         fragment_base_directory = os.path.abspath(
-            os.path.join(base_directory, config["package_dir"], config["package"])
+            os.path.join(base_directory, config.package_dir, config.package)
         )
         fragment_directory = "newsfragments"
 
     fragments = {
         os.path.normpath(path)
         for path in find_fragments(
             fragment_base_directory,
-            config["sections"],
+            config.sections,
             fragment_directory,
-            config["types"],
+            config.types.keys(),
         )[1]
     }
     fragments_in_branch = fragments & files