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

feat: support bring cloud #242

Merged
merged 28 commits into from
Oct 13, 2022
Merged

feat: support bring cloud #242

merged 28 commits into from
Oct 13, 2022

Conversation

yoav-steinberg
Copy link
Contributor

@yoav-steinberg yoav-steinberg commented Oct 9, 2022

Bsically #154:
This enables bring cloud command. Its the basis for bringing any JSII into wing but currently includes some hard coded hack which make it wingsdk.cloud only. To get this working there was lots of other work done on grammar, compiler features, code structure. I'll try to summarize:

  • Support for struct literals.
  • Support map literals.
  • Updated sdk_capture_test.w to use literals as prop args until we support default arguments.
  • Misc grammar cleanups.
  • Move all type system code under /type_check, and added a jsii_importer.rs module that uses wingii.
  • Removed old bring cloud hack that treated all AST method calls under an anything typed object as a capture. Now we can detect these as real method calls of the imported JSII type.
  • Support bring cloud by importing the wingsdk JSII types into our type system during the type-checking-inference stage.
  • Added namespace support to the type system, this is used to insert imported types into their own namespace (cloud).
  • Support structs including mutiple inheritance, I used a parent flattening approach with some verifications, I hope it's good enough for now.
  • Fixed type checker where we didn't handle resource inheritance (only class inheritance).

yoav-steinberg and others added 12 commits September 22, 2022 14:51
Merge jsii resource clients into generate wing resources.
Handle JSII Void return types as no return type in wing.
* Added support fro `struct`s in parser/type_checker/jsification.
* Import constructors (including skipping scope/id on resources).
* Pass "self" argument to imported class methods.
* refactor: moved jsii importing logic into submodule under a new type_check module.
* resolve namespaced custom types in type checker.
* corretly handle parent resource in resource type checking.
* JSII imported handles references to other JSII types (not only primitives).
* Also added support for multiple struct inheritance in when importing JSII props.
* Support importing wingsdk `Duration` types.
also:
Cleaned up method_call vs function_call in grammar/ast
Fixed return type parsing
* Updated sdk_capture tests to use explicit Props ars used by sdk resources.
* Support type annotation in container literals.
* Added map literals support.
* de-dup captured method names in capture mechanism.
* removed auto capturing of everything looking like a mehtod call on an anything object (AKA wingsdk support hack).
@yoav-steinberg yoav-steinberg changed the title Support bring cloud <feature>Support bring cloud Oct 9, 2022
@yoav-steinberg
Copy link
Contributor Author

@MarkMcCulloh can you look at why the build's failing?

@winglang winglang deleted a comment from vercel bot Oct 9, 2022
@MarkMcCulloh MarkMcCulloh changed the title <feature>Support bring cloud feat: support bring cloud Oct 9, 2022
@MarkMcCulloh
Copy link
Contributor

@yoav-steinberg 2 small things:

  1. Updated title to match the "conventional PR" format
  2. The branch needed to pull from main to get some changes to prevent that "vercel" error from popping up. Updating the branch also seemed to fix the wingc test error (idk why).

@MarkMcCulloh
Copy link
Contributor

jk wingc tests are still failing 🙂 @yoav-steinberg

Copy link
Contributor

@eladb eladb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incredibly exciting!

examples/simple/sdk_capture_test.w Show resolved Hide resolved
libs/tree-sitter-wing/grammar.js Show resolved Hide resolved
libs/wingc/src/capture.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Outdated Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
@yoav-steinberg
Copy link
Contributor Author

This is incredibly exciting!

Forgot to give credis to @3p3r for wingii and @Chriscbr for two very productive pair programming sessions.

Copy link
Contributor

@Chriscbr Chriscbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome work Yoav! 🤩

libs/tree-sitter-wing/grammar.js Show resolved Hide resolved
libs/wingc/Cargo.toml Show resolved Hide resolved
libs/wingc/src/capture.rs Outdated Show resolved Hide resolved
libs/wingc/src/capture.rs Outdated Show resolved Hide resolved
libs/wingc/src/type_check/type_env.rs Outdated Show resolved Hide resolved
libs/wingc/src/type_check/type_env.rs Outdated Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Show resolved Hide resolved
libs/wingc/src/type_check/jsii_importer.rs Outdated Show resolved Hide resolved
libs/wingc/src/type_check.rs Outdated Show resolved Hide resolved
Co-authored-by: Chris Rybicki <chrisr@monada.co>
libs/tree-sitter-wing/grammar.js Outdated Show resolved Hide resolved
libs/tree-sitter-wing/grammar.js Show resolved Hide resolved
libs/tree-sitter-wing/grammar.js Outdated Show resolved Hide resolved
libs/tree-sitter-wing/grammar.js Outdated Show resolved Hide resolved
@3p3r
Copy link
Contributor

