From 2e7049511d6102323d2a0ab88dbfcaaf5d77bc0f Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Sun, 5 Oct 2025 21:34:31 -0400 Subject: [PATCH 1/3] Add custom jsonfieldname linter to ensure Go field name matches JSON tag name Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- .custom-gcl.yml | 4 +- .golangci.yml | 7 +- github/orgs.go | 2 +- tools/jsonfieldname/go.mod | 13 + tools/jsonfieldname/go.sum | 10 + tools/jsonfieldname/jsonfieldname.go | 270 ++++++++++++++++++ tools/jsonfieldname/jsonfieldname_test.go | 20 ++ .../testdata/src/has-warnings/main.go | 15 + .../testdata/src/no-warnings/main.go | 15 + 9 files changed, 353 insertions(+), 3 deletions(-) create mode 100644 tools/jsonfieldname/go.mod create mode 100644 tools/jsonfieldname/go.sum create mode 100644 tools/jsonfieldname/jsonfieldname.go create mode 100644 tools/jsonfieldname/jsonfieldname_test.go create mode 100644 tools/jsonfieldname/testdata/src/has-warnings/main.go create mode 100644 tools/jsonfieldname/testdata/src/no-warnings/main.go diff --git a/.custom-gcl.yml b/.custom-gcl.yml index 0a36cedd21d..b1ec685e83c 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -1,4 +1,6 @@ version: v2.2.2 plugins: - - module: "github.com/google/go-github/v68/tools/sliceofpointers" + - module: "github.com/google/go-github/v75/tools/jsonfieldname" + path: ./tools/jsonfieldname + - module: "github.com/google/go-github/v75/tools/sliceofpointers" path: ./tools/sliceofpointers diff --git a/.golangci.yml b/.golangci.yml index b745b8d85a7..248bfd4572b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,6 +14,7 @@ linters: - godot - goheader - gosec + - jsonfieldname - misspell - nakedret - paralleltest @@ -136,10 +137,14 @@ linters: os-create-temp: true os-temp-dir: true custom: + jsonfieldname: + type: module + description: Reports mismatches between Go field and JSON tag names. + original-url: github.com/google/go-github/v75/tools/jsonfieldname sliceofpointers: type: module description: Reports usage of []*string and slices of structs without pointers. - original-url: github.com/google/go-github/v68/tools/sliceofpointers + original-url: github.com/google/go-github/v75/tools/sliceofpointers exclusions: rules: - linters: diff --git a/github/orgs.go b/github/orgs.go index bec49bbff5f..ed34198a14c 100644 --- a/github/orgs.go +++ b/github/orgs.go @@ -100,7 +100,7 @@ type Organization struct { // MembersCanDeleteRepositories toggles whether members with admin permissions can delete a repository. MembersCanDeleteRepositories *bool `json:"members_can_delete_repositories,omitempty"` // MembersCanChangeRepoVisibility toggles whether members with admin permissions can change the visibility for a repository. - MembersCanChangeRepoVisibility *bool `json:"members_can_change_repo_visiblilty,omitempty"` + MembersCanChangeRepoVisibility *bool `json:"members_can_change_repo_visibility,omitempty"` // MembersCanInviteOutsideCollaborators toggles whether members with admin permissions can invite outside collaborators. MembersCanInviteOutsideCollaborators *bool `json:"members_can_invite_outside_collaborators,omitempty"` // MembersCanDeleteIssues toggles whether members with admin permissions can delete issues. diff --git a/tools/jsonfieldname/go.mod b/tools/jsonfieldname/go.mod new file mode 100644 index 00000000000..3ae53f27ac4 --- /dev/null +++ b/tools/jsonfieldname/go.mod @@ -0,0 +1,13 @@ +module tools/jsonfieldname + +go 1.24.0 + +require ( + github.com/golangci/plugin-module-register v0.1.1 + golang.org/x/tools v0.29.0 +) + +require ( + golang.org/x/mod v0.22.0 // indirect + golang.org/x/sync v0.10.0 // indirect +) diff --git a/tools/jsonfieldname/go.sum b/tools/jsonfieldname/go.sum new file mode 100644 index 00000000000..c2f7392bb23 --- /dev/null +++ b/tools/jsonfieldname/go.sum @@ -0,0 +1,10 @@ +github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c= +github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= +golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= +golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ= +golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/tools v0.29.0 h1:Xx0h3TtM9rzQpQuR4dKLrdglAmCEN5Oi+P74JdhdzXE= +golang.org/x/tools v0.29.0/go.mod h1:KMQVMRsVxU6nHCFXrBPhDB8XncLNLM0lIy/F14RP588= diff --git a/tools/jsonfieldname/jsonfieldname.go b/tools/jsonfieldname/jsonfieldname.go new file mode 100644 index 00000000000..7267c845e61 --- /dev/null +++ b/tools/jsonfieldname/jsonfieldname.go @@ -0,0 +1,270 @@ +// Copyright 2025 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package jsonfieldname is a custom linter to be used by +// golangci-lint to find instances where the Go field name +// of a struct does not match the JSON tag name. +// It honors idiomatic Go initialisms and handles the +// special case of `Github` vs `GitHub` as agreed upon +// by the original author of the repo. +package jsonfieldname + +import ( + "go/ast" + "go/token" + "reflect" + "regexp" + "strings" + + "github.com/golangci/plugin-module-register/register" + "golang.org/x/tools/go/analysis" +) + +func init() { + register.Plugin("jsonfieldname", New) +} + +// JSONFieldNamePlugin is a custom linter plugin for golangci-lint. +type JSONFieldNamePlugin struct{} + +// New returns an analysis.Analyzer to use with golangci-lint. +func New(_ any) (register.LinterPlugin, error) { + return &JSONFieldNamePlugin{}, nil +} + +// BuildAnalyzers builds the analyzers for the JSONFieldNamePlugin. +func (f *JSONFieldNamePlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { + return []*analysis.Analyzer{ + { + Name: "jsonfieldname", + Doc: "Reports mismatches between Go field and JSON tag names.", + Run: run, + }, + }, nil +} + +// GetLoadMode returns the load mode for the JSONFieldNamePlugin. +func (f *JSONFieldNamePlugin) GetLoadMode() string { + return register.LoadModeSyntax +} + +func run(pass *analysis.Pass) (any, error) { + for _, file := range pass.Files { + ast.Inspect(file, func(n ast.Node) bool { + if n == nil { + return false + } + + switch t := n.(type) { + case *ast.TypeSpec: + structType, ok := t.Type.(*ast.StructType) + if !ok { + return true + } + structName := t.Name.Name + + // Only check exported structs. + if !ast.IsExported(structName) { + return true + } + + for _, field := range structType.Fields.List { + if field.Tag == nil || len(field.Names) == 0 { + continue + } + + goField := field.Names[0] + tagValue := strings.Trim(field.Tag.Value, "`") + structTag := reflect.StructTag(tagValue) + jsonTagName, ok := structTag.Lookup("json") + if !ok || jsonTagName == "-" { + continue + } + jsonTagName = strings.TrimSuffix(jsonTagName, ",omitempty") + + checkGoFieldName(structName, goField.Name, jsonTagName, goField.Pos(), pass) + } + } + + return true + }) + } + return nil, nil +} + +func checkGoFieldName(structName, goFieldName, jsonTagName string, tokenPos token.Pos, pass *analysis.Pass) { + fullName := structName + "." + goFieldName + if allowedExceptions[fullName] { + return + } + + want, alternate := jsonTagToPascal(jsonTagName) + if goFieldName != want && goFieldName != alternate { + const msg = "change Go field name %q to %q for JSON tag %q in struct %q" + pass.Reportf(tokenPos, msg, goFieldName, want, jsonTagName, structName) + } +} + +func splitJSONTag(jsonTagName string) []string { + if strings.Contains(jsonTagName, "_") { + return strings.Split(jsonTagName, "_") + } + + if strings.Contains(jsonTagName, "-") { + return strings.Split(jsonTagName, "-") + } + + if strings.ToLower(jsonTagName) == jsonTagName { // single word + return []string{jsonTagName} + } + + s := camelCaseRE.ReplaceAllString(jsonTagName, "$1 $2") + parts := strings.Fields(s) + for i, part := range parts { + parts[i] = strings.ToLower(part) + } + + return parts +} + +var camelCaseRE = regexp.MustCompile(`([a-z0-9])([A-Z])`) + +func jsonTagToPascal(jsonTagName string) (want, alternate string) { + parts := splitJSONTag(jsonTagName) + alt := make([]string, len(parts)) + for i, part := range parts { + alt[i] = part + if part == "" { + continue + } + upper := strings.ToUpper(part) + if initialisms[upper] { + parts[i] = upper + alt[i] = upper + } else if specialCase, ok := specialCases[upper]; ok { + parts[i] = specialCase + alt[i] = specialCase + } else if possibleAlternate, ok := possibleAlternates[upper]; ok { + parts[i] = possibleAlternate + alt[i] = strings.ToUpper(part[:1]) + part[1:] + } else { + parts[i] = strings.ToUpper(part[:1]) + part[1:] + alt[i] = parts[i] + } + } + return strings.Join(parts, ""), strings.Join(alt, "") +} + +var allowedExceptions = map[string]bool{ + "ActionsCacheUsageList.RepoCacheUsage": true, // TODO: RepoCacheUsages ? + "AuditEntry.ExternalIdentityNameID": true, + "AuditEntry.Timestamp": true, + "CheckSuite.AfterSHA": true, + "CheckSuite.BeforeSHA": true, + "CodeSearchResult.CodeResults": true, + "CodeSearchResult.Total": true, + "CommitAuthor.Login": true, + "CommitsSearchResult.Commits": true, + "CommitsSearchResult.Total": true, + "CreateOrgInvitationOptions.TeamID": true, // TODO: TeamIDs + "DependencyGraphSnapshot.Sha": true, // TODO: SHA + "Discussion.DiscussionCategory": true, // TODO: Category ? + "EditOwner.OwnerInfo": true, + "Event.RawPayload": true, + "HookRequest.RawPayload": true, + "HookResponse.RawPayload": true, + "Issue.PullRequestLinks": true, // TODO: PullRequest + "IssueImportRequest.IssueImport": true, // TODO: Issue + "IssuesSearchResult.Issues": true, // TODO: Items + "IssuesSearchResult.Total": true, + "LabelsSearchResult.Labels": true, // TODO: Items + "LabelsSearchResult.Total": true, + "ListCheckRunsResults.Total": true, + "ListCheckSuiteResults.Total": true, + "ListCustomDeploymentRuleIntegrationsResponse.AvailableIntegrations": true, + "ListDeploymentProtectionRuleResponse.ProtectionRules": true, + "OrganizationCustomRepoRoles.CustomRepoRoles": true, // TODO: CustomRoles + "OrganizationCustomRoles.CustomRepoRoles": true, // TODO: Roles + "PreReceiveHook.ConfigURL": true, + "ProjectV2ItemEvent.ProjectV2Item": true, // TODO: ProjectsV2Item + "Protection.RequireLinearHistory": true, // TODO: RequiredLinearHistory + "ProtectionRequest.RequireLinearHistory": true, // TODO: RequiredLinearHistory + "PullRequestComment.InReplyTo": true, // TODO: InReplyToID + "PullRequestReviewsEnforcementRequest.BypassPullRequestAllowancesRequest": true, // TODO: BypassPullRequestAllowances + "PullRequestReviewsEnforcementRequest.DismissalRestrictionsRequest": true, // TODO: DismissalRestrictions + "PullRequestReviewsEnforcementUpdate.BypassPullRequestAllowancesRequest": true, // TODO: BypassPullRequestAllowances + "PullRequestReviewsEnforcementUpdate.DismissalRestrictionsRequest": true, // TODO: DismissalRestrictions + "Reactions.MinusOne": true, + "Reactions.PlusOne": true, + "RepositoriesSearchResult.Repositories": true, + "RepositoriesSearchResult.Total": true, + "RepositoryVulnerabilityAlert.GitHubSecurityAdvisoryID": true, + "SCIMDisplayReference.Ref": true, + "SecretScanningAlertLocationDetails.Startline": true, // TODO: StartLine + "SecretScanningPatternOverride.Bypassrate": true, // TODO: BypassRate + "StarredRepository.Repository": true, // TODO: Repo + "Timeline.Requester": true, // TODO: ReviewRequester + "Timeline.Reviewer": true, // TODO: RequestedReviewer + "TopicsSearchResult.Topics": true, // TODO: Items + "TopicsSearchResult.Total": true, + "TotalCacheUsage.TotalActiveCachesUsageSizeInBytes": true, // TODO: TotalActiveCachesSizeInBytes + "TransferRequest.TeamID": true, // TODO: TeamIDs + "Tree.Entries": true, + "User.LdapDn": true, // TODO: LDAPDN + "UsersSearchResult.Total": true, + "UsersSearchResult.Users": true, + "WeeklyStats.Additions": true, + "WeeklyStats.Commits": true, + "WeeklyStats.Deletions": true, + "WeeklyStats.Week": true, +} + +// Common Go initialisms that should be all caps. +var initialisms = map[string]bool{ + "API": true, "ASCII": true, + "CAA": true, "CAS": true, "CNAME": true, "CPU": true, + "CSS": true, "CWE": true, "CVE": true, "CVSS": true, + "DN": true, "DNS": true, + "EOF": true, "EPSS": true, + "GB": true, "GHSA": true, "GPG": true, "GUID": true, + "HTML": true, "HTTP": true, "HTTPS": true, + "ID": true, "IDE": true, "IDP": true, "IP": true, "JIT": true, + "JSON": true, + "LDAP": true, "LFS": true, "LHS": true, + "MD5": true, "MS": true, "MX": true, + "NPM": true, "NTP": true, "NVD": true, + "OID": true, "OS": true, + "PEM": true, "PR": true, "QPS": true, + "RAM": true, "RHS": true, "RPC": true, + "SAML": true, "SBOM": true, "SCIM": true, + "SHA": true, "SHA1": true, "SHA256": true, + "SKU": true, "SLA": true, "SMTP": true, "SNMP": true, + "SPDX": true, "SPDXID": true, "SQL": true, "SSH": true, + "SSL": true, "SSO": true, "SVN": true, + "TCP": true, "TFVC": true, "TLS": true, "TTL": true, + "UDP": true, "UI": true, "UID": true, "UUID": true, + "URI": true, "URL": true, "UTF8": true, + "VCF": true, "VCS": true, "VM": true, + "XML": true, "XMPP": true, "XSRF": true, "XSS": true, +} + +var specialCases = map[string]string{ + "CPUS": "CPUs", + "CWES": "CWEs", + "GRAPHQL": "GraphQL", + "HREF": "HRef", + "IDS": "IDs", + "IPS": "IPs", + "OAUTH": "OAuth", + "OPENAPI": "OpenAPI", + "URLS": "URLs", +} + +var possibleAlternates = map[string]string{ + "ORGANIZATION": "Org", + "ORGANIZATIONS": "Orgs", + "REPOSITORY": "Repo", + "REPOSITORIES": "Repos", +} diff --git a/tools/jsonfieldname/jsonfieldname_test.go b/tools/jsonfieldname/jsonfieldname_test.go new file mode 100644 index 00000000000..fa5044c7aba --- /dev/null +++ b/tools/jsonfieldname/jsonfieldname_test.go @@ -0,0 +1,20 @@ +// Copyright 2025 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package jsonfieldname + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestRun(t *testing.T) { + t.Parallel() + testdata := analysistest.TestData() + plugin, _ := New(nil) + analyzers, _ := plugin.BuildAnalyzers() + analysistest.Run(t, testdata, analyzers[0], "has-warnings", "no-warnings") +} diff --git a/tools/jsonfieldname/testdata/src/has-warnings/main.go b/tools/jsonfieldname/testdata/src/has-warnings/main.go new file mode 100644 index 00000000000..3043b48ed42 --- /dev/null +++ b/tools/jsonfieldname/testdata/src/has-warnings/main.go @@ -0,0 +1,15 @@ +// Copyright 2025 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +type Example struct { + GitHubThing string `json:"github_thing"` // want `change Go field name "GitHubThing" to "GithubThing" for JSON tag "github_thing" in struct "Example"` + Id string `json:"id,omitempty"` // want `change Go field name "Id" to "ID" for JSON tag "id" in struct "Example"` + strings string `json:"strings,omitempty"` // want `change Go field name "strings" to "Strings" for JSON tag "strings" in struct "Example"` +} + +func main() { +} diff --git a/tools/jsonfieldname/testdata/src/no-warnings/main.go b/tools/jsonfieldname/testdata/src/no-warnings/main.go new file mode 100644 index 00000000000..dd297daae36 --- /dev/null +++ b/tools/jsonfieldname/testdata/src/no-warnings/main.go @@ -0,0 +1,15 @@ +// Copyright 2025 The go-github AUTHORS. All rights reserved. +// +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +type Example struct { + GithubThing string `json:"github_thing"` // Should not be flagged + ID string `json:"id,omitempty"` // Should not be flagged + Strings string `json:"strings,omitempty"` // Should not be flagged +} + +func main() { +} From 9652f22ddf06a702ed4d7731b2004c4f4291b846 Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Fri, 17 Oct 2025 11:31:19 -0400 Subject: [PATCH 2/3] Address review feedback Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- .golangci.yml | 62 +++++++++++++++++++ tools/jsonfieldname/jsonfieldname.go | 2 +- .../testdata/src/has-warnings/main.go | 10 ++- .../testdata/src/no-warnings/main.go | 3 - 4 files changed, 67 insertions(+), 10 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d3d09dea6de..df31eeebaf3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -148,6 +148,68 @@ linters: type: module description: Reports mismatches between Go field and JSON tag names. original-url: github.com/google/go-github/v75/tools/jsonfieldname + allowed-exceptions: + ActionsCacheUsageList.RepoCacheUsage # TODO: RepoCacheUsages ? + AuditEntry.ExternalIdentityNameID + AuditEntry.Timestamp + CheckSuite.AfterSHA + CheckSuite.BeforeSHA + CodeSearchResult.CodeResults + CodeSearchResult.Total + CommitAuthor.Login + CommitsSearchResult.Commits + CommitsSearchResult.Total + CreateOrgInvitationOptions.TeamID # TODO: TeamIDs + DependencyGraphSnapshot.Sha # TODO: SHA + Discussion.DiscussionCategory # TODO: Category ? + EditOwner.OwnerInfo + Event.RawPayload + HookRequest.RawPayload + HookResponse.RawPayload + Issue.PullRequestLinks # TODO: PullRequest + IssueImportRequest.IssueImport # TODO: Issue + IssuesSearchResult.Issues # TODO: Items + IssuesSearchResult.Total + LabelsSearchResult.Labels # TODO: Items + LabelsSearchResult.Total + ListCheckRunsResults.Total + ListCheckSuiteResults.Total + ListCustomDeploymentRuleIntegrationsResponse.AvailableIntegrations + ListDeploymentProtectionRuleResponse.ProtectionRules + OrganizationCustomRepoRoles.CustomRepoRoles # TODO: CustomRoles + OrganizationCustomRoles.CustomRepoRoles # TODO: Roles + PreReceiveHook.ConfigURL + ProjectV2ItemEvent.ProjectV2Item # TODO: ProjectsV2Item + Protection.RequireLinearHistory # TODO: RequiredLinearHistory + ProtectionRequest.RequireLinearHistory # TODO: RequiredLinearHistory + PullRequestComment.InReplyTo # TODO: InReplyToID + PullRequestReviewsEnforcementRequest.BypassPullRequestAllowancesRequest # TODO: BypassPullRequestAllowances + PullRequestReviewsEnforcementRequest.DismissalRestrictionsRequest # TODO: DismissalRestrictions + PullRequestReviewsEnforcementUpdate.BypassPullRequestAllowancesRequest # TODO: BypassPullRequestAllowances + PullRequestReviewsEnforcementUpdate.DismissalRestrictionsRequest # TODO: DismissalRestrictions + Reactions.MinusOne + Reactions.PlusOne + RepositoriesSearchResult.Repositories + RepositoriesSearchResult.Total + RepositoryVulnerabilityAlert.GitHubSecurityAdvisoryID + SCIMDisplayReference.Ref + SecretScanningAlertLocationDetails.Startline # TODO: StartLine + SecretScanningPatternOverride.Bypassrate # TODO: BypassRate + StarredRepository.Repository # TODO: Repo + Timeline.Requester # TODO: ReviewRequester + Timeline.Reviewer # TODO: RequestedReviewer + TopicsSearchResult.Topics # TODO: Items + TopicsSearchResult.Total + TotalCacheUsage.TotalActiveCachesUsageSizeInBytes # TODO: TotalActiveCachesSizeInBytes + TransferRequest.TeamID # TODO: TeamIDs + Tree.Entries + User.LdapDn # TODO: LDAPDN + UsersSearchResult.Total + UsersSearchResult.Users + WeeklyStats.Additions + WeeklyStats.Commits + WeeklyStats.Deletions + WeeklyStats.Week sliceofpointers: type: module description: Reports usage of []*string and slices of structs without pointers. diff --git a/tools/jsonfieldname/jsonfieldname.go b/tools/jsonfieldname/jsonfieldname.go index 7267c845e61..e23bce034e3 100644 --- a/tools/jsonfieldname/jsonfieldname.go +++ b/tools/jsonfieldname/jsonfieldname.go @@ -39,7 +39,7 @@ func (f *JSONFieldNamePlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { return []*analysis.Analyzer{ { Name: "jsonfieldname", - Doc: "Reports mismatches between Go field and JSON tag names.", + Doc: "Reports mismatches between Go field and JSON tag names. Note that the JSON tag name is the source-of-truth and the Go field name needs to match it.", Run: run, }, }, nil diff --git a/tools/jsonfieldname/testdata/src/has-warnings/main.go b/tools/jsonfieldname/testdata/src/has-warnings/main.go index 3043b48ed42..04670e20b1d 100644 --- a/tools/jsonfieldname/testdata/src/has-warnings/main.go +++ b/tools/jsonfieldname/testdata/src/has-warnings/main.go @@ -6,10 +6,8 @@ package main type Example struct { - GitHubThing string `json:"github_thing"` // want `change Go field name "GitHubThing" to "GithubThing" for JSON tag "github_thing" in struct "Example"` - Id string `json:"id,omitempty"` // want `change Go field name "Id" to "ID" for JSON tag "id" in struct "Example"` - strings string `json:"strings,omitempty"` // want `change Go field name "strings" to "Strings" for JSON tag "strings" in struct "Example"` -} - -func main() { + GitHubThing string `json:"github_thing"` // want `change Go field name "GitHubThing" to "GithubThing" for JSON tag "github_thing" in struct "Example"` + Id string `json:"id,omitempty"` // want `change Go field name "Id" to "ID" for JSON tag "id" in struct "Example"` + strings string `json:"strings,omitempty"` // want `change Go field name "strings" to "Strings" for JSON tag "strings" in struct "Example"` + camelcaseexample *int `json:"camelCaseExample,omitempty"` // want `change Go field name "camelcaseexample" to "CamelCaseExample" for JSON tag "camelCaseExample" in struct "Example"` } diff --git a/tools/jsonfieldname/testdata/src/no-warnings/main.go b/tools/jsonfieldname/testdata/src/no-warnings/main.go index dd297daae36..73119c771f0 100644 --- a/tools/jsonfieldname/testdata/src/no-warnings/main.go +++ b/tools/jsonfieldname/testdata/src/no-warnings/main.go @@ -10,6 +10,3 @@ type Example struct { ID string `json:"id,omitempty"` // Should not be flagged Strings string `json:"strings,omitempty"` // Should not be flagged } - -func main() { -} From 54a2ba53462a74989e99f3b96174a5addafdab5a Mon Sep 17 00:00:00 2001 From: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> Date: Fri, 17 Oct 2025 12:25:47 -0400 Subject: [PATCH 3/3] Address review feedback Signed-off-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com> --- .custom-gcl.yml | 6 +- .golangci.yml | 132 ++++++++++++++------------- tools/jsonfieldname/jsonfieldname.go | 104 +++++++-------------- 3 files changed, 103 insertions(+), 139 deletions(-) diff --git a/.custom-gcl.yml b/.custom-gcl.yml index b03078a28dc..c116a82b1df 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -1,8 +1,8 @@ version: v2.2.2 plugins: - - module: "github.com/google/go-github/v75/tools/fmtpercentv" + - module: "github.com/google/go-github/v76/tools/fmtpercentv" path: ./tools/fmtpercentv - - module: "github.com/google/go-github/v75/tools/jsonfieldname" + - module: "github.com/google/go-github/v76/tools/jsonfieldname" path: ./tools/jsonfieldname - - module: "github.com/google/go-github/v75/tools/sliceofpointers" + - module: "github.com/google/go-github/v76/tools/sliceofpointers" path: ./tools/sliceofpointers diff --git a/.golangci.yml b/.golangci.yml index df31eeebaf3..e64f5c11961 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -143,77 +143,79 @@ linters: fmtpercentv: type: module description: Reports usage of %d or %s in format strings. - original-url: github.com/google/go-github/v75/tools/fmtpercentv + original-url: github.com/google/go-github/v76/tools/fmtpercentv jsonfieldname: type: module description: Reports mismatches between Go field and JSON tag names. - original-url: github.com/google/go-github/v75/tools/jsonfieldname - allowed-exceptions: - ActionsCacheUsageList.RepoCacheUsage # TODO: RepoCacheUsages ? - AuditEntry.ExternalIdentityNameID - AuditEntry.Timestamp - CheckSuite.AfterSHA - CheckSuite.BeforeSHA - CodeSearchResult.CodeResults - CodeSearchResult.Total - CommitAuthor.Login - CommitsSearchResult.Commits - CommitsSearchResult.Total - CreateOrgInvitationOptions.TeamID # TODO: TeamIDs - DependencyGraphSnapshot.Sha # TODO: SHA - Discussion.DiscussionCategory # TODO: Category ? - EditOwner.OwnerInfo - Event.RawPayload - HookRequest.RawPayload - HookResponse.RawPayload - Issue.PullRequestLinks # TODO: PullRequest - IssueImportRequest.IssueImport # TODO: Issue - IssuesSearchResult.Issues # TODO: Items - IssuesSearchResult.Total - LabelsSearchResult.Labels # TODO: Items - LabelsSearchResult.Total - ListCheckRunsResults.Total - ListCheckSuiteResults.Total - ListCustomDeploymentRuleIntegrationsResponse.AvailableIntegrations - ListDeploymentProtectionRuleResponse.ProtectionRules - OrganizationCustomRepoRoles.CustomRepoRoles # TODO: CustomRoles - OrganizationCustomRoles.CustomRepoRoles # TODO: Roles - PreReceiveHook.ConfigURL - ProjectV2ItemEvent.ProjectV2Item # TODO: ProjectsV2Item - Protection.RequireLinearHistory # TODO: RequiredLinearHistory - ProtectionRequest.RequireLinearHistory # TODO: RequiredLinearHistory - PullRequestComment.InReplyTo # TODO: InReplyToID - PullRequestReviewsEnforcementRequest.BypassPullRequestAllowancesRequest # TODO: BypassPullRequestAllowances - PullRequestReviewsEnforcementRequest.DismissalRestrictionsRequest # TODO: DismissalRestrictions - PullRequestReviewsEnforcementUpdate.BypassPullRequestAllowancesRequest # TODO: BypassPullRequestAllowances - PullRequestReviewsEnforcementUpdate.DismissalRestrictionsRequest # TODO: DismissalRestrictions - Reactions.MinusOne - Reactions.PlusOne - RepositoriesSearchResult.Repositories - RepositoriesSearchResult.Total - RepositoryVulnerabilityAlert.GitHubSecurityAdvisoryID - SCIMDisplayReference.Ref - SecretScanningAlertLocationDetails.Startline # TODO: StartLine - SecretScanningPatternOverride.Bypassrate # TODO: BypassRate - StarredRepository.Repository # TODO: Repo - Timeline.Requester # TODO: ReviewRequester - Timeline.Reviewer # TODO: RequestedReviewer - TopicsSearchResult.Topics # TODO: Items - TopicsSearchResult.Total - TotalCacheUsage.TotalActiveCachesUsageSizeInBytes # TODO: TotalActiveCachesSizeInBytes - TransferRequest.TeamID # TODO: TeamIDs - Tree.Entries - User.LdapDn # TODO: LDAPDN - UsersSearchResult.Total - UsersSearchResult.Users - WeeklyStats.Additions - WeeklyStats.Commits - WeeklyStats.Deletions - WeeklyStats.Week + original-url: github.com/google/go-github/v76/tools/jsonfieldname + settings: + allowed-exceptions: + - ActionsCacheUsageList.RepoCacheUsage # TODO: RepoCacheUsages ? + - AuditEntry.ExternalIdentityNameID + - AuditEntry.Timestamp + - CheckSuite.AfterSHA + - CheckSuite.BeforeSHA + - CodeSearchResult.CodeResults + - CodeSearchResult.Total + - CommitAuthor.Login + - CommitsSearchResult.Commits + - CommitsSearchResult.Total + - CreateOrgInvitationOptions.TeamID # TODO: TeamIDs + - DependencyGraphSnapshot.Sha # TODO: SHA + - Discussion.DiscussionCategory # TODO: Category ? + - EditOwner.OwnerInfo + - EnterpriseLicensedUsers.GithubComSamlNameID # TODO: GithubComSAMLNameID + - Event.RawPayload + - HookRequest.RawPayload + - HookResponse.RawPayload + - Issue.PullRequestLinks # TODO: PullRequest + - IssueImportRequest.IssueImport # TODO: Issue + - IssuesSearchResult.Issues # TODO: Items + - IssuesSearchResult.Total + - LabelsSearchResult.Labels # TODO: Items + - LabelsSearchResult.Total + - ListCheckRunsResults.Total + - ListCheckSuiteResults.Total + - ListCustomDeploymentRuleIntegrationsResponse.AvailableIntegrations + - ListDeploymentProtectionRuleResponse.ProtectionRules + - OrganizationCustomRepoRoles.CustomRepoRoles # TODO: CustomRoles + - OrganizationCustomRoles.CustomRepoRoles # TODO: Roles + - PreReceiveHook.ConfigURL + - ProjectV2ItemEvent.ProjectV2Item # TODO: ProjectsV2Item + - Protection.RequireLinearHistory # TODO: RequiredLinearHistory + - ProtectionRequest.RequireLinearHistory # TODO: RequiredLinearHistory + - PullRequestComment.InReplyTo # TODO: InReplyToID + - PullRequestReviewsEnforcementRequest.BypassPullRequestAllowancesRequest # TODO: BypassPullRequestAllowances + - PullRequestReviewsEnforcementRequest.DismissalRestrictionsRequest # TODO: DismissalRestrictions + - PullRequestReviewsEnforcementUpdate.BypassPullRequestAllowancesRequest # TODO: BypassPullRequestAllowances + - PullRequestReviewsEnforcementUpdate.DismissalRestrictionsRequest # TODO: DismissalRestrictions + - Reactions.MinusOne + - Reactions.PlusOne + - RepositoriesSearchResult.Repositories + - RepositoriesSearchResult.Total + - RepositoryVulnerabilityAlert.GitHubSecurityAdvisoryID + - SCIMDisplayReference.Ref + - SecretScanningAlertLocationDetails.Startline # TODO: StartLine + - SecretScanningPatternOverride.Bypassrate # TODO: BypassRate + - StarredRepository.Repository # TODO: Repo + - Timeline.Requester # TODO: ReviewRequester + - Timeline.Reviewer # TODO: RequestedReviewer + - TopicsSearchResult.Topics # TODO: Items + - TopicsSearchResult.Total + - TotalCacheUsage.TotalActiveCachesUsageSizeInBytes # TODO: TotalActiveCachesSizeInBytes + - TransferRequest.TeamID # TODO: TeamIDs + - Tree.Entries + - User.LdapDn # TODO: LDAPDN + - UsersSearchResult.Total + - UsersSearchResult.Users + - WeeklyStats.Additions + - WeeklyStats.Commits + - WeeklyStats.Deletions + - WeeklyStats.Week sliceofpointers: type: module description: Reports usage of []*string and slices of structs without pointers. - original-url: github.com/google/go-github/v75/tools/sliceofpointers + original-url: github.com/google/go-github/v76/tools/sliceofpointers exclusions: rules: - linters: diff --git a/tools/jsonfieldname/jsonfieldname.go b/tools/jsonfieldname/jsonfieldname.go index e23bce034e3..e4c117e3cb6 100644 --- a/tools/jsonfieldname/jsonfieldname.go +++ b/tools/jsonfieldname/jsonfieldname.go @@ -27,11 +27,35 @@ func init() { } // JSONFieldNamePlugin is a custom linter plugin for golangci-lint. -type JSONFieldNamePlugin struct{} +type JSONFieldNamePlugin struct { + allowedExceptions map[string]bool +} + +// Settings is the configuration for the jsonfieldname linter. +type Settings struct { + AllowedExceptions []string `json:"allowed-exceptions" yaml:"allowed-exceptions"` +} // New returns an analysis.Analyzer to use with golangci-lint. -func New(_ any) (register.LinterPlugin, error) { - return &JSONFieldNamePlugin{}, nil +// It parses the "allowed-exceptions" section to determine which warnings to skip. +func New(cfg any) (register.LinterPlugin, error) { + allowedExceptions := map[string]bool{} + + if cfg != nil { + if settingsMap, ok := cfg.(map[string]any); ok { + if exceptionsRaw, ok := settingsMap["allowed-exceptions"]; ok { + if exceptionsList, ok := exceptionsRaw.([]any); ok { + for _, item := range exceptionsList { + if exception, ok := item.(string); ok { + allowedExceptions[exception] = true + } + } + } + } + } + } + + return &JSONFieldNamePlugin{allowedExceptions: allowedExceptions}, nil } // BuildAnalyzers builds the analyzers for the JSONFieldNamePlugin. @@ -40,7 +64,9 @@ func (f *JSONFieldNamePlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { { Name: "jsonfieldname", Doc: "Reports mismatches between Go field and JSON tag names. Note that the JSON tag name is the source-of-truth and the Go field name needs to match it.", - Run: run, + Run: func(pass *analysis.Pass) (any, error) { + return run(pass, f.allowedExceptions) + }, }, }, nil } @@ -50,7 +76,7 @@ func (f *JSONFieldNamePlugin) GetLoadMode() string { return register.LoadModeSyntax } -func run(pass *analysis.Pass) (any, error) { +func run(pass *analysis.Pass, allowedExceptions map[string]bool) (any, error) { for _, file := range pass.Files { ast.Inspect(file, func(n ast.Node) bool { if n == nil { @@ -84,7 +110,7 @@ func run(pass *analysis.Pass) (any, error) { } jsonTagName = strings.TrimSuffix(jsonTagName, ",omitempty") - checkGoFieldName(structName, goField.Name, jsonTagName, goField.Pos(), pass) + checkGoFieldName(structName, goField.Name, jsonTagName, goField.Pos(), pass, allowedExceptions) } } @@ -94,7 +120,7 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } -func checkGoFieldName(structName, goFieldName, jsonTagName string, tokenPos token.Pos, pass *analysis.Pass) { +func checkGoFieldName(structName, goFieldName, jsonTagName string, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { fullName := structName + "." + goFieldName if allowedExceptions[fullName] { return @@ -157,70 +183,6 @@ func jsonTagToPascal(jsonTagName string) (want, alternate string) { return strings.Join(parts, ""), strings.Join(alt, "") } -var allowedExceptions = map[string]bool{ - "ActionsCacheUsageList.RepoCacheUsage": true, // TODO: RepoCacheUsages ? - "AuditEntry.ExternalIdentityNameID": true, - "AuditEntry.Timestamp": true, - "CheckSuite.AfterSHA": true, - "CheckSuite.BeforeSHA": true, - "CodeSearchResult.CodeResults": true, - "CodeSearchResult.Total": true, - "CommitAuthor.Login": true, - "CommitsSearchResult.Commits": true, - "CommitsSearchResult.Total": true, - "CreateOrgInvitationOptions.TeamID": true, // TODO: TeamIDs - "DependencyGraphSnapshot.Sha": true, // TODO: SHA - "Discussion.DiscussionCategory": true, // TODO: Category ? - "EditOwner.OwnerInfo": true, - "Event.RawPayload": true, - "HookRequest.RawPayload": true, - "HookResponse.RawPayload": true, - "Issue.PullRequestLinks": true, // TODO: PullRequest - "IssueImportRequest.IssueImport": true, // TODO: Issue - "IssuesSearchResult.Issues": true, // TODO: Items - "IssuesSearchResult.Total": true, - "LabelsSearchResult.Labels": true, // TODO: Items - "LabelsSearchResult.Total": true, - "ListCheckRunsResults.Total": true, - "ListCheckSuiteResults.Total": true, - "ListCustomDeploymentRuleIntegrationsResponse.AvailableIntegrations": true, - "ListDeploymentProtectionRuleResponse.ProtectionRules": true, - "OrganizationCustomRepoRoles.CustomRepoRoles": true, // TODO: CustomRoles - "OrganizationCustomRoles.CustomRepoRoles": true, // TODO: Roles - "PreReceiveHook.ConfigURL": true, - "ProjectV2ItemEvent.ProjectV2Item": true, // TODO: ProjectsV2Item - "Protection.RequireLinearHistory": true, // TODO: RequiredLinearHistory - "ProtectionRequest.RequireLinearHistory": true, // TODO: RequiredLinearHistory - "PullRequestComment.InReplyTo": true, // TODO: InReplyToID - "PullRequestReviewsEnforcementRequest.BypassPullRequestAllowancesRequest": true, // TODO: BypassPullRequestAllowances - "PullRequestReviewsEnforcementRequest.DismissalRestrictionsRequest": true, // TODO: DismissalRestrictions - "PullRequestReviewsEnforcementUpdate.BypassPullRequestAllowancesRequest": true, // TODO: BypassPullRequestAllowances - "PullRequestReviewsEnforcementUpdate.DismissalRestrictionsRequest": true, // TODO: DismissalRestrictions - "Reactions.MinusOne": true, - "Reactions.PlusOne": true, - "RepositoriesSearchResult.Repositories": true, - "RepositoriesSearchResult.Total": true, - "RepositoryVulnerabilityAlert.GitHubSecurityAdvisoryID": true, - "SCIMDisplayReference.Ref": true, - "SecretScanningAlertLocationDetails.Startline": true, // TODO: StartLine - "SecretScanningPatternOverride.Bypassrate": true, // TODO: BypassRate - "StarredRepository.Repository": true, // TODO: Repo - "Timeline.Requester": true, // TODO: ReviewRequester - "Timeline.Reviewer": true, // TODO: RequestedReviewer - "TopicsSearchResult.Topics": true, // TODO: Items - "TopicsSearchResult.Total": true, - "TotalCacheUsage.TotalActiveCachesUsageSizeInBytes": true, // TODO: TotalActiveCachesSizeInBytes - "TransferRequest.TeamID": true, // TODO: TeamIDs - "Tree.Entries": true, - "User.LdapDn": true, // TODO: LDAPDN - "UsersSearchResult.Total": true, - "UsersSearchResult.Users": true, - "WeeklyStats.Additions": true, - "WeeklyStats.Commits": true, - "WeeklyStats.Deletions": true, - "WeeklyStats.Week": true, -} - // Common Go initialisms that should be all caps. var initialisms = map[string]bool{ "API": true, "ASCII": true,