From 5d5d46ee60ed948cdfa2e3de40cd1ec080418e21 Mon Sep 17 00:00:00 2001 From: yoav-steinberg Date: Sun, 27 Aug 2023 12:49:41 +0300 Subject: [PATCH] fix(compiler): capture and lifting cleanups and fixes (#3899) * Types aren't expressions anymore. This reverts a change in #3213. Back then we turned all user defined types into expressions so we can lift/capture them the same way we handle expressions. The downside was that the type system didn't make sense anymore (we added a type variable kind and the symbol env wasn't normalized). A side effect is better error spans on unknown type errors. * To handle capturing/lifting of Types I created a `Liftable` enum that represents AST elements that can be lifted. A lift token now points to a `Liftable` and not to an expression. Currently a `Liftable` can be either an expression or a `UserDefinedType`. * Simplified and normalized the `Lifts` data structure stored per class. It now contains: * `lifts_qualifications` - Stores information for each class inflight method which preflight code it lifts and for that code which ops are performed on it. It's a map of map: "method" -> "preflight js code" -> "set of ops". This data structure is used for generating the lift bindings. * `captures` - Stores capture tokens and for each token the preflight js code used for capture and also whether this is a free capture or an object field. This is used to initialize the tokens in preflight. * `token_for_liftable` - Maps capturable AST elements (`Liftable`s) to a capture token. This is used for generating inflight code when we encounter a captured type or expression during jsification. * Fixed a bug where a type referencing itself but calling an inflight static method wasn't properly lifted. No bindings were generated for it leading to potential permission issues. Added a snapshot test. * Fixed bug parsing static method calls on json `Json.something()` generated a wrong AST expression where we wrapped a `TypeMember` reference inside a `InstanceMemeber` reference (instead of just being a `TypeMemeber` reference). So the end jsification result was `Json.something().something()`?? * Fixed visitor type annotation's default implementation to call `visit_user_defined_type` for user defined types. ## 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)*. --- libs/wingc/src/ast.rs | 62 ++-- libs/wingc/src/closure_transform.rs | 19 +- libs/wingc/src/fold.rs | 9 +- libs/wingc/src/jsify.rs | 115 ++++--- ...class_tries_to_extend_preflight_class.snap | 1 + .../jsify/snapshots/lift_self_reference.snap | 82 +++++ libs/wingc/src/jsify/tests.rs | 15 + libs/wingc/src/lifting.rs | 280 +++++++++++------- libs/wingc/src/lsp/completions.rs | 6 +- libs/wingc/src/lsp/hover.rs | 115 ++++--- libs/wingc/src/lsp/signature.rs | 5 +- .../hovers/static_stdtype_method.snap | 2 +- .../hovers/user_defined_type_annotation.snap | 14 + .../user_defined_type_reference_property.snap | 14 + .../user_defined_type_reference_type.snap | 14 + libs/wingc/src/parser.rs | 73 ++--- libs/wingc/src/type_check.rs | 139 ++++----- libs/wingc/src/type_check/lifts.rs | 167 ++++------- libs/wingc/src/type_check_assert.rs | 2 +- libs/wingc/src/valid_json_visitor.rs | 2 +- libs/wingc/src/visit.rs | 18 +- libs/wingc/src/visit_context.rs | 52 +++- tools/hangar/__snapshots__/invalid.ts.snap | 42 ++- .../call_static_of_myself.w_compile_tf-aws.md | 12 + ...ling_inflight_variants.w_compile_tf-aws.md | 6 +- .../capture_containers.w_compile_tf-aws.md | 2 +- .../capture_mutables.w_compile_tf-aws.md | 2 +- .../valid/capture_tokens.w_compile_tf-aws.md | 1 + .../extern_implementation.w_compile_tf-aws.md | 6 + .../valid/resource.w_compile_tf-aws.md | 1 + ...ource_captures_globals.w_compile_tf-aws.md | 2 +- 31 files changed, 737 insertions(+), 543 deletions(-) create mode 100644 libs/wingc/src/jsify/snapshots/lift_self_reference.snap create mode 100644 libs/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap create mode 100644 libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap create mode 100644 libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap diff --git a/libs/wingc/src/ast.rs b/libs/wingc/src/ast.rs index 3c0046ecd7a..2ee52902908 100644 --- a/libs/wingc/src/ast.rs +++ b/libs/wingc/src/ast.rs @@ -148,13 +148,26 @@ pub enum TypeAnnotationKind { // In the future this may be an enum for type-alias, class, etc. For now its just a nested name. // Also this root,fields thing isn't really useful, should just turn in to a Vec. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Eq)] pub struct UserDefinedType { pub root: Symbol, pub fields: Vec, pub span: WingSpan, } +impl Hash for UserDefinedType { + fn hash(&self, state: &mut H) { + self.root.hash(state); + self.fields.hash(state); + } +} + +impl PartialEq for UserDefinedType { + fn eq(&self, other: &Self) -> bool { + self.root == other.root && self.fields == other.fields + } +} + impl UserDefinedType { pub fn for_class(class: &Class) -> Self { Self { @@ -173,13 +186,6 @@ impl UserDefinedType { pub fn full_path_str(&self) -> String { self.full_path().iter().join(".") } - - pub fn to_expression(&self) -> Expr { - Expr::new( - ExprKind::Reference(Reference::TypeReference(self.clone())), - self.span.clone(), - ) - } } impl Display for UserDefinedType { @@ -346,21 +352,12 @@ pub struct Class { pub methods: Vec<(Symbol, FunctionDefinition)>, pub initializer: FunctionDefinition, pub inflight_initializer: FunctionDefinition, - pub parent: Option, // base class (the expression is a reference to a user defined type) + pub parent: Option, // base class (the expression is a reference to a user defined type) pub implements: Vec, pub phase: Phase, } impl Class { - /// Returns the `UserDefinedType` of the parent class, if any. - pub fn parent_udt(&self) -> Option<&UserDefinedType> { - let Some(expr) = &self.parent else { - return None; - }; - - expr.as_type_reference() - } - /// Returns all methods, including the initializer and inflight initializer. pub fn all_methods(&self, include_initializers: bool) -> Vec<&FunctionDefinition> { let mut methods: Vec<&FunctionDefinition> = vec![]; @@ -606,19 +603,11 @@ impl Expr { let id = EXPR_COUNTER.fetch_add(1, Ordering::SeqCst); Self { id, kind, span } } - - /// Returns the user defined type if the expression is a reference to a type. - pub fn as_type_reference(&self) -> Option<&UserDefinedType> { - match &self.kind { - ExprKind::Reference(Reference::TypeReference(t)) => Some(t), - _ => None, - } - } } #[derive(Debug)] pub struct NewExpr { - pub class: Box, // expression must be a reference to a user defined type + pub class: UserDefinedType, // expression must be a reference to a user defined type pub obj_id: Option>, pub obj_scope: Option>, pub arg_list: ArgList, @@ -724,10 +713,11 @@ pub enum Reference { property: Symbol, optional_accessor: bool, }, - /// A reference to a type (e.g. `std.Json` or `MyResource` or `aws.s3.Bucket`) - TypeReference(UserDefinedType), /// A reference to a member inside a type: `MyType.x` or `MyEnum.A` - TypeMember { typeobject: Box, property: Symbol }, + TypeMember { + type_name: UserDefinedType, + property: Symbol, + }, } impl Spanned for Reference { @@ -739,8 +729,7 @@ impl Spanned for Reference { property, optional_accessor: _, } => object.span().merge(&property.span()), - Reference::TypeReference(type_) => type_.span(), - Reference::TypeMember { typeobject, property } => typeobject.span().merge(&property.span()), + Reference::TypeMember { type_name, property } => type_name.span().merge(&property.span()), } } } @@ -760,13 +749,8 @@ impl Display for Reference { }; write!(f, "{}.{}", obj_str, property.name) } - Reference::TypeReference(type_) => write!(f, "{}", type_), - Reference::TypeMember { typeobject, property } => { - let ExprKind::Reference(ref r) = typeobject.kind else { - return write!(f, ".{}", property.name); - }; - - write!(f, "{}.{}", r, property.name) + Reference::TypeMember { type_name, property } => { + write!(f, "{}.{}", type_name, property.name) } } } diff --git a/libs/wingc/src/closure_transform.rs b/libs/wingc/src/closure_transform.rs index bbb32b85440..34b1516c220 100644 --- a/libs/wingc/src/closure_transform.rs +++ b/libs/wingc/src/closure_transform.rs @@ -190,14 +190,11 @@ impl Fold for ClosureTransformer { ExprKind::Call { callee: CalleeKind::Expr(Box::new(Expr::new( ExprKind::Reference(Reference::TypeMember { - typeobject: Box::new(Expr::new( - ExprKind::Reference(Reference::TypeReference(UserDefinedType { - root: Symbol::new("std", WingSpan::for_file(file_id)), - fields: vec![Symbol::new("Node", WingSpan::for_file(file_id))], - span: WingSpan::for_file(file_id), - })), - WingSpan::for_file(file_id), - )), + type_name: UserDefinedType { + root: Symbol::new("std", WingSpan::for_file(file_id)), + fields: vec![Symbol::new("Node", WingSpan::for_file(file_id))], + span: WingSpan::for_file(file_id), + }, property: Symbol::new("of", WingSpan::for_file(file_id)), }), WingSpan::for_file(file_id), @@ -304,7 +301,7 @@ impl Fold for ClosureTransformer { // ``` let new_class_instance = Expr::new( ExprKind::New(NewExpr { - class: Box::new(class_udt.to_expression()), + class: class_udt, arg_list: ArgList { named_args: IndexMap::new(), pos_args: vec![], @@ -365,9 +362,7 @@ impl<'a> Fold for RenameThisTransformer<'a> { Reference::Identifier(ident) } } - Reference::InstanceMember { .. } | Reference::TypeMember { .. } | Reference::TypeReference(_) => { - fold::fold_reference(self, node) - } + Reference::InstanceMember { .. } | Reference::TypeMember { .. } => fold::fold_reference(self, node), } } } diff --git a/libs/wingc/src/fold.rs b/libs/wingc/src/fold.rs index 9264ae7e934..92df85e9cd2 100644 --- a/libs/wingc/src/fold.rs +++ b/libs/wingc/src/fold.rs @@ -210,7 +210,7 @@ where .map(|(name, def)| (f.fold_symbol(name), f.fold_function_definition(def))) .collect(), initializer: f.fold_function_definition(node.initializer), - parent: node.parent.map(|parent| f.fold_expr(parent)), + parent: node.parent.map(|parent| f.fold_user_defined_type(parent)), implements: node .implements .into_iter() @@ -342,7 +342,7 @@ where F: Fold + ?Sized, { NewExpr { - class: Box::new(f.fold_expr(*node.class)), + class: f.fold_user_defined_type(node.class), obj_id: node.obj_id, arg_list: f.fold_args(node.arg_list), obj_scope: node.obj_scope, @@ -386,9 +386,8 @@ where property: f.fold_symbol(property), optional_accessor, }, - Reference::TypeReference(udt) => Reference::TypeReference(f.fold_user_defined_type(udt)), - Reference::TypeMember { typeobject, property } => Reference::TypeMember { - typeobject: Box::new(f.fold_expr(*typeobject)), + Reference::TypeMember { type_name, property } => Reference::TypeMember { + type_name: f.fold_user_defined_type(type_name), property: f.fold_symbol(property), }, } diff --git a/libs/wingc/src/jsify.rs b/libs/wingc/src/jsify.rs index 57474367b1a..9d72fb7264f 100644 --- a/libs/wingc/src/jsify.rs +++ b/libs/wingc/src/jsify.rs @@ -9,7 +9,6 @@ use std::{ borrow::Borrow, cell::RefCell, cmp::Ordering, - collections::BTreeMap, path::{Path, PathBuf}, vec, }; @@ -25,8 +24,10 @@ use crate::{ diagnostic::{report_diagnostic, Diagnostic, WingSpan}, files::Files, type_check::{ - lifts::Lifts, resolve_super_method, symbol_env::SymbolEnv, ClassLike, Type, TypeRef, Types, VariableKind, - CLASS_INFLIGHT_INIT_NAME, + lifts::{Liftable, Lifts}, + resolve_super_method, resolve_user_defined_type, + symbol_env::SymbolEnv, + ClassLike, Type, TypeRef, Types, VariableKind, CLASS_INFLIGHT_INIT_NAME, }, visit_context::VisitContext, MACRO_REPLACE_ARGS, MACRO_REPLACE_ARGS_TEXT, MACRO_REPLACE_SELF, WINGSDK_ASSEMBLY_NAME, WINGSDK_RESOURCE, @@ -243,9 +244,8 @@ impl<'a> JSifier<'a> { property, optional_accessor, } => self.jsify_expression(object, ctx) + (if *optional_accessor { "?." } else { "." }) + &property.to_string(), - Reference::TypeReference(udt) => self.jsify_user_defined_type(&udt), - Reference::TypeMember { typeobject, property } => { - let typename = self.jsify_expression(typeobject, ctx); + Reference::TypeMember { type_name, property } => { + let typename = self.jsify_user_defined_type(type_name, ctx); typename + "." + &property.to_string() } } @@ -288,21 +288,21 @@ impl<'a> JSifier<'a> { } } - fn jsify_type(&self, typ: &TypeAnnotationKind) -> Option { + fn jsify_type(&self, typ: &TypeAnnotationKind, ctx: &mut JSifyContext) -> Option { match typ { - TypeAnnotationKind::UserDefined(t) => Some(self.jsify_user_defined_type(&t)), + TypeAnnotationKind::UserDefined(t) => Some(self.jsify_user_defined_type(&t, ctx)), TypeAnnotationKind::String => Some("string".to_string()), TypeAnnotationKind::Number => Some("number".to_string()), TypeAnnotationKind::Bool => Some("boolean".to_string()), TypeAnnotationKind::Array(t) => { - if let Some(inner) = self.jsify_type(&t.kind) { + if let Some(inner) = self.jsify_type(&t.kind, ctx) { Some(format!("{}[]", inner)) } else { None } } TypeAnnotationKind::Optional(t) => { - if let Some(inner) = self.jsify_type(&t.kind) { + if let Some(inner) = self.jsify_type(&t.kind, ctx) { Some(format!("{}?", inner)) } else { None @@ -312,17 +312,17 @@ impl<'a> JSifier<'a> { } } - fn jsify_struct_field_to_json_schema_type(&self, typ: &TypeAnnotationKind) -> String { + fn jsify_struct_field_to_json_schema_type(&self, typ: &TypeAnnotationKind, ctx: &mut JSifyContext) -> String { match typ { TypeAnnotationKind::Bool | TypeAnnotationKind::Number | TypeAnnotationKind::String => { - format!("type: \"{}\"", self.jsify_type(typ).unwrap()) + format!("type: \"{}\"", self.jsify_type(typ, ctx).unwrap()) } TypeAnnotationKind::UserDefined(udt) => { format!("\"$ref\": \"#/$defs/{}\"", udt.root.name) } TypeAnnotationKind::Json => "type: \"object\"".to_string(), TypeAnnotationKind::Map(t) => { - let map_type = self.jsify_type(&t.kind); + let map_type = self.jsify_type(&t.kind, ctx); // Ensure all keys are of some type format!( "type: \"object\", patternProperties: {{ \".*\": {{ type: \"{}\" }} }}", @@ -336,15 +336,22 @@ impl<'a> JSifier<'a> { TypeAnnotationKind::Set(_) => "uniqueItems: true,".to_string(), _ => "".to_string(), }, - self.jsify_struct_field_to_json_schema_type(&t.kind) + self.jsify_struct_field_to_json_schema_type(&t.kind, ctx) ) } - TypeAnnotationKind::Optional(t) => self.jsify_struct_field_to_json_schema_type(&t.kind), + TypeAnnotationKind::Optional(t) => self.jsify_struct_field_to_json_schema_type(&t.kind, ctx), _ => "type: \"null\"".to_string(), } } - fn jsify_user_defined_type(&self, udt: &UserDefinedType) -> String { + pub fn jsify_user_defined_type(&self, udt: &UserDefinedType, ctx: &mut JSifyContext) -> String { + if ctx.visit_ctx.current_phase() == Phase::Inflight { + if let Some(lifts) = &ctx.lifts { + if let Some(t) = lifts.token_for_liftable(&Liftable::Type(udt.clone())) { + return t.clone(); + } + } + } udt.full_path_str() } @@ -355,7 +362,7 @@ impl<'a> JSifier<'a> { // then emit the token instead of the expression. if ctx.visit_ctx.current_phase() == Phase::Inflight { if let Some(lifts) = &ctx.lifts { - if let Some(t) = lifts.token_for_expr(&expression.id) { + if let Some(t) = lifts.token_for_liftable(&Liftable::Expr(expression.id)) { return t.clone(); } } @@ -399,7 +406,7 @@ impl<'a> JSifier<'a> { // user-defined types), we simply instantiate the type directly (maybe in the future we will // allow customizations of user-defined types as well, but for now we don't). - let ctor = self.jsify_expression(class, ctx); + let ctor = self.jsify_user_defined_type(class, ctx); let scope = if is_preflight_class && class_type.std_construct_args { if let Some(scope) = obj_scope { @@ -654,7 +661,12 @@ impl<'a> JSifier<'a> { } } - pub fn jsify_struct_properties(&self, fields: &Vec, extends: &Vec) -> CodeMaker { + pub fn jsify_struct_properties( + &self, + fields: &Vec, + extends: &Vec, + ctx: &mut JSifyContext, + ) -> CodeMaker { let mut code = CodeMaker::default(); // Any parents we need to get their properties @@ -669,7 +681,7 @@ impl<'a> JSifier<'a> { code.line(format!( "{}: {{ {} }},", field.name.name, - self.jsify_struct_field_to_json_schema_type(&field.member_type.kind) + self.jsify_struct_field_to_json_schema_type(&field.member_type.kind, ctx) )); } @@ -682,6 +694,7 @@ impl<'a> JSifier<'a> { fields: &Vec, extends: &Vec, _env: &SymbolEnv, + ctx: &mut JSifyContext, ) -> CodeMaker { // To allow for struct validation at runtime this will generate a JS class that has a static // getValidator method that will create a json schema validator. @@ -701,7 +714,7 @@ impl<'a> JSifier<'a> { code.open("properties: {"); - code.add_code(self.jsify_struct_properties(fields, extends)); + code.add_code(self.jsify_struct_properties(fields, extends, ctx)); // determine which fields are required, and which schemas need to be added to validator for field in fields { @@ -844,7 +857,8 @@ impl<'a> JSifier<'a> { fn jsify_statement(&self, env: &SymbolEnv, statement: &Stmt, ctx: &mut JSifyContext) -> CodeMaker { CompilationContext::set(CompilationPhase::Jsifying, &statement.span); - match &statement.kind { + ctx.visit_ctx.push_stmt(statement.idx); + let code = match &statement.kind { StmtKind::Bring { source, identifier } => match source { BringSource::BuiltinModule(name) => CodeMaker::one_line(format!("const {} = {}.{};", name, STDLIB, name)), BringSource::JsiiModule(name) => CodeMaker::one_line(format!( @@ -879,11 +893,11 @@ impl<'a> JSifier<'a> { type_: _, } => { let initial_value = self.jsify_expression(initial_value, ctx); - return if *reassignable { + if *reassignable { CodeMaker::one_line(format!("let {var_name} = {initial_value};")) } else { CodeMaker::one_line(format!("const {var_name} = {initial_value};")) - }; + } } StmtKind::ForLoop { iterator, @@ -1031,7 +1045,7 @@ impl<'a> JSifier<'a> { CodeMaker::default() } StmtKind::Struct { name, fields, extends } => { - let mut code = self.jsify_struct(name, fields, extends, env); + let mut code = self.jsify_struct(name, fields, extends, env, ctx); // Emits struct class file self.emit_struct_file(name, code, ctx); @@ -1086,7 +1100,9 @@ impl<'a> JSifier<'a> { code } StmtKind::CompilerDebugEnv => CodeMaker::default(), - } + }; + ctx.visit_ctx.pop_stmt(); + code } fn jsify_enum(&self, values: &IndexSet) -> CodeMaker { @@ -1231,13 +1247,12 @@ impl<'a> JSifier<'a> { self.emit_inflight_file(&class, inflight_class_code, ctx); // lets write the code for the preflight side of the class + // TODO: why would we want to do this for inflight classes?? maybe return here in that case? let mut code = CodeMaker::default(); // default base class for preflight classes is `core.Resource` let extends = if let Some(parent) = &class.parent { - let base = parent.as_type_reference().expect("resolve parent type"); - - format!(" extends {}", base) + format!(" extends {}", self.jsify_user_defined_type(parent, ctx)) } else { format!(" extends {}", STDLIB_CORE_RESOURCE) }; @@ -1337,10 +1352,9 @@ impl<'a> JSifier<'a> { code.open(format!("require(\"{client_path}\")({{")); if let Some(lifts) = &ctx.lifts { - for capture in lifts.captures() { - let preflight = capture.code.clone(); - let lift_type = format!("context._lift({})", preflight); - code.line(format!("{}: ${{{}}},", capture.token, lift_type)); + for (token, capture) in lifts.captures.iter().filter(|(_, cap)| !cap.is_field) { + let lift_type = format!("context._lift({})", capture.code); + code.line(format!("{}: ${{{}}},", token, lift_type)); } } @@ -1399,7 +1413,7 @@ impl<'a> JSifier<'a> { class_code.open(format!( "class {name}{} {{", if let Some(parent) = &class.parent { - format!(" extends {}", self.jsify_expression(&parent, &mut ctx)) + format!(" extends {}", self.jsify_user_defined_type(&parent, ctx)) } else { "".to_string() } @@ -1444,7 +1458,11 @@ impl<'a> JSifier<'a> { let mut code = CodeMaker::default(); let inputs = if let Some(lifts) = &ctx.lifts { - lifts.captures().iter().map(|c| c.token.clone()).join(", ") + lifts + .captures + .iter() + .filter_map(|(token, cap)| if !cap.is_field { Some(token) } else { None }) + .join(", ") } else { Default::default() }; @@ -1475,7 +1493,12 @@ impl<'a> JSifier<'a> { }; let parent_fields = if let Some(parent) = &class.parent { - let parent_type = self.types.get_expr_type(parent); + let parent_type = resolve_user_defined_type( + parent, + ctx.visit_ctx.current_env().expect("an env"), + ctx.visit_ctx.current_stmt_idx(), + ) + .expect("resolved type"); if let Some(parent_lifts) = &parent_type.as_class().unwrap().lifts { parent_lifts.lifted_fields().keys().map(|f| f.clone()).collect_vec() } else { @@ -1532,13 +1555,12 @@ impl<'a> JSifier<'a> { let class_name = class.name.to_string(); - let lifts_per_method = if let Some(lifts) = &ctx.lifts { - lifts.lifts_per_method() - } else { - BTreeMap::default() + let Some(lifts) = ctx.lifts else { + return bind_method; }; - let lifts = lifts_per_method + let lift_qualifications = lifts + .lifts_qualifications .iter() .filter(|(m, _)| { let var_kind = &class_type @@ -1554,19 +1576,18 @@ impl<'a> JSifier<'a> { .collect_vec(); // Skip jsifying this method if there are no lifts (in this case we'll use super's register bind method) - if lifts.is_empty() { + if lift_qualifications.is_empty() { return bind_method; } bind_method.open(format!("{modifier}{bind_method_name}(host, ops) {{")); - for (method_name, method_lifts) in lifts { + for (method_name, method_qual) in lift_qualifications { bind_method.open(format!("if (ops.includes(\"{method_name}\")) {{")); - for lift in method_lifts { - let ops_strings = lift.ops.iter().map(|op| format!("\"{}\"", op)).join(", "); - let field = lift.code.clone(); + for (code, method_lift_qual) in method_qual { + let ops_strings = method_lift_qual.ops.iter().map(|op| format!("\"{}\"", op)).join(", "); bind_method.line(format!( - "{class_name}._registerBindObject({field}, host, [{ops_strings}]);", + "{class_name}._registerBindObject({code}, host, [{ops_strings}]);", )); } bind_method.close("}"); diff --git a/libs/wingc/src/jsify/snapshots/fails_when_inflight_class_tries_to_extend_preflight_class.snap b/libs/wingc/src/jsify/snapshots/fails_when_inflight_class_tries_to_extend_preflight_class.snap index 67184370974..e8403443f54 100644 --- a/libs/wingc/src/jsify/snapshots/fails_when_inflight_class_tries_to_extend_preflight_class.snap +++ b/libs/wingc/src/jsify/snapshots/fails_when_inflight_class_tries_to_extend_preflight_class.snap @@ -3,3 +3,4 @@ source: libs/wingc/src/jsify/tests.rs --- ## Errors Class "Derived" is an inflight class and cannot extend preflight class "Base" 2:35 +Cannot qualify access to a lifted type "Base" (see https://github.com/winglang/wing/issues/76 for more details) 2:35 diff --git a/libs/wingc/src/jsify/snapshots/lift_self_reference.snap b/libs/wingc/src/jsify/snapshots/lift_self_reference.snap new file mode 100644 index 00000000000..ab70ce1363f --- /dev/null +++ b/libs/wingc/src/jsify/snapshots/lift_self_reference.snap @@ -0,0 +1,82 @@ +--- +source: libs/wingc/src/jsify/tests.rs +--- +## Code + +```w + + class Foo { + static inflight static_method() {} + + inflight bar() { + Foo.static_method(); + } + } + +``` + +## inflight.Foo-1.js + +```js +module.exports = function({ }) { + class Foo { + constructor({ }) { + } + static async static_method() { + } + async bar() { + (await Foo.static_method()); + } + } + return Foo; +} +``` + +## preflight.js + +```js +const $stdlib = require('@winglang/sdk'); +const $plugins = ((s) => !s ? [] : s.split(';'))(process.env.WING_PLUGIN_PATHS); +const $outdir = process.env.WING_SYNTH_DIR ?? "."; +const $wing_is_test = process.env.WING_IS_TEST === "true"; +const std = $stdlib.std; +class $Root extends $stdlib.std.Resource { + constructor(scope, id) { + super(scope, id); + class Foo extends $stdlib.std.Resource { + constructor(scope, id, ) { + super(scope, id); + } + static _toInflightType(context) { + return ` + require("./inflight.Foo-1.js")({ + }) + `; + } + _toInflight() { + return ` + (await (async () => { + const FooClient = ${Foo._toInflightType(this)}; + const client = new FooClient({ + }); + if (client.$inflight_init) { await client.$inflight_init(); } + return client; + })()) + `; + } + _getInflightOps() { + return ["static_method", "bar", "$inflight_init"]; + } + _registerBind(host, ops) { + if (ops.includes("bar")) { + Foo._registerBindObject(Foo, host, ["static_method"]); + } + super._registerBind(host, ops); + } + } + } +} +const $App = $stdlib.core.App.for(process.env.WING_TARGET); +new $App({ outdir: $outdir, name: "main", rootConstruct: $Root, plugins: $plugins, isTestEnvironment: $wing_is_test, entrypointDir: process.env['WING_SOURCE_DIR'], rootId: process.env['WING_ROOT_ID'] }).synth(); +``` + diff --git a/libs/wingc/src/jsify/tests.rs b/libs/wingc/src/jsify/tests.rs index 4d7d6bc1991..bd496b91158 100644 --- a/libs/wingc/src/jsify/tests.rs +++ b/libs/wingc/src/jsify/tests.rs @@ -1992,3 +1992,18 @@ fn implicit_lift_inflight_init() { "# ); } + +#[test] +fn lift_self_reference() { + assert_compile_ok!( + r#" + class Foo { + static inflight static_method() {} + + inflight bar() { + Foo.static_method(); + } + } + "# + ); +} diff --git a/libs/wingc/src/lifting.rs b/libs/wingc/src/lifting.rs index 69cfce91b93..5e131ab4bc3 100644 --- a/libs/wingc/src/lifting.rs +++ b/libs/wingc/src/lifting.rs @@ -1,13 +1,18 @@ use crate::{ - ast::{Class, Expr, ExprKind, FunctionBody, FunctionDefinition, Phase, Reference, Scope, Stmt, UserDefinedType}, + ast::{ + Class, Expr, ExprKind, FunctionBody, FunctionDefinition, Phase, Reference, Scope, Stmt, Symbol, UserDefinedType, + }, comp_ctx::{CompilationContext, CompilationPhase}, - diagnostic::{report_diagnostic, Diagnostic, WingSpan}, + diagnostic::{report_diagnostic, Diagnostic}, jsify::{JSifier, JSifyContext}, type_check::{ - lifts::Lifts, resolve_user_defined_type, symbol_env::LookupResult, TypeRef, CLOSURE_CLASS_HANDLE_METHOD, + lifts::{Liftable, Lifts}, + resolve_user_defined_type, + symbol_env::LookupResult, + ClassLike, TypeRef, CLOSURE_CLASS_HANDLE_METHOD, }, visit::{self, Visit}, - visit_context::VisitContext, + visit_context::{VisitContext, VisitorWithContext}, }; pub struct LiftVisitor<'a> { @@ -25,69 +30,47 @@ impl<'a> LiftVisitor<'a> { } } - fn is_self_type_reference(&self, node: &Expr) -> bool { + fn is_self_type_reference(&self, udt: &UserDefinedType) -> bool { let Some(current_class_udt) = self.ctx.current_class() else { return false; }; - let Some(udt) = node.as_type_reference() else { - return false; - }; - udt.full_path_str() == current_class_udt.full_path_str() } - fn is_defined_in_current_env(&self, fullname: &str, span: &WingSpan) -> bool { - // if the symbol is defined later in the current environment, it means we can't capture a - // reference to a symbol with the same name from a parent so bail out. - // notice that here we are looking in the current environment and not in the method's environment - if let Some(env) = self.ctx.current_env() { - let lookup = env.lookup_nested_str(&fullname, Some(self.ctx.current_stmt_idx())); - - match lookup { - LookupResult::Found(_, e) => { - // if we found the symbol in the current environment, it means we don't need to capture it at all - if e.env.is_same(env) { - return true; - } - } - LookupResult::DefinedLater => { - report_diagnostic(Diagnostic { - span: Some(span.clone()), - message: format!( - "Cannot capture symbol \"{fullname}\" because it is shadowed by another symbol with the same name" - ), - }); - return true; - } - LookupResult::NotFound(_) | LookupResult::ExpectedNamespace(_) => {} + fn verify_defined_in_current_env(&mut self, symbol: &Symbol) { + // Skip this check if we're in an "unresolved" expression (e.g. type errors) to avoid cascading errors + if let Some(expr_id) = self.ctx().current_expr() { + if self.jsify.types.get_expr_id_type(expr_id).is_unresolved() { + return; } } - return false; + // If the symbol is defined later in the current environment, it means we can't reference an identicatl symbol + // from an outter scope (and we can't capture it either). In that case we'll report and error. + // Note: + // In theory this can be supported, but it'll fail later on when running the JS code because of how + // JS creates all symbols *before* running the code of the current scope (https://tc39.es/ecma262/#sec-let-and-const-declarations). + // Solving this will require renaming the symbols in the current scope to avoid the conflict, so the east way is just to adopt + // the JS limitation. Will be good to improve on this in the future. + if let Some(env) = self.ctx.current_env() { + if matches!( + env.lookup_ext(symbol, Some(self.ctx.current_stmt_idx())), + LookupResult::DefinedLater + ) { + report_diagnostic(Diagnostic { + span: Some(symbol.span.clone()), + message: format!("Cannot access \"{symbol}\" because it is shadowed by another symbol with the same name"), + }); + } + } } - fn should_capture_expr(&self, node: &Expr) -> bool { - let ExprKind::Reference(ref r) = node.kind else { - return false; - }; - - let (fullname, span) = match r { - Reference::Identifier(ref symb) => (symb.name.clone(), symb.span.clone()), - Reference::TypeReference(ref t) => (t.full_path_str(), t.span.clone()), - _ => return false, - }; + fn should_capture_type(&self, node: &UserDefinedType) -> bool { + let fullname = node.full_path_str(); // skip "This" (which is the type of "this") - if let Some(class) = &self.ctx.current_class() { - if class.full_path_str() == fullname { - return false; - } - } - - // if the symbol is defined in the *current* environment (which could also be a nested scope - // inside the current method), we don't need to capture it - if self.is_defined_in_current_env(&fullname, &span) { + if self.is_self_type_reference(node) { return false; } @@ -126,8 +109,8 @@ impl<'a> LiftVisitor<'a> { return true; } - fn jsify_expr(&mut self, node: &Expr, phase: Phase) -> String { - self.ctx.push_phase(phase); + fn jsify_expr(&mut self, node: &Expr) -> String { + self.ctx.push_phase(Phase::Preflight); let res = self.jsify.jsify_expression( &node, &mut JSifyContext { @@ -138,101 +121,176 @@ impl<'a> LiftVisitor<'a> { self.ctx.pop_phase(); res } + + fn jsify_udt(&mut self, node: &UserDefinedType) -> String { + let res = self.jsify.jsify_user_defined_type( + &node, + &mut JSifyContext { + lifts: None, + visit_ctx: &mut self.ctx, + }, + ); + res + } } impl<'a> Visit<'a> for LiftVisitor<'a> { fn visit_reference(&mut self, node: &'a Reference) { match node { Reference::InstanceMember { property, .. } => { - self.ctx.push_property(property.name.clone()); + self.ctx.push_property(property); visit::visit_reference(self, &node); self.ctx.pop_property(); } Reference::TypeMember { property, .. } => { - self.ctx.push_property(property.name.clone()); + self.ctx.push_property(property); visit::visit_reference(self, &node); self.ctx.pop_property(); } - _ => visit::visit_reference(self, &node), + Reference::Identifier(symb) => { + self.verify_defined_in_current_env(symb); + visit::visit_reference(self, &node); + } } } + fn visit_type_annotation(&mut self, node: &'a crate::ast::TypeAnnotation) { + self.ctx.push_type_annotation(); + visit::visit_type_annotation(self, node); + self.ctx.pop_type_annotation(); + } + fn visit_expr(&mut self, node: &'a Expr) { CompilationContext::set(CompilationPhase::Lifting, &node.span); + self.with_expr(node.id, |v| { + let expr_phase = self.jsify.types.get_expr_phase(&node).unwrap(); + let expr_type = v.jsify.types.get_expr_type(&node); - let expr_phase = self.jsify.types.get_expr_phase(&node).unwrap(); - let expr_type = self.jsify.types.get_expr_type(&node); + // Skip expressions of an unresoved type (type errors) + if expr_type.is_unresolved() { + visit::visit_expr(v, node); + return; + } + + // this whole thing only applies to inflight code + if v.ctx.current_phase() == Phase::Preflight { + visit::visit_expr(v, node); + return; + } + + //--------------- + // LIFT + if expr_phase == Phase::Preflight { + // jsify the expression so we can get the preflight code + let code = v.jsify_expr(&node); + + let property = if let Some(property) = v.ctx.current_property() { + Some(property) + } else if expr_type.is_closure() { + // this is the case where we are lifting a "closure class" (e.g. a class that has a "handle" + // method) the reason we might not have "property" set is because closure classes might be + // syntheticaly generated by the compiler from closures. + Some(Symbol::global(CLOSURE_CLASS_HANDLE_METHOD)) + } else { + None + }; + + // check that we can qualify the lift (e.g. determine which property is being accessed) + if property.is_none() && expr_type.is_preflight_class() { + report_diagnostic(Diagnostic { + message: format!( + "Cannot qualify access to a lifted object of type \"{}\" (see https://github.com/winglang/wing/issues/76 for more details)", + expr_type.to_string() + ), + span: Some(node.span.clone()), + }); + + return; + } + + // if this is an inflight field of "this" no need to lift it + if is_inflight_field(&node, expr_type, &property) { + return; + } + + let mut lifts = v.lifts_stack.pop().unwrap(); + let is_field = code.contains("this."); // TODO: starts_with? + lifts.lift(v.ctx.current_method(), property, &code, is_field); + lifts.capture(&Liftable::Expr(node.id), &code, is_field); + v.lifts_stack.push(lifts); + return; + } + + visit::visit_expr(v, node); + }); + } + + fn visit_user_defined_type(&mut self, node: &'a UserDefinedType) { + // If we're inside a type annotation we currently don't need to capture type since our target compilation + // is typeless (javascript). For typed targes we may need to also capture the types used in annotations. + if self.ctx.in_type_annotation() { + visit::visit_user_defined_type(self, node); + return; + } - // this whole thing only applies to inflight expressions + // this whole thing only applies to inflight code if self.ctx.current_phase() == Phase::Preflight { - visit::visit_expr(self, node); + visit::visit_user_defined_type(self, node); return; } - // if this expression represents the current class, no need to capture it (it is by definition - // available in the current scope) - if self.is_self_type_reference(&node) { - visit::visit_expr(self, node); + // Get the type of the udt + let udt_type = resolve_user_defined_type( + node, + self.ctx.current_env().expect("an env"), + self.ctx.current_stmt_idx(), + ) + .unwrap_or(self.jsify.types.error()); + + // Since our target languages is isn't statically typed, we don't need to capture interfaces + if udt_type.as_interface().is_some() { + visit::visit_user_defined_type(self, node); return; } //--------------- // LIFT - if expr_phase == Phase::Preflight { + if udt_type.is_preflight_class() { // jsify the expression so we can get the preflight code - let code = self.jsify_expr(&node, Phase::Preflight); - - let property = if let Some(property) = self.ctx.current_property() { - Some(property) - } else if expr_type.is_closure() { - // this is the case where we are lifting a "closure class" (e.g. a class that has a "handle" - // method) the reason we might not have "property" set is because closure classes might be - // syntheticaly generated by the compiler from closures. - Some(CLOSURE_CLASS_HANDLE_METHOD.to_string()) - } else { - None - }; + let code = self.jsify_udt(&node); + + let property = self.ctx.current_property(); // check that we can qualify the lift (e.g. determine which property is being accessed) - if property.is_none() && expr_type.as_preflight_class().is_some() { + if property.is_none() { report_diagnostic(Diagnostic { message: format!( - "Cannot qualify access to a lifted object of type \"{}\" (see https://github.com/winglang/wing/issues/76 for more details)", - expr_type.to_string() - ), + "Cannot qualify access to a lifted type \"{udt_type}\" (see https://github.com/winglang/wing/issues/76 for more details)"), span: Some(node.span.clone()), }); return; } - // if this is an inflight property, no need to lift it - if is_inflight_field(&node, expr_type, &property) { - return; - } - let mut lifts = self.lifts_stack.pop().unwrap(); - lifts.lift(node.id, self.ctx.current_method(), property, &code); + lifts.lift(self.ctx.current_method(), property, &code, false); self.lifts_stack.push(lifts); - - return; } //--------------- // CAPTURE - - if self.should_capture_expr(&node) { - // jsify the expression so we can get the preflight code - let code = self.jsify_expr(&node, Phase::Inflight); + if self.should_capture_type(&node) { + // jsify the type so we can get the preflight code + let code = self.jsify_udt(&node); let mut lifts = self.lifts_stack.pop().unwrap(); - lifts.capture(&node.id, &code); + lifts.capture(&Liftable::Type(node.clone()), &code, false); self.lifts_stack.push(lifts); return; } - visit::visit_expr(self, node); + visit::visit_user_defined_type(self, node); } // State Tracking @@ -257,6 +315,10 @@ impl<'a> Visit<'a> for LiftVisitor<'a> { // nothing to do if we are emitting an inflight class from within an inflight scope if self.ctx.current_phase() == Phase::Inflight && node.phase == Phase::Inflight { self.visit_symbol(&node.name); + + visit::visit_function_definition(self, &node.initializer); + visit::visit_function_definition(self, &node.inflight_initializer); + for field in node.fields.iter() { self.visit_symbol(&field.name); self.visit_type_annotation(&field.member_type); @@ -265,14 +327,12 @@ impl<'a> Visit<'a> for LiftVisitor<'a> { self.visit_symbol(&name); visit::visit_function_definition(self, &def); } - visit::visit_function_definition(self, &node.initializer); if let Some(parent) = &node.parent { - self.visit_expr(&parent); + self.visit_user_defined_type(&parent); } for interface in node.implements.iter() { self.visit_user_defined_type(&interface); } - visit::visit_function_definition(self, &node.inflight_initializer); return; } @@ -297,7 +357,7 @@ impl<'a> Visit<'a> for LiftVisitor<'a> { if let Some(parent) = &node.parent { let mut lifts = self.lifts_stack.pop().unwrap(); - lifts.capture(&parent.id, &self.jsify_expr(&parent, Phase::Inflight)); + lifts.capture(&Liftable::Type(parent.clone()), &self.jsify_udt(&parent), false); self.lifts_stack.push(lifts); } @@ -332,17 +392,13 @@ impl<'a> Visit<'a> for LiftVisitor<'a> { /// Check if an expression is a reference to an inflight field (`this.`). /// in this case, we don't need to lift the field because it is already available -fn is_inflight_field(expr: &Expr, expr_type: TypeRef, property: &Option) -> bool { +fn is_inflight_field(expr: &Expr, expr_type: TypeRef, property: &Option) -> bool { if let ExprKind::Reference(Reference::Identifier(symb)) = &expr.kind { if symb.name == "this" { if let (Some(cls), Some(property)) = (expr_type.as_preflight_class(), property) { - if let LookupResult::Found(kind, _) = cls.env.lookup_nested_str(&property, None) { - if let Some(var) = kind.as_variable() { - if !var.type_.is_closure() { - if var.phase != Phase::Preflight { - return true; - } - } + if let Some(var) = cls.get_field(&property) { + if var.phase != Phase::Preflight { + return true; } } } @@ -351,3 +407,9 @@ fn is_inflight_field(expr: &Expr, expr_type: TypeRef, property: &Option) return false; } + +impl VisitorWithContext for LiftVisitor<'_> { + fn ctx(&mut self) -> &mut VisitContext { + &mut self.ctx + } +} diff --git a/libs/wingc/src/lsp/completions.rs b/libs/wingc/src/lsp/completions.rs index 7d09bd0ac70..4b86d819fdd 100644 --- a/libs/wingc/src/lsp/completions.rs +++ b/libs/wingc/src/lsp/completions.rs @@ -16,7 +16,7 @@ use crate::lsp::sync::{JSII_TYPES, PROJECT_DATA, WING_TYPES}; use crate::type_check::symbol_env::{LookupResult, StatementIdx}; use crate::type_check::{ fully_qualify_std_type, import_udt_from_jsii, resolve_super_method, resolve_user_defined_type, ClassLike, Namespace, - Struct, SymbolKind, Type, Types, UnsafeRef, VariableKind, CLASS_INFLIGHT_INIT_NAME, CLASS_INIT_NAME, + Struct, SymbolKind, Type, TypeRef, Types, UnsafeRef, VariableKind, CLASS_INFLIGHT_INIT_NAME, CLASS_INIT_NAME, }; use crate::visit::{visit_expr, visit_type_annotation, Visit}; use crate::wasm_util::{ptr_to_string, string_to_combined_ptr, WASM_RETURN_ERROR}; @@ -309,7 +309,7 @@ pub fn on_completion(params: lsp_types::CompletionParams) -> CompletionResponse }, arg_list, )), - ExprKind::New(new_expr) => Some((types.get_expr_type(&new_expr.class), &new_expr.arg_list)), + ExprKind::New(new_expr) => Some((types.get_expr_type(&e), &new_expr.arg_list)), _ => None, }) { let mut completions = @@ -646,7 +646,7 @@ fn get_inner_struct_completions(struct_: &Struct, existing_fields: &Vec) /// Gets accessible properties on a type as a list of CompletionItems fn get_completions_from_type( - type_: &UnsafeRef, + type_: &TypeRef, types: &Types, current_phase: Option, is_instance: bool, diff --git a/libs/wingc/src/lsp/hover.rs b/libs/wingc/src/lsp/hover.rs index b41df8cc23a..b6cf333bf26 100644 --- a/libs/wingc/src/lsp/hover.rs +++ b/libs/wingc/src/lsp/hover.rs @@ -1,12 +1,15 @@ use crate::ast::{ CalleeKind, Class, Expr, ExprKind, FunctionBody, FunctionDefinition, Phase, Reference, Scope, Stmt, StmtKind, Symbol, - TypeAnnotation, TypeAnnotationKind, + UserDefinedType, }; use crate::diagnostic::WingSpan; use crate::docs::Documented; use crate::lsp::sync::PROJECT_DATA; use crate::type_check::symbol_env::LookupResult; -use crate::type_check::{resolve_super_method, ClassLike, Type, Types, CLASS_INFLIGHT_INIT_NAME, CLASS_INIT_NAME}; +use crate::type_check::{ + resolve_super_method, resolve_user_defined_type, ClassLike, Type, TypeRef, Types, CLASS_INFLIGHT_INIT_NAME, + CLASS_INIT_NAME, +}; use crate::visit::{self, Visit}; use crate::wasm_util::WASM_RETURN_ERROR; use crate::wasm_util::{ptr_to_string, string_to_combined_ptr}; @@ -74,12 +77,7 @@ impl<'a> HoverVisitor<'a> { } } - fn visit_reference_with_member(&mut self, object: &'a Expr, property: &'a Symbol) { - if object.span.contains(&self.position) { - self.visit_expr(object); - return; - } - let obj_type = self.types.get_expr_type(object); + fn visit_type_with_member(&mut self, obj_type: TypeRef, property: &'a Symbol) { if property.span.contains(&self.position) { let new_span = self.current_expr.unwrap().span.clone(); match &**obj_type.maybe_unwrap_option() { @@ -204,20 +202,6 @@ impl<'a> Visit<'a> for HoverVisitor<'a> { }); } - fn visit_type_annotation(&mut self, node: &'a TypeAnnotation) { - if self.found.is_some() { - return; - } - - if let TypeAnnotationKind::UserDefined(t) = &node.kind { - if t.span.contains(&self.position) { - self.found = Some((t.span.clone(), self.lookup_docs(&t.full_path_str(), None))); - } - } - - visit::visit_type_annotation(self, node); - } - fn visit_symbol(&mut self, node: &'a Symbol) { if self.found.is_some() { return; @@ -371,6 +355,26 @@ impl<'a> Visit<'a> for HoverVisitor<'a> { } } + fn visit_user_defined_type(&mut self, node: &'a UserDefinedType) { + if self.found.is_some() { + return; + } + + if node.span.contains(&self.position) { + // Only lookup string up to the position + let mut partial_path = vec![]; + node.full_path().iter().for_each(|p| { + if p.span.start <= self.position.into() { + partial_path.push(p.name.clone()); + } + }); + let lookup_str = partial_path.join("."); + self.found = Some((node.span.clone(), self.lookup_docs(&lookup_str, None))); + } + + visit::visit_user_defined_type(self, node); + } + fn visit_reference(&mut self, node: &'a Reference) { if self.found.is_some() { return; @@ -382,23 +386,29 @@ impl<'a> Visit<'a> for HoverVisitor<'a> { self.found = Some((sym.span.clone(), self.lookup_docs(&sym.name, None))); } } - Reference::TypeReference(t) => { - if t.span.contains(&self.position) { - // Only lookup string up to the position - let mut partial_path = vec![]; - t.full_path().iter().for_each(|p| { - if p.span.start <= self.position.into() { - partial_path.push(p.name.clone()); - } - }); - let lookup_str = partial_path.join("."); - self.found = Some((t.span.clone(), self.lookup_docs(&lookup_str, None))); + Reference::InstanceMember { object, property, .. } => { + if object.span.contains(&self.position) { + self.visit_expr(object) + } else { + self.visit_type_with_member(self.types.get_expr_type(object), property) + } + } + Reference::TypeMember { type_name, property } => { + if type_name.span.contains(&self.position) { + self.visit_user_defined_type(type_name) + } else { + self.visit_type_with_member( + resolve_user_defined_type( + type_name, + &self.types.get_scope_env(self.current_scope), + self.current_statement_index, + ) + .unwrap_or(self.types.error()), + property, + ) } } - Reference::InstanceMember { object, property, .. } => self.visit_reference_with_member(object, property), - Reference::TypeMember { typeobject, property } => self.visit_reference_with_member(&typeobject, property), } - visit::visit_reference(self, node); } } @@ -602,11 +612,42 @@ new cloud.Bucket(); test_hover_list!( user_defined_types, r#" -class Foo { }; +class Foo { } //^ "# ); + test_hover_list!( + user_defined_type_annotation, + r#" +class Foo { } +let a: Foo = new Foo(); + //^ +"# + ); + + test_hover_list!( + user_defined_type_reference_property, + r#" +class Foo { + static static_method() { } +} +Foo.static_method(); + //^ +"# + ); + + test_hover_list!( + user_defined_type_reference_type, + r#" +class Foo { + static static_method() { } +} +Foo.static_method(); +//^ +"# + ); + test_hover_list!( static_method, r#" diff --git a/libs/wingc/src/lsp/signature.rs b/libs/wingc/src/lsp/signature.rs index 736a8cafedd..22ee19e6656 100644 --- a/libs/wingc/src/lsp/signature.rs +++ b/libs/wingc/src/lsp/signature.rs @@ -48,11 +48,8 @@ pub fn on_signature_help(params: lsp_types::SignatureHelpParams) -> Option { let NewExpr { class, arg_list, .. } = new_expr; - let Some(udt) = class.as_type_reference() else { - return None; - }; - let Some(t) = resolve_user_defined_type(&udt, &types.get_scope_env(&root_scope), 0).ok() else { + let Some(t) = resolve_user_defined_type(class, &types.get_scope_env(&root_scope), 0).ok() else { return None; }; diff --git a/libs/wingc/src/lsp/snapshots/hovers/static_stdtype_method.snap b/libs/wingc/src/lsp/snapshots/hovers/static_stdtype_method.snap index d457f74d5ed..98cf7181825 100644 --- a/libs/wingc/src/lsp/snapshots/hovers/static_stdtype_method.snap +++ b/libs/wingc/src/lsp/snapshots/hovers/static_stdtype_method.snap @@ -10,5 +10,5 @@ range: character: 0 end: line: 1 - character: 14 + character: 4 diff --git a/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap b/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap new file mode 100644 index 00000000000..07461b084da --- /dev/null +++ b/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_annotation.snap @@ -0,0 +1,14 @@ +--- +source: libs/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\nclass Foo\n```\n---" +range: + start: + line: 2 + character: 7 + end: + line: 2 + character: 10 + diff --git a/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap b/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap new file mode 100644 index 00000000000..fe9305daba1 --- /dev/null +++ b/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_property.snap @@ -0,0 +1,14 @@ +--- +source: libs/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\nstatic preflight static_method: preflight (): void\n```" +range: + start: + line: 4 + character: 0 + end: + line: 4 + character: 17 + diff --git a/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap b/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap new file mode 100644 index 00000000000..c081d40d46b --- /dev/null +++ b/libs/wingc/src/lsp/snapshots/hovers/user_defined_type_reference_type.snap @@ -0,0 +1,14 @@ +--- +source: libs/wingc/src/lsp/hover.rs +--- +contents: + kind: markdown + value: "```wing\nclass Foo\n```\n---" +range: + start: + line: 4 + character: 0 + end: + line: 4 + character: 3 + diff --git a/libs/wingc/src/parser.rs b/libs/wingc/src/parser.rs index d509522d234..9d038ae6c72 100644 --- a/libs/wingc/src/parser.rs +++ b/libs/wingc/src/parser.rs @@ -1015,10 +1015,7 @@ impl<'s> Parser<'s> { let parent = if let Some(parent_node) = statement_node.child_by_field_name("parent") { let parent_type = self.build_type_annotation(Some(parent_node), class_phase)?; match parent_type.kind { - TypeAnnotationKind::UserDefined(parent_type) => Some(Expr::new( - ExprKind::Reference(Reference::TypeReference(parent_type)), - self.node_span(&parent_node), - )), + TypeAnnotationKind::UserDefined(parent_type) => Some(parent_type), _ => { self.with_error::( format!("Parent type must be a user defined type, found {}", parent_type), @@ -1411,37 +1408,34 @@ impl<'s> Parser<'s> { let object_expr = self.get_child_field(nested_node, "object")?; if let Some(property) = nested_node.child_by_field_name("property") { - let object_expr = if object_expr.kind() == "json_container_type" { - Expr::new( + if object_expr.kind() == "json_container_type" { + Ok(Expr::new( ExprKind::Reference(Reference::TypeMember { - typeobject: Box::new( - UserDefinedType { - root: Symbol::global(WINGSDK_STD_MODULE), - fields: vec![self.node_symbol(&object_expr)?], - span: self.node_span(&object_expr), - } - .to_expression(), - ), + type_name: UserDefinedType { + root: Symbol::global(WINGSDK_STD_MODULE), + fields: vec![self.node_symbol(&object_expr)?], + span: self.node_span(&object_expr), + }, property: self.node_symbol(&property)?, }), self.node_span(&object_expr), - ) + )) } else { - self.build_expression(&object_expr, phase)? - }; - let accessor_sym = self.node_symbol(&self.get_child_field(nested_node, "accessor_type")?)?; - let optional_accessor = match accessor_sym.name.as_str() { - "?." => true, - _ => false, - }; - Ok(Expr::new( - ExprKind::Reference(Reference::InstanceMember { - object: Box::new(object_expr), - property: self.node_symbol(&property)?, - optional_accessor, - }), - self.node_span(&nested_node), - )) + let object_expr = self.build_expression(&object_expr, phase)?; + let accessor_sym = self.node_symbol(&self.get_child_field(nested_node, "accessor_type")?)?; + let optional_accessor = match accessor_sym.name.as_str() { + "?." => true, + _ => false, + }; + Ok(Expr::new( + ExprKind::Reference(Reference::InstanceMember { + object: Box::new(object_expr), + property: self.node_symbol(&property)?, + optional_accessor, + }), + self.node_span(&nested_node), + )) + } } else { // we are missing the last property, but we can still parse the rest of the expression self.add_error( @@ -1544,10 +1538,6 @@ impl<'s> Parser<'s> { match expression_node.kind() { "new_expression" => { let class_udt = self.build_udt(&expression_node.child_by_field_name("class").unwrap())?; - let class_udt_exp = Expr::new( - ExprKind::Reference(Reference::TypeReference(class_udt)), - expression_span.clone(), - ); let arg_list = if let Ok(args_node) = self.get_child_field(expression_node, "args") { self.build_arg_list(&args_node, phase) @@ -1568,7 +1558,7 @@ impl<'s> Parser<'s> { Ok(Expr::new( ExprKind::New(NewExpr { - class: Box::new(class_udt_exp), + class: class_udt, obj_id, arg_list: arg_list?, obj_scope, @@ -2098,14 +2088,11 @@ impl<'s> Parser<'s> { let type_span = self.node_span(&statement_node.child(0).unwrap()); Ok(StmtKind::Expression(Expr::new( ExprKind::New(NewExpr { - class: Box::new(Expr::new( - ExprKind::Reference(Reference::TypeReference(UserDefinedType { - root: Symbol::global(WINGSDK_STD_MODULE), - fields: vec![Symbol::global(WINGSDK_TEST_CLASS_NAME)], - span: type_span.clone(), - })), - type_span.clone(), - )), + class: UserDefinedType { + root: Symbol::global(WINGSDK_STD_MODULE), + fields: vec![Symbol::global(WINGSDK_TEST_CLASS_NAME)], + span: type_span.clone(), + }, obj_id: Some(test_id), obj_scope: None, arg_list: ArgList { diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index 3108b103ee0..4f77f87ffde 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -91,12 +91,6 @@ pub enum VariableKind { /// a class member (or an enum member) StaticMember, - /// a type (e.g. `std.Json`) - Type, - - /// a namespace (e.g. `cloud`) - Namespace, - /// an error placeholder Error, } @@ -388,7 +382,7 @@ pub trait ClassLike { .0 .as_variable() .expect("class env should only contain variables"); - if v.type_.as_function_sig().is_some() { + if v.type_.is_closure() { Some(v) } else { None @@ -396,7 +390,18 @@ pub trait ClassLike { } fn get_field(&self, name: &Symbol) -> Option<&VariableInfo> { - self.get_env().lookup_ext(name, None).ok()?.0.as_variable() + let v = self + .get_env() + .lookup_ext(name, None) + .ok()? + .0 + .as_variable() + .expect("class env should only contain variables"); + if !v.type_.is_closure() { + Some(v) + } else { + None + } } } @@ -1499,8 +1504,13 @@ impl Types { /// Obtain the type of a given expression node. Will panic if the expression has not been type checked yet. pub fn get_expr_type(&self, expr: &Expr) -> TypeRef { + self.get_expr_id_type(expr.id) + } + + /// Obtain the type of a given expression id. Will panic if the expression has not been type checked yet. + pub fn get_expr_id_type(&self, expr_id: ExprId) -> TypeRef { self - .try_get_expr_type(expr) + .try_get_expr_type(expr_id) .expect("All expressions should have a type") } @@ -1527,12 +1537,12 @@ impl Types { } } - /// Obtain the type of a given expression node. Returns None if the expression has not been type checked yet. If + /// Obtain the type of a given expression id. Returns None if the expression has not been type checked yet. If /// this is called after type checking, it should always return Some. - pub fn try_get_expr_type(&self, expr: &Expr) -> Option { + pub fn try_get_expr_type(&self, expr_id: ExprId) -> Option { self .type_for_expr - .get(expr.id) + .get(expr_id) .and_then(|t| t.as_ref().map(|t| t.type_)) } @@ -1856,21 +1866,13 @@ impl<'a> TypeChecker<'a> { obj_scope, } = new_expr; // Type check everything - let class_type = self.type_check_exp(&class, env).0; + let class_type = self + .resolve_user_defined_type(class, env, self.statement_idx) + .unwrap_or_else(|e| self.type_error(e)); let obj_scope_type = obj_scope.as_ref().map(|x| self.type_check_exp(x, env).0); let obj_id_type = obj_id.as_ref().map(|x| self.type_check_exp(x, env).0); let arg_list_types = self.type_check_arg_list(arg_list, env); - let ExprKind::Reference(ref r) = class.kind else { - self.spanned_error(exp,"Must be a reference to a class"); - return (self.types.error(), Phase::Independent); - }; - - let Reference::TypeReference(_) = r else { - self.spanned_error(exp,"Must be a type reference to a class"); - return (self.types.error(), Phase::Independent); - }; - // Lookup the class's type in the env let (class_env, class_symbol) = match *class_type { Type::Class(ref class) => { @@ -2371,7 +2373,7 @@ impl<'a> TypeChecker<'a> { } } - fn resolved_error(&mut self) -> (UnsafeRef, Phase) { + fn resolved_error(&mut self) -> (TypeRef, Phase) { (self.types.error(), Phase::Independent) } @@ -2381,7 +2383,7 @@ impl<'a> TypeChecker<'a> { func_sig: &FunctionSignature, exp: &impl Spanned, arg_list_types: ArgListTypes, - ) -> Option> { + ) -> Option { // Verify arity let pos_args_count = arg_list.pos_args.len(); let min_args = func_sig.min_parameters(); @@ -2491,7 +2493,7 @@ impl<'a> TypeChecker<'a> { None } - fn type_check_closure(&mut self, func_def: &ast::FunctionDefinition, env: &SymbolEnv) -> (UnsafeRef, Phase) { + fn type_check_closure(&mut self, func_def: &ast::FunctionDefinition, env: &SymbolEnv) -> (TypeRef, Phase) { // TODO: make sure this function returns on all control paths when there's a return type (can be done by recursively traversing the statements and making sure there's a "return" statements in all control paths) // https://github.com/winglang/wing/issues/457 // Create a type_checker function signature from the AST function definition @@ -3770,7 +3772,7 @@ impl<'a> TypeChecker<'a> { fn type_check_super_constructor_against_parent_initializer( &mut self, scope: &Scope, - class_type: UnsafeRef, + class_type: TypeRef, class_env: &mut SymbolEnv, init_name: &str, ) { @@ -4361,15 +4363,11 @@ impl<'a> TypeChecker<'a> { _ => return None, } } - Reference::TypeReference(type_) => { - return Some(type_.clone()); - } - Reference::TypeMember { typeobject, property } => { + Reference::TypeMember { type_name, property } => { path.push(property.clone()); - current_reference = match &typeobject.kind { - ExprKind::Reference(r) => r, - _ => return None, - } + type_name.fields.iter().rev().for_each(|f| path.push(f.clone())); + path.push(type_name.root.clone()); + break; } } } @@ -4466,7 +4464,7 @@ impl<'a> TypeChecker<'a> { // We can't get here twice, we can safely assume that if we're here the `object` part of the reference doesn't have and evaluated type yet. // Create a type reference out of this nested reference and call ourselves again let new_ref = Reference::TypeMember { - typeobject: Box::new(user_type_annotation.to_expression()), + type_name: user_type_annotation, property: property.clone(), }; // Replace the reference with the new one, this is unsafe because `reference` isn't mutable and theoretically someone may @@ -4533,42 +4531,10 @@ impl<'a> TypeChecker<'a> { (property_variable, property_phase) } - Reference::TypeReference(udt) => { - let result = self.resolve_user_defined_type(udt, env, self.statement_idx); - let t = match result { - Err(e) => return self.spanned_error_with_var(udt, e.message), - Ok(t) => t, - }; - - let phase = if let Some(c) = t.as_class() { - c.phase - } else { - Phase::Independent - }; - - ( - VariableInfo { - name: Symbol::global(udt.full_path_str()), - type_: t, - reassignable: false, - phase, - kind: VariableKind::Type, - docs: None, - }, - phase, - ) - } - Reference::TypeMember { typeobject, property } => { - let (type_, _) = self.type_check_exp(typeobject, env); - - let ExprKind::Reference(typeref) = &typeobject.kind else { - return self.spanned_error_with_var(typeobject, "Expecting a reference"); - }; - - let Reference::TypeReference(_) = typeref else { - return self.spanned_error_with_var(typeobject, "Expecting a reference to a type"); - }; - + Reference::TypeMember { type_name, property } => { + let type_ = self + .resolve_user_defined_type(type_name, env, self.statement_idx) + .unwrap_or_else(|e| self.type_error(e)); match *type_ { Type::Enum(ref e) => { if e.values.contains(property) { @@ -4641,7 +4607,7 @@ impl<'a> TypeChecker<'a> { fn resolve_variable_from_instance_type( &mut self, - instance_type: UnsafeRef, + instance_type: TypeRef, property: &Symbol, env: &SymbolEnv, // only used for recursion @@ -4796,12 +4762,12 @@ impl<'a> TypeChecker<'a> { fn extract_parent_class( &mut self, - parent_expr: Option<&Expr>, + parent: Option<&UserDefinedType>, phase: Phase, name: &Symbol, env: &mut SymbolEnv, ) -> (Option, Option) { - let Some(parent_expr) = parent_expr else { + let Some(parent) = parent else { if phase == Phase::Preflight { // if this is a preflight and we don't have a parent, then we implicitly set it to `std.Resource` let t = self.types.resource_base_type(); @@ -4812,19 +4778,20 @@ impl<'a> TypeChecker<'a> { } }; - let (parent_type, _) = self.type_check_exp(&parent_expr, env); + let parent_type = self + .resolve_user_defined_type(parent, env, self.statement_idx) + .unwrap_or_else(|e| { + self.type_error(e); + self.types.error() + }); // bail out if we could not resolve the parent type if parent_type.is_unresolved() { return (None, None); } - // Safety: we return from the function above so parent_udt cannot be None - let parent_udt = parent_expr.as_type_reference().unwrap(); - - if &parent_udt.root == name && parent_udt.fields.is_empty() { - self.spanned_error(parent_udt, "Class cannot extend itself".to_string()); - self.types.assign_type_to_expr(parent_expr, self.types.error(), phase); + if &parent.root == name && parent.fields.is_empty() { + self.spanned_error(parent, "Class cannot extend itself".to_string()); return (None, None); } @@ -4837,17 +4804,15 @@ impl<'a> TypeChecker<'a> { "Class \"{}\" is an {} class and cannot extend {} class \"{}\"", name, phase, parent_class.phase, parent_class.name ), - span: Some(parent_expr.span.clone()), + span: Some(parent.span.clone()), }); - self.types.assign_type_to_expr(parent_expr, self.types.error(), phase); (None, None) } } else { report_diagnostic(Diagnostic { - message: format!("Expected \"{}\" to be a class", parent_udt), - span: Some(parent_expr.span.clone()), + message: format!("Expected \"{}\" to be a class", parent), + span: Some(parent.span.clone()), }); - self.types.assign_type_to_expr(parent_expr, self.types.error(), phase); (None, None) } } diff --git a/libs/wingc/src/type_check/lifts.rs b/libs/wingc/src/type_check/lifts.rs index 84fb0658765..f5f2f4c824c 100644 --- a/libs/wingc/src/type_check/lifts.rs +++ b/libs/wingc/src/type_check/lifts.rs @@ -1,140 +1,93 @@ -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap}; -use itertools::Itertools; - -use crate::ast::Symbol; +use crate::ast::{Symbol, UserDefinedType}; use super::{ExprId, CLASS_INFLIGHT_INIT_NAME}; /// A repository of lifts and captures at the class level. #[derive(Debug)] pub struct Lifts { - /// All the lifts. The key is "/" - lifts: BTreeMap, - - /// All the captures. The key is token. - captures: BTreeMap, + // TODO: make all these private and add accessors+helper logic + /// All the lifts. Map from method to a map from inflight code to lift qualifications. + pub lifts_qualifications: BTreeMap>, - /// Map from token to lift - lift_by_token: BTreeMap, + /// All the captures. The key is token the value is the preflight code. + /// Used for preflight setup of inflight captures. + pub captures: BTreeMap, - /// Map between expression id and a lift token. - token_by_expr_id: BTreeMap, + /// Map between liftable AST element and a lift token (used for inflight jsification of captures) + pub token_for_liftable: HashMap, } -/// A record that describes a capture. -#[derive(Debug, Clone)] -pub struct Capture { - /// Lifting token (the symbol used in inflight code) - pub token: String, - - /// The javascript code to capture - pub code: String, +/// Ast elements that may be lifted +#[derive(Debug, PartialEq, Eq, Hash, Clone)] +pub enum Liftable { + Expr(ExprId), + Type(UserDefinedType), } /// A record that describes a single lift from a method. #[derive(Debug)] -pub struct MethodLift { - /// The method name - pub method: String, - - /// Lifting token (the symbol used in inflight code) - pub token: String, - - /// The javascript code to lift (preflight) - pub code: String, - +pub struct LiftQualification { /// The operations that qualify the lift (the property names) pub ops: BTreeSet, - - /// Indicates if this is a lift for a field or a free variable - pub is_field: bool, } /// A record that describes a lift from a class. #[derive(Debug)] -pub struct Lift { - /// Lifting token (the symbol used in inflight code) - pub token: String, +pub struct Capture { + /// Whether this is a field capture (`this.foo`) + pub is_field: bool, - /// The javascript code to lift (preflight) + /// The javascript code to capture (preflight) pub code: String, - - /// Whether this is a field lift (`this.foo`) - pub is_field: bool, } impl Lifts { pub fn new() -> Self { Self { - lifts: BTreeMap::new(), - lift_by_token: BTreeMap::new(), + lifts_qualifications: BTreeMap::new(), captures: BTreeMap::new(), - token_by_expr_id: BTreeMap::new(), + token_for_liftable: HashMap::new(), } } - /// Returns the list of all lifts from this class. - pub fn lifts(&self) -> Vec<&Lift> { - self.lift_by_token.values().collect_vec() - } - fn render_token(&self, code: &str) -> String { - if code == "this" { - return code.to_string(); - } - format!("${}", replace_non_alphanumeric(code)) } /// Adds a lift for an expression. - pub fn lift(&mut self, expr_id: ExprId, method: Option, property: Option, code: &str) { - let is_field = code.contains("this."); - - let token = self.render_token(code); - - self.token_by_expr_id.entry(expr_id).or_insert(token.clone()); - - self.lift_by_token.entry(token.clone()).or_insert(Lift { - token: token.clone(), - is_field, - code: code.to_string(), - }); - + pub fn lift(&mut self, method: Option, property: Option, code: &str, is_field: bool) { let method = method.map(|m| m.name).unwrap_or(Default::default()); - self.add_lift(method, token.clone(), code, property, is_field); + self.add_lift(method, code, property.as_ref().map(|s| s.name.clone())); // add a lift to the inflight initializer or capture it if its not a field if is_field { - self.add_lift(CLASS_INFLIGHT_INIT_NAME.to_string(), token, code, None, true); - } else { - self.capture(&expr_id, code); + self.add_lift(CLASS_INFLIGHT_INIT_NAME.to_string(), code, None); } } - fn add_lift(&mut self, method: String, token: String, code: &str, property: Option, is_field: bool) { - let key = format!("{}/{}", method.clone(), token); - let lift = self.lifts.entry(key).or_insert(MethodLift { - code: code.to_string(), - token: token.clone(), - method: method.clone(), - ops: BTreeSet::new(), - is_field, - }); + fn add_lift(&mut self, method: String, code: &str, property: Option) { + let lift = self + .lifts_qualifications + .entry(method) + .or_default() + .entry(code.to_string()) + .or_insert(LiftQualification { ops: BTreeSet::new() }); if let Some(op) = &property { lift.ops.insert(op.clone()); } } - /// Returns the token for an expression. Called by the jsifier when emitting inflight code. - pub fn token_for_expr(&self, expr_id: &ExprId) -> Option { - let Some(token) = self.token_by_expr_id.get(expr_id) else { + /// Returns the token for a liftable. Called by the jsifier when emitting inflight code. + pub fn token_for_liftable(&self, lifted_thing: &Liftable) -> Option { + let Some(token) = self.token_for_liftable.get(lifted_thing) else { return None; }; - let is_field = if let Some(lift) = self.lift_by_token.get(token) { + let is_field = if let Some(lift) = self.captures.get(token) { lift.is_field } else { false @@ -147,8 +100,8 @@ impl Lifts { } } - /// Captures an expression. - pub fn capture(&mut self, expr_id: &ExprId, code: &str) { + /// Captures a liftable piece of code. + pub fn capture(&mut self, lifted_thing: &Liftable, code: &str, is_field: bool) { // no need to capture this (it's already in scope) if code == "this" { return; @@ -156,46 +109,28 @@ impl Lifts { let token = self.render_token(code); - self.token_by_expr_id.entry(*expr_id).or_insert(token.clone()); + self + .token_for_liftable + .entry(lifted_thing.clone()) + .or_insert(token.clone()); self.captures.entry(token.clone()).or_insert(Capture { - token: token.to_string(), + is_field, code: code.to_string(), }); } - /// The list of captures. - pub fn captures(&self) -> Vec<&Capture> { - self.captures.values().collect_vec() - } - - /// List of all lifted fields in the class. + /// List of all lifted fields in the class. (map from lift token to preflight code) pub fn lifted_fields(&self) -> BTreeMap { let mut result: BTreeMap = BTreeMap::new(); - for (_, lift) in &self.lifts { - if !lift.is_field { - continue; - } - - result.insert(lift.token.clone(), lift.code.clone()); - } - - // self.lifted_fields_by_token.clone() - result - } - - /// List of all lifts per method. Key is the method name and the value is a list of lifts. - pub fn lifts_per_method(&self) -> BTreeMap> { - let mut result: BTreeMap> = BTreeMap::new(); - - for (_, method_lift) in &self.lifts { - if method_lift.method.is_empty() { - continue; - } - result.entry(method_lift.method.clone()).or_default().push(method_lift); - } - + self + .captures + .iter() + .filter(|(_, lift)| lift.is_field) + .for_each(|(token, lift)| { + result.insert(token.clone(), lift.code.clone()); + }); result } } diff --git a/libs/wingc/src/type_check_assert.rs b/libs/wingc/src/type_check_assert.rs index a1315dd4208..d29c06b5512 100644 --- a/libs/wingc/src/type_check_assert.rs +++ b/libs/wingc/src/type_check_assert.rs @@ -25,7 +25,7 @@ impl<'a> TypeCheckAssert<'a> { impl<'a> Visit<'_> for TypeCheckAssert<'a> { fn visit_expr(&mut self, expr: &Expr) { - if let Some(t) = self.types.try_get_expr_type(expr) { + if let Some(t) = self.types.try_get_expr_type(expr.id) { assert!( self.tc_found_errors || !t.is_unresolved(), "Expr's type was not resolved: {:?}", diff --git a/libs/wingc/src/valid_json_visitor.rs b/libs/wingc/src/valid_json_visitor.rs index fbbf5794107..e62fda9e3e6 100644 --- a/libs/wingc/src/valid_json_visitor.rs +++ b/libs/wingc/src/valid_json_visitor.rs @@ -26,7 +26,7 @@ impl<'a> ValidJsonVisitor<'a> { impl<'a> Visit<'_> for ValidJsonVisitor<'a> { fn visit_expr(&mut self, expr: &Expr) { - if let Some(t) = self.types.try_get_expr_type(expr) { + if let Some(t) = self.types.try_get_expr_type(expr.id) { // if the type is json with known values, then we may need to validate that the values are legal json values if let Type::Json(Some(JsonData { kind, expression_id })) = &*t { // if this json expr is not being cast to something else, then it must be a legal json value diff --git a/libs/wingc/src/visit.rs b/libs/wingc/src/visit.rs index 57c00e0e171..95bc7117385 100644 --- a/libs/wingc/src/visit.rs +++ b/libs/wingc/src/visit.rs @@ -248,7 +248,7 @@ where } if let Some(extend) = &node.parent { - v.visit_expr(&extend); + v.visit_user_defined_type(extend); } for implement in &node.implements { @@ -276,7 +276,7 @@ pub fn visit_new_expr<'ast, V>(v: &mut V, node: &'ast NewExpr) where V: Visit<'ast> + ?Sized, { - v.visit_expr(&node.class); + v.visit_user_defined_type(&node.class); v.visit_args(&node.arg_list); if let Some(id) = &node.obj_id { v.visit_expr(&id); @@ -408,11 +408,8 @@ where v.visit_expr(object); v.visit_symbol(property); } - Reference::TypeReference(type_) => { - v.visit_user_defined_type(type_); - } - Reference::TypeMember { typeobject, property } => { - v.visit_expr(typeobject); + Reference::TypeMember { type_name, property } => { + v.visit_user_defined_type(type_name); v.visit_symbol(property); } } @@ -487,12 +484,7 @@ where } v.visit_type_annotation(&f.return_type); } - TypeAnnotationKind::UserDefined(t) => { - v.visit_symbol(&t.root); - for field in &t.fields { - v.visit_symbol(field); - } - } + TypeAnnotationKind::UserDefined(t) => v.visit_user_defined_type(t), } } diff --git a/libs/wingc/src/visit_context.rs b/libs/wingc/src/visit_context.rs index c95abf41000..2ea815724b5 100644 --- a/libs/wingc/src/visit_context.rs +++ b/libs/wingc/src/visit_context.rs @@ -1,5 +1,5 @@ use crate::{ - ast::{Phase, Symbol, UserDefinedType}, + ast::{ExprId, Phase, Symbol, UserDefinedType}, type_check::symbol_env::SymbolEnvRef, }; @@ -7,11 +7,13 @@ pub struct VisitContext { phase: Vec, env: Vec, method_env: Vec>, - property: Vec, + property: Vec, method: Vec>, class: Vec, statement: Vec, in_json: Vec, + in_type_annotation: Vec, + expression: Vec, } impl VisitContext { @@ -25,11 +27,27 @@ impl VisitContext { statement: vec![], method: vec![], in_json: vec![], + in_type_annotation: vec![], + expression: vec![], } } } impl VisitContext { + pub fn push_type_annotation(&mut self) { + self.in_type_annotation.push(true); + } + + pub fn pop_type_annotation(&mut self) { + self.in_type_annotation.pop(); + } + + pub fn in_type_annotation(&self) -> bool { + *self.in_type_annotation.last().unwrap_or(&false) + } + + // -- + pub fn push_stmt(&mut self, stmt: usize) { self.statement.push(stmt); } @@ -44,6 +62,20 @@ impl VisitContext { // -- + fn push_expr(&mut self, expr: ExprId) { + self.expression.push(expr); + } + + fn pop_expr(&mut self) { + self.expression.pop(); + } + + pub fn current_expr(&self) -> Option { + self.expression.last().map(|id| *id) + } + + // -- + pub fn push_class(&mut self, class: UserDefinedType, phase: &Phase, initializer_env: Option) { self.class.push(class); self.push_phase(*phase); @@ -93,15 +125,15 @@ impl VisitContext { // -- - pub fn push_property(&mut self, property: String) { - self.property.push(property); + pub fn push_property(&mut self, property: &Symbol) { + self.property.push(property.clone()); } pub fn pop_property(&mut self) { self.property.pop(); } - pub fn current_property(&self) -> Option { + pub fn current_property(&self) -> Option { self.property.last().cloned() } @@ -143,3 +175,13 @@ impl VisitContext { self.phase.pop(); } } + +pub trait VisitorWithContext { + fn ctx(&mut self) -> &mut VisitContext; + + fn with_expr(&mut self, expr: usize, f: impl FnOnce(&mut Self)) { + self.ctx().push_expr(expr); + f(self); + self.ctx().pop_expr(); + } +} diff --git a/tools/hangar/__snapshots__/invalid.ts.snap b/tools/hangar/__snapshots__/invalid.ts.snap index 7676d6d9de0..d53ba19c913 100644 --- a/tools/hangar/__snapshots__/invalid.ts.snap +++ b/tools/hangar/__snapshots__/invalid.ts.snap @@ -5,7 +5,7 @@ exports[`access_hidden_namespace.w 1`] = ` --> ../../../examples/tests/invalid/access_hidden_namespace.w:7:5 | 7 | new core.NodeJsCode(\\"/tmp/test.txt\\"); // This should fail even though \`fs.TextFile\` extends \`core.FileBase\` because we didn't bring in \`core\` explicitly. - | ^^^^^^^^^^^^^^^ Unknown symbol \\"core\\" + | ^^^^ Unknown symbol \\"core\\" @@ -150,7 +150,7 @@ error: Unknown symbol \\"stuff\\" --> ../../../examples/tests/invalid/bring_local_variables.w:3:5 | 3 | new stuff.Bar(); - | ^^^^^^^^^ Unknown symbol \\"stuff\\" + | ^^^^^ Unknown symbol \\"stuff\\" @@ -212,11 +212,25 @@ Duration " `; exports[`capture_redefinition.w 1`] = ` -"error: Cannot capture symbol \\"y\\" because it is shadowed by another symbol with the same name +"error: Cannot access \\"y\\" because it is shadowed by another symbol with the same name + --> ../../../examples/tests/invalid/capture_redefinition.w:5:7 + | +5 | log(y); + | ^ Cannot access \\"y\\" because it is shadowed by another symbol with the same name + + +error: Cannot access \\"y\\" because it is shadowed by another symbol with the same name --> ../../../examples/tests/invalid/capture_redefinition.w:14:9 | 14 | log(y); - | ^ Cannot capture symbol \\"y\\" because it is shadowed by another symbol with the same name + | ^ Cannot access \\"y\\" because it is shadowed by another symbol with the same name + + +error: Cannot access \\"x\\" because it is shadowed by another symbol with the same name + --> ../../../examples/tests/invalid/capture_redefinition.w:22:7 + | +22 | log(x); + | ^ Cannot access \\"x\\" because it is shadowed by another symbol with the same name @@ -1112,11 +1126,11 @@ error: Cannot create inflight class \\"Foo\\" in preflight phase | ^^^^^^^^^ Cannot create inflight class \\"Foo\\" in preflight phase -error: Cannot qualify access to a lifted object of type \\"PreflightClass\\" (see https://github.com/winglang/wing/issues/76 for more details) - --> ../../../examples/tests/invalid/inflight_class_created_in_preflight.w:19:3 +error: Cannot qualify access to a lifted type \\"PreflightClass\\" (see https://github.com/winglang/wing/issues/76 for more details) + --> ../../../examples/tests/invalid/inflight_class_created_in_preflight.w:19:7 | 19 | new PreflightClass(); - | ^^^^^^^^^^^^^^^^^^^^ Cannot qualify access to a lifted object of type \\"PreflightClass\\" (see https://github.com/winglang/wing/issues/76 for more details) + | ^^^^^^^^^^^^^^ Cannot qualify access to a lifted type \\"PreflightClass\\" (see https://github.com/winglang/wing/issues/76 for more details) @@ -1919,11 +1933,11 @@ exports[`resource_inflight.w 1`] = ` | ^^^^^^^^^^^^^^^^^^ Cannot create preflight class \\"Bucket\\" in inflight phase -error: Cannot qualify access to a lifted object of type \\"Bucket\\" (see https://github.com/winglang/wing/issues/76 for more details) - --> ../../../examples/tests/invalid/resource_inflight.w:4:3 +error: Cannot qualify access to a lifted type \\"Bucket\\" (see https://github.com/winglang/wing/issues/76 for more details) + --> ../../../examples/tests/invalid/resource_inflight.w:4:7 | 4 | new cloud.Bucket(); // Should fail because we can't create resources inflight - | ^^^^^^^^^^^^^^^^^^ Cannot qualify access to a lifted object of type \\"Bucket\\" (see https://github.com/winglang/wing/issues/76 for more details) + | ^^^^^^^^^^^^ Cannot qualify access to a lifted type \\"Bucket\\" (see https://github.com/winglang/wing/issues/76 for more details) @@ -2309,10 +2323,10 @@ error: Struct \\"Showtime\\" extends \\"Dazzle\\" which introduces a conflicting error: Cannot instantiate type \\"BucketProps\\" because it is a struct and not a class. Use struct instantiation instead. - --> ../../../examples/tests/invalid/structs.w:47:9 + --> ../../../examples/tests/invalid/structs.w:47:13 | 47 | let x = new cloud.BucketProps(1); - | ^^^^^^^^^^^^^^^^^^^^^^^^ Cannot instantiate type \\"BucketProps\\" because it is a struct and not a class. Use struct instantiation instead. + | ^^^^^^^^^^^^^^^^^ Cannot instantiate type \\"BucketProps\\" because it is a struct and not a class. Use struct instantiation instead. @@ -2503,14 +2517,14 @@ error: Unknown symbol \\"clod\\" --> ../../../examples/tests/invalid/unknown_symbol.w:3:18 | 3 | let bucket = new clod.Bucket(); - | ^^^^^^^^^^^ Unknown symbol \\"clod\\" + | ^^^^ Unknown symbol \\"clod\\" error: Unknown symbol \\"cloudy\\" --> ../../../examples/tests/invalid/unknown_symbol.w:6:17 | 6 | let funky = new cloudy.Funktion(inflight () => { }); - | ^^^^^^^^^^^^^^^ Unknown symbol \\"cloudy\\" + | ^^^^^^ Unknown symbol \\"cloudy\\" error: Unknown symbol \\"y\\" diff --git a/tools/hangar/__snapshots__/test_corpus/valid/call_static_of_myself.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/call_static_of_myself.w_compile_tf-aws.md index 0cafda8e3e0..ab82616d342 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/call_static_of_myself.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/call_static_of_myself.w_compile_tf-aws.md @@ -223,6 +223,18 @@ class $Root extends $stdlib.std.Resource { _getInflightOps() { return ["foo", "bar", "callThis", "$inflight_init"]; } + _registerBind(host, ops) { + if (ops.includes("callThis")) { + Foo._registerBindObject(Foo, host, ["bar"]); + } + super._registerBind(host, ops); + } + static _registerTypeBind(host, ops) { + if (ops.includes("bar")) { + Foo._registerBindObject(Foo, host, ["foo"]); + } + super._registerTypeBind(host, ops); + } } class Bar extends $stdlib.std.Resource { constructor(scope, id, ) { diff --git a/tools/hangar/__snapshots__/test_corpus/valid/calling_inflight_variants.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/calling_inflight_variants.w_compile_tf-aws.md index 90e3ac6798d..32509e0f39c 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/calling_inflight_variants.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/calling_inflight_variants.w_compile_tf-aws.md @@ -266,19 +266,19 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("$inflight_init")) { - Foo._registerBindObject(this.inflight1, host, []); Foo._registerBindObject(this, host, ["inflight2"]); + Foo._registerBindObject(this.inflight1, host, []); } if (ops.includes("callFn")) { Foo._registerBindObject(this, host, ["makeFn"]); } if (ops.includes("callFn2")) { - Foo._registerBindObject(this.inflight1, host, ["handle"]); Foo._registerBindObject(this, host, ["inflight2"]); + Foo._registerBindObject(this.inflight1, host, ["handle"]); } if (ops.includes("makeFn")) { - Foo._registerBindObject(this.inflight1, host, ["handle"]); Foo._registerBindObject(this, host, ["inflight2"]); + Foo._registerBindObject(this.inflight1, host, ["handle"]); } super._registerBind(host, ops); } diff --git a/tools/hangar/__snapshots__/test_corpus/valid/capture_containers.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/capture_containers.w_compile_tf-aws.md index 848d5e1b022..bd1c6dfc691 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/capture_containers.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/capture_containers.w_compile_tf-aws.md @@ -197,13 +197,13 @@ class $Root extends $stdlib.std.Resource { } _registerBind(host, ops) { if (ops.includes("handle")) { - $Closure1._registerBindObject(Object.keys(myMap).length, host, []); $Closure1._registerBindObject(("bang" in ((arrOfMap.at(0)))), host, []); $Closure1._registerBindObject(("world" in (myMap)), host, []); $Closure1._registerBindObject((arr.at(0)), host, []); $Closure1._registerBindObject((arr.at(1)), host, []); $Closure1._registerBindObject((j)["b"], host, []); $Closure1._registerBindObject((mySet.has("my")), host, []); + $Closure1._registerBindObject(Object.keys(myMap).length, host, []); $Closure1._registerBindObject(arr.length, host, []); $Closure1._registerBindObject(mySet.size, host, []); } diff --git a/tools/hangar/__snapshots__/test_corpus/valid/capture_mutables.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/capture_mutables.w_compile_tf-aws.md index e4c74ddaf59..1bbb98c7945 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/capture_mutables.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/capture_mutables.w_compile_tf-aws.md @@ -205,8 +205,8 @@ class $Root extends $stdlib.std.Resource { _registerBind(host, ops) { if (ops.includes("handle")) { $Closure1._registerBindObject(Object.keys(m).length, host, []); - $Closure1._registerBindObject(aCloned.length, host, []); $Closure1._registerBindObject(a.length, host, []); + $Closure1._registerBindObject(aCloned.length, host, []); $Closure1._registerBindObject(s.size, host, []); } super._registerBind(host, ops); diff --git a/tools/hangar/__snapshots__/test_corpus/valid/capture_tokens.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/capture_tokens.w_compile_tf-aws.md index ac529b918f2..96046af2514 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/capture_tokens.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/capture_tokens.w_compile_tf-aws.md @@ -388,6 +388,7 @@ class $Root extends $stdlib.std.Resource { MyResource._registerBindObject(this.url, host, []); } if (ops.includes("foo")) { + MyResource._registerBindObject(MyResource, host, ["isValidUrl"]); MyResource._registerBindObject(this.api.url, host, []); MyResource._registerBindObject(this.url, host, []); } diff --git a/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md index 4e9fed02d6a..e3f59932f67 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/extern_implementation.w_compile_tf-aws.md @@ -300,6 +300,12 @@ class $Root extends $stdlib.std.Resource { _getInflightOps() { return ["regexInflight", "getUuid", "getData", "print", "call", "$inflight_init"]; } + _registerBind(host, ops) { + if (ops.includes("call")) { + Foo._registerBindObject(Foo, host, ["getData", "getUuid", "regexInflight"]); + } + super._registerBind(host, ops); + } } class $Closure1 extends $stdlib.std.Resource { constructor(scope, id, ) { diff --git a/tools/hangar/__snapshots__/test_corpus/valid/resource.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/resource.w_compile_tf-aws.md index ed59edee046..e4edf2f63a2 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/resource.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/resource.w_compile_tf-aws.md @@ -934,6 +934,7 @@ class $Root extends $stdlib.std.Resource { Bar._registerBindObject(this.foo, host, ["fooGet", "fooInc"]); } if (ops.includes("testTypeAccess")) { + Bar._registerBindObject(Bar, host, ["barStatic"]); Bar._registerBindObject(Foo, host, ["fooStatic"]); Bar._registerBindObject(this.e, host, []); } diff --git a/tools/hangar/__snapshots__/test_corpus/valid/resource_captures_globals.w_compile_tf-aws.md b/tools/hangar/__snapshots__/test_corpus/valid/resource_captures_globals.w_compile_tf-aws.md index fa7bcb4660f..42bfe43527d 100644 --- a/tools/hangar/__snapshots__/test_corpus/valid/resource_captures_globals.w_compile_tf-aws.md +++ b/tools/hangar/__snapshots__/test_corpus/valid/resource_captures_globals.w_compile_tf-aws.md @@ -672,10 +672,10 @@ class $Root extends $stdlib.std.Resource { MyResource._registerBindObject(this.localTopic, host, []); } if (ops.includes("myPut")) { - MyResource._registerBindObject(Another, host, ["myStaticMethod"]); MyResource._registerBindObject((globalArrayOfStr.at(0)), host, []); MyResource._registerBindObject((globalMapOfNum)["a"], host, []); MyResource._registerBindObject((globalSetOfStr.has("a")), host, []); + MyResource._registerBindObject(Another, host, ["myStaticMethod"]); MyResource._registerBindObject(globalAnother, host, ["myMethod"]); MyResource._registerBindObject(globalAnother.first.myResource, host, ["put"]); MyResource._registerBindObject(globalAnother.myField, host, []);