diff --git a/.custom-gcl.yml b/.custom-gcl.yml index 2aa94b1bb08..b03078a28dc 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -1,6 +1,8 @@ version: v2.2.2 plugins: - - module: "github.com/google/go-github/v75/tools/sliceofpointers" - path: ./tools/sliceofpointers - module: "github.com/google/go-github/v75/tools/fmtpercentv" path: ./tools/fmtpercentv + - 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 f35bafec21e..0e001c1c6c8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -15,6 +15,7 @@ linters: - godot - goheader - gosec + - jsonfieldname - misspell - nakedret - paralleltest @@ -141,6 +142,10 @@ linters: type: module description: Reports usage of %d or %s in format strings. original-url: github.com/google/go-github/v75/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 sliceofpointers: type: module description: Reports usage of []*string and slices of structs without pointers. diff --git a/github/orgs.go b/github/orgs.go index c810bfbb852..c7e4c412606 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() { +}