Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit invalid error(want 2471, but 2322) #745

Closed
sunrabbit123 opened this issue Feb 23, 2023 · 5 comments
Closed

Emit invalid error(want 2471, but 2322) #745

sunrabbit123 opened this issue Feb 23, 2023 · 5 comments
Assignees

Comments

@sunrabbit123
Copy link
Collaborator

Do you know this test file? (tests/conformance/types/typeRelationships/assignmentCompatibility/unionTypesAssignability)

class C { }
class D extends C { foo1() { } }
class E extends C { foo2() { } }
var unionDE: D | E;

var num: number;
var str: string;
var c: C;
var d: D;
var e: E;

// A union type U is assignable to a type T if each type in U is assignable to T
c = d;
c = e;
c = unionDE; // ok
d = d;
d = e;
d = unionDE; // error e is not assignable to d
e = d;
e = e;

Except for allocations related to the Union,
There is a difference in properties, so it should be 2471.

But stc emit 2322
because the logic returns 2322.

https://github.com/dudykr/stc/blob/main/crates/stc_ts_file_analyzer/src/analyzer/assign/class.rs#L299

I think that we should touch only assign/mod.rs and assign/class.rs
I think it's an easy issue except for coordinating the order.

@kdy1 kdy1 added this to the v0.0.1: Correctness milestone Feb 23, 2023
@kdy1 kdy1 added good first issue Good for newcomers and removed good first issue Good for newcomers labels Feb 23, 2023
@kdy1
Copy link
Member

kdy1 commented Feb 23, 2023

Not sure if this can be a GFI. In my experience 2322 vs 2741 was quite complex

@awareness481
Copy link
Contributor

Im pretty sure this also depends on #741

@sunrabbit123
Copy link
Collaborator Author

Im pretty sure this also depends on #741

It'd be nice to combine the two together,
Their internal behavior is different.

@kdy1
Copy link
Member

kdy1 commented Feb 26, 2023

I tried this, but this requires implementing missing property logic for class assignments.
I think we should delegate to type element assign logic, though.

@kdy1
Copy link
Member

kdy1 commented Feb 28, 2023

Closed by #769

@kdy1 kdy1 closed this as completed Feb 28, 2023
@kdy1 kdy1 self-assigned this Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants