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

Change span/error code of assignment failure due to wrong properties #215

Closed
kdy1 opened this issue Nov 5, 2022 · 1 comment
Closed
Labels
bug Something isn't working

Comments

@kdy1
Copy link
Member

kdy1 commented Nov 5, 2022

Problem

Wanted: [RefError { line: 16, column: 5, code: "TS2411" }, RefError { line: 23, column: 5, code: "TS2411" }, RefError { line: 24, column: 5, code: "TS2411" }, RefError { line: 31, column: 5, code: "TS2411" }, RefError { line: 32, column: 5, code: "TS2411" }, RefError { line: 37, column: 8, code: "TS2741" }, RefError { line: 38, column: 8, code: "TS2741" }]
Unwanted: [(36, "TS2322")]

All required errors: [RefError { line: 15, column: 5, code: "TS2411" }, RefError { line: 16, column: 5, code: "TS2411" }, RefError { line: 23, column: 5, code: "TS2411" }, RefError { line: 24, column: 5, code: "TS2411" }, RefError { line: 31, column: 5, code: "TS2411" }, RefError { line: 32, column: 5, code: "TS2411" }, RefError { line: 37, column: 8, code: "TS2741" }, RefError { line: 38, column: 8, code: "TS2741" }]
All actual errors: [(15, "TS2411"), (36, "TS2322")]', crates/stc_ts_type_checker/tests/tsc.rs:742:17

We report an error on line 36. To be exact, we are reporting an assignment failure (TS2322) on VarDeclarator.

image

But we should report multiple TS2741 (missing properties) instead and on line 37 and 38.

Actually, we have logic for creating TS2741.

if !missing_fields.is_empty() {
errors.push(Error::MissingFields {
span,
fields: missing_fields,
});
}

But because tsc uses TS2322 for many kinds of missing properties, we have extra logic for converting it to TS2322.
We should change it not to use such logic for assignments to index signatures

Solution

We should preserve MissingFields in more cases and call nested assign with the span of properties in the RHS of assignments.

Note

This issue is hard to fix I guess

@kdy1 kdy1 added the bug Something isn't working label Nov 5, 2022
@kdy1 kdy1 added this to the 1. Correctness milestone Nov 5, 2022
@kdy1 kdy1 self-assigned this Nov 10, 2022
@kdy1 kdy1 removed their assignment Nov 10, 2022
@kdy1 kdy1 self-assigned this Nov 17, 2022
@kdy1 kdy1 removed their assignment Nov 26, 2022
@kdy1
Copy link
Member Author

kdy1 commented Mar 4, 2023

Closing as postponed. See #768

@kdy1 kdy1 closed this as completed Mar 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

1 participant