Skip to content

Re-add undefined assignability check #53561

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

Closed
wants to merge 2 commits into from

Conversation

MariaSolOs
Copy link
Contributor

Follow up to #53490 (refer to @ahejlsberg's comment).

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Mar 28, 2023
@MariaSolOs MariaSolOs requested a review from ahejlsberg March 28, 2023 18:00
@@ -221,6 +222,8 @@ tests/cases/conformance/types/unknown/unknownType1.ts(181,5): error TS2322: Type
// Functions with unknown return type don't need return expressions

function f27(): unknown {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just to be sure: This error should be emitted because noImplicitReturns is true by default right?

Copy link
Member

Choose a reason for hiding this comment

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

Actually no, this error is issued regardless of noImplicitReturns. It occurs when there are no return statements at all in the function body (i.e. when hasExplicitReturn is false).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait then this comment is incorrect.

Copy link
Member

Choose a reason for hiding this comment

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

I'm so confused. Can we get a playground link of what was desirable in TS 5.0, but is now incorrect in nightly?

@MariaSolOs MariaSolOs marked this pull request as ready for review March 28, 2023 18:51
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@jakebailey
Copy link
Member

Can you explicitly add the test from: #53490 (comment)

@MariaSolOs
Copy link
Contributor Author

MariaSolOs commented Mar 28, 2023

Can you explicitly add the test from: #53490 (comment)

Will do, but why didn't this test catch it?

@jakebailey
Copy link
Member

Hm, that is effectively the right test, yes. It sounds like maybe everyone has a different idea about the desired behavior? Or, is this test's settings matrix missing noImplicitReturns?

@MariaSolOs
Copy link
Contributor Author

Hmmm. I think it would make sense to add // @noImplicitReturns: true, false to these and these.

@MariaSolOs MariaSolOs closed this Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants