Skip to content

internal access modifier #4156

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

Closed
yoav-steinberg opened this issue Sep 12, 2023 · 4 comments · Fixed by #6980
Closed

internal access modifier #4156

yoav-steinberg opened this issue Sep 12, 2023 · 4 comments · Fixed by #6980
Assignees
Labels
🛠️ compiler Compiler ✨ enhancement New feature or request 📜 lang-spec-impl Appears in the language spec roadmap 📐 language-design Language architecture roadmap

Comments

@yoav-steinberg
Copy link
Contributor

yoav-steinberg commented Sep 12, 2023

Feature Spec

internal access modifier can be used anywhere the pub access modifier can be applied (on type definitions, methods, fields, etc.).
When a type or method or other API element has the internal access modifier, it behaves like it's public (pub) within the current Wing library or module, but it will appear private when the library or module is imported by other Wing source code.

Use Cases

class A {
  internal static do_a() {};
}
class B {
  public static do_b() {
    A.do_a(); // We don't want access to `do_a` outside this file, but we do need access to it in this exported public method
  }
}

Implementation Notes

No response

Component

Language Design, Compiler

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@yoav-steinberg yoav-steinberg added the ✨ enhancement New feature or request label Sep 12, 2023
@monadabot monadabot added this to Wing Sep 12, 2023
@github-project-automation github-project-automation bot moved this to 🆕 New - not properly defined in Wing Sep 12, 2023
@yoav-steinberg yoav-steinberg added 📜 lang-spec-impl Appears in the language spec roadmap 📐 language-design Language architecture 🛠️ compiler Compiler labels Sep 12, 2023
@staycoolcall911 staycoolcall911 moved this from Todo - p2 to Todo - out of scope for beta in Wing Language Roadmap Sep 12, 2023
@staycoolcall911 staycoolcall911 moved this from 🆕 New - not properly defined to 🤝 Backlog - handoff to owners in Wing Sep 12, 2023
mergify bot pushed a commit that referenced this issue Sep 15, 2023
See #108 
This PR adds `public` and `protected` support to class members and while doing so makes all other members **private** by default. It also verifies interface method implementations must be `public`. It also includes support for JSII access modifiers. To get this working I did the following refactors:

* Cleaned up fields in the `SymbolEnv` and instead added a `kind` field distinguishing between 3 types of environments: a simple code block (`Scope`) a function/method/closure environemt (`Function`) and an environment used to store members of a type (`Type`). This was primarily needed so I can use the result of a symbol env lookup to figure out in what ancestor class a given member was defined. The added benefit was code clarity and sanity checks which, btw, found a non-related bug in super constructor type checking (see below). One piece of such clarity was the long awaited removal of `return_type` from `SymbolEnv`.
* Cleaned up the `ContextVisitor` which I needed to add to the `TypeChecker`. Here I removed the requirement to store the `init_env` in `push_class` which didn't make sense, and is probably a leftover of some iteration on our lifting code. I also cleaned up the `push_method` code to make more sense and now it's a more general `push_function` stack with a helper for when we're just interested in methods.
* I added some more stuff to `VisitorWithContext` trait, for easy "push -> do stuff or even return -> pop" pattern. Specifically I needed the `TypeChecker` to push function definitions and current statement safely.
* Use a `VisitorContext` in `TypeChecker` to track context so I know in what class I am for access modifier checking. This approach also provides a better way of tracking the current `return_type` so I could remove it from the env. This also removed all sorts of dirt from the `TypeChecker` struct (like `in_json`, etc.).
* The `type_check_statement` method was getting way to big, so I moved the contents of its match statements into separate methods.
* The sanity checks when creating new `SymbolEnv`s detected that we're creating a weird env in `type_check_super_constructor_against_parent_initializer`. I removed a large bunch of code there that didn't seem correct to me.
* The actual access modifier verification takes place inside `get_property_from_class_like` (**<= this is where you want to look**). This way I avoid cascading errors and "unhydrated" classes. To make this work for all cases I updated `resolve_referece` for static type references to also use this method instead of doing its own thing. I think this is cleaner.


Still left and possible candidate for followup PRs:
- [x] Tests for JSII imports with access modifiers
- [x] Handle method overriding rules
- [x] Support `public` for types (export types from module) -> out of scope, will be handled added by our module system.
- [x] Decide if we allow `public` fields: tracking issue #4180
- [x] Decide about `internal`: tracking issue #4156

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@staycoolcall911 staycoolcall911 added the needs-discussion Further discussion is needed prior to impl label Oct 4, 2023
@Chriscbr Chriscbr self-assigned this Oct 24, 2023
@Chriscbr Chriscbr removed the needs-discussion Further discussion is needed prior to impl label Oct 24, 2023
@Chriscbr Chriscbr removed their assignment Oct 27, 2023
Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

Copy link

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label May 27, 2024
@Chriscbr Chriscbr changed the title support internal access modifier internal access modifier Jul 25, 2024
@Chriscbr Chriscbr self-assigned this Aug 1, 2024
@Chriscbr Chriscbr moved this to Backlog in Winglang team Aug 1, 2024
@Chriscbr Chriscbr moved this from Backlog to In Progress in Winglang team Aug 1, 2024
@mergify mergify bot closed this as completed in #6980 Aug 5, 2024
@mergify mergify bot closed this as completed in 9c36ce5 Aug 5, 2024
@github-project-automation github-project-automation bot moved this from 🤝 Backlog - handoff to owners to ✅ Done in Wing Aug 5, 2024
@github-project-automation github-project-automation bot moved this from Todo - out of scope for beta to Done in Wing Language Roadmap Aug 5, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Winglang team Aug 5, 2024
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.81.5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ compiler Compiler ✨ enhancement New feature or request 📜 lang-spec-impl Appears in the language spec roadmap 📐 language-design Language architecture roadmap
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants