Skip to content

Commit

Permalink
fix(compiler): can't pass inflight closure as parameter to super()
Browse files Browse the repository at this point in the history
…call (#6599)

fixes #4925

Now you can pass inflight closures and also any `new PreflightClass()` as parameters to a `super()` call in a ctor.
What I did is detect these cases and made sure the implicit scope passed to the construct ctor is `$scope` and not `this` which isn't available before `super()` is called.


## 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)
- [ ] 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 Jun 2, 2024
1 parent 5ff5635 commit ccdd51b
Show file tree
Hide file tree
Showing 9 changed files with 480 additions and 11 deletions.
116 changes: 116 additions & 0 deletions docs/docs/02-concepts/02-application-tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ id: application-tree
title: Application tree
---

## Instance names

Instances of preflight classes in Wing are identified by a unique name.
The name is used to identify the resource in the Wing Console, and is used to determine the logical name assigned to the resource in the infrastructure provisioning engine (such as Terraform or CloudFormation), and the physical name of the resource in the target cloud provider (such as AWS, Azure, or GCP).

Expand Down Expand Up @@ -34,6 +36,120 @@ new Group1();
new Group2();
```

## Instance scope

Instances of preflight classes define the application's construct tree. This way you can think about the generated cloud infrastructure as a logical tree where each node is some logical piece of infrastructure which may contain children nodes which are also part of your application's infrastructure (composition).

In Wing this tree structure is automatically generated. Any class instantiated at the top level (global scope) is a child of the "root" node of of the tree.
Any class instantiated inside another class (in its constructor or one of its other preflight methods) will be placed as a child of that other class.

```js
class ThumbnailBucket {
//...
}

class ImageStorage {
new() {
new ThumbnailBucket(); // This ThumbnailBucket will be a child of a ImageStorage instance in the construct tree
}
}

new ImageStorage(); // This ImageStorage will be a child of the root in the construct tree
new ThumbnailBucket(); // This Counter will be a child of of the root in the construct tree

// Here's a tree view of the generated infrastructure:
//
// root
// /\
// ImageStorage ThumbnailBucket
// /
// ThumbnailBucket
```

As mentioned in the [previous section](#instance-names) each instance must have a unique name within its scope.
And the name will be automatically generated based on the class name.
So the tree shown above also shows the correct names for each infrastructure piece of our application.
You may query information about the construct tree using the `nodeof(someInstance)` intrinsic function:
```js
let b = new cloud.Bucket();
log(nodeof(b).path); // Will log something like "/root/Bucket"
```

### Controlling instance scope

You may define an explicit scope for an instance instead of using Wing's default of placing it inside the instance where it was created using the `in` keyword:

```js
class ThumbnailBucket {
//...
}

class ImageStorage {
new() {
new ThumbnailBucket(); // This ThumbnailBucket will be a child of a ImageStorage instance in the construct tree
}
}

let imageStorage = new ImageStorage(); // This goes in root
let defaultThumbnails = new ThumbnailBucket() as "defaultThumbs" in imageStorage; // This is explicitly named "defaultThumbs" and explicitly placed inside imageStorage

// Here's a tree view of the generated infrastructure:
//
// root
// /
// ImageStorage
// / \
// ThumbnailBucket defaultsThumbs
```

### Instances created inside static methods

Preflight classes instantiated inside static methods, or instantiated inside constructors before `this` is available will use the
scope of the caller by default:

```js
class Factory {
pub static make() {
new cloud.Bucket(); // We're in a static, so we don't know where to place this bucket
}
}

class MyBucket() {
new() {
Factory.make(); // Bucket will be placed inside `this` instance of `MyBucket`
}
}
new MyBucket();

// tree:
// root
// /
// MyBucket
// /
// Bucket
```

Similarly, consider this case where we instantiate a class inside a parameter in a `super()` constructor call before the
the class is creates and its scope becomes valid:

```js
class Base {
new(b: cloud.Bucket) {}
}
class Derived extends Base {
new() {
super(new cloud.Bucket()); // Bucket create before `Derived` so scope defaults to caller
}
}
new Derived();

// tree:
// root
// /\
// Bucket Derived
```

## Interop
Classes in Wing are an extension of the [Construct Programming Model] and as such any [AWS Constructs] can be natively used in Wing applications.

[Construct Programming Model]: https://docs.aws.amazon.com/cdk/v2/guide/constructs.html
Expand Down
35 changes: 35 additions & 0 deletions examples/tests/valid/inflight_closure_as_super_param.test.w
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
class Foo {}

class Base {
pub h: inflight (): str;
pub f_base: Foo;
new(handler: inflight (): str, f: Foo) {
this.h = handler;
this.f_base = f;
}
}

class Derived extends Base {
pub f: Foo;
new() {
super(inflight (): str => {
return "boom!";
}, new Foo() as "in_root");
this.f = new Foo() as "in_derived";
}
}

let c = new Derived() as "derived";
// Make sure that instances created in a ctor are scoped to the instance they were created in
// This is related to this test because the transformed inflight closure is also an instance
// created in a ctor, but it's special cased to to be scoped in the class because `this` isn't
// available during the closures instantiation.
assert(nodeof(c.f).path.endsWith("derived/in_derived"));
// Make sure the instance created in the super call is scoped to the parent (root)
assert(!nodeof(c.f_base).path.endsWith("derived/in_root"));
let appPath = nodeof(this).path;
assert(nodeof(c.f_base).path == "{appPath}/in_root");

test "boom!" {
assert(c.h() == "boom!");
}
11 changes: 9 additions & 2 deletions libs/wingc/src/jsify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,14 @@ impl<'a> JSifier<'a> {
}) {
Some(SCOPE_PARAM.to_string())
} else {
Some("this".to_string())
// By default use `this` as the scope.
// If we're inside an argument to a `super()` call then `this` isn't avialble, in which case
// we can safely use the ctor's `$scope` arg.
if ctx.visit_ctx.current_stmt_is_super_call() {
Some(SCOPE_PARAM.to_string())
} else {
Some("this".to_string())
}
}
}
} else {
Expand Down Expand Up @@ -1146,7 +1153,7 @@ impl<'a> JSifier<'a> {
let mut code = CodeMaker::with_source(&statement.span);

CompilationContext::set(CompilationPhase::Jsifying, &statement.span);
ctx.visit_ctx.push_stmt(statement.idx);
ctx.visit_ctx.push_stmt(statement);
match &statement.kind {
StmtKind::Bring { source, identifier } => match source {
BringSource::BuiltinModule(name) => {
Expand Down
2 changes: 1 addition & 1 deletion libs/wingc/src/lifting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ impl<'a> Visit<'a> for LiftVisitor<'a> {
fn visit_stmt(&mut self, node: &'a Stmt) {
CompilationContext::set(CompilationPhase::Lifting, &node.span);

self.ctx.push_stmt(node.idx);
self.ctx.push_stmt(node);

// If this is an explicit lift statement then add the explicit lift
if let StmtKind::ExplicitLift(explicit_lift) = &node.kind {
Expand Down
2 changes: 1 addition & 1 deletion libs/wingc/src/lsp/symbol_locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ impl<'a> Visit<'a> for SymbolLocator<'a> {
return;
}

self.ctx.push_stmt(node.idx);
self.ctx.push_stmt(node);

// Handle situations where symbols are actually defined in inner scopes
match &node.kind {
Expand Down
2 changes: 1 addition & 1 deletion libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4124,7 +4124,7 @@ new cloud.Function(@inflight("./handler.ts"), lifts: { bucket: ["put"] });
CompilationContext::set(CompilationPhase::TypeChecking, &stmt.span);

// Set the current statement index for symbol lookup checks.
self.with_stmt(stmt.idx, |tc| match &stmt.kind {
self.with_stmt(stmt, |tc| match &stmt.kind {
StmtKind::Let {
reassignable,
var_name,
Expand Down
25 changes: 19 additions & 6 deletions libs/wingc/src/visit_context.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use itertools::Itertools;

use crate::{
ast::{Class, ExprId, FunctionSignature, Phase, Symbol, UserDefinedType},
ast::{Class, ExprId, FunctionSignature, Phase, Stmt, StmtKind, Symbol, UserDefinedType},
type_check::symbol_env::SymbolEnvRef,
};

Expand All @@ -18,6 +18,12 @@ pub enum PropertyObject {
Instance(ExprId),
}

#[derive(Clone)]
pub struct StmtContext {
pub idx: usize,
pub super_call: bool,
}

#[derive(Clone)]
pub struct VisitContext {
phase: Vec<Phase>,
Expand All @@ -26,7 +32,7 @@ pub struct VisitContext {
property: Vec<(PropertyObject, Symbol)>,
function: Vec<FunctionContext>,
class: Vec<UserDefinedType>,
statement: Vec<usize>,
statement: Vec<StmtContext>,
in_json: Vec<bool>,
in_type_annotation: Vec<bool>,
expression: Vec<ExprId>,
Expand Down Expand Up @@ -64,16 +70,23 @@ impl VisitContext {

// --

pub fn push_stmt(&mut self, stmt: usize) {
self.statement.push(stmt);
pub fn push_stmt(&mut self, stmt: &Stmt) {
self.statement.push(StmtContext {
idx: stmt.idx,
super_call: matches!(stmt.kind, StmtKind::SuperConstructor { .. }),
});
}

pub fn pop_stmt(&mut self) {
self.statement.pop();
}

pub fn current_stmt_idx(&self) -> usize {
*self.statement.last().unwrap_or(&0)
self.statement.last().map_or(0, |s| s.idx)
}

pub fn current_stmt_is_super_call(&self) -> bool {
self.statement.last().map_or(false, |s| s.super_call)
}

// --
Expand Down Expand Up @@ -230,7 +243,7 @@ pub trait VisitorWithContext {
self.ctx().pop_expr();
}

fn with_stmt(&mut self, stmt: usize, f: impl FnOnce(&mut Self)) {
fn with_stmt(&mut self, stmt: &Stmt, f: impl FnOnce(&mut Self)) {
self.ctx().push_stmt(stmt);
f(self);
self.ctx().pop_stmt();
Expand Down
Loading

0 comments on commit ccdd51b

Please sign in to comment.