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.

Sign up for GitHub

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

Jump to bottom

Make tuple and custom array declared as interface compatible #216

Open
kdy1 opened this issue Nov 6, 2022 · 9 comments · May be fixed by #413
Open

Make tuple and custom array declared as interface compatible #216

kdy1 opened this issue Nov 6, 2022 · 9 comments · May be fixed by #413
Assignees
@devgony
Labels
bug Something isn't working good first issue Good for newcomers
Milestone
v0.0.1: Correctness

Comments

@kdy1
Copy link
Member

kdy1 commented Nov 6, 2022

Problem

stc/crates/stc_ts_type_checker/tests/conformance/types/tuple/arityAndOrderCompatibility01.ts

Lines 1 to 5 in 56ae129

interface StrNum extends Array<string|number> {
0: string;
1: number;
length: 2;
}

This type is treated almost as a tuple by tsc, but stc fails to verify it.
It's because it extends Array.

image

Solution

I think we should handle it from Analyzer.normalize.
Implementing special rules to each function for handling such edge looks like overengineering.

We can add logic for checking if the ty is an interface which extends an array-like builtin interface at

stc/crates/stc_ts_file_analyzer/src/analyzer/types/mod.rs

Lines 83 to 99 in 56ae129

match ty.normalize() {
Type::Lit(..)
| Type::TypeLit(..)
| Type::Interface(..)
| Type::Class(..)
| Type::ClassDef(..)
| Type::Tuple(..)
| Type::Function(..)
| Type::Constructor(..)
| Type::EnumVariant(..)
| Type::Enum(..)
| Type::Param(_)
| Type::Module(_)
| Type::Tpl(..) => return Ok(ty),
_ => {}
}

@kdy1 kdy1 added bug Something isn't working good first issue Good for newcomers labels Nov 6, 2022
@kdy1 kdy1 added this to the 1. Correctness milestone Nov 6, 2022
@b4s36t4
Copy link

b4s36t4 commented Nov 7, 2022

@kdy1 want to work on this. Just wanted to understand adding the check for array like extending to the match block should solve this right?!

@kdy1
Copy link
Member Author

kdy1 commented Nov 7, 2022

Thank you and yes I think you need to check the length property too. Without it it can't be treated as a tuple (Tuple with unknown length is Array, not Tuple)

@kdy1
Copy link
Member Author

kdy1 commented Nov 15, 2022

@b4s36t4 Are you working on it? If not, feel free to tell me. I'm asking because this is a good-first-issue

@devgony
Copy link

devgony commented Dec 17, 2022

Let me try this issue!
Is it right way to return Type::Tpl instead of Type::Array in Type::Interface.extends?

@kdy1
Copy link
Member Author

kdy1 commented Dec 17, 2022

I think instead you should add logic to assign_without_wrapping.
You can check if the other one is an interface with special extends after checking that one type is an Type::Array.

The other way is to add logic to Analyzer.normalize, and if you select this path you can add logic to match on a special interface and return Type::Arrray

@kdy1
Copy link
Member Author

kdy1 commented Dec 17, 2022

@devgony And thank you!

@devgony devgony linked a pull request Dec 18, 2022 that will close this issue
Open
@kdy1
Copy link
Member Author

kdy1 commented Mar 23, 2023

@devgony Are you still willing to work on this?

@devgony
Copy link

devgony commented Mar 23, 2023

@kdy1 Yes! I'm still working on it at local! i will push to github first.

@kdy1
Copy link
Member Author

kdy1 commented Mar 23, 2023

Thank you! I just wanted to check 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@devgony devgony

Labels
bug Something isn't working good first issue Good for newcomers
Milestone
v0.0.1: Correctness
Development

Successfully merging a pull request may close this issue.

Make tuple and custom array declared as interface compatible devgony/stc
3 participants
@kdy1 @devgony @b4s36t4