-
-
Notifications
You must be signed in to change notification settings - Fork 74
Fix: Parameter with assignation provide type annotations (fixes #146) #147
Conversation
LGTM |
Thanks so much for your efforts, @weirdpattern! I've been so swamped in January but things are set to improve from next week. I will try my best to get to this at the weekend, but it will be by Monday at the latest. |
Perfect, could you please also take a look at issue #146, just want your input on that before I submit the PR |
@weirdpattern I think there are some more cases that need to include type annotations //Missing
function iii(...name: string): string {
return name;
}
//Handled correctly
function jjj(name: string): string {
return name;
}
//Fixed by this PR
function kkk(name: string = 'hello'): string {
return name;
}
//Missing
function lll(...name: string = 'hh'): string {
return name;
} Maybe we can avoid the duplicate code by setting a variable to the argument and then setting typeAnnotation on that. |
Hey @soda0289
I will look into this later today (I believe this was already being handled by the current code, but I can double check).
I don't think this even compiles, rest operators cannot have initializers here Are you seeing something that I'm not? |
You are right @weirdpattern those examples I gave are invalid typescript. I was just reading the code and noticied rest paremeters were missing the type annotation conversion. I did not know initializers with the rest parameter were invalid, or that it had to be an array but that makes sense. With the following code
The output I get for function declation is
For the rest element
I think type annotations are missing from the argument property of the RestElement node and would also need to have type annotations included. |
@soda0289 I think you are right. I will fix this today. Thanks! |
LGTM |
@soda0289 ok, I've included rest arguments in this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
…t#146) (eslint#147) * Fix: Parameter with assignation provide type annotations (fixes eslint#146) * Adding support for rest arguments
Adds code to process the type annotations on parameters that have assignations