From b456dc5342359b3e5e48e55d42f029fe8240dd06 Mon Sep 17 00:00:00 2001
From: Markus Hofbauer <markus.hofbauer@tum.de>
Date: Tue, 22 Nov 2022 17:57:15 +0100
Subject: [PATCH] print all collisions found before failing

---
 .../alias_collision/check_alias_collision.py  | 57 ++++++++++++-------
 .../tests/test_check_alias_collision.py       | 27 ++++++---
 2 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/tools/alias_collision/check_alias_collision.py b/tools/alias_collision/check_alias_collision.py
index e6cbd8586..ca0fc671f 100644
--- a/tools/alias_collision/check_alias_collision.py
+++ b/tools/alias_collision/check_alias_collision.py
@@ -2,16 +2,14 @@
 
 from argparse import ArgumentDefaultsHelpFormatter, ArgumentParser
 from pathlib import Path
-from typing import List
 from dataclasses import dataclass
 import itertools
 import re
 
 
-ERROR_MESSAGE_TEMPLATE = """Found alias collision
-Alias `%s` defined in `%s` already exists as alias `%s` in `%s`.
-Consider renaming your alias.
-"""
+ERROR_MESSAGE_TEMPLATE = (
+    "Alias `%s` defined in `%s` already exists as alias `%s` in `%s`."
+)
 
 
 def dir_path(path_string: str) -> Path:
@@ -42,41 +40,62 @@ class Alias:
     module: Path
 
 
-def find_aliases_in_file(file: Path) -> List[Alias]:
+@dataclass(frozen=True)
+class Collision:
+
+    existing_alias: Alias
+    new_alias: Alias
+
+
+def find_aliases_in_file(file: Path) -> list[Alias]:
     matches = re.findall(r"^alias (.*)='(.*)'", file.read_text(), re.M)
     return [Alias(match[0], match[1], file) for match in matches]
 
 
-def find_all_aliases(path: Path) -> List:
+def find_all_aliases(path: Path) -> list:
     files = list(path.rglob("*.zsh"))
     aliases = [find_aliases_in_file(file) for file in files]
     return list(itertools.chain(*aliases))
 
 
-def check_for_duplicates(aliases: List[Alias]) -> None:
+def check_for_duplicates(aliases: list[Alias]) -> list[Collision]:
     elements = {}
+    collisions = []
     for alias in aliases:
         if alias.alias in elements:
             existing = elements[alias.alias]
-            raise ValueError(
-                ERROR_MESSAGE_TEMPLATE
-                % (
-                    f"{alias.alias}={alias.value}",
-                    alias.module.name,
-                    f"{existing.alias}={existing.value}",
-                    existing.module.name,
-                )
-            )
+            collisions.append(Collision(existing, alias))
         else:
             elements[alias.alias] = alias
+    return collisions
+
+
+def print_collisions(collisions: dict[Alias, Alias]) -> None:
+    if collisions:
+        print(f"Found {len(collisions)} alias collisions:\n")
+        for collision in collisions:
+            print(
+                ERROR_MESSAGE_TEMPLATE
+                % (
+                    f"{collision.new_alias.alias}={collision.new_alias.value}",
+                    collision.new_alias.module.name,
+                    f"{collision.existing_alias.alias}={collision.existing_alias.value}",
+                    collision.existing_alias.module.name,
+                )
+            )
+        print("\nConsider renaming your aliases.")
+    else:
+        print("Found no collisions")
 
 
 def main():
     """main"""
     args = parse_arguments()
     aliases = find_all_aliases(args.folder)
-    check_for_duplicates(aliases)
-    print("Found no collisions")
+    collisions = check_for_duplicates(aliases)
+    print_collisions(collisions)
+    if collisions:
+        exit(-1)
 
 
 if __name__ == "__main__":
diff --git a/tools/alias_collision/tests/test_check_alias_collision.py b/tools/alias_collision/tests/test_check_alias_collision.py
index 0183ed901..4eb0ae343 100644
--- a/tools/alias_collision/tests/test_check_alias_collision.py
+++ b/tools/alias_collision/tests/test_check_alias_collision.py
@@ -7,6 +7,7 @@ from check_alias_collision import (
     find_aliases_in_file,
     check_for_duplicates,
     Alias,
+    Collision,
 )
 
 
@@ -73,21 +74,29 @@ class CheckAliasCollisionTest(TestCase):
         result = find_aliases_in_file(Path("conditional.zsh"))
         self.assertListEqual([], result)
 
-    def test_check_for_duplicates__no_duplicates_should_not_raise(self) -> None:
-        check_for_duplicates(
+    def test_check_for_duplicates__no_duplicates_should_return_empty_dict(self) -> None:
+        result = check_for_duplicates(
             [
                 Alias("g", "git", Path("git.zsh")),
                 Alias("ga", "git add", Path("git.zsh")),
                 Alias("gaa", "git add --all", Path("git.zsh")),
             ]
         )
-        self.assertTrue(True)
+        self.assertListEqual(result, [])
 
-    def test_check_for_duplicates__duplicates_should_raise(self) -> None:
-        with self.assertRaises(ValueError):
-            check_for_duplicates(
-                [
+    def test_check_for_duplicates__duplicates_should_have_one_collision(self) -> None:
+        result = check_for_duplicates(
+            [
+                Alias("gc", "git commit", Path("git.zsh")),
+                Alias("gc", "git clone", Path("git.zsh")),
+            ]
+        )
+        self.assertListEqual(
+            result,
+            [
+                Collision(
                     Alias("gc", "git commit", Path("git.zsh")),
                     Alias("gc", "git clone", Path("git.zsh")),
-                ]
-            )
+                )
+            ],
+        )