cli: Introduce new --format argument (and remove --xunit-file)

Any output (text or xUnit) is now directed to the standard output. It
will be easier to add new formats (such as CSV or JSON, which I plan).

CHANGES.rst

@@ -1,7 +1,14 @@
-0.8.11 (unreleased)
--------------------
+1.0.0 (unreleased)
+------------------
+
+- |backward-incompatible| Remove ``--xunit-file`` argument from
+  ``check-branches`` and ``check-fixmes`` commands. It can be replaced
+  by a new ``--format=xunit`` argument and redirecting the standard
+  output to a file.
 
-- Nothing changed yet.
+  This change is needed to properly introduce the ``--format``
+  argument that controls the formatting output, which is now directed
+  to the standard output.
 
 
 0.8.10 (2023-08-16)

src/check_oldies/annotations.py

@@ -8,6 +8,7 @@
 import pkg_resources
 
 from . import commands
+from . import output
 
 
 IGNORE_PRAGMA = "no-check-fixmes"
@@ -20,8 +21,8 @@ class Config:
     path: str = "."
     max_age: int = 180
 
+    output_format: output.OutputFormat = output.OutputFormat.TEXT
     colorize_errors: bool = True
-    xunit_file: str = None
 
     annotations: typing.Sequence = ("todo", "fixme", )  # no-check-fixmes
     ignored_orphans_annotations: typing.Sequence = ("wontfix", "xxx")  # annotation which won't trigger orphans checks
@@ -79,6 +80,16 @@ class Annotation:
     assignee: str = None
     is_old: bool = None
 
+    @property
+    def must_warn(self):
+        return self.is_old
+
+    def to_text(self):
+        return (
+            f"{self.assignee: <15} - {self.age: >4} days - "
+            f"{self.filename}:{self.line_no}: {self.line_content.strip()}"
+        )
+
 
 @dataclasses.dataclass
 class FutureTag:
@@ -87,6 +98,14 @@ class FutureTag:
     tag: str
     author: str = None
 
+    must_warn = True
+
+    def to_text(self):
+        return (
+            f"{self.author: <15} -   ORPHAN  - "
+            f"{self.path}:{self.line_no}: Unknown tag {self.tag}"
+        )
+
 
 def get_annotation_candidates(directory, annotation_regex, whitelist):
     """Return lines (with filename and line number) that contains an annotation."""
@@ -151,7 +170,7 @@ def get_annotations(config: Config):
         config.path, config.annotation_regex, config.whitelist
     ):
         filename, line_no, line_content = candidate.split(":", 2)
-        # FIXME (dbaty, 2020-10-21): it should be possible to apply
+        # FIXME (dbaty, 2024-03-28): it should be possible to apply
         # the right regex (with boundaries) directly in git grep.
         if config.py_annotation_regex.search(line_content):
             annotations.append(Annotation(filename, int(line_no), line_content))

src/check_oldies/branches.py

@@ -8,6 +8,7 @@
 
 from . import commands
 from . import githost
+from . import output
 
 
 TODAY = datetime.date.today()
@@ -40,8 +41,8 @@ class Config:
     path: str = "."
     max_age: int = 90
 
+    output_format: output.OutputFormat = output.OutputFormat.TEXT
     colorize_errors: bool = True
-    xunit_file: str = None
 
     calm_branches: typing.Sequence = ("gh-pages", "master", "main", "prod", "maint(enance)?/.*")
     ignore_branches_without_pull_request: bool = False
@@ -88,6 +89,10 @@ class BranchInfo:
     is_old: bool
     pull_request: githost.PullRequestInfo = None
 
+    @property
+    def must_warn(self):
+        return self.is_old
+
     @property
     def name_and_details(self):
         details = f"{self.name} ({self.url})"
@@ -96,6 +101,11 @@ def name_and_details(self):
             details += f", linked to {pr.state} PR/MR #{pr.number} ({pr.url})"
         return details
 
+    def to_text(self):
+        return (
+            f"{self.author[:30]: <30} - {self.age: >4} days - {self.name_and_details}"
+        )
+
 
 def get_repository_info(path):
     """Extract the repository owner and name from the origin remote."""
@@ -131,12 +141,12 @@ def get_branches(config: Config):
         branch = branch.strip()[len("origin/") :]
         if config.ignore_branch(branch) or "->" in branch:
             continue
