fix(zoom-html): show business domains in projects organised by type

graphify/zoom_html.py

@@ -10,9 +10,13 @@
     ``method`` edge whose source is a class.
   * Layers come from folder names only (Symfony by type or by domain, Laravel).
     A class whose folder names no layer goes to ``other``, never guessed.
-  * The domain is the first folder under the code root; inside a layer folder
-    (``src/Controller/Account/``) it is the folder below the layer. A class
-    placed directly in a layer folder has no known domain.
+  * Type folders are the names framework generators give to a kind of class
+    (``Controller``, ``Entity``, ``Command``, ``Form``, ``Enum``...). A code root
+    where most classes sit below a type folder is organised by type: the domain
+    is the first non-type folder below (``src/Service/Billing/`` gives Billing).
+    Otherwise it is organised by domain: the domain is the first folder under
+    the root, unless that folder is a type folder. A class with no domain folder
+    goes to the explicit "Common" group, never to a guessed domain.
   * A template belongs to a domain only through a ``renders`` edge from one of
     that domain's classes.
   * Only EXTRACTED calls can mark a dependency as going against the layer or
@@ -51,6 +55,25 @@
     "entity": "entity", "entities": "entity", "model": "entity", "models": "entity",
     "view": "view", "views": "view",
 }
+# Folder names framework generators give to a kind of class (Symfony maker, Laravel make:*), never to a business domain.
+_TYPE_DIRS = set(_LAYER_DIRS) | {
+    "abstract", "abstracts", "attribute", "attributes", "authenticator", "authenticators",
+    "cast", "casts", "channel", "channels", "concerns", "function", "functions", "handler", "handlers",
+    "purger", "purgers",
+    "command", "commands", "component", "components", "console", "contract", "contracts",
+    "datafixtures", "datatransformer", "datatransformers", "doctrine", "dto", "dtos", "enum", "enums",
+    "event", "events", "eventlistener", "eventlisteners", "eventsubscriber", "eventsubscribers",
+    "exception", "exceptions", "extension", "extensions", "factory", "factories", "fixtures",
+    "form", "forms", "helper", "helpers", "http", "interface", "interfaces", "job", "jobs",
+    "listener", "listeners", "mail", "message", "messages", "messagehandler", "messagehandlers",
+    "middleware", "migration", "migrations", "normalizer", "normalizers", "notification", "notifications",
+    "observer", "observers", "policies", "policy", "provider", "providers", "request", "requests",
+    "resource", "resources", "rule", "rules", "scope", "scopes", "security", "serializer",
+    "subscriber", "subscribers", "support", "trait", "traits", "twig", "type", "types",
+    "util", "utils", "validator", "validators", "voter", "voters",
+}
+LAYOUT_BY_TYPE = "by-type"
+LAYOUT_BY_DOMAIN = "by-domain"
 _TEST_DIRS = {"test", "tests", "spec", "specs", "__tests__"}
 _TEST_FILE = re.compile(r"(Test|Tests|_test|\.test|\.spec)\.[A-Za-z]+$|^test_[^/]*\.py$")
 _VENDOR_DIRS = {"vendor", "vendors", "node_modules", "bower_components", "third_party", "third-party"}
@@ -90,27 +113,32 @@ def classify_layer(source_file: str, root: Optional[str]) -> str:
     return "other"
 
 
-def classify_domain(source_file: str, root: Optional[str]) -> Optional[str]:
-    """Domain folder of a class under *root*, or None when no folder names one."""
+def detect_layout(class_files: list[str], root: Optional[str]) -> str:
+    """``by-type`` when most classes under *root* sit below a type folder, else ``by-domain``."""
+    nested = [_parts(f) for f in class_files
+              if f and not is_test_path(f) and not is_vendor_path(f)]
+    nested = [p for p in nested if root and len(p) >= 3 and p[0] == root]
+    typed = sum(1 for p in nested if p[1].lower() in _TYPE_DIRS)
+    return LAYOUT_BY_TYPE if nested and typed * 2 >= len(nested) else LAYOUT_BY_DOMAIN
+
+
+def classify_domain(source_file: str, root: Optional[str], layout: str = LAYOUT_BY_DOMAIN) -> Optional[str]:
+    """Business domain folder of a class, or None when only type folders hold it."""
     parts = _parts(source_file)
     if not root or len(parts) < 3 or parts[0] != root:
         return None
