10000 Fix handling variadic functions by ilya-hontarau · Pull Request #56 · fe3dback/go-arch-lint · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Conversation

ilya-hontarau
Copy link
Contributor
@ilya-hontarau ilya-hontarau commented Jun 8, 2024

Closes #55

Copy link
Owner
@fe3dback fe3dback left a 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 {
Copy link
Owner

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++ {
Copy link
Owner

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?

Copy link
Contributor Author

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
Copy link
Owner

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))

< 8000 /span>

@ilya-hontarau ilya-hontarau requested a review from fe3dback June 9, 2024 14:37
@fe3dback fe3dback merged commit 28c8ccb into fe3dback:master Jun 12, 2024
@fe3dback
Copy link
Owner

thx, merged in v1.11.3

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.

Incorrect handling variadic parameters
2 participants
0