Skip to content
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.

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

feat: implement namespaces on sol! side #693

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented Jul 25, 2024

Motivation

Makes sol! macro respect namespaces.

Solution

  • current_namespace context variable is added to ExpCtxt which is switched when we enter/exit contract body during AST traversal or expansion.
  • all_items, custom_types and overloaded_items are wrapped into NamespacedMap which resolves full SolPaths depending on current namespace

cc @DaniPopes please take a look and lmk if I should switch base to master instead

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

this should work,
pending @DaniPopes

Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

lgtm

@DaniPopes DaniPopes merged commit 3c59f3a into alloy-rs:dani/to-sol-contract-specifiers Jul 25, 2024
29 checks passed
DaniPopes added a commit that referenced this pull request Aug 12, 2024
* wip

* wip

* feat: implement namespaces on `sol!` side (#693)

* respect namespaces

* un-ignore test

* clippy

* clippy

* fix doc

* rm debug prints

* chore: cleanup

* fix: correctly expand derives for namespaced JSON outputs (#704)

* fix

* newline

* chore: clippy

* chore: use mem::replace

* fix: respect namespaces when resolving overloads

* test: add regression test

* chore: link the test

---------

Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com>
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.

3 participants