Epic: Add visibility system to hide implementation details internal to modules #4515
Open
3 tasks done
Labels
Milestone
Comments
TomAFrench
changed the title
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 2, 2024
# Description ## Problem Part of #4515 ## Summary We recently added a warning for unused imports... but only for bin and contract packages. We didn't enable it for lib packages because a `use` could also be used as `pub use`, something we don't have yet. I thought it would be really nice if we had `pub use`, and warned on unused imports in libs too. I checked the code and we already track visibility for any item, in general, it's just that for things that don't allow a visibility modifier we just consider it's public. So I tried to see how difficult it would be to implement it, and it turned out it wasn't that hard or time-consuming. That said, visibility for `use` involves some more logic, particularly for autocompletion, because now `pub use` should be suggested, but the "parent" module of that item isn't the actual parent (it's the module where the `pub use` is defined) but that was relatively striaght-forward to implement too. ## Additional Context If we decide to go forward with this, any existing `use` that was used as `pub use` will likely start producing a warning for libs (a lot of them in Aztec-Packages), but now that can be silenced by changing them to `pub use`. Where should this new feature be documented? I'm not sure if it should go in `dependencies.md` or `modules.md`. ## Documentation\* Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
5 tasks
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 17, 2024
# Description ## Problem Part of #4515 Part of #4775 ## Summary Allows marking structs as pub or pub(crate). Also will now report unused structs. ## Additional Context Let me know if some of the structs I marked as `pub` shouldn't actually be `pub`. I didn't mark some as pub, I think only the ones used for tests. ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 17, 2024
# Description ## Problem Part of #4515 Fixes #4775 ## Summary Traits can now be marked as `pub` or `pub(crate)`, and a warning will happen if a trait is unused. ## Additional Context ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 25, 2024
# Description ## Problem Part of #4515 ## Summary ## Additional Context ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 26, 2024
# Description ## Problem Part of #4515 ## Summary ## Additional Context I left all of the stdlib globals as private except one which needed to be `pub(crate)` to avoid a warning. I don't know if some of these globals should actually be public. ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this issue
Sep 27, 2024
# Description ## Problem Part of #4515 ## Summary ## Additional Context I made most of the stdlib modules public. These are all except the test modules and some modules that I thought were just implementation details of some other modules. I'm adding very short documentation for each of these. I'm thinking that once we get all visibility done it should be documented in its own section, with examples. ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 3, 2024
# Description ## Problem Part of #4515 ## Summary ~After searching for a while I found that to make this work we only needed to change one line~ ## Additional Context I didn't include tests for now, but I'll add them later because we first need to figure out which `impl` functions should be marked as `pub` or `pub(crate)` in the stdlib. I'm not familiar with the stdlib though I probably can't do it. That said, if you open the standard library with a nargo compiled from this PR you already get some warnings, so those functions should at least not be private. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: Tom French <tom@tomfren.ch>
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 9, 2024
# Description ## Problem Resolves #4837 Part of #4515 ## Summary This checks struct visibility in member access, constructors and patterns. ## Additional Context Private fields are still suggested in LSP. I plan to hide those fields in a follow-up PR. ## Documentation Check one: - [ ] No documentation needed. - [x] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
github-merge-queue bot
pushed a commit
that referenced
this issue
Oct 9, 2024
# Description ## Problem Part of #4515 Apparently in #6179 I only made it work for calls like `foo::Bar::baz()` but no checks were done for method calls (where `self` is involved). ## Summary This PR now warns when calling private or `pub(crate)` methods, either on structs or primitive types, when they are not accessible from the current module. We also now don't suggest non-accessible methods from LSP. ## Additional Context I also removed an unused global that was introduced some time ago. ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
|
Good to close? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
Currently all types/values defined in Noir are public, this causes an issue where libraries cannot provide a restricted interface as users can reach in an import internal implementation details.
Happy Case
Noir should have a visibility system similar to Rust where by default functions, globals, traits, etc. are private to the modules they're defined in.
Tasks
The text was updated successfully, but these errors were encountered: