-
Notifications
You must be signed in to change notification settings - Fork 64
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
Refactor the contract analyzer #239
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #239 +/- ##
==========================================
- Coverage 87.55% 87.55% -0.01%
==========================================
Files 63 63
Lines 7828 7825 -3
==========================================
- Hits 6854 6851 -3
Misses 796 796
Partials 178 178 ☔ View full report in Codecov by Sentry. |
ebec5a0
to
045d6f0
Compare
045d6f0
to
4c694b0
Compare
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.
Looks good. The refactors in this PR make sense to me.
This PR simply refactors the contract analyzer to prepare for further updates. Specifically, it
(1) Defines a separate type for
[]Contract
such that it can be made to be exported (i.e., satisfies theanalysis.Fact
interface) in the future.(2) Replaces a few places where we were returning initialized empty slices with simple
nil
. In principle we should not distinguish nil slice vs empty nonnil slice.(3) Shortens a few variable names to help improve readability without introducing ambiguity (since the variables are all under the
functioncontracts
package already).(4) Moved test cases from
src/go.uber.org/functioncontracts/xxx
tosrc/go.uber.org/xxx
insidefunctioncontracts
package. Those tests are only meant to be used forfunctioncontracts
package, such that an extra folder only increases nesting but does not help test organization.Note that this PR does not bring any functionality changes.