Skip to content

Commit

Permalink
feat(compiler): access modifiers (#4133)
Browse files Browse the repository at this point in the history
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)*.
  • Loading branch information
yoav-steinberg authored Sep 15, 2023
1 parent 3083cc1 commit 3f09a3e
Show file tree
Hide file tree
Showing 117 changed files with 2,079 additions and 1,278 deletions.
119 changes: 92 additions & 27 deletions docs/docs/03-language-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,73 @@ Static class fields are not supported yet, see https://github.com/winglang/wing/
---
### 1.5 Reassignability
### 1.5 Access Modifiers (member visibility)
Class members, by default, can only be accessed from within the implementation code of the same class (private).
Inner classes or closures can access private members of their containing class.
```TS
class Foo {
private_field: num; // This is private by default
init() {this.private_field = 1;}
method() {
log(this.private_field); // We can access `private_field` since we're in Foo
class InnerFoo {
method(f: Foo) {
log(f.private_field); // We can access `private_field` since we're in Foo
}
}
}
}
```
Accessing class members of a super class can be done by adding the the `protected` access modifier.
```TS
class Foo {
protected protected_method() {}; // This is a `protected` method
}
class Bar extends Foo {
method() {
this.protected_method(); // We can access `protected_method` from a subclass
}
}
```
The `pub` access modifier makes the class member accessible from anywhere.
Interface members are always public.
Implementing interface members in a class requires explicitly flagging them as `pub`.
```TS
interface FooInterface {
interface_method(): void; // Interface definitions are always implicitly `pub`
}
class Foo impl FooInterface {
pub public_method() {} // This can be accessed from outside of the class implemenetation
pub interface_method() {} // This must be explicitly defined as `pub` since it's an interface implementation
}
let f = new Foo();
f.public_method(); // We can call this method from outside the class - it's public
```
Access modifier rules apply for both fields and methods of a class.
Struct fields are always public and do not have access modifiers.
#### 1.5.1 Method overriding and access modifiers
Private methods cannot be overriden.
Overriding a method of a parent class requires the parent class's method to be either `pub` or `protected`.
The overriding method can have either the same access modifier as the original method or a more permissive one.
You cannot "decrease" the access level down the inheritence hierarchy, only "increase" it.
In practice this means:
* `protected` methods can be overidden by either a `protected` or a `pub` method.
* `pub` methods can be overriden by a `pub` method.
Note that method overriding only applies to instance methods. `static` methods are not treated as part of the inheritence hierarcy.
[`top`][top]
---
### 1.6 Reassignability
Re-assignment to variables that are defined with `let` is not allowed in Wing.
Expand Down Expand Up @@ -700,7 +766,7 @@ let f = (arg1: num, var arg2: num) => {
---
### 1.6 Optionality
### 1.7 Optionality
Nullity is a primary source of bugs in software. Being able to guarantee that a value will never be
null makes it easier to write safe code without constantly having to take nullity into account.
Expand All @@ -723,9 +789,9 @@ Here's a quick summary of how optionality works in Wing:
* The keyword `nil` can be used in assignment scenarios to indicate that an optional doesn't have a
value. It cannot be used to test if an optional has a value or not.
#### 1.6.1 Declaration
#### 1.7.1 Declaration
##### 1.6.1.1 Struct fields
##### 1.7.1.1 Struct fields
One of the more common use cases for optionals is to use them in struct declarations.
Expand All @@ -746,7 +812,7 @@ assert(david.address? == false);
assert(jonathan.address? == true);
```
##### 1.6.1.2 Variables
##### 1.7.1.2 Variables
Use `T?` to indicate that a variable is optional. To initialize it without a value use `= nil`.
Expand All @@ -764,7 +830,7 @@ x = nil;
assert(x? == false);
```
##### 1.6.1.3 Class fields
##### 1.7.1.3 Class fields
Similarly to struct fields, fields of classes can be also defined as optional using `T?`:
Expand All @@ -784,7 +850,7 @@ class Foo {
}
```
##### 1.6.1.4 Function arguments
##### 1.7.1.4 Function arguments
In the following example, the argument `by` is optional, so it is possible to call `increment()`
without supplying a value for `by`:
Expand Down Expand Up @@ -826,7 +892,7 @@ f(myRequired: "hello");
f(myOptional: 12, myRequired: "dang");
```
##### 1.6.1.5 Function return types
##### 1.7.1.5 Function return types
If a function returns an optional type, use the `return nil;` statement to indicate that the value
is not defined.
Expand All @@ -852,7 +918,7 @@ if let name = tryParseName("Neo Matrix") {
}
```
#### 1.6.2 Testing using `x?`
#### 1.7.2 Testing using `x?`
To test if an optional has a value or not, you can either use `x == nil` or `x != nil` or the
special syntax `x?`.
Expand Down Expand Up @@ -883,7 +949,7 @@ if myPerson.address == nil {
}
```
#### 1.6.3 Unwrapping using `if let`
#### 1.7.3 Unwrapping using `if let`
The `if let` statement (or `if let var` for a reassignable variable) can be used to test if an
optional is defined and *unwrap* it into a non-optional variable defined inside the block:
Expand All @@ -899,7 +965,7 @@ if let address = myPerson.address {
> multiple conditions, or unwrapping multiple optionals. This is something we might consider in the
> future.
#### 1.6.4 Unwrapping or default value using `??`
#### 1.7.4 Unwrapping or default value using `??`
The `??` operator can be used to unwrap or provide a default value. This returns a value of `T` that
can safely be used.
Expand All @@ -908,7 +974,7 @@ can safely be used.
let address: str = myPerson.address ?? "Planet Earth";
```
#### 1.6.5 Optional chaining using `?.`
#### 1.7.5 Optional chaining using `?.`
The `?.` syntax can be used for optional chaining. Optional chaining returns a value of type `T?`
which must be unwrapped in order to be used.
Expand All @@ -925,7 +991,7 @@ if let ip = ipAddress {
---
#### 1.6.6 Roadmap
#### 1.7.6 Roadmap
The following features are not yet implemented, but we are planning to add them in the future:
Expand All @@ -944,7 +1010,7 @@ The following features are not yet implemented, but we are planning to add them
* Support `??` for different types if they have a common ancestor (and also think of interfaces).
See https://github.com/winglang/wing/issues/2103 to track.
### 1.7 Type Inference
### 1.8 Type Inference
Type can optionally be put between name and the equal sign, using a colon.
Partial type inference is allowed while using the `?` keyword immediately after
Expand Down Expand Up @@ -972,7 +1038,7 @@ Type annotations are required for method arguments and their return value but op
---
### 1.8 Error Handling
### 1.9 Error Handling
Exceptions and `try/catch/finally` are the error mechanism. Mechanics directly
translate to JavaScript. You can create a new exception with a `throw` call.
Expand All @@ -997,7 +1063,7 @@ In the presence of `catch` the variable holding the exception (`e` in the exampl
---
### 1.9 Recommended Formatting
### 1.10 Recommended Formatting
Wing recommends the following formatting and naming conventions:
Expand All @@ -1013,7 +1079,7 @@ Wing recommends the following formatting and naming conventions:
---
### 1.10 Memory Management
### 1.11 Memory Management
There is no implicit memory de-allocation function, dynamic memory is managed by
Wing and is garbage collected (relying on JSII target GC for the meantime).
Expand All @@ -1022,7 +1088,7 @@ Wing and is garbage collected (relying on JSII target GC for the meantime).
---
### 1.11 Execution Model
### 1.12 Execution Model
Execution model currently is delegated to the JSII target. This means if you are
targeting JSII with Node, Wing will use the event based loop that Node offers.
Expand All @@ -1043,7 +1109,7 @@ case of CDK for Terraform target.
---
### 1.12 Asynchronous Model
### 1.13 Asynchronous Model
Wing builds upon the asynchronous model of JavaScript currently and expands upon
it with new keywords and concepts. The `async` keyword of JavaScript is replaced
Expand All @@ -1056,17 +1122,17 @@ Main concepts to understand:
Contrary to JavaScript, any call to an async function is implicitly awaited in Wing.
#### 1.12.1 Roadmap
#### 1.13.1 Roadmap
The following features are not yet implemented, but we are planning to add them in the future:
* `await`/`defer` statements - see https://github.com/winglang/wing/issues/116 to track.
* Promise function type - see https://github.com/winglang/wing/issues/1004 to track.
### 1.13 Roadmap
### 1.14 Roadmap
Access modifiers (`private`/`public`/`internal`/`protected`) are not yet implemented.
See https://github.com/winglang/wing/issues/108 to track.
* Module type visibility (exports/`pub` types) is not implemented yet - see https://github.com/winglang/wing/issues/130 to track.
* `internal` access modifier is not yet implemented - see https://github.com/winglang/wing/issues/4156 to track.
## 2. Statements
Expand Down Expand Up @@ -1310,7 +1376,7 @@ class Bar {
this.z = new Foo();
this.log(); // OK to call here
}
public log() {
pub log() {
log("${this.y}");
}
}
Expand All @@ -1331,7 +1397,7 @@ their "strict" mode.
class Foo {
x: num;
init() { this.x = 0; }
public method() { }
pub method() { }
}
class Boo extends Foo {
init() {
Expand All @@ -1349,7 +1415,7 @@ Classes can implement interfaces iff the interfaces do not contain `inflight`.
class Foo {
x: num;
init() { this.x = 0; }
public method() { }
pub method() { }
}
class Boo extends Foo {
init() { super(); this.x = 10; }
Expand All @@ -1372,7 +1438,6 @@ In methods if return type is missing, `: void` is assumed.
The following features are not yet implemented, but we are planning to add them in the future:
* Overloading class methods (including `init`) - see https://github.com/winglang/wing/issues/3123 to track.
* Overriding class methods - see https://github.com/winglang/wing/issues/1124 to track.
* Using the `final` keyword to stop the inheritance chain - see https://github.com/winglang/wing/issues/460 to track.
[`▲ top`][top]
Expand Down
16 changes: 8 additions & 8 deletions docs/docs/07-examples/08-classes.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Foo {
log("at inflight init");
}

inflight doStuff() {
pub inflight doStuff() {
// all code is async and runs on the cloud
log("field3[0]='${this.field3.at(0)}'");
util.sleep(1s);
Expand All @@ -55,7 +55,7 @@ interface IProfile {
}

inflight class WingPerson impl IProfile {
inflight name(): str {
pub inflight name(): str {
return "Fairy Wing";
}
}
Expand Down Expand Up @@ -84,10 +84,10 @@ class BucketBasedKeyValueStore impl IKVStore {
init() {
this.bucket = new cloud.Bucket();
}
inflight get(key: str): Json {
pub inflight get(key: str): Json {
return this.bucket.getJson(key);
}
inflight set(key: str, value: Json): void {
pub inflight set(key: str, value: Json): void {
this.bucket.putJson(key, value);
}
}
Expand All @@ -108,10 +108,10 @@ class BucketBasedKeyValueStore impl IKVStore {
init() {
this.bucket = new cloud.Bucket();
}
inflight get(key: str): Json {
pub inflight get(key: str): Json {
return this.bucket.getJson(key);
}
inflight set(key: str, value: Json): void {
pub inflight set(key: str, value: Json): void {
this.bucket.putJson(key, value);
}
}
Expand All @@ -127,10 +127,10 @@ class TableBasedKeyValueStore impl IKVStore {
}
);
}
inflight get(key: str): Json {
pub inflight get(key: str): Json {
return this.table.get(key);
}
inflight set(key: str, value: Json): str {
pub inflight set(key: str, value: Json): str {
this.table.insert(key, value);
}
}
Expand Down
2 changes: 1 addition & 1 deletion examples/proposed/cacheable-function.w
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ resource CachableFunction extends cloud.Function {
this._cache = new cloud.Bucket();
}

public override inflight invoke(event: any): any {
pub override inflight invoke(event: any): any {
let key = hashof(event);
let cached = this._cache.try_get(key);
if cached {
Expand Down
8 changes: 4 additions & 4 deletions examples/proposed/counting-semaphore.w
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ struct CountingSemaphoreProps {
}

resource CountingSemaphore {
public limit: num;
pub limit: num;
_counter: cloud.Counter;

// need some ttl solution here,
Expand All @@ -19,7 +19,7 @@ resource CountingSemaphore {
// some stable unique instance id is wanted in the inflight context
// so that a resource instance can be properly claimed and later released
// when used in conjunction with a key-value store
public inflight try_acquire(): bool {
pub inflight try_acquire(): bool {
if this.is_at_capacity() {
return false;
}
Expand All @@ -35,15 +35,15 @@ resource CountingSemaphore {
// probably key-value store is wanted,
// so that a specific resource instance can be released
// rather than naively releasing a count
public inflight release() {
pub inflight release() {
if this._counter.peek() <= 0 {
return;
}

this._counter.dec();
}

public inflight is_at_capacity(): bool {
pub inflight is_at_capacity(): bool {
return this._counter.peek() >= this.limit;
}
}
Expand Down
Loading

0 comments on commit 3f09a3e

Please sign in to comment.