8000 Fix element access exception caused by invalid TS by Perryvw · Pull Request #1410 · TypeScriptToLua/TypeScriptToLua · GitHub
[go: up one dir, main page]

Skip to content

Fix element access exception caused by invalid TS #1410

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

Merged
merged 2 commits into from
Feb 26, 2023

Conversation

Perryvw
Copy link
Member
@Perryvw Perryvw commented Feb 26, 2023

No description provided.

@@ -225,3 +226,14 @@ describe("array destructuring optimization", () => {
.expectToMatchJsResult();
});
});

test("no exception from invalid TS syntax", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do consider these "sequence expressions" (a, b execute a, then use b as a result) valid syntax and the code below is just let [a, b] = testFunc(5)[a, b] = testFunc(b)

This test is more "can use sequence expressions inside an element access expression without fatal errors" which isn't related to destructuring

Don't want to cover all potential invalid TS code!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to at least not throw as long as a valid AST is passed into tstl. I guess I should not call it invalid syntax because the syntax itself is valid, it's just that there are also semantic errors reported by TS, in which case we will not guarantee we produce correct output, but there should be no exception.

@Perryvw Perryvw merged commit 4511e17 into master Feb 26, 2023
@Perryvw Perryvw deleted the exception-elementaccess branch February 26, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0