fix(api): tighten public surface before the 1.0 freeze

.agents/plans/A50-api-pre-freeze.md

@@ -0,0 +1,82 @@
+---
+id: A50
+title: "API pre-freeze fixes: struct encoding, dead guard, option consistency"
+issue: null
+pr: 380
+branch: fix/api-pre-freeze
+base: main
+status: review
+direction: A
+---
+
+## Goal
+Fix the public-API sharp edges that would become breaking changes once 1.0
+freezes the surface: bare-struct encoding, a dead compat guard, and option
+inconsistencies.
+
+## Out of scope
+- The `Lua.Encoder` protocol itself (1.1, issue #341).
+- Renaming closure tags or any tag-tuple restructuring.
+- Docs-only fixes elsewhere (sibling PR A49 handles those).
+- Version bump / release mechanics.
+
+## Success criteria
+- [ ] Bare struct encoding raises a helpful error; `Map.from_struct/1` path works.
+- [ ] `is_mfa/1` guard and its import removed; no repo references remain.
+- [ ] Closure tags documented as unstable (guard + `unwrap/1`).
+- [ ] `:max_string_bytes` accepts `:infinity`, uniform with siblings; test added.
+- [ ] `eval!/3` chunk clause accepts `:source`; test added.
+- [ ] `call_function/3` error shape documented with an example.
+- [ ] `Lua.CompilerException` `:state` field removed (was always nil).
+- [ ] `Lua.VM.execute/3` moduledoc reworded as internal.
+- [ ] CHANGELOG Unreleased entries added for items 1, 2, 4, 5, 7.
+- [ ] `mix format`, `mix compile --warnings-as-errors`, `mix test`,
+      `mix test --only lua53` (20 passed / 9 skipped), `mix docs --warnings-as-errors` all pass.
+
+## Implementation notes
+- `lib/lua/vm/value.ex`: add a `%mod{}` struct clause before the `is_map`
+  clause that raises `Lua.RuntimeException`.
+- `lib/lua/api.ex`: remove `is_mfa/1` guard + import entry; document closure
+  tags as unstable.
+- `lib/lua.ex`: `validate_max_string_bytes!` accepts `:infinity`; `eval!/3`
+  chunk clause accepts `:source`; `call_function/3` error-shape doc; `unwrap/1`
+  tag-stability note; `:max_string_bytes` option doc mentions `:infinity`.
+- `lib/lua/vm/state.ex`: widen `max_string_bytes` typespec.
+- `lib/lua/compiler_exception.ex`: drop `:state` field, update moduledoc.
+- `lib/lua/vm.ex`: reword moduledoc as internal.
+- `CHANGELOG.md`: Unreleased entries.
+
+## Verification
+```
+mix format
+mix compile --warnings-as-errors
+mix test
+mix test --only lua53
+mix docs --warnings-as-errors
+```
+
+## Risks
+- Struct encoding raise could break a caller that relied on the accidental
+  `__struct__`-key behaviour. Audit found no legitimate struct encoding in
+  lib/ or test/, so risk is low; it is exactly the pre-freeze fix.
+
+## What changed
+- `lib/lua/vm/value.ex`: `%mod{}` clause raises `Lua.RuntimeException` before
+  the `is_map` clause.
+- `lib/lua/api.ex`: removed `is_mfa/1` guard + import; unstable-tuple warnings
+  on `is_lua_func/1` and `is_erl_func/1`.
+- `lib/lua.ex`: `validate_max_string_bytes!` accepts `:infinity`; chunk `eval!/3`
+  accepts `:source`; `call_function/3` error-shape doctests; `unwrap/1` and
+  `:max_string_bytes` doc notes.
+- `lib/lua/vm/state.ex`: widened `max_string_bytes` typespec.
+- `lib/lua/vm/limits.ex`: documented `:infinity` term-ordering behaviour.
+- `lib/lua/compiler_exception.ex`: dropped `:state` field.
+- `lib/lua/vm.ex`: moduledoc reframed as internal.
+- `CHANGELOG.md`: Unreleased entries for items 1, 2, 4, 5, 7.
+- Tests: struct-encode raise + `Map.from_struct/1` path (`test/lua_test.exs`),
+  `:infinity` acceptance (`test/lua/vm/limits_test.exs`), chunk `:source`
+  (`test/lua_test.exs`).
+- **Audit finding:** no structs are legitimately encoded today — the plan's
+  carve-out concern is moot; the raise carves out nothing.
+- Verification: `mix test` 2580 passed / 7 skipped; `mix test --only lua53`
+  20 passed / 9 skipped; `mix docs --warnings-as-errors` clean.

CHANGELOG.md

@@ -7,7 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+<!-- A50: API pre-freeze fixes — breaking-before-1.0 changes are called out
+     explicitly so they never surprise anyone after the surface freezes. -->
+
+### Changed (breaking, before 1.0 freeze)
+- Encoding a bare Elixir struct now raises. Previously `Lua.encode!/2` (and
+  `Lua.set!/3`) matched a struct as a plain map and silently encoded a Lua
+  table carrying a `"__struct__"` key — a lossy, accidental conversion.
+  Convert structs explicitly first (e.g. `Map.from_struct/1`), selecting the
+  fields Lua needs. This lands before 1.0 so the planned `Lua.Encoder`
+  protocol (#341) can be added without breaking a behaviour people relied on.
+- Removed the `is_mfa` guard from `Lua.API`. It was a Luerl-era compatibility
+  shim that always returned `false` and was imported into every `use Lua.API`
+  module; the VM has no MFA references. Remove any `when is_mfa(value)` clauses
+  (they never matched).
+
+### Changed
+- `Lua.new/1`'s `:max_string_bytes` now accepts `:infinity` for no limit,
+  making it uniform with its siblings `:max_call_depth` and
+  `:max_instructions`.
+- `Lua.CompilerException` no longer has a `:state` field. It was never
+  populated (always `nil`); `:errors` carries all the formatted diagnostics.
+
 ### Fixed
+- `Lua.eval!/3` now accepts a `:source` option when evaluating a pre-compiled
+  `Lua.Chunk`, matching the string-script clause. Previously
+  `eval!(lua, chunk, source: "x")` raised via `Keyword.validate!`. For a chunk
+  the option is accepted but ignored — the chunk already carries the source
+  name it was compiled with.
 - `Lua.encode!/2` now maps Elixir `nil` to Lua `nil` instead of the string
   `"nil"`. Previously top-level `nil` fell into the atom-encoding head and
   became `"nil"`, which is truthy in Lua — silently inverting `if not value

lib/lua.ex

@@ -105,7 +105,8 @@ defmodule Lua do
   * `:max_string_bytes` - (default 256 MiB) ceiling for any single string the VM will build,
     whether via `..`, `string.rep`, or a `load` reader. An oversized result raises a catchable
     `"resulting string too large"` error — the size is computed *before* allocating, so the
-    bomb is refused rather than detected after the fact. Accepts a positive integer.
+    bomb is refused rather than detected after the fact. Accepts a positive integer or
+    `:infinity` for no limit (matching `:max_call_depth` and `:max_instructions`).
 
     When running the VM inside a process capped with `:max_heap_size`, set this comfortably
     below the heap cap: the default ceiling permits strings large enough to trip a smaller
@@ -169,11 +170,12 @@ defmodule Lua do
           ":max_call_depth must be a positive integer or :infinity, got: #{inspect(other)}"
   end
 
+  defp validate_max_string_bytes!(:infinity), do: :infinity
   defp validate_max_string_bytes!(bytes) when is_integer(bytes) and bytes > 0, do: bytes
 
   defp validate_max_string_bytes!(other) do
     raise ArgumentError,
-          ":max_string_bytes must be a positive integer, got: #{inspect(other)}"
+          ":max_string_bytes must be a positive integer or :infinity, got: #{inspect(other)}"
   end
 
   defp validate_max_instructions!(:infinity), do: :infinity
@@ -475,7 +477,9 @@ defmodule Lua do
                 Pass `decode: false` as an option to return encoded values
   * `:source` - (default `"<eval>"`) Source name attached to the compiled chunk. Surfaces in
                 runtime errors as `at <source>:<line>:` and in stack traces. Pick a name that
-                identifies where this script came from (e.g. `"my_script.lua"`).
+                identifies where this script came from (e.g. `"my_script.lua"`). When the script
+                is a pre-compiled `t:Lua.Chunk.t/0`, `:source` is accepted but ignored — the
+                chunk already carries the source name it was compiled with.
   """
   @spec eval!(String.t() | Lua.Chunk.t()) :: {[term()], t()}
   @spec eval!(t() | String.t() | Lua.Chunk.t(), String.t() | Lua.Chunk.t() | keyword()) ::
@@ -540,7 +544,10 @@ defmodule Lua do
   end
 
   def eval!(%__MODULE__{state: state} = lua, %Lua.Chunk{prototype: proto}, opts) do
-    opts = Keyword.validate!(opts, decode: true)
+    # `:source` is accepted for symmetry with the string clause but ignored: a
+    # chunk was already compiled with its source name baked into the prototype,
+    # and that name wins. Pass `:source` at `load_chunk!/2`/compile time to set it.
+    opts = Keyword.validate!(opts, decode: true, source: nil)
 
     {:ok, results, new_state} = Lua.VM.execute(proto, state)
 
@@ -698,6 +705,26 @@ defmodule Lua do
       iex> ret
       42
 
+  ## Errors
+
+  When the function raises, `call_function/3` returns `{:error, reason, lua}`.
+  Unlike the raising `call_function!/3`, `reason` is the raw Lua-facing error
+  value — exactly what `pcall` would hand back — not a terminal-formatted
+  message. A non-string error value passes through verbatim:
+
+      iex> {[ref], lua} = Lua.eval!(Lua.new(), "return function() error(42) end", decode: false)
+      iex> {:error, reason, _lua} = Lua.call_function(lua, ref, [])
+      iex> reason
+      42
+
+  A string passed to `error` comes back with Lua's `source:line:` position
+  prefix as a single-line string (no ANSI, no `"Lua runtime error:"` header):
+
+      iex> {[ref], lua} = Lua.eval!(Lua.new(), "return function() error('boom') end", decode: false)
+      iex> {:error, reason, _lua} = Lua.call_function(lua, ref, [])
+      iex> reason =~ "boom"
+      true
+
   """
   @spec call_function(t(), term(), [term()]) ::
           {:ok, [term()], t()} | {:error, term(), t()}
@@ -848,6 +875,13 @@ defmodule Lua do
   Useful when you need to pass an `eval`-returned closure or table
   reference to a tool that expects the raw VM tag (for example, an
   internal helper or a custom `deflua`).
+
+  > #### Tag tuples are not stable API {: .warning}
+  > The `{:tref, _}`, `{:lua_closure, _, _}`, `{:compiled_closure, _, _}`, and
+  > `{:native_func, _}` shapes returned here are VM internals and may change
+  > between releases. Use the `Lua.API` guards (`is_table/1`, `is_lua_func/1`,
+  > `is_erl_func/1`, `is_userdata/1`) to classify unwrapped values rather than
+  > pattern-matching the tuples directly.
   """
   @spec unwrap(term()) :: term()
   defdelegate unwrap(value), to: Display

lib/lua/api.ex

@@ -102,8 +102,7 @@ defmodule Lua.API do
           is_table: 1,
           is_userdata: 1,
           is_lua_func: 1,
-          is_erl_func: 1,
-          is_mfa: 1
+          is_erl_func: 1
         ]
 
       Module.register_attribute(__MODULE__, :lua_function, accumulate: true)
@@ -139,24 +138,28 @@ defmodule Lua.API do
 
   @doc """
   Is the value a reference to a Lua function?
+
+  > #### Internal tuple shape {: .warning}
+  > The `:lua_closure` / `:compiled_closure` tags this guard checks are VM
+  > internals and are **not** part of the stable API — they may change between
+  > releases. Match with this guard; do not pattern-match the tag tuples
+  > directly.
   """
   defguard is_lua_func(value)
            when is_tuple(value) and tuple_size(value) == 3 and
                   (elem(value, 0) == :lua_closure or elem(value, 0) == :compiled_closure)
 
   @doc """
   Is the value a reference to an Erlang / Elixir function?
+
+  > #### Internal tuple shape {: .warning}
+  > The `:native_func` tag this guard checks is a VM internal and is **not**
+  > part of the stable API. Match with this guard; do not pattern-match the tag
+  > tuple directly.
   """
   defguard is_erl_func(value)
            when is_tuple(value) and tuple_size(value) == 2 and elem(value, 0) == :native_func
 
-  @doc """
-  Is the value a reference to an Erlang / Elixir mfa?
-
-  Note: MFA references are not used in the new VM. This guard always returns false.
-  """
-  defguard is_mfa(_value) when false
-
   @doc """
   Raises a runtime exception inside an API function, displaying contextual
   information about where the exception was raised.

lib/lua/compiler_exception.ex

@@ -5,7 +5,7 @@ defmodule Lua.CompilerException do
   """
   alias Lua.Util
 
-  defexception [:errors, :state]
+  defexception [:errors]
 
   def exception(formatted: errors) when is_list(errors) do
     %__MODULE__{errors: errors}

lib/lua/vm.ex

@@ -1,8 +1,12 @@
 defmodule Lua.VM do
   @moduledoc """
-  Public API for the Lua Virtual Machine.
+  Internal entry point for the Lua Virtual Machine — executes compiled
+  prototypes.
 
-  Executes compiled Lua prototypes.
+  This module is not part of the public, stable API (it is filtered out of
+  the published HexDocs) and is kept readable for source readers and IEx
+  exploration only. Reach for `Lua.eval!/2` and friends instead; its shape
+  may change between releases without notice.
   """
 
   alias Lua.Compiler.Prototype
@@ -17,7 +21,8 @@ defmodule Lua.VM do
   ## Options
 
     * `:reset_instructions` - (default `true`) reset the `:max_instructions` instruction
-      tally to 0 before executing. True at a genuine top-level evaluation
+      tally to 0 before executing. This is internal require-reentrancy
+      machinery, not user API. It is `true` at a genuine top-level evaluation
       (`Lua.eval`/`eval!`). Internal re-entries that run mid-evaluation —
       notably `require`, which loads and runs a module's chunk through this
       function — must pass `false` so the module body accumulates against the

lib/lua/vm/limits.ex

@@ -46,7 +46,11 @@ defmodule Lua.VM.Limits do
   bound here). Raises a catchable "resulting string too large" runtime
   error otherwise.
   """
-  @spec check_string_size!(integer(), pos_integer()) :: :ok
+  # `max` may be `:infinity` (from `Lua.new(max_string_bytes: :infinity)`).
+  # Erlang term ordering places every number below every atom, so an integer
+  # `bytes <= :infinity` is always true and the check passes unconditionally —
+  # no separate clause is needed.
+  @spec check_string_size!(integer(), pos_integer() | :infinity) :: :ok
   def check_string_size!(bytes, max \\ @max_string_bytes)
 
   def check_string_size!(bytes, max) when is_integer(bytes) and bytes <= max, do: :ok

lib/lua/vm/state.ex

@@ -62,7 +62,7 @@ defmodule Lua.VM.State do
           call_stack: list(),
           call_depth: non_neg_integer(),
           max_call_depth: pos_integer() | :infinity,
-          max_string_bytes: pos_integer(),
+          max_string_bytes: pos_integer() | :infinity,
           max_instructions: pos_integer() | :infinity,
           instruction_count: non_neg_integer(),
           metatables: map(),

lib/lua/vm/value.ex

@@ -241,6 +241,18 @@ defmodule Lua.VM.Value do
     State.alloc_userdata(state, value)
   end
 
+  # Structs are maps, so without this clause a bare `%MyStruct{}` would encode
+  # to a Lua table carrying a `"__struct__"` key — a silent, lossy conversion.
+  # Refuse it explicitly: the caller must decide how the struct maps to a Lua
+  # value. Protocol-based struct encoding is planned (see issue #341); raising
+  # now keeps that future addition additive rather than a breaking change.
+  def encode(%mod{}, _state, _fun_wrapper) do
+    raise Lua.RuntimeException,
+          "cannot encode #{inspect(mod)} struct into a Lua value. Convert it to a " <>
+            "plain map first (e.g. `Map.from_struct/1`), selecting only the fields " <>
+            "Lua needs. Protocol-based struct encoding is planned (issue #341)."
+  end
+
   def encode(map, state, fun_wrapper) when is_map(map) do
     {data, state} =
       Enum.reduce(map, {%{}, state}, fn {k, v}, {data, state} ->

test/lua/api_test.exs

@@ -370,8 +370,7 @@ defmodule Lua.APITest do
   describe "guards" do
     @tag :pending
     test "can use in functions" do
-      # Requires userdata support, string.lower (mfa), and is_userdata/is_mfa guards
-      # (currently stubbed as `when false`).
+      # Requires userdata support and the is_userdata guard.
       # Original implementation:
       # assert [{module, _}] =
       #          Code.compile_string("""
@@ -394,10 +393,6 @@ defmodule Lua.APITest do
       #              "erl function"
       #            end
       #
-      #            deflua type(value) when is_mfa(value) do
-      #              "mfa"
-      #            end
-      #
       #            deflua type(_value) do
       #              "other"
       #            end
@@ -419,7 +414,6 @@ defmodule Lua.APITest do
       #          """)
       #
       # assert {["erl function"], _} = Lua.eval!(lua, "return guard.type(guard.type)")
-      # assert {["mfa"], _} = Lua.eval!(lua, "return guard.type(string.lower)")
       # assert {["other"], _} = Lua.eval!(lua, "return guard.type(5)")
     end
   end

test/lua/vm/limits_test.exs

@@ -100,12 +100,23 @@ defmodule Lua.VM.LimitsTest do
     end
 
     test "rejects non-positive and non-integer values" do
-      for bad <- [0, -1, :infinity, "16m", 1.5] do
-        assert_raise ArgumentError, ~r/:max_string_bytes must be a positive integer/, fn ->
+      for bad <- [0, -1, "16m", 1.5] do
+        assert_raise ArgumentError, ~r/:max_string_bytes must be a positive integer or :infinity/, fn ->
           Lua.new(max_string_bytes: bad)
         end
       end
     end
+
+    test "accepts :infinity to disable the ceiling, uniform with its siblings" do
+      # `:infinity` is a valid value (like `:max_call_depth`/`:max_instructions`)
+      # and lifts the string-size guard entirely.
+      unbounded = Lua.new(sandboxed: [], max_string_bytes: :infinity)
+
+      # A build that trips the 1 KB ceiling above succeeds with no limit.
+      assert {[8192], _} = Lua.eval!(unbounded, ~s|return #string.rep("x", 8192)|)
+      assert {[result], _} = Lua.eval!(unbounded, ~s|return string.rep("x", 4) .. "y"|)
+      assert result == "xxxxy"
+    end
   end
 
   describe "string.format" do