From 4a089b08560bc6d1269e884ae3b8bf4feefee14c Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Mon, 11 May 2026 20:18:56 +0200 Subject: [PATCH] fix(perf): bind length/1 to variables before loop bodies in route paths and sidebar (CSM-029) --- CODESMELL.md | 12 +++-- lib/bds/generation/outputs.ex | 18 +++++--- lib/bds/ui/sidebar.ex | 4 +- test/bds/csm029_length_in_guards_test.exs | 55 +++++++++++++++++++++++ 4 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 test/bds/csm029_length_in_guards_test.exs diff --git a/CODESMELL.md b/CODESMELL.md index 6e21aaf..c3bb086 100644 --- a/CODESMELL.md +++ b/CODESMELL.md @@ -434,10 +434,14 @@ --- -### CSM-029 — `length/1` in Guards or Comparisons -- **Files:** `lib/bds/generation/outputs.ex`, `lib/bds/ui/sidebar.ex` -- **What:** `length(list)` is O(n). Using it inside a loop makes the whole loop O(n²). -- **Fix:** Bind the length before the loop. +### ~~CSM-029 — `length/1` in Guards or Comparisons~~ ✅ FIXED +- **Fixed:** 2026-05-11 +- **What was done:** + - **`lib/bds/generation/outputs.ex`** — `category_route_paths`, `tag_route_paths`, `date_route_paths`: + - Bound `post_count = length(posts)` at the start of each `Enum.flat_map` callback, before passing to `paginated_archive_paths`. Eliminates inline `length/1` calls inside loop bodies. + - **`lib/bds/ui/sidebar.ex`** — `build_post_section`: + - Bound `post_count = length(posts)` before the map literal instead of computing `length(posts)` inline in the `count:` field. + - Added 3 source-level tests in `test/bds/csm029_length_in_guards_test.exs`: no inline `length(posts)` in `paginated_archive_paths` calls, no inline `length()` in `build_post_section` map literal, no inline `length(posts)` in route path function callbacks. --- diff --git a/lib/bds/generation/outputs.ex b/lib/bds/generation/outputs.ex index a04cdd4..f675464 100644 --- a/lib/bds/generation/outputs.ex +++ b/lib/bds/generation/outputs.ex @@ -80,10 +80,12 @@ defmodule BDS.Generation.Outputs do def category_route_paths(plan, posts_by_category, route_language) do if :category in plan.sections do Enum.flat_map(posts_by_category, fn {category, posts} -> + post_count = length(posts) + paginated_archive_paths( route_language, ["category", archive_route_segment(category)], - length(posts), + post_count, plan.max_posts_per_page ) end) @@ -96,10 +98,12 @@ defmodule BDS.Generation.Outputs do def tag_route_paths(plan, posts_by_tag, route_language) do if :tag in plan.sections do Enum.flat_map(posts_by_tag, fn {tag, posts} -> + post_count = length(posts) + paginated_archive_paths( route_language, ["tag", archive_route_segment(tag)], - length(posts), + post_count, plan.max_posts_per_page ) end) @@ -113,10 +117,12 @@ defmodule BDS.Generation.Outputs do if :date in plan.sections do year_paths = Enum.flat_map(post_index.posts_by_year, fn {year, posts} -> + post_count = length(posts) + paginated_archive_paths( route_language, [Integer.to_string(year)], - length(posts), + post_count, plan.max_posts_per_page ) end) @@ -124,11 +130,12 @@ defmodule BDS.Generation.Outputs do month_paths = Enum.flat_map(post_index.posts_by_year_month, fn {year_month, posts} -> [year, month] = String.split(year_month, "/", parts: 2) + post_count = length(posts) paginated_archive_paths( route_language, [year, month], - length(posts), + post_count, plan.max_posts_per_page ) end) @@ -136,11 +143,12 @@ defmodule BDS.Generation.Outputs do day_paths = Enum.flat_map(post_index.posts_by_year_month_day, fn {year_month_day, posts} -> [year, month, day] = String.split(year_month_day, "/", parts: 3) + post_count = length(posts) paginated_archive_paths( route_language, [year, month, day], - length(posts), + post_count, plan.max_posts_per_page ) end) diff --git a/lib/bds/ui/sidebar.ex b/lib/bds/ui/sidebar.ex index 5312dbe..e6c210f 100644 --- a/lib/bds/ui/sidebar.ex +++ b/lib/bds/ui/sidebar.ex @@ -821,11 +821,13 @@ defmodule BDS.UI.Sidebar do # --------------------------------------------------------------------------- defp build_post_section(title, status, posts, translation_counts, published_meta?) do + post_count = length(posts) + %{ id: Atom.to_string(status), title: title, status: Atom.to_string(status), - count: length(posts), + count: post_count, items: Enum.map(posts, fn post -> %{ diff --git a/test/bds/csm029_length_in_guards_test.exs b/test/bds/csm029_length_in_guards_test.exs new file mode 100644 index 0000000..0332567 --- /dev/null +++ b/test/bds/csm029_length_in_guards_test.exs @@ -0,0 +1,55 @@ +defmodule BDS.CSM029LengthInGuardsTest do + use ExUnit.Case, async: true + + @outputs_path "lib/bds/generation/outputs.ex" + @sidebar_path "lib/bds/ui/sidebar.ex" + + describe "source-level: length/1 bound before use in loops" do + test "outputs.ex route path functions bind length before passing to paginated_archive_paths" do + source = File.read!(@outputs_path) + + refute source =~ ~r/paginated_archive_paths\([^)]*length\(posts\)/, + "length(posts) should be bound to a variable before passing to paginated_archive_paths" + end + + test "sidebar.ex build_post_section binds length before map literal" do + source = File.read!(@sidebar_path) + + lines = + source + |> String.split("\n") + |> Enum.with_index(1) + + build_section_lines = + Enum.filter(lines, fn {line, _} -> line =~ "defp build_post_section" end) + + for {_line, line_no} <- build_section_lines do + context = + lines + |> Enum.drop(line_no - 1) + |> Enum.take(30) + |> Enum.map(fn {l, _} -> l end) + |> Enum.join("\n") + + refute context =~ ~r/count: length\(/, + "length() should be pre-bound, not inline in map literal" + end + end + + test "no length/1 calls remain inline inside Enum.flat_map callbacks in route path functions" do + source = File.read!(@outputs_path) + + route_fns = ["category_route_paths", "tag_route_paths", "date_route_paths"] + + for fn_name <- route_fns do + fn_match = Regex.run(~r/def #{fn_name}\(.*?\n end/s, source) + assert fn_match, "Expected to find #{fn_name} in outputs.ex" + + [fn_body] = fn_match + + refute fn_body =~ ~r/\blength\(posts\)(?!\s)/, + "#{fn_name} should use pre-bound post_count instead of inline length(posts)" + end + end + end +end