-
Notifications
You must be signed in to change notification settings - Fork 145
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
feat: function signature detection #1699
feat: function signature detection #1699
Conversation
Adds an enumeration of base accepted method signatures, and implements a detector for go functions.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lkingland The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1699 +/- ##
==========================================
+ Coverage 62.84% 62.87% +0.02%
==========================================
Files 93 94 +1
Lines 11967 12298 +331
==========================================
+ Hits 7521 7732 +211
- Misses 3757 3865 +108
- Partials 689 701 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
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.
Overall looks good to me. Just a few small nits to address for consistency across the codebase. Currently, none of the other runtimes has instanced based functions, but I suppose that shouldn't really matter and may come over time.
return | ||
} | ||
for _, file := range files { | ||
filename := filepath.Join(dir, file.Name()) |
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.
I wonder if it could be a condition that the filename be handle.go
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.
Since most languages don't have any file name requirements, I didn't want us to impose anything.
Scanning each file in the directory looking for the expected signature should be trivially fast.
Also, I personally think that, now that we support "instanced" function signatures, the filename for those should be "function.go". So this is compatible with both templates: static handle.go
and an instanced function.go
. Or, if a user is creating an HTTP Rest API at api.example.com
, perhapse they want their code in api.go
🤷🏻
} | ||
|
||
func (d goDetector) hasFunctionDeclaration(filename, function string) bool { | ||
astFile, err := parser.ParseFile(token.NewFileSet(), filename, nil, parser.SkipObjectResolution) |
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.
I wonder if there's a golang parser for all of our runtimes! :)
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.
Wouldn't that be helpful! I wouldn't be suprised if there were a parser for all of them already out there.
Fortunately, we don't need a full parser; we just need to be able to split on tokens and have enough of a state machine to know if it's inside of a block comment or not. So even if we need to write a small parser for each language ourselves; it should be fairly straightforward. But let's hope they already exist! 🤞🏻
Co-authored-by: Lance Ball <lball@redhat.com>
Co-authored-by: Lance Ball <lball@redhat.com>
Co-authored-by: Lance Ball <lball@redhat.com>
Co-authored-by: Lance Ball <lball@redhat.com>
Co-authored-by: Lance Ball <lball@redhat.com>
/lgtm |
Adds an enumeration of base accepted method signatures, and implements a detector for go functions.
Introduces the concept of "Instanced" functions.
Mocks up where additional detectors would be added using a stubbed Python detector.
/kind enhancement