Skip to content

fix module scoping#4

Merged
lyninx merged 1 commit into
mainfrom
module-scoping
Oct 29, 2025
Merged

fix module scoping#4
lyninx merged 1 commit into
mainfrom
module-scoping

Conversation

@lyninx

@lyninx lyninx commented Oct 29, 2025

Copy link
Copy Markdown
Owner

No description provided.

@lyninx lyninx force-pushed the module-scoping branch 2 times, most recently from 76abe38 to 4cf9fd7 Compare October 29, 2025 02:40
@lyninx lyninx requested a review from Copilot October 29, 2025 02:42
@lyninx lyninx marked this pull request as ready for review October 29, 2025 02:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a scoping issue where module functions with the same name as local identifiers were incorrectly shadowed by those local variables. The fix ensures that when calling module functions, the function scope (containing module function definitions) takes precedence over the outer scope (containing local variables).

Key changes:

  • Reversed the order of Map.merge arguments in func_access evaluation to prioritize function scope over outer scope
  • Added test coverage for the specific case of module functions sharing names with local identifiers
  • Uncommented a previously skipped test for nested module functions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/lunary.ex Fixed scope merging order in func_access evaluation to prioritize function scope
test/unit/module_test.exs Added test case for module functions with same name as local identifiers
test/unit/chain_test.exs Uncommented previously skipped test for nested module functions
Comments suppressed due to low confidence (1)

lib/lunary.ex:400

  • The merge order is inconsistent with the fix at line 133. This should be Map.merge(scope, func_scope) to ensure function scope takes priority, matching the behavior in the func_access evaluation clause. The current order would cause the same shadowing bug in chained module function calls.
        merged_scope = Map.merge(func_scope, scope) # is this overly broad?

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lyninx lyninx merged commit 59bfede into main Oct 29, 2025
7 checks passed
@lyninx lyninx deleted the module-scoping branch October 29, 2025 02:46
@lyninx lyninx restored the module-scoping branch February 8, 2026 00:28
@lyninx lyninx deleted the module-scoping branch February 8, 2026 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants