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

Improve Interface type params #956

Closed
1 of 2 tasks
awareness481 opened this issue Mar 25, 2023 · 7 comments · Fixed by #1011
Closed
1 of 2 tasks

Improve Interface type params #956

awareness481 opened this issue Mar 25, 2023 · 7 comments · Fixed by #1011
Assignees

Comments

@awareness481
Copy link
Contributor

awareness481 commented Mar 25, 2023

  • Interface type params not validated
    interface A<T extends string> {}
    type a = A<number> // required error, not reported
  • If two interfaces of the same name and the second one has more type params, there's a panic.
    interface A<T extends string> {}
    interface A<T extends number, U extends number> {}
    type a = A<string, number>; // panics
    thread 'main' panicked at 'index out of bounds: the len is 1 but the index is 1', 
    crates/stc_ts_file_analyzer/src/analyzer/decl_merging.rs:109:43
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@awareness481
Copy link
Contributor Author

If you agree with the issue, please assign me as I want to try fixing this, thanks ^^

@kdy1 kdy1 added this to the v0.0.1: Correctness milestone Mar 26, 2023
@kdy1
Copy link
Member

kdy1 commented Mar 26, 2023

Thank you!

@sunrabbit123
Copy link
Collaborator

@awareness481
Are you running Interface type params not validated?
I'm getting an error on that part, so I'm guessing you need to assign a worker to it.

The problem is caused by propagating generics and not checking for type.

I think the right place to work on it is in analyzer/generic/mod.rs:122.

If you're working on it, you can use the
How far along are you?

@awareness481
Copy link
Contributor Author

@sunrabbit123 I was actually thinking of removing the assignment from myself because when I tried it I couldn't get it working. Do you want to give it a go yourself? Please feel free to

@sunrabbit123
Copy link
Collaborator

sunrabbit123 commented Apr 25, 2023

I've been playing around with it a bit now, and I've found that
Unfortunately, it's working fine, but I'm getting a lot of regressions. 😭
image


It wasn't a good location.
As you can see from the picture, the value of left should have been string | number | symbol, but instead it was
All we could get from that location was an Index type.

kdy1 pushed a commit that referenced this issue May 18, 2023
**Description:**
```ts
interface A<T extends string> {}
type a = A<number> // required error, not reported
```

**Related issue:**

 - Closes #956.
@sunrabbit123
Copy link
Collaborator

revert this issue
#1029

@sunrabbit123
Copy link
Collaborator

#1053

Close the issue because it has been implemented in that location.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants