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

Access Modifiers #108

Closed
Tracked by #194 ...
staycoolcall911 opened this issue Sep 20, 2022 · 4 comments · Fixed by #4133
Closed
Tracked by #194 ...

Access Modifiers #108

staycoolcall911 opened this issue Sep 20, 2022 · 4 comments · Fixed by #4133
Assignees
Labels
🛠️ compiler Compiler 📜 lang-spec-impl Appears in the language spec roadmap
Milestone

Comments

@staycoolcall911
Copy link
Contributor

staycoolcall911 commented Sep 20, 2022

The following section should be added to the language reference:

1.5 Access Modifiers

Visibility inference is done with the following rules:

  • Default visibility is private for all members. If modifiers are missing, the
    symbol is assumed private by the compiler and not exported.
  • pub is to declare a symbol that is visible for and exported publicly.
  • protected is to declare a symbol that is visible for and exported publicly
    but only for the class and its subclasses.
  • internal is to declare a symbol that is visible for and exported publicly
    but only for the current compilation unit.
    Accessing fields, members, or structured data is done with ..
    Visibility modifiers can be applied to members of classes.
    Mixing protected and internal is not allowed.
@staycoolcall911 staycoolcall911 added the 📜 lang-spec-impl Appears in the language spec roadmap label Sep 20, 2022
@Chriscbr Chriscbr reopened this Sep 22, 2022
@Chriscbr Chriscbr added the 🛠️ compiler Compiler label Sep 28, 2022
@github-actions
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!

@staycoolcall911
Copy link
Contributor Author

The bare minimum is adding pub - let's see if that makes sense when we implement

@staycoolcall911
Copy link
Contributor Author

Another requirement (from @eladb): all fields must be private (access should be done via getter/setter)

mergify bot pushed a commit that referenced this issue Jun 29, 2023
This PR introduces an up-to-date language reference - all wishlist/roadmap feature mentions were deleted and instead put into new/existing issues, with a link in a newly added "Roadmap" paragraph at the end of relevant sections..
All code samples (Wing and TypeScript) were tested and should successfully compile.

[Rendered version of the language reference](https://github.com/winglang/wing/blob/urib/lang-ref/docs/docs/03-language-guide/90-reference.md#1149-roadmap).

Fixes #2420 

### TODO
- [ ] I'll remove all typescript code samples in a follow-up PR

### Misc
- [x] Changed the rust debugger to debug the current open `.w` file by default (relevant contributor guide doc updated).
- [x] Small fix of a couple of tree-sitter test headlines.
- [x] Fixes #2696 - by defining the behavior of `for` loops.

### I simply removed the following spec features/requirements:
1. In 1.1.4.8 Json logging
“It is also legal to just log a json object”
2. In 1.2 Utility Functions:
“The above functions can accept variadic arguments of any type except `throw` which only accepts one argument and that is the message to be contained in the error.”
3. In 1.4 Storage modifiers:
“The name of any static data member and static member function must be different from the name of the containing class regardless of the casing.”
4. In 2.6 For:
“Type annotation after an iteratee (left hand side of **in**) is optional.”
5. In 3.2 Classes:
“Optionals are initialized to `nil` if omitted, unless the type is `nil?`, which in that case, absent initialization is a compile error.<br/>
Member function and field access in constructor with the "this" keyword before
all fields are initialized is invalid and would throw a compile error.<br/>
In other words, the `this` keyword is immutable to its field access operator `.`
before all the member fields are properly initialized. The behavior is similar to JavaScript and TypeScript in their "strict" mode.”
6. In 3.3 Preflight classes:
Scope and ID are “both overrideable by user-defined ones in constructor.”
7. In 3.8 Enumeration:
“Last comma is optional in single line definitions but required in multi line definitions.”
“`nameof` operator”
8. Entire section: 6.1.2 Shell strings
9. Entire section: 6.4 Kitchen Sink
10. Entire section: Inspiration (last section)

### Issues opened/changed to track spec completeness:
- [x] Opened issues:
	- [x] #3121 
	- [x] #3123 
	- [x] #3129
	- [x] #3139 
	- [x] #3140 
	- [x] #3142 
- [x] Changed description:
	- [x] #108 
	- [x] #2103 
	- [x] #116 
	- [x] #125 
	- [x] #128
	- [x] #129 
	- [x] #130
	- [x] #1737 

## Checklist

- [x] Title matches [Winglang's style guide](https://docs.winglang.io/contributors/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [ ] 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 [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
revitalbarletz pushed a commit that referenced this issue Jul 3, 2023
This PR introduces an up-to-date language reference - all wishlist/roadmap feature mentions were deleted and instead put into new/existing issues, with a link in a newly added "Roadmap" paragraph at the end of relevant sections..
All code samples (Wing and TypeScript) were tested and should successfully compile.

[Rendered version of the language reference](https://github.com/winglang/wing/blob/urib/lang-ref/docs/docs/03-language-guide/90-reference.md#1149-roadmap).

Fixes #2420 

### TODO
- [ ] I'll remove all typescript code samples in a follow-up PR

### Misc
- [x] Changed the rust debugger to debug the current open `.w` file by default (relevant contributor guide doc updated).
- [x] Small fix of a couple of tree-sitter test headlines.
- [x] Fixes #2696 - by defining the behavior of `for` loops.

### I simply removed the following spec features/requirements:
1. In 1.1.4.8 Json logging
“It is also legal to just log a json object”
2. In 1.2 Utility Functions:
“The above functions can accept variadic arguments of any type except `throw` which only accepts one argument and that is the message to be contained in the error.”
3. In 1.4 Storage modifiers:
“The name of any static data member and static member function must be different from the name of the containing class regardless of the casing.”
4. In 2.6 For:
“Type annotation after an iteratee (left hand side of **in**) is optional.”
5. In 3.2 Classes:
“Optionals are initialized to `nil` if omitted, unless the type is `nil?`, which in that case, absent initialization is a compile error.<br/>
Member function and field access in constructor with the "this" keyword before
all fields are initialized is invalid and would throw a compile error.<br/>
In other words, the `this` keyword is immutable to its field access operator `.`
before all the member fields are properly initialized. The behavior is similar to JavaScript and TypeScript in their "strict" mode.”
6. In 3.3 Preflight classes:
Scope and ID are “both overrideable by user-defined ones in constructor.”
7. In 3.8 Enumeration:
“Last comma is optional in single line definitions but required in multi line definitions.”
“`nameof` operator”
8. Entire section: 6.1.2 Shell strings
9. Entire section: 6.4 Kitchen Sink
10. Entire section: Inspiration (last section)

### Issues opened/changed to track spec completeness:
- [x] Opened issues:
	- [x] #3121 
	- [x] #3123 
	- [x] #3129
	- [x] #3139 
	- [x] #3140 
	- [x] #3142 
- [x] Changed description:
	- [x] #108 
	- [x] #2103 
	- [x] #116 
	- [x] #125 
	- [x] #128
	- [x] #129 
	- [x] #130
	- [x] #1737 

## Checklist

- [x] Title matches [Winglang's style guide](https://docs.winglang.io/contributors/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [ ] 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 [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*.
@staycoolcall911 staycoolcall911 added this to the Containers milestone Sep 11, 2023
@yoav-steinberg yoav-steinberg linked a pull request Sep 14, 2023 that will close this issue
10 tasks
@mergify mergify bot closed this as completed in #4133 Sep 15, 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)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.30.9.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ compiler Compiler 📜 lang-spec-impl Appears in the language spec roadmap
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants