-
Notifications
You must be signed in to change notification settings - Fork 16
Fix handling variadic functions #56
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
Fix handling variadic functions #56
Conversation
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.
Hi, thanks for your contribution, the code looks good, but there are a couple of nuances that should be corrected
// unknown injection type, possible generics or other not known | ||
// go features on current moment | ||
// we can extend this function later for new cases | ||
if gate.IsVariadic && len(callExpr.Args) <= gate.Index { |
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.
It's worth adding a clarifying commentary on what is being tested here and why
// possible is some not importable std const like `errors` | ||
// or not known ast at this moment | ||
continue | ||
for idx := gate.Index; idx <= maxIdx; idx++ { |
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.
this idx
is not used anywhere?
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.
Oh, thank you, it should be used instead of a gate.Index.
Also, do we actually have a test coverage for this part? Maybe it's worth adding it there.
@@ -19,6 +19,7 @@ type ( | |||
ArgumentDefinition Source // where method param type defined (func (a,b,c _int_)) | |||
Interface Interface // used interface for injection | |||
Implementations []Implementation // all code links to this param | |||
IsVariadic bool // function param is variadic |
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.
It would be great to add an example in the comments, like:
IsVariadic bool // function param is variadic (func (a bool, nums ...int))
thx, merged in v1.11.3 |
Closes #55