Add custom jsonfieldname linter to ensure Go field name matches JSON tag name #3757
Conversation
…tag name Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3757 +/- ##
=======================================
Coverage 91.11% 91.11%
=======================================
Files 187 187
Lines 16702 16702
=======================================
Hits 15218 15218
Misses 1296 1296
Partials 188 188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
|
@stevehipwell - @alexandear - @zyfy29 - might you have time for a code review? Thank you! |
| jsonfieldname: | ||
| type: module | ||
| description: Reports mismatches between Go field and JSON tag names. | ||
| original-url: github.com/google/go-github/v75/tools/jsonfieldname |
There was a problem hiding this comment.
Did you try @ldez tagliatelle that is bundled with golangci-lint?
There was a problem hiding this comment.
@gmlewis I'm happy to review but this seems like a good suggestion?
There was a problem hiding this comment.
Let's try enabling tagliatelle
There was a problem hiding this comment.
Did you try @ldez tagliatelle that is bundled with golangci-lint?
Yes, I did, but couldn't coerce it to do what I wanted, where the JSON tag is the source-of-truth, not the Go field name.
Others are welcome to try to get the same results as this PR if you wish, but I was unable to.
There was a problem hiding this comment.
Let's try enabling tagliatelle
OK, @alexandear, then I will mark this PR as "DO NOT MERGE" until you are able to get comparable results with tagliatelle.
There was a problem hiding this comment.
I may add an option about that. Don't hesitate to open an issue https://github.com/ldez/tagliatelle.
There was a problem hiding this comment.
Note, @alexandear - that your PR should make NO breaking API changes (as I have done here), but instead should flag inconsistencies with // TODO: ... (as I've also done) so that they can be addressed in separate "Breaking API Change" PRs.
The majority of the existing inconsistencies are actually due to agreed-upon discussions where the renaming was deemed to be clearer for end-users of this client library (although you can see that I flagged many with TODO to investigate as some appear to be mistakes).
gmlewis
added
waiting for reply
NeedsInvestigation
DO NOT MERGE
Note that there are many Go field names that do not match the JSON tag names, but changing them all would be a massive breaking API change that is probably unnecessary, and can instead be addressed on a one-by-one basis.
Additionally, one JSON tag had a typo which I've fixed in this PR.