Complain when a non-void function lacks a return expresson.#147
Complain when a non-void function lacks a return expresson.#147DanielRosenwasser merged 5 commits intomasterfrom
Conversation
There was a problem hiding this comment.
This seems like an unfortunate consequence of #139.
|
Baselines 🆗 |
src/compiler/checker.ts
Outdated
There was a problem hiding this comment.
Did we used to support a getter with a single throw statement?
There was a problem hiding this comment.
Yes; however, we erroneously permitted the following in both the old compiler and the new one prior to this commit.
class C {
get foo() {
return;
}
}There was a problem hiding this comment.
Can you add an explanation why this is error? Per TypeScript spec (and ECMA 262) return statement that lacks expression returns the value undefined
There was a problem hiding this comment.
Yes, This is crazy world of JavaScript. Also restricting this is a potential breaking change. I'd say that these issues should be traced by some sort of linter tool
There was a problem hiding this comment.
Well yes, but doesn't the following also implicitly return undefined?
class C {
get foo() {
}
}Additionally, the following should be permissible by the same reasoning, meaning that the entire check should be removed.
function f(): string {
return;
}I think that the rules for the check should be the same between get accessors and functions with type annotations. If not, we may need to open this up to discussion, because then maybe we don't need the check.
Edit: Sorry @vladima, didn't see your last response, and I decided to remove my answer and think the matter over a bit more before posting this.
There was a problem hiding this comment.
Adding 'Breaking Change' label to the original issue then
There was a problem hiding this comment.
I have opened up #162 to prevent this issue from blocking the fix.
I'll simply amend this pull request so that it does not cause a breaking change.
|
Looks great! |
There was a problem hiding this comment.
This is legal JavaScript code and should not be an error. Return statement without expression returns undefined
src/compiler/checker.ts
Outdated
There was a problem hiding this comment.
no need for parens around the parameter.
In effect this fixes #62. Also - Changes the error message for get accessors lacking return expressions. - Actually checks for return expressions instead of return statements for get-accessors. - Removes fancy quotes. - Corrects errors in the compiler caught by the new check. - Simplified `checkAndAggregateReturnTypes` by extracting it out to `visitReturnStatements`.
|
👍 |
Complain when a non-void function lacks a return expresson.
There was a problem hiding this comment.
no parens around (returnStatement)
As per feedback in pull request #147.
In effect this fixes #62
Also
- Changes the error message for get accessors lacking return expressions.
- Actually checks for return expressions instead of return statements for get-accessors.
- Removes fancy quotes.
- Corrects errors in the compiler caught by the new check.
- Simplified
checkAndAggregateReturnTypesby extracting it out tovisitReturnStatements.