fix(dsl): tighten compile-time error reporting and call-site precision

Apply four fixes from PR #85 review feedback:

1. arc.ex `decompose_args/4`: raise CompileError when a positional expression
   is followed by a `do ... end` block. Previously `input :p, bind({1, x}) do
   ... end` silently dropped the bind expression and used the block body as
   the inscription, hiding user mistakes.

2. termination.ex: reject multiple `termination` blocks at macro-expansion
   time. The previous flow accumulated criteria and `pull_one` returned the
   stalest entry (List.last on a prepend-accumulated list), silently dropping
   later declarations. Builder's `pull_one` is simplified to `List.first/1`
   now that uniqueness is enforced upstream.

3. builder.ex: when `Validators.run/1` returns an identifiable error
   (`UniqueNameViolationError`, `MissingColourSetError`), point the raised
   CompileError at the offending declaration's source location instead of
   the `defmodule` line. Each accumulating macro (`colset`, `var`, `val`,
   `place`, `function`, `transition`) now also stores `{name, file, line}`
   metadata so Builder can resolve scope+name back to a callsite. Falls
   back to the `defmodule` callsite for unmappable error types.

4. Add `error.file` and `error.line` assertions across existing
   duplicate-detection tests (place, transition, guard, action,
   on_markings, multiple termination, arc options-before-do, validator
   errors) to lock the call-site precision into the test contract.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: fahchen

lib/coloured_flow/dsl.ex

@@ -66,6 +66,15 @@ defmodule ColouredFlow.DSL do
       Module.register_attribute(__MODULE__, :cf_name, accumulate: false)
       Module.register_attribute(__MODULE__, :cf_version, accumulate: false)
 
+      # Per-declaration metadata: accumulates `{name, file, line}` triples so
+      # `Builder` can map validator-driven errors back to the offending callsite.
+      Module.register_attribute(__MODULE__, :cf_colour_sets_meta, accumulate: true)
+      Module.register_attribute(__MODULE__, :cf_variables_meta, accumulate: true)
+      Module.register_attribute(__MODULE__, :cf_constants_meta, accumulate: true)
+      Module.register_attribute(__MODULE__, :cf_functions_meta, accumulate: true)
+      Module.register_attribute(__MODULE__, :cf_places_meta, accumulate: true)
+      Module.register_attribute(__MODULE__, :cf_transitions_meta, accumulate: true)
+
       @before_compile ColouredFlow.DSL.Builder
     end
   end
@@ -109,12 +118,15 @@ defmodule ColouredFlow.DSL do
   """
   defmacro colset(declaration) do
     {name, type} = ColouredFlow.Notation.Colset.__colset__(declaration)
+    caller_file = __CALLER__.file
+    caller_line = __CALLER__.line
 
     quote do
       @cf_colour_sets %ColourSet{
         name: unquote(name),
         type: unquote(type)
       }
+      @cf_colour_sets_meta {unquote(name), unquote(caller_file), unquote(caller_line)}
     end
   end
 
@@ -128,12 +140,15 @@ defmodule ColouredFlow.DSL do
   """
   defmacro var(declaration) do
     {name, colour_set} = ColouredFlow.Notation.Var.__var__(declaration)
+    caller_file = __CALLER__.file
+    caller_line = __CALLER__.line
 
     quote do
       @cf_variables %Variable{
         name: unquote(name),
         colour_set: unquote(colour_set)
       }
+      @cf_variables_meta {unquote(name), unquote(caller_file), unquote(caller_line)}
     end
   end
 
