Skip to content

Commit

Permalink
fix(compiler): unknown property diagnostic is same as unknown symbol (#…
Browse files Browse the repository at this point in the history
…5807)

Fixes #2769
A revisit of #5761 
`x_instance.unkonwn_prop` resulted in:
```
Unknown symbol "unkonwn_prop"
```
And now results in:
```
Property "unkonwn_prop" doesn't exist in "X"
```

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
yoav-steinberg authored Mar 2, 2024
1 parent 13b79ea commit 365827a
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 90 deletions.
18 changes: 9 additions & 9 deletions examples/tests/invalid/immutable_container_types.test.w
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
let m1 = Map<str>{"a" => "hi"};

m1.set("a", "bye");
// ^^^ Unknown symbol "set" (TODO: better error message https://github.com/winglang/wing/issues/1660)
// ^^^ Member "set" doesn't exist in "Map" (TODO: better error message https://github.com/winglang/wing/issues/1660)

let m2: Map<str> = MutMap<str> {};
// ^^^^^^^^^^^^^^ Expected type to be "Map<str>", but got "MutMap<str>" instead
let m3 = Map<bool> { "a" => "A" };
// ^^^ Expected type to be "bool", but got "str" instead
let m4 = {"1" => 1, "2" => 2 };
m4.set("2", 3);
// ^^^ Unknown symbol "set"
// ^^^ Member "set" doesn't exist in "Map"
let m5 = m4.copy();
// ^^^^ Unknown symbol "copy"
// ^^^^ Member "copy" doesn't exist in "Map"
m4.delete("1");
// ^^^^^^ Unknown symbol "delete"
// ^^^^^^ Member "delete" doesn't exist in "Map"
m4.clear();
// ^^^^^ Unknown symbol "clear"
// ^^^^^ Member "clear" doesn't exist in "Map"


let s1: Set<Array<num>> = MutSet<Array<num>>[[1]];
// ^^^^^^^^^^^^^^^^^^^^^^^^ Expected type to be "Set<Array<num>>", but got "MutSet<Array<num>>" instead
let s2 = Set<str> ["a", "b", "c"];
s2.delete("a");
// ^^^^^^ Unknown symbol "delete"
// ^^^^^^ Member "delete" doesn't exist in "Set"
s2.add("d");
// ^^^ Unknown symbol "add"
// ^^^ Member "add" doesn't exist in "Set"
let s3 = s2.copy();
// ^^^^ Unknown symbol "copy"
// ^^^^ Member "copy" doesn't exist in "Set"
s2.clear();
// ^^^^^ Unknown symbol "clear"
// ^^^^^ Member "clear" doesn't exist in "Set"
let s4: Set<bool> = Set<bool>[[3]];
// ^^^^^ Expected type to be "Set<bool>", but got "Set<Array<num>>" instead
2 changes: 1 addition & 1 deletion examples/tests/invalid/json.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ let a: Array<str> = j;
// Immutable Json
let foreverJson = Json {a: "hello"};
foreverJson.set("a", "world!");
// ^^^ Unknown symbol "set" (TODO: better error message https://github.com/winglang/wing/issues/1660)
// ^^^ Member "set" doesn't exist in "Json" (TODO: better error message https://github.com/winglang/wing/issues/1660)

let bkt = new cloud.Bucket();
let jArr = Json [bkt];
Expand Down
2 changes: 1 addition & 1 deletion examples/tests/invalid/json_static.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ let immutObj = Json {a: 123, b: "hello", c: [1, 2, 3]};
let mutObj = Json.deepCopyMut(immutObj);

immutObj.set("a", "foo");
// ^^^ Unknown symbol "set" (TODO: better error message https://github.com/winglang/wing/issues/1660)
// ^^^ Member "set" doesn't exist in "Json" (TODO: better error message https://github.com/winglang/wing/issues/1660)
4 changes: 2 additions & 2 deletions examples/tests/invalid/primitives.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ log(y[0]);

let arr = [1, 2, 3];
let join = arr.blabla(",");
//^ blabla is not a valid method
//^ Property "blabla" doesn't not exist in "Array"
arr.push(4);
//^ push is not a valid method (array is immutable)
//^ Property "push" doesn't not exist in "Array"
let n: str = arr.at(0);
//^ expected str, got num
2 changes: 1 addition & 1 deletion examples/tests/invalid/structs.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ let a = A {
x: "Sup"
};
log(a.badField);
// ^^^^^^^^ Unknown symbol "badField"
// ^^^^^^^^ Property "badField" doesn't exist on "A"

// two inherits with same field name but different type
struct Razzle {
Expand Down
12 changes: 10 additions & 2 deletions examples/tests/invalid/unknown_symbol.test.w
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,25 @@ class SomeResource {

inflight getTask(id: str): str {
this.bucket.assert(2 + "2");
//^ Unknown symbol
//^ Member "assert" doesn't exist in "Bucket"
//^ Expected type to be "num", but got "str" instead
return this.bucket.methodWhichIsNotPartOfBucketApi(id);
//^ Unknown symbol
//^ Member "methodWhichIsNotPartOfBucketApi" doesn't exist in "Bucket"
}
}

class A extends B {
//^ Unknown symbol
}

class SomeResourceChild extends SomeResource {
}

let src = new SomeResourceChild();
// Make sure the error states the class and not its parent
src.dodo();
//^ Member "dodo" doesn't exist in "SomeResourceChild"

unknown = 1;
//^ Unknown symbol

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
source: libs/wingc/src/jsify/tests.rs
---
## Errors
Unknown symbol "b" 3:13
Member "b" doesn't exist in "MyInflightClass" 3:13
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
source: libs/wingc/src/jsify/tests.rs
---
## Errors
Unknown symbol "print" 3:13
Member "print" doesn't exist in "Construct" 3:13
17 changes: 12 additions & 5 deletions libs/wingc/src/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6021,11 +6021,18 @@ where
T: Spanned + Display,
{
match lookup_result {
LookupResult::NotFound(s) => TypeError {
message: format!("Unknown symbol \"{s}\""),
span: s.span(),
annotations: vec![],
},
LookupResult::NotFound(s, maybe_t) => {
let message = if let Some(env_type) = maybe_t {
format!("Member \"{s}\" doesn't exist in \"{env_type}\"")
} else {
format!("Unknown symbol \"{s}\"")
};
TypeError {
message,
span: s.span(),
annotations: vec![],
}
}
LookupResult::NotPublic(kind, lookup_info) => TypeError {
message: {
let access = lookup_info.access.to_string();
Expand Down
36 changes: 25 additions & 11 deletions libs/wingc/src/type_check/symbol_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ pub enum LookupResult<'a> {
/// A matching symbol was found but it's not public
NotPublic(reference([a], [SymbolKind]), SymbolLookupInfo),
/// The symbol was not found in the environment, contains the name of the symbol or part of it that was not found
NotFound(Symbol),
/// If the lookup environment was a type environment (SymbolEnvKind::Type) such as a class, the type is also included
NotFound(Symbol, Option<TypeRef>),
/// A symbol with a matching name was found in multiple environments.
MultipleFound,
/// The symbol exists in the environment but it's not defined yet (based on the statement
Expand All @@ -134,7 +135,7 @@ impl<'a> LookupResult<'a> {
match self {
LookupResult::Found(kind, info) => (kind, info),
LookupResult::NotPublic(x, _) => panic!("LookupResult::unwrap({x}) called on LookupResult::NotPublic"),
LookupResult::NotFound(x) => panic!("LookupResult::unwrap({x}) called on LookupResult::NotFound"),
LookupResult::NotFound(x, ..) => panic!("LookupResult::unwrap({x}) called on LookupResult::NotFound"),
LookupResult::MultipleFound => panic!("LookupResult::unwrap() called on LookupResult::MultipleFound"),
LookupResult::DefinedLater(_) => panic!("LookupResult::unwrap() called on LookupResult::DefinedLater"),
LookupResult::ExpectedNamespace(symbol) => panic!(
Expand Down Expand Up @@ -336,13 +337,26 @@ impl SymbolEnv {
);
}

// Get the type of this env (if it's a type env) to include in the result
let env_type = if let SymbolEnvKind::Type(t) = self.kind {
Some(t)
} else {
None
};

// we couldn't find the symbol in the current environment, let's look up in the parent
// environment.
if let Some(ref_annotation([parent_env])) = self.parent {
return parent_env.lookup_ext(symbol, not_after_stmt_idx.map(|_| self.statement_idx));
let res = parent_env.lookup_ext(symbol, not_after_stmt_idx.map(|_| self.statement_idx));
// The `NotFound` result needs to include the type of the original (non-parent) environment (if applicable)
let res = match res {
LookupResult::NotFound(s, ..) => LookupResult::NotFound(s, env_type),
_ => res,
};
return res;
}

LookupResult::NotFound(symbol.clone())
LookupResult::NotFound(symbol.clone(), env_type)
}

#[allow(clippy::needless_arbitrary_self_type)]
Expand Down Expand Up @@ -384,7 +398,7 @@ impl SymbolEnv {

// Look up the result in each env. If there are multiple results, throw a special error
// otherwise proceed normally
let mut lookup_result = LookupResult::NotFound((*next_symb).clone());
let mut lookup_result = LookupResult::NotFound((*next_symb).clone(), None);
for env in ns.envs.vec_iter() {
// invariant: lookup_result is never "ExpectedNamespace" or "MultipleFound"

Expand Down Expand Up @@ -416,15 +430,15 @@ impl SymbolEnv {
if (matches!(partial_result, LookupResult::Found(_, _))
|| matches!(partial_result, LookupResult::DefinedLater(_)))
&& (matches!(lookup_result, LookupResult::NotPublic(_, _))
|| matches!(lookup_result, LookupResult::NotFound(_)))
|| matches!(lookup_result, LookupResult::NotFound(..)))
{
lookup_result = partial_result;
}
// if we found a symbol but it wasn't public, we can update our
// result if we're currently "NotFound". "Found", "DefinedLater", and any
// existing "NotPublic" results take precedence.
else if (matches!(partial_result, LookupResult::NotPublic(_, _)))
&& (matches!(lookup_result, LookupResult::NotFound(_)))
&& (matches!(lookup_result, LookupResult::NotFound(..)))
{
lookup_result = partial_result;
}
Expand Down Expand Up @@ -610,7 +624,7 @@ mod tests {
// Lookup non-existent variable
assert!(matches!(
parent_env.lookup_nested_str("non_existent_var", None),
LookupResult::NotFound(_)
LookupResult::NotFound(..)
));

// Lookup globally visible variable
Expand Down Expand Up @@ -658,7 +672,7 @@ mod tests {
// Lookup for a child var in the parent env
assert!(matches!(
parent_env.lookup_nested_str("child_global_var", None),
LookupResult::NotFound(_)
LookupResult::NotFound(..)
));

// Lookup for a var in the child env
Expand Down Expand Up @@ -770,7 +784,7 @@ mod tests {
// Perform a nested lookup for a non-existent var
let res = child_env.lookup_nested_str("ns1.ns2.non_existent", None);
match res {
LookupResult::NotFound(s) => {
LookupResult::NotFound(s, ..) => {
assert!(s.name == "non_existent")
}
_ => panic!("Expected LookupResult::NotFount(_) but got {:?}", res),
Expand All @@ -779,7 +793,7 @@ mod tests {
// Perform a nested lookup through a non-existent namespace
let res = child_env.lookup_nested_str("ns1.non_existent.ns2_var", None);
match res {
LookupResult::NotFound(s) => {
LookupResult::NotFound(s, ..) => {
assert!(s.name == "non_existent")
}
_ => panic!("Expected LookupResult::NotFount(_) but got {:?}", res),
Expand Down
Loading

0 comments on commit 365827a

Please sign in to comment.