fix(rendering): replace inline try/rescue with with-chains and safe_liquex_render helpers (CSM-031)

CODESMELL.md

@@ -457,9 +457,14 @@
 
 ---
 
-### CSM-031 — `try/rescue` Instead of `with` and Error Tuples
+### ~~CSM-031 — `try/rescue` Instead of `with` and Error Tuples~~ ✅ FIXED
-- **Files:** `lib/bds/rendering/filters.ex`, `lib/bds/rendering/template_selection.ex`, `lib/bds/desktop/shell_data.ex`
+- **Fixed:** 2026-05-27
-- **Fix:** Replace `try/rescue` around expected failures with non-bang functions and `with` chains.
+- **What was done:**
+  - **`lib/bds/rendering/filters.ex`** — Extracted `safe_liquex_render/3` private helper that isolates the unavoidable `Liquex.render!` rescue into a single function returning `{:ok, binary} | {:error, String.t()}`. Replaced inline `try/rescue` in `render_macro_source/4` with a `with` chain using the helper.
+  - **`lib/bds/rendering/template_selection.ex`** — Same pattern: extracted `safe_liquex_render/3` helper, replaced inline `try/rescue` in `render_template/3` with a `with` chain.
+  - **`lib/bds/rendering/template_selection.ex`** — `load_bundled_template_source/3`: Replaced raising `Liquex.FileSystem.read_template_file` with `FileSystem.try_read` (returns `{:ok, source} | {:error, :enoent}`), eliminating the function-level `rescue` block entirely. Uses a `with` chain for control flow.
+  - **`lib/bds/desktop/shell_data.ex`** — Already fixed by CSM-010; no `try/rescue` blocks remain.
+  - Added 7 tests in `test/bds/csm031_try_rescue_test.exs`: source-level assertions that no inline `try/rescue` around `Liquex.render!` exists in either file, both files define `safe_liquex_render` helpers, `load_bundled_template_source` has no rescue block and uses `FileSystem.try_read`, and `shell_data.ex` has no try/rescue.
 
 ---
 

lib/bds/rendering/filters.ex

@@ -148,27 +148,31 @@ defmodule BDS.Rendering.Filters do
   end
 
   defp render_macro_source(template_path, template_source, assigns, context) do
-    case Liquex.parse(template_source) do
+    with {:ok, template_ast} <- Liquex.parse(template_source),
-      {:ok, template_ast} ->
+         {:ok, rendered} <- safe_liquex_render(template_ast, context, assigns) do
-        isolated_context = Liquex.Context.new_isolated_subscope(context, assigns)
+      rendered
-
+    else
-        try do
-          {result, _context} = Liquex.render!(template_ast, isolated_context)
-          IO.iodata_to_binary(result)
-        rescue
-          e in Liquex.Error ->
-            require Logger
-            Logger.warning("Macro template render failed (#{template_path}): #{e.message}")
-            ""
-        end
-
       {:error, reason, line} ->
         require Logger
         Logger.warning("Macro template parse failed (#{template_path}): #{reason} at line #{line}")
         ""
+
+      {:error, message} ->
+        require Logger
+        Logger.warning("Macro template render failed (#{template_path}): #{message}")
+        ""
     end
   end
 
+  defp safe_liquex_render(template_ast, context, assigns) do
+    isolated_context = Liquex.Context.new_isolated_subscope(context, assigns)
+
+    {result, _context} = Liquex.render!(template_ast, isolated_context)
+    {:ok, IO.iodata_to_binary(result)}
+  rescue
+    e in Liquex.Error -> {:error, e.message}
+  end
+
   defp render_markdown_html(markdown) do
     case Earmark.as_html(markdown) do
       {:ok, html, _messages} -> html

lib/bds/rendering/template_selection.ex

@@ -93,7 +93,16 @@ defmodule BDS.Rendering.TemplateSelection do
   @spec render_template(String.t(), String.t(), map()) ::
           {:ok, String.t()} | {:error, String.t()}
   def render_template(project_id, source, assigns) do
-    with {:ok, template_ast} <- Liquex.parse(source) do
+    with {:ok, template_ast} <- Liquex.parse(source),
+         {:ok, _rendered} = ok <- safe_liquex_render(template_ast, project_id, assigns) do
+      ok
+    else
+      {:error, reason, line} when is_integer(line) -> {:error, "#{reason} at line #{line}"}
+      {:error, _message} = error -> error
+    end
+  end
+
+  defp safe_liquex_render(template_ast, project_id, assigns) do
     project = Projects.get_project!(project_id)
 
     context =
@@ -103,35 +112,26 @@ defmodule BDS.Rendering.TemplateSelection do
         file_system: FileSystem.new(StarterTemplates.template_roots(project))
       )
 
-      try do
     {result, _context} = Liquex.render!(template_ast, context)
     {:ok, IO.iodata_to_binary(result)}
   rescue
     e in Liquex.Error -> {:error, e.message}
   end
-    else
-      {:error, reason, line} -> {:error, "#{reason} at line #{line}"}
-    end
-  end
 
   defp load_bundled_template_source(project, kind, slug) do
     desired_slug = bundled_template_slug(kind, slug)
 
-    if is_binary(desired_slug) do
+    with true <- is_binary(desired_slug),
-      file_system = project |> StarterTemplates.template_roots() |> FileSystem.new()
+         file_system = project |> StarterTemplates.template_roots() |> FileSystem.new(),
-      source = Liquex.FileSystem.read_template_file(file_system, desired_slug)
+         {:ok, source} <- FileSystem.try_read(file_system, desired_slug) do
-
       case Frontmatter.parse_document(source) do
         {:ok, %{body: body}} -> {:ok, body}
         {:error, :invalid_frontmatter} -> {:ok, source}
       end
     else
-      {:error, :template_not_found}
+      false -> {:error, :template_not_found}
+      {:error, :enoent} -> {:error, :template_not_found}
     end
-  rescue
-    error in [Liquex.Error] ->
-      _ = error
-      {:error, :template_not_found}
   end
 
   defp maybe_load_bundled_template_source(project, kind, slug, template, reason, error)

test/bds/csm031_try_rescue_test.exs

@@ -0,0 +1,68 @@
+defmodule BDS.CSM031TryRescueTest do
+  use ExUnit.Case, async: true
+
+  describe "source-level: no inline try/rescue around Liquex.render!" do
+    test "filters.ex has no try/rescue block in render_macro_source" do
+      source = File.read!("lib/bds/rendering/filters.ex")
+      refute source =~ ~r/try do\s+.*Liquex\.render!/s,
+             "render_macro_source should use safe_liquex_render helper, not inline try/rescue"
+    end
+
+    test "filters.ex isolates Liquex.render! rescue in safe_liquex_render" do
+      source = File.read!("lib/bds/rendering/filters.ex")
+      assert source =~ "defp safe_liquex_render"
+    end
+
+    test "template_selection.ex has no try/rescue block in render_template" do
+      source = File.read!("lib/bds/rendering/template_selection.ex")
+      lines = String.split(source, "\n")
+
+      in_render_template =
+        lines
+        |> Enum.drop_while(&(not String.contains?(&1, "def render_template(")))
+        |> Enum.take_while(&(not String.match?(&1, ~r/^\s+def[p]?\s/)))
+
+      body = Enum.join(in_render_template, "\n")
+      refute body =~ "try do", "render_template should not contain inline try/rescue"
+    end
+
+    test "template_selection.ex isolates Liquex.render! rescue in safe_liquex_render" do
+      source = File.read!("lib/bds/rendering/template_selection.ex")
+      assert source =~ "defp safe_liquex_render"
+    end
+  end
+
+  describe "source-level: no function-level rescue in load_bundled_template_source" do
+    test "template_selection.ex load_bundled_template_source has no rescue block" do
+      source = File.read!("lib/bds/rendering/template_selection.ex")
+      lines = String.split(source, "\n")
+
+      in_load_bundled =
+        lines
+        |> Enum.drop_while(&(not String.contains?(&1, "defp load_bundled_template_source(")))
+        |> Enum.take_while(fn line ->
+          not (String.match?(line, ~r/^\s+def[p]?\s/) and
+                 not String.contains?(line, "load_bundled_template_source"))
+        end)
+
+      body = Enum.join(in_load_bundled, "\n")
+      refute body =~ ~r/^\s+rescue\b/m, "load_bundled_template_source should use with, not rescue"
+    end
+
+    test "template_selection.ex uses FileSystem.try_read instead of read_template_file" do
+      source = File.read!("lib/bds/rendering/template_selection.ex")
+      refute source =~ "read_template_file",
+             "should use FileSystem.try_read, not the raising read_template_file"
+
+      assert source =~ "FileSystem.try_read"
+    end
+  end
+
+  describe "source-level: shell_data.ex has no try/rescue" do
+    test "shell_data.ex contains no try/rescue blocks" do
+      source = File.read!("lib/bds/desktop/shell_data.ex")
+      refute source =~ ~r/\btry\s+do\b/, "shell_data.ex should not contain try/rescue blocks"
+      refute source =~ ~r/\brescue\b/, "shell_data.ex should not contain rescue clauses"
+    end
+  end
+end