@@ -147,13 +162,16 @@ defmodule ColouredFlow.DSL do
   """
   defmacro val(declaration) do
     {name, colour_set, value} = ColouredFlow.Notation.Val.__val__(declaration)
+    caller_file = __CALLER__.file
+    caller_line = __CALLER__.line
 
     quote do
       @cf_constants %Constant{
         name: unquote(name),
         colour_set: unquote(colour_set),
         value: unquote(value)
       }
+      @cf_constants_meta {unquote(name), unquote(caller_file), unquote(caller_line)}
     end
   end
 end

lib/coloured_flow/dsl/arc.ex

@@ -100,8 +100,24 @@ defmodule ColouredFlow.DSL.Arc do
         {arg2 ++ opts_after_do, body}
 
       Keyword.has_key?(arg3, :do) ->
-        {body, _opts} = Keyword.pop!(arg3, :do)
-        {[], body}
+        raise CompileError,
+          description: """
+          Invalid `#{label}` arguments: options must come before the `do ... end` block.
+
+          The expression is the block body for multi-line arcs; any positional \
+          argument before the block (other than a keyword list of options) is \
+          ambiguous and would be silently dropped.
+
+          Got: #{Macro.to_string(arg2)} as the second argument before the `do` block.
+
+          Use one of:
+              #{label} :place, expression                    # single-line
+              #{label} :place, expression, label: "..."      # single-line, with options
+              #{label} :place, do: expression                # multi-line
+              #{label} :place, label: "..." do ... end       # multi-line, with options
+          """,
+          file: caller.file,
+          line: caller.line
 
       keyword?(arg3) ->
         {arg3, arg2}

lib/coloured_flow/dsl/builder.ex

@@ -12,6 +12,9 @@ defmodule ColouredFlow.DSL.Builder do
   alias ColouredFlow.Definition.Procedure
   alias ColouredFlow.Enactment.Marking
 
+  alias ColouredFlow.Validators.Exceptions.MissingColourSetError
+  alias ColouredFlow.Validators.Exceptions.UniqueNameViolationError
+
   @doc false
   defmacro __before_compile__(env) do
     cpnet = build_cpnet(env.module)
@@ -55,14 +58,153 @@ defmodule ColouredFlow.DSL.Builder do
         end
 
       {:error, exception} ->
+        {file, line} = locate_error(exception, env)
+
         raise CompileError,
           description: """
           ColouredFlow.DSL: invalid workflow definition.
 
           #{Exception.message(exception)}
           """,
-          file: env.file,
-          line: env.line
+          file: file,
+          line: line
+    end
+  end
+
+  # Map a validator-driven error back to the originating declaration's
+  # `{file, line}`. Falls back to the `defmodule` callsite when the offending
+  # declaration cannot be identified.
+  @spec locate_error(Exception.t(), Macro.Env.t()) :: {String.t(), non_neg_integer()}
+  defp locate_error(%UniqueNameViolationError{scope: scope, name: name}, env) do
+    locate_unique_violation(scope, name, env)
+  end
+
+  defp locate_error(%MissingColourSetError{colour_set: colour_set}, env) do
+    locate_missing_colour_set(colour_set, env)
+  end
+
+  defp locate_error(_other, env) do
+    {env.file, env.line}
+  end
+
+  # `UniqueNameValidator` halts on the *second* occurrence of a duplicate name.
+  # The metadata accumulator stores entries in reverse declaration order
+  # (most-recent first), so reverse to source order and pick the second
+  # occurrence — that's the duplicate the validator rejected. The validator
+  # uses `:variable_and_constant` to lump variables and constants together,
+  # despite that not appearing in the exception's declared scope type.
+  @unique_scope_attrs %{
+    colour_set: [:cf_colour_sets_meta],
+    place: [:cf_places_meta],
+    transition: [:cf_transitions_meta],
+    function: [:cf_functions_meta],
+    variable_and_constant: [:cf_variables_meta, :cf_constants_meta]
+  }
+
+  defp locate_unique_violation(scope, name, env) do
+    case Map.fetch(@unique_scope_attrs, scope) do
+      {:ok, attrs} ->
+        attrs
+        |> Enum.flat_map(&source_order_meta(env.module, &1))
+        |> locate_duplicate(name, env)
+
+      :error ->
+        {env.file, env.line}
+    end
+  end
+
+  # `MissingColourSetError` is raised by the places/functions/variables/
+  # constants validators when a referenced colour set is not declared. The
+  # exception carries the missing colour set name. We probe each declaration
+  # scope in the same order the validator pipeline does and return the first
+  # match.
+  defp locate_missing_colour_set(colour_set, env) do
+    locate_missing_in_places(colour_set, env) ||
+      locate_missing_in_functions(colour_set, env) ||
+      locate_missing_in_constants(colour_set, env) ||
+      locate_missing_in_variables(colour_set, env) ||
+      {env.file, env.line}
+  end
+
+  defp locate_missing_in_places(colour_set, env) do
+    env.module
+    |> zip_meta(:cf_places, :cf_places_meta)
+    |> Enum.find_value(fn
+      {%Place{colour_set: ^colour_set}, {_name, file, line}} -> {file, line}
+      _other -> nil
+    end)
+  end
+
+  defp locate_missing_in_functions(colour_set, env) do
+    env.module
+    |> zip_meta(:cf_functions, :cf_functions_meta)
+    |> Enum.find_value(fn
+      {%Procedure{result: result}, {_name, file, line}} ->
+        if descr_references?(result, colour_set), do: {file, line}, else: nil
+
+      _other ->
+        nil
+    end)
+  end
+
+  defp locate_missing_in_constants(colour_set, env) do
+    env.module
+    |> zip_meta(:cf_constants, :cf_constants_meta)
+    |> Enum.find_value(fn
+      {%{colour_set: ^colour_set}, {_name, file, line}} -> {file, line}
+      _other -> nil
+    end)
+  end
+
+  defp locate_missing_in_variables(colour_set, env) do
+    env.module
+    |> zip_meta(:cf_variables, :cf_variables_meta)
+    |> Enum.find_value(fn
+      {%{colour_set: ^colour_set}, {_name, file, line}} -> {file, line}
+      _other -> nil
+    end)
+  end
+
+  defp zip_meta(module, items_attr, meta_attr) do
+    items = source_order_meta(module, items_attr)
+    metas = source_order_meta(module, meta_attr)
+    Enum.zip(items, metas)
+  end
+
+  # Recursively check whether a `ColourSet.descr()` references the given
+  # colour-set name as a leaf node.
+  defp descr_references?({name, []}, target) when is_atom(name), do: name == target
+
+  defp descr_references?({:tuple, types}, target) when is_list(types) do
+    Enum.any?(types, &descr_references?(&1, target))
+  end
+
+  defp descr_references?({:list, type}, target), do: descr_references?(type, target)
+
+  defp descr_references?({:map, types}, target) when is_map(types) do
+    Enum.any?(types, fn {_key, type} -> descr_references?(type, target) end)
+  end
+
+  defp descr_references?({:union, types}, target) when is_map(types) do
+    Enum.any?(types, fn {_tag, type} -> descr_references?(type, target) end)
+  end
+
+  defp descr_references?(_other, _target), do: false
+
+  defp locate_duplicate(meta_list, name, env) do
+    meta_list
+    |> Enum.filter(fn {entry_name, _file, _line} -> entry_name == name end)
+    |> case do
+      [_first, {_name, file, line} | _rest] -> {file, line}
+      [{_name, file, line}] -> {file, line}
+      [] -> {env.file, env.line}
+    end
+  end
+
+  defp source_order_meta(module, attr) do
+    case Module.get_attribute(module, attr) do
+      nil -> []
+      list when is_list(list) -> Enum.reverse(list)
     end
   end
 
@@ -98,10 +240,14 @@ defmodule ColouredFlow.DSL.Builder do
     end
   end
 
+  # `Module.put_attribute/3` prepends in accumulate mode, so the head of the list
+  # is the most recent entry. This pulls that entry. Macros that should only
+  # appear once (e.g. `termination/1`) enforce uniqueness at macro-expansion
+  # time, so this list will contain at most one element.
   defp pull_one(module, attr) do
     case Module.get_attribute(module, attr) do
       nil -> nil
-      list when is_list(list) -> List.last(list)
+      list when is_list(list) -> List.first(list)
       other -> other
     end
   end

lib/coloured_flow/dsl/function.ex

@@ -51,6 +51,7 @@ defmodule ColouredFlow.DSL.Function do
         expression: unquote(Macro.escape(expression)),
         result: unquote(Macro.escape(result))
       }
+      @cf_functions_meta {unquote(name), unquote(caller_file), unquote(caller_line)}
     end
   end
 

lib/coloured_flow/dsl/place.ex

@@ -17,12 +17,16 @@ defmodule ColouredFlow.DSL.Place do
   defmacro place(name, colour_set) do
     name_value = unquote_atom!(name, "place name", __CALLER__)
     colour_set_value = unquote_atom!(colour_set, "place colour set", __CALLER__)
+    name_str = Atom.to_string(name_value)
+    caller_file = __CALLER__.file
+    caller_line = __CALLER__.line
 
     quote do
       @cf_places %Place{
-        name: unquote(Atom.to_string(name_value)),
+        name: unquote(name_str),
         colour_set: unquote(colour_set_value)
       }
+      @cf_places_meta {unquote(name_str), unquote(caller_file), unquote(caller_line)}
     end
   end
 

lib/coloured_flow/dsl/termination.ex

@@ -44,6 +44,12 @@ defmodule ColouredFlow.DSL.Termination do
     end
 
     quote do
+      ColouredFlow.DSL.Termination.__check_unique_termination__!(
+        __MODULE__,
+        unquote(caller_file),
+        unquote(caller_line)
+      )
+
       ColouredFlow.DSL.Termination.__open_termination__!(
         __MODULE__,
         unquote(caller_file),
@@ -85,6 +91,22 @@ defmodule ColouredFlow.DSL.Termination do
     end
   end
 
+  @doc false
+  @spec __check_unique_termination__!(module(), String.t(), non_neg_integer()) :: :ok
+  def __check_unique_termination__!(module, file, line) do
+    case Module.get_attribute(module, :cf_termination_criteria) do
+      [] ->
+        :ok
+
+      list when is_list(list) ->
+        raise CompileError,
+          description:
+            "termination/1 already declared in this workflow; only a single termination block is allowed",
+          file: file,
+          line: line
+    end
+  end
+
   @doc false
   @spec __open_termination__!(module(), String.t(), non_neg_integer()) :: :ok
   def __open_termination__!(module, file, line) do

lib/coloured_flow/dsl/transition.ex

@@ -64,6 +64,8 @@ defmodule ColouredFlow.DSL.Transition do
       unquote(block)
 
       ColouredFlow.DSL.Transition.__close_transition__!(__MODULE__)
+
+      @cf_transitions_meta {unquote(name_str), unquote(caller_file), unquote(caller_line)}
     end
   end
 

test/coloured_flow/dsl/arc_test.exs

@@ -163,5 +163,39 @@ defmodule ColouredFlow.DSL.ArcTest do
         end
       end
     end
+
+    test "rejects do block when arg2 is not a keyword list of options" do
+      source = """
+      defmodule ColouredFlow.DSL.ArcTest.ArcExprBeforeDo do
+        use ColouredFlow.DSL
+
+        name "ArcExprBeforeDo"
+
+        colset int() :: integer()
+
+        var x :: int()
+
+        place :input, :int
+        place :output, :int
+
+        transition :t do
+          input :input, bind({1, x}) do
+            bind({2, x})
+          end
+
+          output :output, {1, x}
+        end
+      end
+      """
+
+      error =
+        assert_raise CompileError, ~r/options.+before.+do/i, fn ->
+          Code.compile_string(source, "arc_expr_before_do.exs")
+        end
+
+      assert error.file == "arc_expr_before_do.exs"
+      # Points at the offending `input :input, bind({1, x}) do ... end` (line 14, 1-indexed).
+      assert error.line == 14
+    end
   end
 end