-        output = commands.get_output(
+        out = commands.get_output(
             ("git", "log", f"origin/{branch}", "-1", "--format=%ae %ci"),
             cwd=config.path,
         )[0]
         # line looks like "[email protected] 2018-12-19 14:18:52 +0100"
-        email, date, *_rest = output.split(" ")
+        email, date, *_rest = out.split(" ")
         date = datetime.date(*[int(s) for s in date.split("-")])
         age = (TODAY - date).days
         branches.append(
@@ -151,7 +161,7 @@ def get_branches(config: Config):
         )
 
     if not branches:
-        return ()
+        return []
 
     if config.host_api_access:
         pr_getter = githost.PullRequestGetter(config.platform, config.host_owner, config.host_api_access)

src/check_oldies/check_branches.py

@@ -2,9 +2,10 @@
 import os
 import sys
 
-from . import branches
+import check_oldies.branches
+
 from . import configuration
-from . import xunit
+from . import output
 
 
 def get_parser():
@@ -26,12 +27,20 @@ def get_parser():
             "Defaults to the working directory."
         ),
     )
+    parser.add_argument(
+        "--format",
+        default=output.OutputFormat.TEXT,
+        dest="output_format",
+        help="Output format. Defaults to human-readable text (one result per line).",
+        choices=sorted(output.OutputFormat),
+        type=output.OutputFormat,
+    )
     parser.add_argument(
         "--max-age",
         type=int,
         help=(
             f"Maximum age in days allowed for a branch, errors otherwise. "
-            f"Defaults to {branches.Config.max_age}."
+            f"Defaults to {check_oldies.branches.Config.max_age}."
         ),
     )
     parser.add_argument(
@@ -41,66 +50,37 @@ def get_parser():
         dest="colorize_errors",
         help="Do not colorize errors. Defaults to colorizing errors in red.",
     )
-    parser.add_argument(
-        "--xunit-file",
-        action="store",
-        help="Path of the xUnit report file to write. Defaults to no xUnit output.",
-    )
     return parser
 
 
-def branch_str(branch):
-    return (
-        f"{branch.author[:30]: <30} - {branch.age: >4} days - {branch.name_and_details}"
-    )
-
-
 def main():
     parser = get_parser()
     config = configuration.get_config(
-        "check-branches", parser, sys.argv[1:], branches.Config
+        "check-branches", parser, sys.argv[1:], check_oldies.branches.Config
     )
     if not configuration.is_git_directory(config.path):
         sys.exit(f'Invalid path: "{config.path}" is not a Git repository.')
 
-    if config.colorize_errors:
-        warn = "\033[91m{}\033[0m".format
-    else:
-        warn = lambda text: text  # pylint: disable=unnecessary-lambda-assignment
-
-    all_branches = branches.get_branches(config)
-
-    out = []
-    uncolorized_out = []
-    for branch in sorted(
-        all_branches, key=lambda branch: (branch.author, -branch.age, branch.name)
-    ):
-        line = branch_str(branch)
-        out.append(warn(line) if branch.is_old else line)
-        uncolorized_out.append(line if branch.is_old else line)
-    has_old_branches = any(branch for branch in all_branches if branch.is_old)
+    branches = check_oldies.branches.get_branches(config)
+    branches.sort(key=lambda branch: (branch.author, -branch.age, branch.name))
+    has_old_branches = any(branch for branch in branches if branch.is_old)
 
-    out = os.linesep.join(out)
+    ok_msg = err_msg = ""
     if has_old_branches:
         err_msg = "NOK: Some branches are too old."
-        print(err_msg)
     else:
-        err_msg = ""
-        print("OK: All branches are fresh.")
-    if out:
-        print(out)
+        ok_msg = "OK: All branches are fresh."
 
-    if config.xunit_file:
-        uncolorized_out = os.linesep.join(uncolorized_out)
-        xunit.create_xunit_file(
-            os.path.abspath(config.xunit_file),
-            "check-branches",
-            "branches",
-            "CheckBranches",
-            err_msg=err_msg,
-            stdout=uncolorized_out,
-            stderr="",
-        )
+    output.printer(
+        branches,
+        config.output_format,
+        ok_message=ok_msg,
+        error_message=err_msg,
+        colorize_errors=config.colorize_errors,
+        xunit_suite_name="check-branches",
+        xunit_case_name="branches",
+        xunit_class_name="CheckBranches",
+    )
 
     sys.exit(os.EX_DATAERR if has_old_branches else os.EX_OK)
 

src/check_oldies/check_fixmes.py

@@ -2,23 +2,9 @@
 import os
 import sys
 
-from . import annotations
+import check_oldies.annotations
 from . import configuration