3p3r commented Oct 10, 2022

@MarkMcCulloh tests failing is a tooling problem. The tests are actually passing:

> nx run wingc:test

> cargo test -p wingc
    Finished test [unoptimized + debuginfo] target(s) in 3m 50s
     Running unittests src/lib.rs (target/debug/deps/wingii-5636e1654a1dac3e)

running 6 tests
test test::tests::can_load_assembly_from_path ... ok
test test::tests::can_load_assembly_from_single_file ... ok
test test::tests::can_correctly_tell_redirects ... ok
test test::tests::can_load_constructs_assembly_with_type_system ... ok
test test::tests::can_query_basic_types ... ok
test test::tests::does_not_blow_up ... ok

test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.01s

   Doc-tests wingii

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s


> nx run wingii:test

> cargo test -p wingii

 

 >  NX   Running target "test" failed

   Failed tasks:
   
   - wingc:test

NX is reporting it a failure despite Rust passing its tests. Maybe it does not like the "running 0 tests" part?

@MarkMcCulloh
Copy link
Contributor

MarkMcCulloh commented Oct 10, 2022

@3p3r No there's an actual wingc test failure. The output is confusing though because multiple test projects are running in parallel. Better test reporting is something I can look into.

---- sanity::can_compile_simple_files stdout ----

=== ../../examples/simple/inflight.w ===

Loaded JSII assembly @monadahq/wingsdk
Adding type BucketBase to namespace
Found property public with type Object {"primitive": String("boolean")}
Adding struct type BucketProps
Found property stateful with type Object {"primitive": String("boolean")}
Resource BucketBase doesn't not seem to have a client
Adding type Bucket to namespace
Adding method get to class
Adding method put to class
Type @monadahq/wingsdk.cloud.BucketInflightMethods is unsupported, skipping
Adding type FunctionBase to namespace
Found property env with type Object {"collection": Object {"elementtype": Object {"primitive": String("string")}, "kind": String("map")}}
Adding struct type FunctionProps
Adding method addEnvironment to class
Found property stateful with type Object {"primitive": String("boolean")}
Resource FunctionBase doesn't not seem to have a client
Adding type Function to namespace
Adding method addEnvironment to class
Adding method invoke to class
Type @monadahq/wingsdk.cloud.FunctionInflightMethods is unsupported, skipping
The JSII interface IBucketClient is not a "datatype", skipping
The JSII interface IFunctionClient is not a "datatype", skipping
The JSII interface IQueueClient is not a "datatype", skipping
Adding type QueueBase to namespace
Found property timeout with type Object {"fqn": String("@monadahq/wingsdk.core.Duration")}
Adding struct type QueueProps
Adding method onMessage to class
Found property batchSize with type Object {"primitive": String("number")}
Adding struct type QueueOnMessageProps
Found property stateful with type Object {"primitive": String("boolean")}
Resource QueueBase doesn't not seem to have a client
Adding type Queue to namespace
Adding method onMessage to class
Adding method push to class
Type @monadahq/wingsdk.cloud.QueueInflightMethods is unsupported, skipping
thread 'sanity::can_compile_simple_files' panicked at 'Unknown symbol c', libs/wingc/src/type_check/type_env.rs:83:43
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    sanity::can_compile_simple_files

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 32.48s

error: test failed, to rerun pass '-p wingc --lib'

It's failing because inflight.w has bring cloud as c which is not something this PR supports.

@3p3r 3p3r mentioned this pull request Oct 11, 2022
4 tasks
Also:
* Fix inflight.w example based on real imported cloud.Bucket type.
* Removed skipping type checking on `new` expressions when the type is resolved to an `Anything`, we don't expect custom types to be `Anything`'s anymore.
This was there because we didn't really know the type of the `new` expression. Now the type is available through a the `Expr.evaluated_type`.
@yoav-steinberg
Copy link
Contributor Author

It's failing because inflight.w has bring cloud as c which is not something this PR supports.

  • Added supporting bring aliases and fixed the example.
  • Cleaned up some more hacks assuming anything objects are cloud resources in the type checker and jsification.

Tests should pass now.

@yoav-steinberg yoav-steinberg merged commit 1f5e867 into main Oct 13, 2022
@yoav-steinberg yoav-steinberg deleted the yoav/jsii_import branch October 13, 2022 08:28
skyrpex pushed a commit that referenced this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants