Skip to content

fix module scoping #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lyninx merged 1 commit into from
Feb 8, 2026
Merged

fix module scoping #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/lunary.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ defmodule Lunary do

defp evaluate({:func_access, index, {:fn, {:identifier, fn_line, fn_id}, fn_args}}, scope, opts) do
{func_scope, _} = evaluate(index, scope, opts)
merged_scope = Map.merge(func_scope, scope)
merged_scope = Map.merge(scope, func_scope) # function scope takes priority

Copilot AI Feb 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the inline comment says "function scope takes priority", but func_scope here is actually the evaluated module scope coming from index (e.g., @example). Consider rewording to avoid confusion when revisiting scoping rules later.

Suggested change
merged_scope = Map.merge(scope, func_scope) # function scope takes priority
merged_scope = Map.merge(scope, func_scope) # evaluated scope from `index` takes priority

Copilot uses AI. Check for mistakes.
evaluate({:fn, {:identifier, fn_line, fn_id}, fn_args}, merged_scope, opts)
Comment on lines 131 to 134

Copilot AI Feb 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scope precedence change here fixes direct module calls, but there is still a second code path for chained module calls ({:chain, ..., {:func_access, ...}} around lib/lunary.ex:396-403) that merges scopes in the old order (Map.merge(func_scope, scope)). That means val |> @mod.fn() can still resolve identifiers using the caller scope ahead of the module scope, making scoping behavior inconsistent and likely leaving the original bug unfixed for pipelines. Update that merge order as well (and consider adding a test that pipes into a module function while a conflicting local identifier exists).

Copilot uses AI. Check for mistakes.
end

Expand Down
1 change: 0 additions & 1 deletion test/unit/chain_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ defmodule ChainTest do
" |> Lunary.Main.eval(%{}, %{ path: "test/fixtures/" }) == 100
end

# @tag :skip
test "can use nested module functions" do
assert "
mod @example (
Expand Down
10 changes: 10 additions & 0 deletions test/unit/module_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,5 +103,15 @@ defmodule ModuleTest do
@example.test(@example.a(5), 1)
" |> Lunary.Main.eval(%{}, %{ path: "test/fixtures/" }) == 106
end

test "can have functions with the same name as a local identifier" do
assert "
mod @example (
fn test (param) -> (param + 10)
)
test = 5
@example.test(5)
" |> Lunary.Main.eval() == 15
end
end
end