Skip to content

Commit

Permalink
fix(compiler): unable to create array of valid heterogeneous values (#…
Browse files Browse the repository at this point in the history
…3885)

Fixes #3883

Removed a function that didn't really do much, added some comments, and generally tweaked the type-checking for literals

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
MarkMcCulloh authored Aug 18, 2023
1 parent 27c6046 commit 31c52b9
Show file tree
Hide file tree
Showing 4 changed files with 243 additions and 132 deletions.
24 changes: 23 additions & 1 deletion examples/tests/valid/std_containers.w
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,26 @@ let immutMap = mutMap.copy();
assert(sMap.get("one") == 1);
assert(sMap.size() == 2);
assert(immutMap.size() == 3);
assert(nestedMap.get("a").get("b").get("c") == "hello");
assert(nestedMap.get("a").get("b").get("c") == "hello");

class Animal {}
class Cat extends Animal {}
class Dog extends Animal {}

let heterogeneousArray = Array<Animal>[
new Cat() as "C1",
new Dog() as "D1",
];
let heterogeneousDoubleArray = Array<Array<Animal>>[
[new Cat() as "C2"],
Array<Animal>[new Cat() as "C3", new Dog() as "D2"],
[new Animal() as "A1"],
];
let heterogeneousSet = Set<Animal>{
new Cat() as "C4",
new Dog() as "D3",
};
let heterogeneousMap = Map<Animal>{
"cat" => new Cat() as "C5",
"dog" => new Dog() as "D4",
};
204 changes: 97 additions & 107 deletions libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2109,43 +2109,114 @@ impl<'a> TypeChecker<'a> {
}
ExprKind::ArrayLiteral { type_, items } => {
// Infer type based on either the explicit type or the value in one of the items
let container_type = if let Some(type_) = type_ {
self.resolve_type_annotation(type_, env)
let (container_type, mut element_type) = if let Some(type_) = type_ {
let container_type = self.resolve_type_annotation(type_, env);
let element_type = match *container_type {
Type::Array(t) | Type::MutArray(t) => t,
_ => {
self.spanned_error(
&type_.span,
format!("Expected \"Array\" or \"MutArray\", found \"{container_type}\""),
);
self.types.error()
}
};
(container_type, element_type)
} else if self.in_json > 0 {
let json_data = JsonData {
expression_id: exp.id,
kind: JsonDataKind::List(vec![]),
};
let inner_type = self.types.add_type(Type::Json(Some(json_data)));
(self.types.add_type(Type::Array(inner_type)), inner_type)
} else {
if self.in_json > 0 {
let json_data = JsonData {
expression_id: exp.id,
kind: JsonDataKind::List(vec![]),
};
let inner_type = self.types.add_type(Type::Json(Some(json_data)));
self.types.add_type(Type::Array(inner_type))
} else {
let inner_type = self.types.make_inference();
self.types.add_type(Type::Array(inner_type))
}
};

let mut element_type = match *container_type {
Type::Array(t) => t,
Type::MutArray(t) => t,
_ => {
self.spanned_error(exp, format!("Expected \"Array\" type, found \"{}\"", container_type));
self.types.error()
}
let inner_type = self.types.make_inference();
(self.types.add_type(Type::Array(inner_type)), inner_type)
};

// Verify all types are the same as the inferred type
for v in items.iter() {
let (t, _) = self.type_check_exp(v, env);
for item in items {
let (t, _) = self.type_check_exp(item, env);

// Augment the json list data with the new element type
if let Type::Json(Some(JsonData { ref mut kind, .. })) = &mut *element_type {
if let JsonDataKind::List(ref mut json_list) = kind {
json_list.push(SpannedTypeInfo {
type_: t,
span: v.span(),
span: item.span(),
});
}
}
element_type = self.check_json_serializable_or_validate_type(t, element_type, v);

if self.in_json == 0 {
// If we're not in a Json literal, validate the type of each element
self.validate_type(t, element_type, item);
element_type = self.types.maybe_unwrap_inference(element_type);
} else if self.is_in_mut_json && !t.is_json_legal_value() {
// if we're in a MutJson literal, we only need to check that each field is legal json
self.spanned_error(
item,
format!("Expected a valid Json value (https://www.json.org/json-en.html), but got \"{t}\""),
);
}
}

(container_type, env.phase)
}
ExprKind::MapLiteral { fields, type_ } => {
// Infer type based on either the explicit type or the value in one of the fields
let (container_type, mut element_type) = if let Some(type_) = type_ {
let container_type = self.resolve_type_annotation(type_, env);
let element_type = match *container_type {
Type::Map(t) | Type::MutMap(t) => t,
_ => {
self.spanned_error(
&type_.span,
format!("Expected \"Map\" or \"MutMap\", found \"{container_type}\""),
);
self.types.error()
}
};
(container_type, element_type)
} else {
let inner_type = self.types.make_inference();
(self.types.add_type(Type::Map(inner_type)), inner_type)
};

// Verify all types are the same as the inferred type
for field in fields.values() {
let (t, _) = self.type_check_exp(field, env);
self.validate_type(t, element_type, field);
element_type = self.types.maybe_unwrap_inference(element_type);
}

(container_type, env.phase)
}
ExprKind::SetLiteral { type_, items } => {
// Infer type based on either the explicit type or the value in one of the items
let (container_type, mut element_type) = if let Some(type_) = type_ {
let container_type = self.resolve_type_annotation(type_, env);
let element_type = match *container_type {
Type::Set(t) | Type::MutSet(t) => t,
_ => {
self.spanned_error(
&type_.span,
format!("Expected \"Set\" or \"MutSet\", found \"{container_type}\""),
);
self.types.error()
}
};
(container_type, element_type)
} else {
let inner_type = self.types.make_inference();
(self.types.add_type(Type::Set(inner_type)), inner_type)
};

// Verify all types are the same as the inferred type
for item in items {
let (t, _) = self.type_check_exp(item, env);
self.validate_type(t, element_type, item);
element_type = self.types.maybe_unwrap_inference(element_type);
}

(container_type, env.phase)
Expand Down Expand Up @@ -2285,58 +2356,6 @@ impl<'a> TypeChecker<'a> {
env.phase,
)
}
ExprKind::MapLiteral { fields, type_ } => {
// Infer type based on either the explicit type or the value in one of the fields
let container_type = if let Some(type_) = type_ {
self.resolve_type_annotation(type_, env)
} else {
let inner_type = self.types.make_inference();
self.types.add_type(Type::Map(inner_type))
};

let mut value_type = match *container_type {
Type::Map(t) => t,
Type::MutMap(t) => t,
_ => {
self.spanned_error(exp, format!("Expected \"Map\" type, found \"{}\"", container_type));
self.types.error()
}
};

// Verify all types are the same as the inferred type
for (_, v) in fields.iter() {
let (t, _) = self.type_check_exp(v, env);
value_type = self.validate_type(t, value_type, v);
}

(container_type, env.phase)
}
ExprKind::SetLiteral { type_, items } => {
// Infer type based on either the explicit type or the value in one of the items
let container_type = if let Some(type_) = type_ {
self.resolve_type_annotation(type_, env)
} else {
let inferred = self.types.make_inference();
self.types.add_type(Type::Set(inferred))
};

let mut element_type = match *container_type {
Type::Set(t) => t,
Type::MutSet(t) => t,
_ => {
self.spanned_error(exp, format!("Expected \"Set\" type, found \"{}\"", container_type));
self.types.error()
}
};

// Verify all types are the same as the inferred type
for v in items.iter() {
let (t, _) = self.type_check_exp(v, env);
element_type = self.validate_type(t, element_type, v);
}

(container_type, env.phase)
}
ExprKind::FunctionClosure(func_def) => self.type_check_closure(func_def, env),
ExprKind::CompilerDebugPanic => {
// Handle the debug panic expression (during type-checking)
Expand Down Expand Up @@ -2565,35 +2584,6 @@ impl<'a> TypeChecker<'a> {
}
}

fn check_json_serializable_or_validate_type(
&mut self,
actual_type: TypeRef,
expected_type: TypeRef,
exp: &Expr,
) -> TypeRef {
// Skip validate if in Json
if self.in_json == 0 {
return self.validate_type(actual_type, expected_type, exp);
}

if self.is_in_mut_json && !actual_type.is_json_legal_value() {
self.spanned_error(
exp,
format!(
"Expected a valid Json value (https://www.json.org/json-en.html), but got \"{}\"",
actual_type
),
);
return self.types.error();
}

if expected_type.is_json() {
expected_type
} else {
actual_type
}
}

/// Validate that the given type is a subtype (or same) as the expected type. If not, add an error
/// to the diagnostics.
///
Expand Down
34 changes: 10 additions & 24 deletions tools/hangar/__snapshots__/invalid.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -488,11 +488,11 @@ error: Expected type to be \\"num\\", but got \\"str\\" instead
| ^^^ Expected type to be \\"num\\", but got \\"str\\" instead
error: Expected \\"Set\\" type, found \\"Array<num>\\"
error: Expected \\"Set\\" or \\"MutSet\\", found \\"Array<num>\\"
--> ../../../examples/tests/invalid/container_types.w:3:12
|
3 | let arr2 = Array<num> {1, 2, 3};
| ^^^^^^^^^^^^^^^^^^^^ Expected \\"Set\\" type, found \\"Array<num>\\"
| ^^^^^^^^^^ Expected \\"Set\\" or \\"MutSet\\", found \\"Array<num>\\"
error: Expected type to be \\"Array<num>\\", but got \\"Array<str>\\" instead
Expand Down Expand Up @@ -551,18 +551,11 @@ error: Expected type to be \\"num\\", but got \\"str\\" instead
| ^^^ Expected type to be \\"num\\", but got \\"str\\" instead
error: Expected \\"Array\\" type, found \\"Set<num>\\"
error: Expected \\"Array\\" or \\"MutArray\\", found \\"Set<num>\\"
--> ../../../examples/tests/invalid/container_types.w:21:10
|
21 | let s2 = Set<num> [1, \\"2\\", 3];
| ^^^^^^^^^^^^^^^^^^^^ Expected \\"Array\\" type, found \\"Set<num>\\"
error: Expected type to be \\"num\\", but got \\"str\\" instead
--> ../../../examples/tests/invalid/container_types.w:21:23
|
21 | let s2 = Set<num> [1, \\"2\\", 3];
| ^^^ Expected type to be \\"num\\", but got \\"str\\" instead
| ^^^^^^^^ Expected \\"Array\\" or \\"MutArray\\", found \\"Set<num>\\"
error: Expected type to be \\"num\\", but got \\"str\\" instead
Expand Down Expand Up @@ -1537,11 +1530,11 @@ exports[`mut_container_types.w 1`] = `
| ^^^ Expected type to be \\"num\\", but got \\"str\\" instead
error: Expected \\"Set\\" type, found \\"MutArray<num>\\"
error: Expected \\"Set\\" or \\"MutSet\\", found \\"MutArray<num>\\"
--> ../../../examples/tests/invalid/mut_container_types.w:3:12
|
3 | let arr2 = MutArray<num>{1, 2, 3};
| ^^^^^^^^^^^^^^^^^^^^^^ Expected \\"Set\\" type, found \\"MutArray<num>\\"
| ^^^^^^^^^^^^^ Expected \\"Set\\" or \\"MutSet\\", found \\"MutArray<num>\\"
error: Expected type to be \\"MutArray<num>\\", but got \\"Array<num>\\" instead
Expand Down Expand Up @@ -1572,18 +1565,11 @@ error: Expected type to be \\"num\\", but got \\"str\\" instead
| ^^^ Expected type to be \\"num\\", but got \\"str\\" instead
error: Expected \\"Array\\" type, found \\"MutSet<num>\\"
error: Expected \\"Array\\" or \\"MutArray\\", found \\"MutSet<num>\\"
--> ../../../examples/tests/invalid/mut_container_types.w:11:10
|
11 | let s2 = MutSet<num>[1, \\"2\\", 3];
| ^^^^^^^^^^^^^^^^^^^^^^ Expected \\"Array\\" type, found \\"MutSet<num>\\"
error: Expected type to be \\"num\\", but got \\"str\\" instead
--> ../../../examples/tests/invalid/mut_container_types.w:11:25
|
11 | let s2 = MutSet<num>[1, \\"2\\", 3];
| ^^^ Expected type to be \\"num\\", but got \\"str\\" instead
| ^^^^^^^^^^^ Expected \\"Array\\" or \\"MutArray\\", found \\"MutSet<num>\\"
error: Expected type to be \\"MutSet<num>\\", but got \\"Set<num>\\" instead
Expand Down Expand Up @@ -1614,11 +1600,11 @@ error: Expected type to be \\"num\\", but got \\"str\\" instead
| ^^^^^^^ Expected type to be \\"num\\", but got \\"str\\" instead
error: Expected \\"Array\\" type, found \\"MutMap<str>\\"
error: Expected \\"Array\\" or \\"MutArray\\", found \\"MutMap<str>\\"
--> ../../../examples/tests/invalid/mut_container_types.w:20:10
|
20 | let m2 = MutMap<str>[\\"hello\\", \\"world\\"];
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected \\"Array\\" type, found \\"MutMap<str>\\"
| ^^^^^^^^^^^ Expected \\"Array\\" or \\"MutArray\\", found \\"MutMap<str>\\"
error: Expected type to be \\"MutMap<num>\\", but got \\"Map<str>\\" instead
Expand Down
Loading

0 comments on commit 31c52b9

Please sign in to comment.