-from . import xunit
-
-
-def annotation_str(annotation):
-    return (
-        f"{annotation.assignee: <15} - {annotation.age: >4} days - "
-        f"{annotation.filename}:{annotation.line_no}: {annotation.line_content.strip()}"
-    )
-
-
-def orphan_str(orphan):
-    return (
-        f"{orphan.author: <15} -   ORPHAN  - "
-        f"{orphan.path}:{orphan.line_no}: Unknown tag {orphan.tag}"
-    )
+from . import output
 
 
 def get_parser():
@@ -40,12 +26,20 @@ def get_parser():
             "Defaults to the working directory."
         ),
     )
+    parser.add_argument(
+        "--format",
+        default=output.OutputFormat.TEXT,
+        dest="output_format",
+        help="Output format. Defaults to human-readable text (one result per line).",
+        choices=sorted(output.OutputFormat),
+        type=output.OutputFormat,
+    )
     parser.add_argument(
         "--max-age",
         type=int,
         help=(
             f"Maximum age in days allowed for an annotation, errors otherwise. "
-            f"Defaults to {annotations.Config.max_age}."
+            f"Defaults to {check_oldies.annotations.Config.max_age}."
         ),
     )
     parser.add_argument(
@@ -55,70 +49,44 @@ def get_parser():
         dest="colorize_errors",
         help="Do not colorize errors. Defaults to colorizing errors in red.",
     )
-    parser.add_argument(
-        "--xunit-file",
-        action="store",
-        help="Path of the xUnit report file to write. Defaults to no xUnit output.",
-    )
     return parser
 
 
 def main():
     parser = get_parser()
     config = configuration.get_config(
-        "check-fixmes", parser, sys.argv[1:], annotations.Config
+        "check-fixmes", parser, sys.argv[1:], check_oldies.annotations.Config
     )
     if not configuration.is_git_directory(config.path):
         sys.exit(f'Invalid path: "{config.path}" is not a Git repository.')
 
-    if config.colorize_errors:
-        warn = "\033[91m{}\033[0m".format
-    else:
-        warn = lambda text: text  # pylint: disable=unnecessary-lambda-assignment
-
     # Look for old annotations
-    out = []
-    uncolorized_out = []
-    all_annotations = sorted(
-        annotations.get_annotations(config),
-        key=lambda f: (f.assignee, -f.age, f.filename, f.line_no),
-    )
-    for annotation in all_annotations:
-        line = annotation_str(annotation)
-        out.append(warn(line) if annotation.is_old else line)
-        uncolorized_out.append(line if annotation.is_old else line)
-    has_old_annotations = any(ann for ann in all_annotations if ann.is_old)
+    annotations = check_oldies.annotations.get_annotations(config)
+    annotations.sort(key=lambda f: (f.assignee, -f.age, f.filename, f.line_no))
+    has_old_annotations = any(ann for ann in annotations if ann.is_old)
 
     # Look for orphan FUTURE tags
-    orphan_futures = annotations.get_orphan_futures(config)
-    for orphan in orphan_futures:
-        out.append(warn(orphan_str(orphan)))
-        uncolorized_out.append(orphan_str(orphan))
+    orphan_futures = check_oldies.annotations.get_orphan_futures(config)
 
-    out = os.linesep.join(out)
+    ok_msg = err_msg = ""
     if has_old_annotations or orphan_futures:
         err_msg = "NOK: Some annotations are too old, or there are orphan FUTURE tags."
-        print(err_msg)
     else:
-        err_msg = ""
-        if all_annotations:
-            print("OK: All annotations are fresh.")
+        if annotations:
+            ok_msg = "OK: All annotations are fresh."
         else:
-            print("OK: No annotations were found.")
-    if out:
-        print(out)
-
-    if config.xunit_file:
-        uncolorized_out = os.linesep.join(uncolorized_out)
-        xunit.create_xunit_file(
-            os.path.abspath(config.xunit_file),
-            suite_name="check-fixmes",
-            case_name="fixmes",
-            class_name="CheckFixmes",
-            err_msg=err_msg,
-            stdout=uncolorized_out,
-            stderr="",
-        )
+            ok_msg = "OK: No annotations were found."
+
+    output.printer(
+        annotations + orphan_futures,
+        config.output_format,
+        ok_message=ok_msg,
+        error_message=err_msg,
+        colorize_errors=config.colorize_errors,
+        xunit_suite_name="check-fixmes",
+        xunit_case_name="fixmes",
+        xunit_class_name="CheckFixmes",
+    )
 
     sys.exit(os.EX_DATAERR if has_old_annotations or orphan_futures else os.EX_OK)