From 6c0026fc34cf43476b94e051942ff538bccec331 Mon Sep 17 00:00:00 2001 From: yoav-steinberg Date: Sun, 22 Jan 2023 20:10:14 +0200 Subject: [PATCH] fix(compiler): no error when reassigning to non-reassignable variable (#1196) Additionally: * import reassignability (let/var) from JSII class properties. * added support for reassignable/non-reassignable function arguments. * Verify reassignablity of class/resource fields. see #1186 *By submitting this pull request, I confirm that my contribution is made under the terms of the [Monada Contribution License](https://docs.winglang.io/terms-and-policies/contribution-license.html)*. Co-authored-by: Chris Rybicki --- docs/04-reference/winglang-spec.md | 13 + .../invalid/reassing_to_nonreassignable.w | 21 ++ examples/tests/valid/anon_function.w | 2 +- examples/tests/valid/reassignment.w | 30 +++ libs/tree-sitter-wing/grammar.js | 6 +- .../test/corpus/statements.txt | 3 +- libs/wingc/src/ast.rs | 17 +- libs/wingc/src/capture.rs | 25 +- libs/wingc/src/jsify.rs | 23 +- libs/wingc/src/lib.rs | 6 +- libs/wingc/src/parser.rs | 25 +- libs/wingc/src/type_check.rs | 236 ++++++++++++------ libs/wingc/src/type_check/jsii_importer.rs | 16 +- libs/wingc/src/type_check/symbol_env.rs | 38 ++- .../hangar/src/__snapshots__/cli.test.ts.snap | 24 ++ 15 files changed, 340 insertions(+), 145 deletions(-) create mode 100644 examples/tests/invalid/reassing_to_nonreassignable.w diff --git a/docs/04-reference/winglang-spec.md b/docs/04-reference/winglang-spec.md index 129ea2ad654..308cc44c041 100644 --- a/docs/04-reference/winglang-spec.md +++ b/docs/04-reference/winglang-spec.md @@ -419,6 +419,19 @@ Examples in the class section below. Assigning `var` to immutables of the same type is allowed. That is similar to assigning non `readonly`s to `readonly`s in TypeScript. +By default function closure arguments are non-reassignable. By prefixing `var` +to an argument definition you can make a re-assignable function argument: + +```ts +// wing +let f = (arg1: num, var arg2: num) => { + if (arg2 > 100) { + // We can reassign a value to arg2 since it's marked `var` + args2 = 100; + } +} +``` + [`▲ top`][top] --- diff --git a/examples/tests/invalid/reassing_to_nonreassignable.w b/examples/tests/invalid/reassing_to_nonreassignable.w new file mode 100644 index 00000000000..31024e9be74 --- /dev/null +++ b/examples/tests/invalid/reassing_to_nonreassignable.w @@ -0,0 +1,21 @@ +// Assign to non-reassignable var +let x = 5; +x = x + 1; + +// Assign to non-reassignable field +resource R { + f: num; + init() { + this.f = 1; + } + + inc() { + this.f = this.f + 1; + } +} + +// Assign to non-reassignable arg +let f = (arg: num):num => { + arg = 0; + return arg; +} diff --git a/examples/tests/valid/anon_function.w b/examples/tests/valid/anon_function.w index 6c2708294c5..4dc8262960a 100644 --- a/examples/tests/valid/anon_function.w +++ b/examples/tests/valid/anon_function.w @@ -1,5 +1,5 @@ // Define a function and assign it to a variable -let myfunc = (x: num) => { +let myfunc = (var x: num) => { print("${x}"); x = x + 1; if (x > 3.14) { diff --git a/examples/tests/valid/reassignment.w b/examples/tests/valid/reassignment.w index b2abc407565..a4bc7a44f69 100644 --- a/examples/tests/valid/reassignment.w +++ b/examples/tests/valid/reassignment.w @@ -3,3 +3,33 @@ assert(x == 5); x = x + 1; assert(x == 6); + +resource R { + var f: num; + f1: num; + init() { + // Initialize fields in `init` but in an inner scope to make sure + // we treat the special case of `this` access from init correctly at all scope levels + if true { + this.f = 1; + this.f1 = 0; // Access non-reassignable field from constructor is valid + } + } + + inc() { + this.f = this.f + 1; + } +} + +let r = new R(); +r.inc(); +assert(r.f == 2); + +let f = (var arg: num):num => { + arg = 0; + return arg; +} + +let y = 1; +assert(f(y) == 0); +assert(y == 1); // y is not modified, since Wing functions are pass-by-value diff --git a/libs/tree-sitter-wing/grammar.js b/libs/tree-sitter-wing/grammar.js index 46bdcac2210..f6672162be6 100644 --- a/libs/tree-sitter-wing/grammar.js +++ b/libs/tree-sitter-wing/grammar.js @@ -373,7 +373,11 @@ module.exports = grammar({ access_modifier: ($) => choice("public", "private", "protected"), parameter_definition: ($) => - seq(field("name", $.identifier), $._type_annotation), + seq( + optional(field("reassignable", $.reassignable)), + field("name", $.identifier), + $._type_annotation + ), parameter_list: ($) => seq("(", commaSep($.parameter_definition), ")"), diff --git a/libs/tree-sitter-wing/test/corpus/statements.txt b/libs/tree-sitter-wing/test/corpus/statements.txt index e39eb461700..7756c0d382f 100644 --- a/libs/tree-sitter-wing/test/corpus/statements.txt +++ b/libs/tree-sitter-wing/test/corpus/statements.txt @@ -76,7 +76,7 @@ return 1; Inflight closure ==================== -inflight (a: num, b: str?, c: bool) => {}; +inflight (a: num, b: str?, var c: bool) => {}; --- @@ -95,6 +95,7 @@ inflight (a: num, b: str?, c: bool) => {}; ) ) (parameter_definition + reassignable: (reassignable) name: (identifier) type: (builtin_type) ) diff --git a/libs/wingc/src/ast.rs b/libs/wingc/src/ast.rs index dcbedba1a3e..588a0680b01 100644 --- a/libs/wingc/src/ast.rs +++ b/libs/wingc/src/ast.rs @@ -134,7 +134,9 @@ pub struct FunctionSignature { #[derive(Derivative)] #[derivative(Debug)] pub struct FunctionDefinition { - pub parameter_names: Vec, // TODO: move into FunctionSignature and make optional + // List of names of function parameters and whether they are reassignable (`var`) or not. + pub parameters: Vec<(Symbol, bool)>, // TODO: move into FunctionSignature and make optional + pub statements: Scope, pub signature: FunctionSignature, #[derivative(Debug = "ignore")] @@ -143,7 +145,9 @@ pub struct FunctionDefinition { #[derive(Debug)] pub struct Constructor { - pub parameters: Vec, + // List of names of constructor parameters and whether they are reassignable (`var`) or not. + pub parameters: Vec<(Symbol, bool)>, + pub statements: Scope, pub signature: FunctionSignature, } @@ -181,7 +185,7 @@ pub enum StmtKind { identifier: Option, }, VariableDef { - kind: VariableKind, + reassignable: bool, var_name: Symbol, initial_value: Expr, type_: Option, @@ -226,16 +230,11 @@ pub enum StmtKind { }, } -#[derive(Debug, Clone, Copy, PartialEq)] -pub enum VariableKind { - Let, - Var, -} - #[derive(Debug)] pub struct ClassMember { pub name: Symbol, pub member_type: Type, + pub reassignable: bool, pub flight: Phase, } diff --git a/libs/wingc/src/capture.rs b/libs/wingc/src/capture.rs index a43956194c0..01bbff60a87 100644 --- a/libs/wingc/src/capture.rs +++ b/libs/wingc/src/capture.rs @@ -86,12 +86,7 @@ pub fn scan_for_inflights_in_scope(scope: &Scope, diagnostics: &mut Diagnostics) } } } - StmtKind::VariableDef { - kind: _, - var_name: _, - initial_value, - type_: _, - } => { + StmtKind::VariableDef { initial_value, .. } => { scan_for_inflights_in_expression(initial_value, diagnostics); } StmtKind::Expression(exp) => { @@ -195,7 +190,8 @@ fn scan_captures_in_call( Ok((prop_type, phase)) => ( prop_type .as_variable() - .expect("Expected resource property to be a variable"), + .expect("Expected resource property to be a variable") + ._type, phase, ), Err(type_error) => { @@ -254,7 +250,7 @@ fn scan_captures_in_expression( // the type checker. if x.is_ok() { - let (var, f) = x.unwrap(); + let (var, si) = x.unwrap(); if var.as_variable().is_none() { diagnostics.push(Diagnostic { @@ -263,10 +259,10 @@ fn scan_captures_in_expression( span: Some(symbol.span.clone()), }); } else { - let t = var.as_variable().unwrap(); + let t = var.as_variable().unwrap()._type; // if the identifier represents a preflight value, then capture it - if f == Phase::Preflight { + if si.flight == Phase::Preflight { if var.is_reassignable() { diagnostics.push(Diagnostic { level: DiagnosticLevel::Error, @@ -405,12 +401,9 @@ fn scan_captures_in_inflight_scope(scope: &Scope, diagnostics: &mut Diagnostics) for s in scope.statements.iter() { match &s.kind { - StmtKind::VariableDef { - kind: _, - var_name: _, - initial_value, - type_: _, - } => res.extend(scan_captures_in_expression(initial_value, env, s.idx, diagnostics)), + StmtKind::VariableDef { initial_value, .. } => { + res.extend(scan_captures_in_expression(initial_value, env, s.idx, diagnostics)) + } StmtKind::ForLoop { iterator: _, iterable, diff --git a/libs/wingc/src/jsify.rs b/libs/wingc/src/jsify.rs index 1dca0ffc8c7..51091109907 100644 --- a/libs/wingc/src/jsify.rs +++ b/libs/wingc/src/jsify.rs @@ -6,7 +6,7 @@ use sha2::{Digest, Sha256}; use crate::{ ast::{ ArgList, BinaryOperator, ClassMember, Expr, ExprKind, FunctionDefinition, InterpolatedStringPart, Literal, Phase, - Reference, Scope, Stmt, StmtKind, Symbol, Type, UnaryOperator, UtilityFunctions, VariableKind, + Reference, Scope, Stmt, StmtKind, Symbol, Type, UnaryOperator, UtilityFunctions, }, capture::CaptureKind, utilities::snake_case_to_camel_case, @@ -438,7 +438,7 @@ impl JSifier { ExprKind::FunctionClosure(func_def) => match func_def.signature.flight { Phase::Inflight => self.jsify_inflight_function(func_def), Phase::Independent => unimplemented!(), - Phase::Preflight => self.jsify_function(None, &func_def.parameter_names, &func_def.statements, phase), + Phase::Preflight => self.jsify_function(None, &func_def.parameters, &func_def.statements, phase), }, } } @@ -466,15 +466,16 @@ impl JSifier { ) } StmtKind::VariableDef { - kind, + reassignable, var_name, initial_value, type_: _, } => { let initial_value = self.jsify_expression(initial_value, phase); - return match kind { - VariableKind::Let => format!("const {} = {};", self.jsify_symbol(var_name), initial_value), - VariableKind::Var => format!("let {} = {};", self.jsify_symbol(var_name), initial_value), + return if *reassignable { + format!("let {} = {};", self.jsify_symbol(var_name), initial_value) + } else { + format!("const {} = {};", self.jsify_symbol(var_name), initial_value) }; } StmtKind::ForLoop { @@ -567,7 +568,7 @@ impl JSifier { .map(|(n, m)| format!( "{} = {}", n.name, - self.jsify_function(None, &m.parameter_names, &m.statements, phase) + self.jsify_function(None, &m.parameters, &m.statements, phase) )) .collect::>() .join("\n") @@ -623,8 +624,8 @@ impl JSifier { fn jsify_inflight_function(&self, func_def: &FunctionDefinition) -> String { let mut parameter_list = vec![]; - for p in func_def.parameter_names.iter() { - parameter_list.push(p.name.clone()); + for p in func_def.parameters.iter() { + parameter_list.push(p.0.name.clone()); } let block = self.jsify_scope(&func_def.statements, Phase::Inflight); let procid = base16ct::lower::encode_string(&Sha256::new().chain_update(&block).finalize()); @@ -696,10 +697,10 @@ impl JSifier { ) } - fn jsify_function(&self, name: Option<&str>, parameters: &[Symbol], body: &Scope, phase: Phase) -> String { + fn jsify_function(&self, name: Option<&str>, parameters: &[(Symbol, bool)], body: &Scope, phase: Phase) -> String { let mut parameter_list = vec![]; for p in parameters.iter() { - parameter_list.push(self.jsify_symbol(p)); + parameter_list.push(self.jsify_symbol(&p.0)); } let (name, arrow) = match name { diff --git a/libs/wingc/src/lib.rs b/libs/wingc/src/lib.rs index a5bb39a14e7..8876aa1b3f2 100644 --- a/libs/wingc/src/lib.rs +++ b/libs/wingc/src/lib.rs @@ -1,7 +1,7 @@ #[macro_use] extern crate lazy_static; -use ast::{Scope, Stmt, Symbol, UtilityFunctions, VariableKind}; +use ast::{Scope, Stmt, Symbol, UtilityFunctions}; use diagnostic::{print_diagnostics, Diagnostic, DiagnosticLevel, Diagnostics, WingSpan}; use jsify::JSifier; use type_check::symbol_env::StatementIdx; @@ -130,7 +130,7 @@ pub fn parse(source_file: &str) -> (Scope, Diagnostics) { } pub fn type_check(scope: &mut Scope, types: &mut Types) -> Diagnostics { - let env = SymbolEnv::new(None, types.void(), false, Phase::Preflight, 0); + let env = SymbolEnv::new(None, types.void(), false, false, Phase::Preflight, 0); scope.set_env(env); add_builtin( @@ -195,7 +195,7 @@ fn add_builtin(name: &str, typ: Type, scope: &mut Scope, types: &mut Types) { .unwrap() .define( &sym, - SymbolKind::Variable(types.add_type(typ), VariableKind::Let), + SymbolKind::make_variable(types.add_type(typ), false), StatementIdx::Top, ) .expect("Failed to add builtin"); diff --git a/libs/wingc/src/parser.rs b/libs/wingc/src/parser.rs index 07649570d37..c612f605aad 100644 --- a/libs/wingc/src/parser.rs +++ b/libs/wingc/src/parser.rs @@ -9,7 +9,7 @@ use tree_sitter_traversal::{traverse, Order}; use crate::ast::{ ArgList, BinaryOperator, ClassMember, Constructor, Expr, ExprKind, FunctionDefinition, FunctionSignature, InterpolatedString, InterpolatedStringPart, Literal, Phase, Reference, Scope, Stmt, StmtKind, Symbol, Type, - UnaryOperator, VariableKind, + UnaryOperator, }; use crate::diagnostic::{Diagnostic, DiagnosticLevel, DiagnosticResult, Diagnostics, WingSpan}; @@ -183,14 +183,8 @@ impl Parser<'_> { None }; - let kind = if let Some(_) = statement_node.child_by_field_name("reassignable") { - VariableKind::Var - } else { - VariableKind::Let - }; - StmtKind::VariableDef { - kind, + reassignable: statement_node.child_by_field_name("reassignable").is_some(), var_name: self.node_symbol(&statement_node.child_by_field_name("name").unwrap())?, initial_value: self.build_expression(&statement_node.child_by_field_name("value").unwrap())?, type_, @@ -316,11 +310,13 @@ impl Parser<'_> { ("class_member", _) => members.push(ClassMember { name: self.node_symbol(&class_element.child_by_field_name("name").unwrap())?, member_type: self.build_type(&class_element.child_by_field_name("type").unwrap())?, + reassignable: class_element.child_by_field_name("reassignable").is_some(), flight: Phase::Preflight, }), ("inflight_class_member", _) => members.push(ClassMember { name: self.node_symbol(&class_element.child_by_field_name("name").unwrap())?, member_type: self.build_type(&class_element.child_by_field_name("type").unwrap())?, + reassignable: class_element.child_by_field_name("reassignable").is_some(), flight: Phase::Inflight, }), ("constructor", _) => { @@ -334,7 +330,7 @@ impl Parser<'_> { } let parameters = self.build_parameter_list(&class_element.child_by_field_name("parameter_list").unwrap())?; constructor = Some(Constructor { - parameters: parameters.iter().map(|p| p.0.clone()).collect(), + parameters: parameters.iter().map(|p| (p.0.clone(), p.2)).collect(), statements: self.build_scope(&class_element.child_by_field_name("block").unwrap()), signature: FunctionSignature { parameters: parameters.iter().map(|p| p.1.clone()).collect(), @@ -394,7 +390,7 @@ impl Parser<'_> { fn build_function_definition(&self, func_def_node: &Node, flight: Phase) -> DiagnosticResult { let parameters = self.build_parameter_list(&func_def_node.child_by_field_name("parameter_list").unwrap())?; Ok(FunctionDefinition { - parameter_names: parameters.iter().map(|p| p.0.clone()).collect(), + parameters: parameters.iter().map(|p| (p.0.clone(), p.2)).collect(), statements: self.build_scope(&func_def_node.child_by_field_name("block").unwrap()), signature: FunctionSignature { parameters: parameters.iter().map(|p| p.1.clone()).collect(), @@ -409,16 +405,23 @@ impl Parser<'_> { }) } - fn build_parameter_list(&self, parameter_list_node: &Node) -> DiagnosticResult> { + /// Builds a vector of all parameters defined in `parameter_list_node`. + /// + /// # Returns + /// A vector of tuples for each parameter in the list. The tuples are the name, type and a bool letting + /// us know whether the parameter is reassignable or not respectively. + fn build_parameter_list(&self, parameter_list_node: &Node) -> DiagnosticResult> { let mut res = vec![]; let mut cursor = parameter_list_node.walk(); for parameter_definition_node in parameter_list_node.named_children(&mut cursor) { if parameter_definition_node.is_extra() { continue; } + res.push(( self.node_symbol(¶meter_definition_node.child_by_field_name("name").unwrap())?, self.build_type(¶meter_definition_node.child_by_field_name("type").unwrap())?, + parameter_definition_node.child_by_field_name("reassignable").is_some(), )) } diff --git a/libs/wingc/src/type_check.rs b/libs/wingc/src/type_check.rs index 3f4b9d3c7be..846258ec651 100644 --- a/libs/wingc/src/type_check.rs +++ b/libs/wingc/src/type_check.rs @@ -53,21 +53,31 @@ pub type TypeRef = UnsafeRef; #[derive(Debug)] pub enum SymbolKind { Type(TypeRef), - Variable(TypeRef, VariableKind), + Variable(VariableInfo), Namespace(Namespace), } +#[derive(Debug, Clone)] +pub struct VariableInfo { + pub _type: TypeRef, + pub reassignable: bool, +} + impl SymbolKind { - pub fn as_variable(&self) -> Option { + pub fn make_variable(_type: TypeRef, reassignable: bool) -> Self { + SymbolKind::Variable(VariableInfo { _type, reassignable }) + } + + pub fn as_variable(&self) -> Option { match &self { - SymbolKind::Variable(t, _) => Some(t.clone()), + SymbolKind::Variable(t) => Some(t.clone()), _ => None, } } pub fn is_reassignable(&self) -> bool { match self { - SymbolKind::Variable(_, VariableKind::Var) => true, + SymbolKind::Variable(VariableInfo { reassignable: true, .. }) => true, _ => false, } } @@ -152,8 +162,8 @@ impl Class { self .env .iter() - .filter(|(_, t)| t.as_variable().unwrap().as_function_sig().is_some()) - .map(|(s, t)| (s.clone(), t.as_variable().unwrap().clone())) + .filter(|(_, t)| t.as_variable().unwrap()._type.as_function_sig().is_some()) + .map(|(s, t)| (s.clone(), t.as_variable().unwrap()._type.clone())) } } @@ -280,7 +290,7 @@ impl Display for SymbolKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { SymbolKind::Type(_) => write!(f, "type"), - SymbolKind::Variable(_, _) => write!(f, "variable"), + SymbolKind::Variable(_) => write!(f, "variable"), SymbolKind::Namespace(_) => write!(f, "namespace"), } } @@ -609,6 +619,19 @@ impl<'a> TypeChecker<'a> { self.types.anything() } + fn variable_error(&self, type_error: &TypeError) -> VariableInfo { + self.diagnostics.borrow_mut().push(Diagnostic { + level: DiagnosticLevel::Error, + message: type_error.message.clone(), + span: Some(type_error.span.clone()), + }); + + VariableInfo { + _type: self.types.anything(), + reassignable: false, + } + } + pub fn get_primitive_type_by_name(&self, name: &str) -> TypeRef { match name { "number" => self.types.number(), @@ -664,7 +687,7 @@ impl<'a> TypeChecker<'a> { self.validate_type(_type, self.types.number(), unary_exp); _type } - ExprKind::Reference(_ref) => self.resolve_reference(_ref, env, statement_idx), + ExprKind::Reference(_ref) => self.resolve_reference(_ref, env, statement_idx)._type, ExprKind::New { class, obj_id: _, // TODO @@ -704,7 +727,7 @@ impl<'a> TypeChecker<'a> { }, None, ) { - Ok(v) => v.as_variable().expect("Expected constructor to be a variable"), + Ok(v) => v.as_variable().expect("Expected constructor to be a variable")._type, Err(type_error) => { self.type_error(&type_error); return self.types.anything(); @@ -765,7 +788,7 @@ impl<'a> TypeChecker<'a> { // If this returns None, this means we're instantiating a resource object in the global scope, which is valid env .try_lookup("this".into(), Some(statement_idx)) - .map(|v| v.as_variable().expect("Expected \"this\" to be a variable")) + .map(|v| v.as_variable().expect("Expected \"this\" to be a variable")._type) }; // Verify the object scope is an actually resource @@ -787,7 +810,7 @@ impl<'a> TypeChecker<'a> { } ExprKind::Call { function, args } => { // Resolve the function's reference (either a method in the class's env or a function in the current env) - let func_type = self.resolve_reference(function, env, statement_idx); + let func_type = self.resolve_reference(function, env, statement_idx)._type; let this_args = if matches!(function, Reference::NestedIdentifier { .. }) { 1 } else { @@ -909,7 +932,8 @@ impl<'a> TypeChecker<'a> { t, field .as_variable() - .expect("Expected struct field to be a variable in the struct env"), + .expect("Expected struct field to be a variable in the struct env") + ._type, v, ); } else { @@ -991,10 +1015,11 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), sig.return_type, false, + false, func_def.signature.flight, statement_idx, ); - self.add_arguments_to_env(&func_def.parameter_names, &sig, &mut function_env); + self.add_arguments_to_env(&func_def.parameters, &sig, &mut function_env); func_def.statements.set_env(function_env); self.inner_scopes.push(&func_def.statements); @@ -1030,7 +1055,8 @@ impl<'a> TypeChecker<'a> { if let Some(field) = field { let field_type = field .as_variable() - .expect("Expected struct field to be a variable in the struct env"); + .expect("Expected struct field to be a variable in the struct env") + ._type; field_map.insert(k.name.clone(), (k, field_type)); } else { self.expr_error(value, format!("\"{}\" is not a field of \"{}\"", k.name, expected_type)); @@ -1042,7 +1068,8 @@ impl<'a> TypeChecker<'a> { ( k, v.as_variable() - .expect("Expected struct field to be a variable in the struct env"), + .expect("Expected struct field to be a variable in the struct env") + ._type, ) }) { if let Some((symb, expected_field_type)) = field_map.get(&k) { @@ -1184,7 +1211,7 @@ impl<'a> TypeChecker<'a> { fn type_check_statement(&mut self, stmt: &Stmt, env: &mut SymbolEnv) { match &stmt.kind { StmtKind::VariableDef { - kind, + reassignable, var_name, initial_value, type_, @@ -1201,7 +1228,7 @@ impl<'a> TypeChecker<'a> { self.validate_type(inferred_type, explicit_type, initial_value); match env.define( var_name, - SymbolKind::Variable(explicit_type, *kind), + SymbolKind::make_variable(explicit_type, *reassignable), StatementIdx::Index(stmt.idx), ) { Err(type_error) => { @@ -1212,7 +1239,7 @@ impl<'a> TypeChecker<'a> { } else { match env.define( var_name, - SymbolKind::Variable(inferred_type, *kind), + SymbolKind::make_variable(inferred_type, *reassignable), StatementIdx::Index(stmt.idx), ) { Err(type_error) => { @@ -1230,12 +1257,8 @@ impl<'a> TypeChecker<'a> { // TODO: Expression must be iterable let exp_type = self.type_check_exp(iterable, env, stmt.idx); - let mut scope_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, env.flight, stmt.idx); - match scope_env.define( - &iterator, - SymbolKind::Variable(exp_type, VariableKind::Let), - StatementIdx::Top, - ) { + let mut scope_env = SymbolEnv::new(Some(env.get_ref()), env.return_type, false, false, env.flight, stmt.idx); + match scope_env.define(&iterator, SymbolKind::make_variable(exp_type, false), StatementIdx::Top) { Err(type_error) => { self.type_error(&type_error); } @@ -1253,6 +1276,7 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), env.return_type, false, + false, env.flight, stmt.idx, )); @@ -1271,6 +1295,7 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), env.return_type, false, + false, env.flight, stmt.idx, )); @@ -1281,6 +1306,7 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), env.return_type, false, + false, env.flight, stmt.idx, )); @@ -1291,10 +1317,12 @@ impl<'a> TypeChecker<'a> { self.type_check_exp(e, env, stmt.idx); } StmtKind::Assignment { variable, value } => { - // TODO: check if variable can be assigned to let exp_type = self.type_check_exp(value, env, stmt.idx); - let var_type = self.resolve_reference(variable, env, stmt.idx); - self.validate_type(exp_type, var_type, value); + let var_info = self.resolve_reference(variable, env, stmt.idx); + if !var_info.reassignable { + self.stmt_error(stmt, format!("Variable {} is not reassignable ", variable)); + } + self.validate_type(exp_type, var_info._type, value); } StmtKind::Use { module_name, @@ -1317,6 +1345,7 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), env.return_type, false, + false, env.flight, stmt.idx, )); @@ -1382,7 +1411,7 @@ impl<'a> TypeChecker<'a> { }; // Create environment representing this class, for now it'll be empty just so we can support referencing ourselves from the class definition. - let dummy_env = SymbolEnv::new(None, self.types.void(), true, env_flight, stmt.idx); + let dummy_env = SymbolEnv::new(None, self.types.void(), true, false, env_flight, stmt.idx); // Create the resource/class type and add it to the current environment (so class implementation can reference itself) let class_spec = Class { @@ -1405,14 +1434,14 @@ impl<'a> TypeChecker<'a> { }; // Create a the real class environment to be filled with the class AST types - let mut class_env = SymbolEnv::new(parent_class_env, self.types.void(), true, env_flight, stmt.idx); + let mut class_env = SymbolEnv::new(parent_class_env, self.types.void(), true, false, env_flight, stmt.idx); // Add members to the class env for member in members.iter() { let member_type = self.resolve_type(&member.member_type, env, stmt.idx); match class_env.define( &member.name, - SymbolKind::Variable(member_type, VariableKind::Let), + SymbolKind::make_variable(member_type, member.reassignable), StatementIdx::Top, ) { Err(type_error) => { @@ -1437,7 +1466,7 @@ impl<'a> TypeChecker<'a> { let method_type = self.resolve_type(&AstType::FunctionSignature(sig), env, stmt.idx); match class_env.define( method_name, - SymbolKind::Variable(method_type, VariableKind::Let), + SymbolKind::make_variable(method_type, false), StatementIdx::Top, ) { Err(type_error) => { @@ -1458,7 +1487,7 @@ impl<'a> TypeChecker<'a> { name: WING_CONSTRUCTOR_NAME.into(), span: name.span.clone(), }, - SymbolKind::Variable(constructor_type, VariableKind::Let), + SymbolKind::make_variable(constructor_type, false), StatementIdx::Top, ) { Err(type_error) => { @@ -1486,6 +1515,7 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), constructor_sig.return_type, false, + true, env_flight, stmt.idx, ); @@ -1497,7 +1527,7 @@ impl<'a> TypeChecker<'a> { name: "this".into(), span: name.span.clone(), }, - SymbolKind::Variable(class_type, VariableKind::Let), + SymbolKind::make_variable(class_type, false), StatementIdx::Top, ) .expect("Expected `this` to be added to constructor env"); @@ -1514,7 +1544,8 @@ impl<'a> TypeChecker<'a> { .lookup(method_name, None) .expect("Expected method to be in class env") .as_variable() - .expect("Expected method to be a variable"); + .expect("Expected method to be a variable") + ._type; let method_sig = method_type .as_function_sig() @@ -1525,15 +1556,19 @@ impl<'a> TypeChecker<'a> { Some(env.get_ref()), method_sig.return_type, false, + false, method_sig.flight, stmt.idx, ); // Add `this` as first argument - let mut actual_parameters = vec![Symbol { - name: "this".into(), - span: method_name.span.clone(), - }]; - actual_parameters.extend(method_def.parameter_names.clone()); + let mut actual_parameters = vec![( + Symbol { + name: "this".into(), + span: method_name.span.clone(), + }, + false, + )]; + actual_parameters.extend(method_def.parameters.clone()); self.add_arguments_to_env(&actual_parameters, method_sig, &mut method_env); method_def.statements.set_env(method_env); self.inner_scopes.push(&method_def.statements); @@ -1545,14 +1580,14 @@ impl<'a> TypeChecker<'a> { // fail type checking. // Create an environment for the struct - let mut struct_env = SymbolEnv::new(None, self.types.void(), true, env.flight, stmt.idx); + let mut struct_env = SymbolEnv::new(None, self.types.void(), true, false, env.flight, stmt.idx); // Add members to the struct env for member in members.iter() { let member_type = self.resolve_type(&member.member_type, env, stmt.idx); match struct_env.define( &member.name, - SymbolKind::Variable(member_type, VariableKind::Let), + SymbolKind::make_variable(member_type, false), StatementIdx::Top, ) { Err(type_error) => { @@ -1638,14 +1673,19 @@ impl<'a> TypeChecker<'a> { } } - fn add_arguments_to_env(&mut self, arg_names: &Vec, sig: &FunctionSignature, env: &mut SymbolEnv) { - assert!(arg_names.len() == sig.args.len()); - for (arg, arg_type) in arg_names.iter().zip(sig.args.iter()) { - match env.define( - &arg, - SymbolKind::Variable(*arg_type, VariableKind::Let), - StatementIdx::Top, - ) { + /// Add function arguments to the function's environment + /// + /// #Arguments + /// + /// * `args` - List of aruments to add, each element is a tuple of the arugment symbol and whether it's + /// reassignable arg or not. Note that the argument types are figured out from `sig`. + /// * `sig` - The function signature (used to figure out the type of each argument). + /// * `env` - The function's environment to prime with the args. + /// + fn add_arguments_to_env(&mut self, args: &Vec<(Symbol, bool)>, sig: &FunctionSignature, env: &mut SymbolEnv) { + assert!(args.len() == sig.args.len()); + for (arg, arg_type) in args.iter().zip(sig.args.iter()) { + match env.define(&arg.0, SymbolKind::make_variable(*arg_type, arg.1), StatementIdx::Top) { Err(type_error) => { self.type_error(&type_error); } @@ -1695,7 +1735,14 @@ impl<'a> TypeChecker<'a> { )); } - let new_env = SymbolEnv::new(None, original_type_class.env.return_type, true, Phase::Independent, 0); + let new_env = SymbolEnv::new( + None, + original_type_class.env.return_type, + true, + false, + Phase::Independent, + 0, + ); let tt = Type::Class(Class { name: original_type_class.name.clone(), env: new_env, @@ -1703,6 +1750,7 @@ impl<'a> TypeChecker<'a> { should_case_convert_jsii: original_type_class.should_case_convert_jsii, type_parameters: Some(type_params.clone()), }); + // TODO: here we add a new type regardless whether we already "hydrated" `original_type` with these `type_params`. Cache! let mut new_type = self.types.add_type(tt); let new_type_class = new_type.as_mut_class_or_resource().unwrap(); @@ -1712,7 +1760,7 @@ impl<'a> TypeChecker<'a> { let new_type_arg = type_params[type_index]; for (name, symbol) in original_type_class.env.iter() { match symbol { - SymbolKind::Variable(v, kind) => { + SymbolKind::Variable(VariableInfo { _type: v, .. }) => { // Replace type params in function signatures if let Some(sig) = v.as_function_sig() { let new_return_type = if sig.return_type == *original_type_param { @@ -1745,7 +1793,7 @@ impl<'a> TypeChecker<'a> { name: name.clone(), span: WingSpan::global(), }, - SymbolKind::Variable(self.types.add_type(Type::Function(new_sig)), VariableKind::Let), + SymbolKind::make_variable(self.types.add_type(Type::Function(new_sig)), false), StatementIdx::Top, ) { Err(type_error) => { @@ -1753,7 +1801,11 @@ impl<'a> TypeChecker<'a> { } _ => {} } - } else if let Some(var) = symbol.as_variable() { + } else if let Some(VariableInfo { + _type: var, + reassignable, + }) = symbol.as_variable() + { let new_var_type = if var == *original_type_param { new_type_arg } else { var }; match new_type_class.env.define( // TODO: Original symbol is not available. SymbolKind::Variable should probably expose it @@ -1761,7 +1813,7 @@ impl<'a> TypeChecker<'a> { name: name.clone(), span: WingSpan::global(), }, - SymbolKind::Variable(new_var_type, *kind), + SymbolKind::make_variable(new_var_type, reassignable), StatementIdx::Top, ) { Err(type_error) => { @@ -1805,20 +1857,20 @@ impl<'a> TypeChecker<'a> { } } - fn resolve_reference(&mut self, reference: &Reference, env: &SymbolEnv, statement_idx: usize) -> TypeRef { + fn resolve_reference(&mut self, reference: &Reference, env: &SymbolEnv, statement_idx: usize) -> VariableInfo { match reference { Reference::Identifier(symbol) => match env.lookup(symbol, Some(statement_idx)) { Ok(var) => { if let Some(var) = var.as_variable() { var } else { - self.type_error(&TypeError { + self.variable_error(&TypeError { message: format!("Expected identifier {}, to be a variable, but it's a {}", symbol, var), span: symbol.span.clone(), }) } } - Err(type_error) => self.type_error(&type_error), + Err(type_error) => self.variable_error(&type_error), }, Reference::NestedIdentifier { object, property } => { // There's a special case where the object is actually a type and the property is either a static method or an enum variant. @@ -1826,27 +1878,45 @@ impl<'a> TypeChecker<'a> { // object as a single reference to the type if let Some(_type) = self.expr_maybe_type(object, env, statement_idx) { // Currently we only support enum field access (no static methods) - match *_type { + let _type = match *_type { Type::Enum(ref e) => { if e.values.contains(property) { - return _type; + _type } else { - return self.general_type_error(format!( + self.general_type_error(format!( "Enum \"{}\" does not contain value \"{}\"", _type, property.name - )); + )) } } - _ => { - return self.general_type_error(format!("Type {} not valid in expression", _type)); + _ => self.general_type_error(format!("Type {} not valid in expression", _type)), + }; + return VariableInfo { + _type, + reassignable: false, + }; + } + + // Special case: if the object expression is a simple reference to `this` and we're inside the init function then + // we'll consider all properties as reassignable regardless of whether they're `var`. + let mut force_reassignable = false; + if let ExprKind::Reference(Reference::Identifier(symb)) = &object.kind { + if symb.name == "this" { + if let Ok((kind, info)) = env.lookup_ext(symb, Some(statement_idx)) { + // `this` resreved symbol should always be a variable + assert!(matches!(kind, SymbolKind::Variable(_))); + force_reassignable = info.init; } } } let instance_type = self.type_check_exp(object, env, statement_idx); - match *instance_type { + let res = match *instance_type { Type::Class(ref class) | Type::Resource(ref class) => self.get_property_from_class(class, property), - Type::Anything => instance_type, + Type::Anything => VariableInfo { + _type: instance_type, + reassignable: false, + }, // Lookup wingsdk std types, hydrating generics if necessary Type::Array(t) => { @@ -1894,22 +1964,34 @@ impl<'a> TypeChecker<'a> { property, ), - _ => self.expr_error( - object, - format!( - "Expression must be a class or resource instance to access property \"{}\", instead found type \"{}\"", - property.name, instance_type + _ => VariableInfo { + _type: self.expr_error( + object, + format!( + "Expression must be a class or resource instance to access property \"{}\", instead found type \"{}\"", + property.name, instance_type + ), ), - ), + reassignable: false, + }, + }; + + if force_reassignable { + VariableInfo { + _type: res._type, + reassignable: true, + } + } else { + res } } } } - fn get_property_from_class(&mut self, class: &Class, property: &Symbol) -> TypeRef { + fn get_property_from_class(&mut self, class: &Class, property: &Symbol) -> VariableInfo { match class.env.lookup(property, None) { Ok(field) => field.as_variable().expect("Expected property to be a variable"), - Err(type_error) => self.type_error(&type_error), + Err(type_error) => self.variable_error(&type_error), } } } @@ -1949,14 +2031,16 @@ fn add_parent_members_to_struct_env( for (parent_member_name, parent_member) in parent_struct.env.iter() { let member_type = parent_member .as_variable() - .expect("Expected struct member to be a variable"); + .expect("Expected struct member to be a variable") + ._type; if let Some(existing_type) = struct_env.try_lookup(&parent_member_name, None) { // We compare types in both directions to make sure they are exactly the same type and not inheriting from each other // TODO: does this make sense? We should add an `is_a()` methdod to `Type` to check if a type is a subtype and use that // when we want to check for subtypes and use equality for strict comparisons. let existing_type = existing_type .as_variable() - .expect("Expected struct member to be a variable"); + .expect("Expected struct member to be a variable") + ._type; if existing_type.ne(&member_type) && member_type.ne(&member_type) { return Err(TypeError { span: name.span.clone(), @@ -1972,7 +2056,7 @@ fn add_parent_members_to_struct_env( name: parent_member_name, span: name.span.clone(), }, - SymbolKind::Variable(member_type, VariableKind::Let), + SymbolKind::make_variable(member_type, false), StatementIdx::Top, )?; } diff --git a/libs/wingc/src/type_check/jsii_importer.rs b/libs/wingc/src/type_check/jsii_importer.rs index fdd79016ad9..43406092527 100644 --- a/libs/wingc/src/type_check/jsii_importer.rs +++ b/libs/wingc/src/type_check/jsii_importer.rs @@ -1,5 +1,5 @@ use crate::{ - ast::{Phase, Symbol, VariableKind}, + ast::{Phase, Symbol}, debug, diagnostic::{CharacterLocation, WingSpan}, type_check::{self, symbol_env::SymbolEnv}, @@ -195,7 +195,7 @@ impl<'a> JsiiImporter<'a> { SymbolKind::Namespace(Namespace { name: namespace_name.to_string(), hidden: namespace_name != self.module_name, - env: SymbolEnv::new(None, self.wing_types.void(), false, self.env.flight, 0), + env: SymbolEnv::new(None, self.wing_types.void(), false, false, self.env.flight, 0), }), StatementIdx::Top, ) @@ -246,6 +246,7 @@ impl<'a> JsiiImporter<'a> { None, self.wing_types.void(), true, + false, self.env.flight, self.import_statement_idx, ); @@ -257,6 +258,7 @@ impl<'a> JsiiImporter<'a> { None, self.wing_types.void(), true, + false, struct_env.flight, self.import_statement_idx, ), // Dummy env, will be replaced below @@ -335,7 +337,7 @@ impl<'a> JsiiImporter<'a> { class_env .define( &Self::jsii_name_to_symbol(&name, &m.location_in_module), - SymbolKind::Variable(method_sig, VariableKind::Let), + SymbolKind::make_variable(method_sig, false), StatementIdx::Top, ) .expect(&format!( @@ -364,7 +366,7 @@ impl<'a> JsiiImporter<'a> { class_env .define( &Self::jsii_name_to_symbol(&camel_case_to_snake_case(&p.name), &p.location_in_module), - SymbolKind::Variable(wing_type, VariableKind::Let), + SymbolKind::make_variable(wing_type, matches!(p.immutable, Some(true))), StatementIdx::Top, ) .expect(&format!( @@ -461,7 +463,7 @@ impl<'a> JsiiImporter<'a> { }; // Create environment representing this class, for now it'll be empty just so we can support referencing ourselves from the class definition. - let dummy_env = SymbolEnv::new(None, self.wing_types.void(), true, self.env.flight, 0); + let dummy_env = SymbolEnv::new(None, self.wing_types.void(), true, false, self.env.flight, 0); let new_type_symbol = Self::jsii_name_to_symbol(type_name, &jsii_class.location_in_module); // Create the new resource/class type and add it to the current environment. // When adding the class methods below we'll be able to reference this type. @@ -504,7 +506,7 @@ impl<'a> JsiiImporter<'a> { .define(&new_type_symbol, SymbolKind::Type(new_type), StatementIdx::Top) .expect(&format!("Invalid JSII library: failed to define class {}", type_name)); // Create class's actual environment before we add properties and methods to it - let mut class_env = SymbolEnv::new(base_class_env, self.wing_types.void(), true, self.env.flight, 0); + let mut class_env = SymbolEnv::new(base_class_env, self.wing_types.void(), true, false, self.env.flight, 0); // Add constructor to the class environment let jsii_initializer = jsii_class.initializer.as_ref(); @@ -533,7 +535,7 @@ impl<'a> JsiiImporter<'a> { })); if let Err(e) = class_env.define( &Self::jsii_name_to_symbol(WING_CONSTRUCTOR_NAME, &initializer.location_in_module), - SymbolKind::Variable(method_sig, VariableKind::Let), + SymbolKind::make_variable(method_sig, false), StatementIdx::Top, ) { panic!("Invalid JSII library, failed to define {}'s init: {}", type_name, e) diff --git a/libs/wingc/src/type_check/symbol_env.rs b/libs/wingc/src/type_check/symbol_env.rs index 0960a73be54..cdb86a20319 100644 --- a/libs/wingc/src/type_check/symbol_env.rs +++ b/libs/wingc/src/type_check/symbol_env.rs @@ -5,15 +5,20 @@ use crate::{ }; use std::collections::{hash_map, HashMap, HashSet}; -use super::UnsafeRef; +use super::{UnsafeRef, VariableInfo}; pub type SymbolEnvRef = UnsafeRef; pub struct SymbolEnv { pub(crate) ident_map: HashMap, parent: Option, + + // TODO: This doesn't make much sense in the context of the "envrioment" but I needed a way to propagate the return type of a function + // down the scopes. Think of a nicer way to do this. pub return_type: TypeRef, + is_class: bool, + pub is_init: bool, pub flight: Phase, statement_idx: usize, } @@ -32,8 +37,8 @@ pub enum StatementIdx { // Possible results for a symbol lookup in the environment enum LookupResult<'a> { - // The type of the symbol and its flight phase - Found((&'a SymbolKind, Phase)), + // The kind of symbol and usefull metadata associated with its lookup + Found((&'a SymbolKind, SymbolInfo)), // The symbol was not found in the environment NotFound, // The symbol exists in the environment but it's not defined yet (based on the statement @@ -41,6 +46,13 @@ enum LookupResult<'a> { DefinedLater, } +pub struct SymbolInfo { + // The phase the symbol was defined in + pub flight: Phase, + // Whether the symbol was defined in an `init`'s environment + pub init: bool, +} + enum LookupMutResult<'a> { // The type of the symbol and its flight phase Found((&'a mut SymbolKind, Phase)), @@ -56,6 +68,7 @@ impl SymbolEnv { parent: Option, return_type: TypeRef, is_class: bool, + is_init: bool, flight: Phase, statement_idx: usize, ) -> Self { @@ -67,6 +80,7 @@ impl SymbolEnv { parent, return_type, is_class, + is_init, flight, statement_idx, } @@ -94,12 +108,12 @@ impl SymbolEnv { if let Some(_parent_env) = self.parent { if let Some(parent_kind) = self.try_lookup(&symbol.name, None) { // If we're a class we allow "symbol shadowing" for methods - let is_function = if let SymbolKind::Variable(t, _) = kind { + let is_function = if let SymbolKind::Variable(VariableInfo { _type: t, .. }) = kind { matches!(*t, Type::Function(_)) } else { false }; - let is_parent_function = if let SymbolKind::Variable(t, _) = *parent_kind { + let is_parent_function = if let SymbolKind::Variable(VariableInfo { _type: t, .. }) = *parent_kind { matches!(*t, Type::Function(_)) } else { false @@ -140,7 +154,13 @@ impl SymbolEnv { } } } - LookupResult::Found((kind.into(), self.flight)) + LookupResult::Found(( + kind.into(), + SymbolInfo { + flight: self.flight, + init: self.is_init, + }, + )) } else if let Some(ref parent_env) = self.parent { parent_env.try_lookup_ext(symbol_name, not_after_stmt_idx.map(|_| self.statement_idx)) } else { @@ -174,11 +194,11 @@ impl SymbolEnv { &self, symbol: &Symbol, not_after_stmt_idx: Option, - ) -> Result<(&SymbolKind, Phase), TypeError> { + ) -> Result<(&SymbolKind, SymbolInfo), TypeError> { let lookup_result = self.try_lookup_ext(&symbol.name, not_after_stmt_idx); match lookup_result { - LookupResult::Found((kind, flight)) => Ok((kind, flight)), + LookupResult::Found((kind, symbol_info)) => Ok((kind, symbol_info)), LookupResult::NotFound => Err(TypeError { message: format!("Unknown symbol \"{}\"", &symbol.name), span: symbol.span.clone(), @@ -234,7 +254,7 @@ impl SymbolEnv { // This is because we currently allow unknown stuff to be referenced under an anything which will // be resolved only in runtime. // TODO: do we still need this? Why? - if let SymbolKind::Variable(t, _) = *t { + if let SymbolKind::Variable(VariableInfo { _type: t, .. }) = *t { if matches!(*t, Type::Anything) { break; } diff --git a/tools/hangar/src/__snapshots__/cli.test.ts.snap b/tools/hangar/src/__snapshots__/cli.test.ts.snap index a2e78edd123..57168187c7e 100644 --- a/tools/hangar/src/__snapshots__/cli.test.ts.snap +++ b/tools/hangar/src/__snapshots__/cli.test.ts.snap @@ -3122,10 +3122,34 @@ class MyApp extends $App { constructor() { super({ outdir: $outdir, name: \\"reassignment\\" }); + class R + { + constructor() { + if (true) { + this.f = 1; + this.f1 = 0; + } + } + f; + f1; + inc = () => { + this.f = (this.f + 1); + } + } let x = 5; {((cond) => {if (!cond) throw new Error(\`assertion failed: '(x === 5)'\`)})((x === 5))}; x = (x + 1); {((cond) => {if (!cond) throw new Error(\`assertion failed: '(x === 6)'\`)})((x === 6))}; + const r = new R(this,\\"R\\"); + (r.inc()); + {((cond) => {if (!cond) throw new Error(\`assertion failed: '(r.f === 2)'\`)})((r.f === 2))}; + const f = (arg) => { + arg = 0; + return arg; + }; + const y = 1; + {((cond) => {if (!cond) throw new Error(\`assertion failed: '((f(y)) === 0)'\`)})(((f(y)) === 0))}; + {((cond) => {if (!cond) throw new Error(\`assertion failed: '(y === 1)'\`)})((y === 1))}; } } new MyApp().synth();"