-    rest = parts[1:-1]
-    first = rest[0]
-    if first.lower() == "http" and len(rest) > 1 and rest[1].lower() in ("controllers", "controller"):
-        rest = rest[1:]
-        first = rest[0]
-    if first.lower() in _LAYER_DIRS:
-        return rest[1] if len(rest) > 1 else None
-    return first
+    dirs = parts[1:-1]
+    if layout == LAYOUT_BY_TYPE:
+        return next((d for d in dirs if d.lower() not in _TYPE_DIRS), None)
+    return None if dirs[0].lower() in _TYPE_DIRS else dirs[0]
 
 
 def _line(location: Any) -> int:
     m = re.match(r"L(\d+)", str(location or ""))
     return int(m.group(1)) if m else 0
 
 
-def _group_of(source_file: str, external: bool, root: Optional[str]) -> tuple[str, str]:
+def _group_of(source_file: str, external: bool, root: Optional[str], layout: str) -> tuple[str, str]:
     """(kind, name) of the level-1 box a class belongs to."""
     if external or not source_file or is_vendor_path(source_file):
         return KIND_VENDOR, "Framework / vendor"
@@ -119,9 +147,9 @@ def _group_of(source_file: str, external: bool, root: Optional[str]) -> tuple[st
     parts = _parts(source_file)
     if not root or not parts or parts[0] != root:
         return KIND_OUTSIDE, f"{parts[0]}/" if len(parts) > 1 else "(repository root)"
-    domain = classify_domain(source_file, root)
+    domain = classify_domain(source_file, root, layout)
     if domain is None:
-        return KIND_NONE, "(no domain)"
+        return KIND_NONE, "Common"
     return KIND_DOMAIN, domain
 
 
@@ -188,7 +216,9 @@ def build_zoom_data(graph: dict, project: str = "") -> dict:
 
     class_ids = sorted(i for i, n in nodes.items() if n.get("_callable_class"))
     class_idx = {cid: k for k, cid in enumerate(class_ids)}
-    root = detect_code_root([str(nodes[c].get("source_file") or "") for c in class_ids])
+    class_files = [str(nodes[c].get("source_file") or "") for c in class_ids]
+    root = detect_code_root(class_files)
+    layout = detect_layout(class_files, root)
 
     files: list[str] = []
     file_idx: dict[str, int] = {}
@@ -205,7 +235,7 @@ def fid(path: str) -> int:
     for cid in class_ids:
         n = nodes[cid]
         src = str(n.get("source_file") or "")
-        key = _group_of(src, bool(n.get("external")), root)
+        key = _group_of(src, bool(n.get("external")), root, layout)
         if key not in group_idx:
             group_idx[key] = len(groups)
             groups.append(key)
@@ -294,6 +324,7 @@ def owner_of(node_id: str) -> tuple[int, int]:
     return {
         "project": project,
         "root": root,
+        "layout": layout,
         "layers": [label for _, label in LAYERS],
         "layerRank": [LAYER_RANK.get(key, -1) for key, _ in LAYERS],
         "files": files,
@@ -425,7 +456,7 @@ def write_zoom_html(graph_path: Path, output_path: Path, *, project_label: Optio
 .cls.is-active { border: 3px solid var(--focus); }
 .tpl { color: var(--ink2); font-style: italic; }
 .icon-against { flex: none; width: 16px; height: 16px; }
-.icon-against path { fill: var(--danger); } .icon-against text { fill: var(--on-danger); font: 700 11px system-ui, sans-serif; }
+.icon-against path { fill: var(--danger); } .icon-against text { fill: var(--on-danger); font: 700 12px system-ui, sans-serif; }
 .more { font: inherit; font-size: .88rem; min-height: 36px; padding: 4px 10px; background: transparent; color: var(--ink); border: 1px solid var(--border); border-radius: 4px; cursor: pointer; }
 .more:hover { border-color: var(--ink); }
 .flows { display: block; margin: 0 0 4px; }
@@ -605,6 +636,7 @@ def write_zoom_html(graph_path: Path, output_path: Path, *, project_label: Optio
   classAgainst: function (s) { s.appendChild(sv("rect", { x: 2, y: 3, width: 40, height: 18, rx: 2, style: "fill:var(--box);stroke:var(--danger);stroke-width:2" })); s.appendChild(sv("path", { d: "M10 6 16 17H4Z", style: "fill:var(--danger)" })); },
   columns: function (s) { [2, 16, 30].forEach(function (x) { s.appendChild(sv("rect", { x: x, y: 3, width: 12, height: 18, rx: 1, style: "fill:var(--panel);stroke:var(--border)" })); }); },
   badge: function (s) { s.appendChild(sv("rect", { x: 6, y: 2, width: 32, height: 20, rx: 10, style: "fill:var(--danger)" })); s.appendChild(sv("text", { x: 22, y: 16, "text-anchor": "middle", style: "fill:var(--on-danger);font-weight:700;font-size:12px" }, "! 2")); },
+  common: function (s) { s.appendChild(sv("rect", { x: 2, y: 4, width: 40, height: 16, rx: 2, style: "fill:var(--box);stroke:var(--border)" })); s.appendChild(sv("text", { x: 22, y: 16, "text-anchor": "middle", style: "font-size:12px;font-weight:650" }, "C")); },
   fold: function (s) { s.appendChild(sv("rect", { x: 2, y: 4, width: 40, height: 16, rx: 2, style: "fill:var(--box);stroke:var(--border);stroke-dasharray:4 3" })); }
 };
 
@@ -649,7 +681,9 @@ def write_zoom_html(graph_path: Path, output_path: Path, *, project_label: Optio
   var against = m.links.filter(function (l) { return l.against; }).length;
   view.appendChild(el("h2", { tabindex: "-1" }, "Domains"));
   var hiddenLinks = m.links.filter(function (l) { return !l.shownByDefault && !l.against; }).length;
-  view.appendChild(el("p", { "class": "lede" }, plural(m.nodes.length, "domain") + " shown, callers above the code they call" +
+  var common = m.nodes.some(function (n) { return n.kind === "none"; });
+  view.appendChild(el("p", { "class": "lede" }, (D.layout === "by-type" ? "Code organised by type (Controller/, Service/...): each domain is the first business folder below them. " : "") +
+    plural(m.nodes.length, "domain") + " shown, callers above the code they call" +
     (hiddenLinks ? ", with the " + TOP_LINKS + " strongest links (" + plural(hiddenLinks, "weaker link") + " under All links). " : ". ") +
     (against ? (against === 1 ? "1 dependency runs back up and closes a cycle: the domain marked “!” starts it." : against + " dependencies run back up and close cycles: the domains marked “!” start them.") : "No dependency runs back up.")));
   var details = el("p", { "class": "details", id: "details" }, "Hover or focus a domain to see its links. Open it to see its classes by layer.");
@@ -723,7 +757,7 @@ def write_zoom_html(graph_path: Path, output_path: Path, *, project_label: Optio
         g.appendChild(badge);
         fitNames[fitNames.length - 1].push(W - bw - 30);
       }
-      g.appendChild(sv("text", { "class": "sub", x: 12, y: 56 }, plural(n.classes, "class", "classes") + (n.kind === "none" ? " · no folder" : n.kind === "fold" ? " · show all" : "")));
+      g.appendChild(sv("text", { "class": "sub", x: 12, y: 56 }, plural(n.classes, "class", "classes") + (n.kind === "fold" ? " · show all" : "")));
       ["mouseenter", "focus"].forEach(function (ev) { g.addEventListener(ev, function () { activate(svg, n, m); }); });
       ["mouseleave", "blur"].forEach(function (ev) { g.addEventListener(ev, function () { activate(svg, null, m); }); });
       gn.appendChild(g);
@@ -751,7 +785,7 @@ def write_zoom_html(graph_path: Path, output_path: Path, *, project_label: Optio
     [SWATCH.badge, "“!” badge: the domain calls back up into a domain above it, closing a dependency cycle. Hover or focus it to see the red links."],
     [SWATCH.against, "Red with “!”: a link running back up."],
     [SWATCH.inferred, "Dotted: only inferred calls (resolved by name, not proven)."],
-    [SWATCH.fold, "Dashed box: the smaller domains, grouped."]]);
+    [SWATCH.fold, "Dashed box: the smaller domains, grouped."]].concat(common ? [[SWATCH.common, "Common: classes placed directly in type folders (Entity/, Enum/...), outside any domain folder."]] : []));
 }
 function freeSpot(path, tw, taken) {
   var len = path.getTotalLength(), best = null, bestScore = Infinity;

tests/test_zoom_html.py

@@ -13,28 +13,70 @@
     build_zoom_data,
     classify_domain,
     classify_layer,
+    detect_layout,
     write_zoom_html,
 )
 
 PYTHON = sys.executable
 
 
-@pytest.mark.parametrize("path,root,layer,domain", [
-    ("src/Item/Controller/ItemController.php", "src", "controller", "Item"),
-    ("src/Item/Entity/Item.php", "src", "entity", "Item"),
-    ("src/Item/Pricing/PriceRule.php", "src", "other", "Item"),
-    ("src/Controller/Item/ItemListController.php", "src", "controller", "Item"),
-    ("src/Repository/ItemRepository.php", "src", "repository", None),
-    ("src/Kernel.php", "src", "other", None),
-    ("app/Http/Controllers/Item/ItemController.php", "app", "controller", "Item"),
-    ("app/Http/Middleware/Locale.php", "app", "other", "Http"),
-    ("app/Models/Item.php", "app", "entity", None),
-    ("app/Services/Item/ItemService.php", "app", "service", "Item"),
-    ("lib/Item/Service/ItemService.php", "src", "service", None),
+@pytest.mark.parametrize("path,root,layout,layer,domain", [
+    ("src/Item/Controller/ItemController.php", "src", "by-domain", "controller", "Item"),
+    ("src/Item/Entity/Item.php", "src", "by-domain", "entity", "Item"),
+    ("src/Item/Pricing/PriceRule.php", "src", "by-domain", "other", "Item"),
+    ("src/Command/ImportCommand.php", "src", "by-domain", "other", None),
+    ("src/Kernel.php", "src", "by-domain", "other", None),
+    ("src/Controller/Item/ItemListController.php", "src", "by-type", "controller", "Item"),
+    ("src/Service/Order/OrderService.php", "src", "by-type", "service", "Order"),
+    ("src/Repository/ItemRepository.php", "src", "by-type", "repository", None),
+    ("src/Form/Type/ItemType.php", "src", "by-type", "other", None),
+    ("src/Security/Voter/ItemVoter.php", "src", "by-type", "other", None),
+    ("src/Sitemap/Provider/ItemUrls.php", "src", "by-type", "other", "Sitemap"),
+    ("app/Http/Controllers/Item/ItemController.php", "app", "by-type", "controller", "Item"),
+    ("app/Http/Middleware/Locale.php", "app", "by-type", "other", None),
+    ("app/Models/Item.php", "app", "by-type", "entity", None),
+    ("app/Services/Item/ItemService.php", "app", "by-type", "service", "Item"),
+    ("lib/Item/Service/ItemService.php", "src", "by-type", "service", None),
 ])
-def test_layer_and_domain_come_from_folder_names_only(path, root, layer, domain):
+def test_layer_and_domain_come_from_folder_names_only(path, root, layout, layer, domain):
     assert classify_layer(path, root) == layer
-    assert classify_domain(path, root) == domain
+    assert classify_domain(path, root, layout) == domain
+
+
+def test_layout_is_by_type_when_most_classes_sit_below_type_folders():
+    by_type = ["src/Controller/Item/A.php", "src/Entity/B.php", "src/Enum/C.php", "src/Sitemap/D.php", "tests/E/F.php"]
+    by_domain = ["src/Item/Controller/A.php", "src/Item/Entity/B.php", "src/Order/C.php", "src/Command/D.php"]
+    assert detect_layout(by_type, "src") == "by-type"
+    assert detect_layout(by_domain, "src") == "by-domain"
+    assert detect_layout([], None) == "by-domain"
+
+
+def _by_type_graph() -> dict:
+    files = {
+        "ctl": ("ItemController", "src/Controller/Item/ItemController.php"),
+        "svc": ("ItemService", "src/Service/Item/ItemService.php"),
+        "osvc": ("OrderService", "src/Service/Order/OrderService.php"),
+        "ent": ("Item", "src/Entity/Item.php"),
+        "repo": ("ItemRepository", "src/Repository/ItemRepository.php"),
+        "cmd": ("ImportItemsCommand", "src/Command/ImportItemsCommand.php"),
+        "form": ("ItemType", "src/Form/Type/ItemType.php"),
+        "enum": ("ItemStatus", "src/Enum/ItemStatus.php"),
+        "fix": ("ItemFixtures", "src/DataFixtures/ItemFixtures.php"),
+        "sub": ("ItemSubscriber", "src/EventSubscriber/ItemSubscriber.php"),
+        "map": ("ItemUrls", "src/Sitemap/Provider/ItemUrls.php"),
+    }
+    return {"nodes": [{"id": k, "label": v[0], "source_file": v[1], "_callable_class": True} for k, v in files.items()],
+            "links": []}
+
+
+def test_by_type_project_shows_business_domains_and_an_explicit_common_group():
+    data = build_zoom_data(_by_type_graph())
+    assert data["layout"] == "by-type"
+    groups = {d["name"]: d["classes"] for d in data["domains"]}
+    assert groups == {"Item": 2, "Order": 1, "Sitemap": 1, "Common": 7}
+    assert {d["kind"] for d in data["domains"] if d["name"] == "Common"} == {"none"}
+    for technical in ("Command", "Form", "Enum", "DataFixtures", "EventSubscriber", "Entity", "Repository"):
+        assert technical not in groups
 
 
 def _cls(cid, label, path):