From e7e528b544cf3d7fbc09af46ad4668432687e6ab Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Mon, 24 Nov 2025 18:39:25 +0200 Subject: [PATCH 1/5] Add custom structfield linter to check struct field names and tags --- .custom-gcl.yml | 4 +- .golangci.yml | 198 +++++++++++++++++- .../testdata/src/has-warnings/main.go | 14 -- .../testdata/src/no-warnings/main.go | 13 -- tools/{jsonfieldname => structfield}/go.mod | 2 +- tools/{jsonfieldname => structfield}/go.sum | 0 .../structfield.go} | 137 +++++++++--- .../structfield_test.go} | 11 +- .../testdata/src/has-warnings/main.go | 22 ++ .../testdata/src/no-warnings/main.go | 33 +++ 10 files changed, 364 insertions(+), 70 deletions(-) delete mode 100644 tools/jsonfieldname/testdata/src/has-warnings/main.go delete mode 100644 tools/jsonfieldname/testdata/src/no-warnings/main.go rename tools/{jsonfieldname => structfield}/go.mod (87%) rename tools/{jsonfieldname => structfield}/go.sum (100%) rename tools/{jsonfieldname/jsonfieldname.go => structfield/structfield.go} (50%) rename tools/{jsonfieldname/jsonfieldname_test.go => structfield/structfield_test.go} (69%) create mode 100644 tools/structfield/testdata/src/has-warnings/main.go create mode 100644 tools/structfield/testdata/src/no-warnings/main.go diff --git a/.custom-gcl.yml b/.custom-gcl.yml index 772345c1cbd..2417cc0eb97 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -2,7 +2,7 @@ version: v2.6.1 plugins: - module: "github.com/google/go-github/v79/tools/fmtpercentv" path: ./tools/fmtpercentv - - module: "github.com/google/go-github/v79/tools/jsonfieldname" - path: ./tools/jsonfieldname + - module: "github.com/google/go-github/v79/tools/structfield" + path: ./tools/structfield - module: "github.com/google/go-github/v79/tools/sliceofpointers" path: ./tools/sliceofpointers diff --git a/.golangci.yml b/.golangci.yml index 139a4a71188..63ad5798cbc 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -16,7 +16,6 @@ linters: - goheader - gosec - intrange - - jsonfieldname - misspell - modernize - musttag @@ -27,6 +26,7 @@ linters: - revive - sliceofpointers - staticcheck + - structfield - tparallel - unconvert - unparam @@ -152,12 +152,12 @@ linters: type: module description: Reports usage of %d or %s in format strings. original-url: github.com/google/go-github/v79/tools/fmtpercentv - jsonfieldname: + structfield: type: module - description: Reports mismatches between Go field and JSON tag names. - original-url: github.com/google/go-github/v79/tools/jsonfieldname + description: Reports mismatches between Go field and JSON, URL tag names and types. + original-url: github.com/google/go-github/v79/tools/structfield settings: - allowed-exceptions: + allowed-tag-name-exceptions: - ActionsCacheUsageList.RepoCacheUsage # TODO: RepoCacheUsages ? - AuditEntry.ExternalIdentityNameID - AuditEntry.Timestamp @@ -186,6 +186,8 @@ linters: - ListCheckSuiteResults.Total - ListCustomDeploymentRuleIntegrationsResponse.AvailableIntegrations - ListDeploymentProtectionRuleResponse.ProtectionRules + - ListIDPGroupsOptions.Query + - ListProjectsOptions.Query - OrganizationCustomRepoRoles.CustomRepoRoles # TODO: CustomRoles - OrganizationCustomRoles.CustomRepoRoles # TODO: Roles - PreReceiveHook.ConfigURL @@ -219,6 +221,189 @@ linters: - WeeklyStats.Commits - WeeklyStats.Deletions - WeeklyStats.Week + allowed-tag-type-exceptions: + - ActivityListStarredOptions.Direction # TODO: Activities + - ActivityListStarredOptions.Sort # TODO: Activities + - AddProjectItemOptions.ID # TODO: Projects + - AddProjectItemOptions.Type # TODO: Projects + - AlertInstancesListOptions.Ref # TODO: CodeScanning + - AlertListOptions.Direction # TODO: CodeScanning + - AlertListOptions.Ref # TODO: CodeScanning + - AlertListOptions.Severity # TODO: CodeScanning + - AlertListOptions.Sort # TODO: CodeScanning + - AlertListOptions.State # TODO: CodeScanning + - AlertListOptions.ToolGUID # TODO: CodeScanning + - AlertListOptions.ToolName # TODO: CodeScanning + - APIMetaArtifactAttestations.TrustDomain # TODO: Meta + - CommitsListOptions.Author # TODO: Repositories + - CommitsListOptions.Path # TODO: Repositories + - CommitsListOptions.SHA # TODO: Repositories + - CommitsListOptions.Since # TODO: Repositories + - CommitsListOptions.Until # TODO: Repositories + - CreateTag.Message # TODO: Git + - CreateTag.Object # TODO: Git + - CreateTag.Tag # TODO: Git + - CreateTag.Type # TODO: Git + - CredentialAuthorizationsListOptions.Login # TODO: Organizations + - DependabotEncryptedSecret.SelectedRepositoryIDs # TODO: Dependabot + - DependabotEncryptedSecret.Visibility # TODO: Dependabot + - DeploymentsListOptions.Environment # TODO: Repositories + - DeploymentsListOptions.Ref # TODO: Repositories + - DeploymentsListOptions.SHA # TODO: Repositories + - DeploymentsListOptions.Task # TODO: Repositories + - DiscussionCommentListOptions.Direction # TODO: Teams + - DiscussionListOptions.Direction # TODO: Teams + - EncryptedSecret.SelectedRepositoryIDs # TODO: Actions + - EncryptedSecret.Visibility # TODO: Actions + - ErrorBlock.Reason # TODO: Common + - ErrorResponse.DocumentationURL # TODO: Common + - GetCodeownersErrorsOptions.Ref # TODO: Repositories + - GistListOptions.Since # TODO: Gists + - HostedRunnerRequest.EnableStaticIP # TODO: Actions + - HostedRunnerRequest.Image # TODO: Actions + - HostedRunnerRequest.ImageVersion # TODO: Actions + - HostedRunnerRequest.MaximumRunners # TODO: Actions + - HostedRunnerRequest.Name # TODO: Actions + - HostedRunnerRequest.RunnerGroupID # TODO: Actions + - HostedRunnerRequest.Size # TODO: Actions + - IssueEvent.Action # TODO: Issues + - IssueListByRepoOptions.Assignee # TODO: Issues + - IssueListByRepoOptions.Assignee # TODO: Issues + - IssueListByRepoOptions.Creator # TODO: Issues + - IssueListByRepoOptions.Creator # TODO: Issues + - IssueListByRepoOptions.Direction # TODO: Issues + - IssueListByRepoOptions.Direction # TODO: Issues + - IssueListByRepoOptions.Mentioned # TODO: Issues + - IssueListByRepoOptions.Mentioned # TODO: Issues + - IssueListByRepoOptions.Milestone # TODO: Issues + - IssueListByRepoOptions.Since # TODO: Issues + - IssueListByRepoOptions.Since # TODO: Issues + - IssueListByRepoOptions.Sort # TODO: Issues + - IssueListByRepoOptions.Sort # TODO: Issues + - IssueListByRepoOptions.State # TODO: Issues + - IssueListOptions.Direction # TODO: Issues + - IssueListOptions.Filter # TODO: Issues + - IssueListOptions.Since # TODO: Issues + - IssueListOptions.Sort # TODO: Issues + - IssueListOptions.State # TODO: Issues + - ListCodespacesOptions.RepositoryID # TODO: Codespaces + - ListCollaboratorsOptions.Affiliation # TODO: Repositories + - ListCollaboratorsOptions.Permission # TODO: Repositories + - ListContributorsOptions.Anon # TODO: Repositories + - ListCursorOptions.After # TODO: Common + - ListCursorOptions.Before # TODO: Common + - ListCursorOptions.Cursor # TODO: Common + - ListCursorOptions.First # TODO: Common + - ListCursorOptions.Last # TODO: Common + - ListCursorOptions.Page # TODO: Common + - ListCursorOptions.PerPage # TODO: Common + - ListCustomPropertyValuesOptions.RepositoryQuery # TODO: Organizations + - ListEnterpriseRunnerGroupOptions.VisibleToOrganization # TODO: Enterprise + - ListFineGrainedPATOptions.Direction # TODO: Organizations + - ListFineGrainedPATOptions.LastUsedAfter # TODO: Organizations + - ListFineGrainedPATOptions.LastUsedBefore # TODO: Organizations + - ListFineGrainedPATOptions.Permission # TODO: Organizations + - ListFineGrainedPATOptions.Repository # TODO: Organizations + - ListFineGrainedPATOptions.Sort # TODO: Organizations + - ListIDPGroupsOptions.Query # TODO: Teams + - ListMembersOptions.Filter # TODO: Organizations + - ListMembersOptions.Role # TODO: Organizations + - ListOptions.Page # TODO: Common + - ListOptions.PerPage # TODO: Common + - ListOrgMembershipsOptions.State # TODO: Organizations + - ListOrgRunnerGroupOptions.VisibleToRepository # TODO: Actions + - ListOutsideCollaboratorsOptions.Filter # TODO: Organizations + - ListProvisionedSCIMGroupsEnterpriseOptions.Count # TODO: Enterprise + - ListProvisionedSCIMGroupsEnterpriseOptions.ExcludedAttributes # TODO: Enterprise + - ListProvisionedSCIMGroupsEnterpriseOptions.Filter # TODO: Enterprise + - ListProvisionedSCIMGroupsEnterpriseOptions.StartIndex # TODO: Enterprise + - ListReactionOptions.Content # TODO: Reactions + - ListRepositoryActivityOptions.ActivityType # TODO: Repositories + - ListRepositoryActivityOptions.Actor # TODO: Repositories + - ListRepositoryActivityOptions.After # TODO: Repositories + - ListRepositoryActivityOptions.Before # TODO: Repositories + - ListRepositoryActivityOptions.Direction # TODO: Repositories + - ListRepositoryActivityOptions.PerPage # TODO: Repositories + - ListRepositoryActivityOptions.Ref # TODO: Repositories + - ListRepositoryActivityOptions.TimePeriod # TODO: Repositories + - ListRepositorySecurityAdvisoriesOptions.Direction # TODO: SecurityAdvisories + - ListRepositorySecurityAdvisoriesOptions.Direction # TODO: SecurityAdvisories + - ListRepositorySecurityAdvisoriesOptions.Sort # TODO: SecurityAdvisories + - ListRepositorySecurityAdvisoriesOptions.Sort # TODO: SecurityAdvisories + - ListRepositorySecurityAdvisoriesOptions.State # TODO: SecurityAdvisories + - ListRepositorySecurityAdvisoriesOptions.State # TODO: SecurityAdvisories + - ListWorkflowJobsOptions.Filter # TODO: Actions + - ListWorkflowRunsOptions.Actor # TODO: Actions + - ListWorkflowRunsOptions.Branch # TODO: Actions + - ListWorkflowRunsOptions.CheckSuiteID # TODO: Actions + - ListWorkflowRunsOptions.Created # TODO: Actions + - ListWorkflowRunsOptions.Event # TODO: Actions + - ListWorkflowRunsOptions.ExcludePullRequests # TODO: Actions + - ListWorkflowRunsOptions.HeadSHA # TODO: Actions + - ListWorkflowRunsOptions.Status # TODO: Actions + - LockIssueOptions.LockReason # TODO: Issues + - MilestoneListOptions.Direction # TODO: Issues + - MilestoneListOptions.Sort # TODO: Issues + - MilestoneListOptions.State # TODO: Issues + - NotificationListOptions.All # TODO: Activities + - NotificationListOptions.Before # TODO: Activities + - NotificationListOptions.Participating # TODO: Activities + - NotificationListOptions.Since # TODO: Activities + - OrganizationsListOptions.Since # TODO: Organizations + - ProjectV2ItemFieldValue.DataType # TODO: Projects + - ProjectV2ItemFieldValue.Name # TODO: Projects + - PullRequestListCommentsOptions.Direction # TODO: PullRequests + - PullRequestListCommentsOptions.Since # TODO: PullRequests + - PullRequestListCommentsOptions.Sort # TODO: PullRequests + - PullRequestListOptions.Base # TODO: PullRequests + - PullRequestListOptions.Direction # TODO: PullRequests + - PullRequestListOptions.Head # TODO: PullRequests + - PullRequestListOptions.Sort # TODO: PullRequests + - PullRequestListOptions.State # TODO: PullRequests + - Rate.Resource # TODO: Common + - RepositoryAddCollaboratorOptions.Permission # TODO: Repositories + - RepositoryContentGetOptions.Ref # TODO: Repositories + - RepositoryCreateForkOptions.DefaultBranchOnly # TODO: Repositories + - RepositoryCreateForkOptions.Name # TODO: Repositories + - RepositoryCreateForkOptions.Organization # TODO: Repositories + - RepositoryListAllOptions.Since # TODO: Repositories + - RepositoryListByAuthenticatedUserOptions.Affiliation # TODO: Repositories + - RepositoryListByAuthenticatedUserOptions.Direction # TODO: Repositories + - RepositoryListByAuthenticatedUserOptions.Sort # TODO: Repositories + - RepositoryListByAuthenticatedUserOptions.Type # TODO: Repositories + - RepositoryListByAuthenticatedUserOptions.Visibility # TODO: Repositories + - RepositoryListByOrgOptions.Direction # TODO: Repositories + - RepositoryListByOrgOptions.Sort # TODO: Repositories + - RepositoryListByOrgOptions.Type # TODO: Repositories + - RepositoryListByUserOptions.Direction # TODO: Repositories + - RepositoryListByUserOptions.Sort # TODO: Repositories + - RepositoryListByUserOptions.Type # TODO: Repositories + - RepositoryListForksOptions.Sort # TODO: Repositories + - RepositoryListOptions.Affiliation # TODO: Repositories + - RepositoryListOptions.Direction # TODO: Repositories + - RepositoryListOptions.Sort # TODO: Repositories + - RepositoryListOptions.Type # TODO: Repositories + - RepositoryListOptions.Visibility # TODO: Repositories + - SearchOptions.Order # TODO: Search + - SearchOptions.Sort # TODO: Search + - Secret.SelectedRepositoriesURL # TODO: Actions + - Secret.Visibility # TODO: Actions + - SecretScanningAlertListOptions.Direction # TODO: SecretScanning + - SecretScanningAlertListOptions.IsMultiRepo # TODO: SecretScanning + - SecretScanningAlertListOptions.IsPubliclyLeaked # TODO: SecretScanning + - SecretScanningAlertListOptions.Resolution # TODO: SecretScanning + - SecretScanningAlertListOptions.SecretType # TODO: SecretScanning + - SecretScanningAlertListOptions.Sort # TODO: SecretScanning + - SecretScanningAlertListOptions.State # TODO: SecretScanning + - SecretScanningAlertListOptions.Validity # TODO: SecretScanning + - TeamAddTeamMembershipOptions.Role # TODO: Teams + - TeamAddTeamRepoOptions.Permission # TODO: Teams + - TeamListTeamMembersOptions.Role # TODO: Teams + - TrafficBreakdownOptions.Per # TODO: Repositories + - UpdateRuleParameters.UpdateAllowsFetchAndMerge # TODO: Rules + - UploadOptions.Label # TODO: Repositories + - UploadOptions.Name # TODO: Repositories + - UserListOptions.Since # TODO: Users sliceofpointers: type: module description: Reports usage of []*string and slices of structs without pointers. @@ -258,6 +443,9 @@ linters: # Because fmt.Sprint(reset.Unix())) is more readable than strconv.FormatInt(reset.Unix(), 10). - linters: [perfsprint] text: fmt.Sprint.* can be replaced with faster strconv.FormatInt +issues: + max-issues-per-linter: 0 + max-same-issues: 0 formatters: enable: - gci diff --git a/tools/jsonfieldname/testdata/src/has-warnings/main.go b/tools/jsonfieldname/testdata/src/has-warnings/main.go deleted file mode 100644 index c1a74c6059a..00000000000 --- a/tools/jsonfieldname/testdata/src/has-warnings/main.go +++ /dev/null @@ -1,14 +0,0 @@ -// 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"` - camelcaseexample *int `json:"camelCaseExample,omitempty"` // want `change Go field name "camelcaseexample" to "CamelCaseExample" for JSON tag "camelCaseExample" in struct "Example"` - DollarRef string `json:"$ref"` // want `change Go field name "DollarRef" to "Ref" for JSON tag "\$ref" in struct "Example"` -} diff --git a/tools/jsonfieldname/testdata/src/no-warnings/main.go b/tools/jsonfieldname/testdata/src/no-warnings/main.go deleted file mode 100644 index c522c7783ea..00000000000 --- a/tools/jsonfieldname/testdata/src/no-warnings/main.go +++ /dev/null @@ -1,13 +0,0 @@ -// 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 - Ref string `json:"$ref,omitempty"` // Should not be flagged -} diff --git a/tools/jsonfieldname/go.mod b/tools/structfield/go.mod similarity index 87% rename from tools/jsonfieldname/go.mod rename to tools/structfield/go.mod index 3ae53f27ac4..22571c72626 100644 --- a/tools/jsonfieldname/go.mod +++ b/tools/structfield/go.mod @@ -1,4 +1,4 @@ -module tools/jsonfieldname +module tools/structfield go 1.24.0 diff --git a/tools/jsonfieldname/go.sum b/tools/structfield/go.sum similarity index 100% rename from tools/jsonfieldname/go.sum rename to tools/structfield/go.sum diff --git a/tools/jsonfieldname/jsonfieldname.go b/tools/structfield/structfield.go similarity index 50% rename from tools/jsonfieldname/jsonfieldname.go rename to tools/structfield/structfield.go index 1de10bc2f8c..151cabe178b 100644 --- a/tools/jsonfieldname/jsonfieldname.go +++ b/tools/structfield/structfield.go @@ -3,15 +3,18 @@ // 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 +// Package structfield 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. +// of a struct does not match the JSON or URL 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 +// It also checks that fields with "omitempty" tags +// are pointer types, except for slices and maps. +package structfield import ( + "fmt" "go/ast" "go/token" "reflect" @@ -23,31 +26,43 @@ import ( ) func init() { - register.Plugin("jsonfieldname", New) + register.Plugin("structfield", New) } -// JSONFieldNamePlugin is a custom linter plugin for golangci-lint. -type JSONFieldNamePlugin struct { - allowedExceptions map[string]bool +// StructFieldPlugin is a custom linter plugin for golangci-lint. +type StructFieldPlugin struct { + allowedTagNameExceptions map[string]bool + allowedTagTypeExceptions map[string]bool } -// Settings is the configuration for the jsonfieldname linter. +// Settings is the configuration for the structfield linter. type Settings struct { - AllowedExceptions []string `json:"allowed-exceptions" yaml:"allowed-exceptions"` + AllowedTagNameExceptions []string `json:"allowed-tag-name-exceptions" yaml:"allowed-tag-name-exceptions"` + AllowedTagTypeExceptions []string `json:"allowed-tag-type-exceptions" yaml:"allowed-tag-type-exceptions"` } // New returns an analysis.Analyzer to use with golangci-lint. -// It parses the "allowed-exceptions" section to determine which warnings to skip. func New(cfg any) (register.LinterPlugin, error) { - allowedExceptions := map[string]bool{} + allowedTagNameExceptions := map[string]bool{} + allowedTagTypeExceptions := map[string]bool{} if cfg != nil { if settingsMap, ok := cfg.(map[string]any); ok { - if exceptionsRaw, ok := settingsMap["allowed-exceptions"]; ok { + if exceptionsRaw, ok := settingsMap["allowed-tag-name-exceptions"]; ok { if exceptionsList, ok := exceptionsRaw.([]any); ok { for _, item := range exceptionsList { if exception, ok := item.(string); ok { - allowedExceptions[exception] = true + allowedTagNameExceptions[exception] = true + } + } + } + } + + if exceptionsRaw, ok := settingsMap["allowed-tag-type-exceptions"]; ok { + if exceptionsList, ok := exceptionsRaw.([]any); ok { + for _, item := range exceptionsList { + if exception, ok := item.(string); ok { + allowedTagTypeExceptions[exception] = true } } } @@ -55,28 +70,33 @@ func New(cfg any) (register.LinterPlugin, error) { } } - return &JSONFieldNamePlugin{allowedExceptions: allowedExceptions}, nil + return &StructFieldPlugin{ + allowedTagNameExceptions: allowedTagNameExceptions, + allowedTagTypeExceptions: allowedTagTypeExceptions, + }, nil } -// BuildAnalyzers builds the analyzers for the JSONFieldNamePlugin. -func (f *JSONFieldNamePlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { +// BuildAnalyzers builds the analyzers for the StructFieldPlugin. +func (f *StructFieldPlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { return []*analysis.Analyzer{ { - 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.", + Name: "structfield", + Doc: `Reports mismatches between Go field and JSON or URL tag names and types. +Note that the JSON or URL tag name is the source-of-truth and the Go field name needs to match it. +If the tag contains "omitempty", then the Go field must be a pointer type except slices and maps.`, Run: func(pass *analysis.Pass) (any, error) { - return run(pass, f.allowedExceptions) + return run(pass, f.allowedTagNameExceptions, f.allowedTagTypeExceptions) }, }, }, nil } -// GetLoadMode returns the load mode for the JSONFieldNamePlugin. -func (f *JSONFieldNamePlugin) GetLoadMode() string { +// GetLoadMode returns the load mode for the StructFieldPlugin. +func (f *StructFieldPlugin) GetLoadMode() string { return register.LoadModeSyntax } -func run(pass *analysis.Pass, allowedExceptions map[string]bool) (any, error) { +func run(pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions map[string]bool) (any, error) { for _, file := range pass.Files { ast.Inspect(file, func(n ast.Node) bool { if n == nil { @@ -102,15 +122,28 @@ func run(pass *analysis.Pass, allowedExceptions map[string]bool) (any, error) { } goField := field.Names[0] + tagValue := strings.Trim(field.Tag.Value, "`") structTag := reflect.StructTag(tagValue) + jsonTagName, ok := structTag.Lookup("json") - if !ok || jsonTagName == "-" { - continue + if ok && jsonTagName != "-" { + if strings.Contains(jsonTagName, ",omitempty") { + checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypeExceptions) + jsonTagName = strings.ReplaceAll(jsonTagName, ",omitempty", "") + } + checkGoFieldName(structName, goField.Name, jsonTagName, goField.Pos(), pass, allowedTagNameExceptions) } - jsonTagName = strings.TrimSuffix(jsonTagName, ",omitempty") - checkGoFieldName(structName, goField.Name, jsonTagName, goField.Pos(), pass, allowedExceptions) + urlTagName, ok := structTag.Lookup("url") + if ok && urlTagName != "-" { + if strings.Contains(urlTagName, ",omitempty") { + checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypeExceptions) + urlTagName = strings.ReplaceAll(urlTagName, ",omitempty", "") + } + urlTagName = strings.ReplaceAll(urlTagName, ",comma", "") + checkGoFieldName(structName, goField.Name, urlTagName, goField.Pos(), pass, allowedTagNameExceptions) + } } } @@ -120,20 +153,58 @@ func run(pass *analysis.Pass, allowedExceptions map[string]bool) (any, error) { return nil, nil } -func checkGoFieldName(structName, goFieldName, jsonTagName string, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { +func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { fullName := structName + "." + goFieldName if allowedExceptions[fullName] { return } - want, alternate := jsonTagToPascal(jsonTagName) + want, alternate := tagNameToPascal(tagName) 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) + const msg = "change Go field name %q to %q for tag %q in struct %q" + pass.Reportf(tokenPos, msg, goFieldName, want, tagName, structName) + } +} + +func checkGoFieldType(structName, goFieldName string, field *ast.Field, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { + fullName := structName + "." + goFieldName + if allowedExceptions[fullName] { + return + } + + var skip bool + switch fieldType := field.Type.(type) { + case *ast.StarExpr, *ast.ArrayType, *ast.MapType: + skip = true + case *ast.SelectorExpr: + // check if type is json.RawMessage + if ident, ok := fieldType.X.(*ast.Ident); ok && ident.Name == "json" && fieldType.Sel.Name == "RawMessage" { + skip = true + } + case *ast.Ident: + // check if type is `any` + if fieldType.Name == "any" { + skip = true + } + } + if !skip { + const msg = `change the %q field type to %q in the struct %q because its tag uses "omitempty"` + pass.Reportf(tokenPos, msg, goFieldName, "*"+exprToString(field.Type), structName) + } +} + +func exprToString(e ast.Expr) string { + switch t := e.(type) { + case *ast.Ident: + return t.Name + case *ast.SelectorExpr: + return exprToString(t.X) + "." + t.Sel.Name + default: + return fmt.Sprintf("%T", e) } } -func splitJSONTag(jsonTagName string) []string { +func splitTag(jsonTagName string) []string { jsonTagName = strings.TrimPrefix(jsonTagName, "$") if strings.Contains(jsonTagName, "_") { @@ -159,8 +230,8 @@ func splitJSONTag(jsonTagName string) []string { var camelCaseRE = regexp.MustCompile(`([a-z0-9])([A-Z])`) -func jsonTagToPascal(jsonTagName string) (want, alternate string) { - parts := splitJSONTag(jsonTagName) +func tagNameToPascal(tagName string) (want, alternate string) { + parts := splitTag(tagName) alt := make([]string, len(parts)) for i, part := range parts { alt[i] = part diff --git a/tools/jsonfieldname/jsonfieldname_test.go b/tools/structfield/structfield_test.go similarity index 69% rename from tools/jsonfieldname/jsonfieldname_test.go rename to tools/structfield/structfield_test.go index fa5044c7aba..54ed61012ab 100644 --- a/tools/jsonfieldname/jsonfieldname_test.go +++ b/tools/structfield/structfield_test.go @@ -3,7 +3,7 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package jsonfieldname +package structfield import ( "testing" @@ -14,7 +14,14 @@ import ( func TestRun(t *testing.T) { t.Parallel() testdata := analysistest.TestData() - plugin, _ := New(nil) + plugin, _ := New(map[string]any{ + "allowed-tag-name-exceptions": []any{ + "Example.Query", + }, + "allowed-tag-type-exceptions": []any{ + "JSONFieldType.Exception", + }, + }) analyzers, _ := plugin.BuildAnalyzers() analysistest.Run(t, testdata, analyzers[0], "has-warnings", "no-warnings") } diff --git a/tools/structfield/testdata/src/has-warnings/main.go b/tools/structfield/testdata/src/has-warnings/main.go new file mode 100644 index 00000000000..088e58f2702 --- /dev/null +++ b/tools/structfield/testdata/src/has-warnings/main.go @@ -0,0 +1,22 @@ +// 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 tag "github_thing" in struct "Example"` + Id *string `json:"id,omitempty"` // want `change Go field name "Id" to "ID" for tag "id" in struct "Example"` + strings *string `json:"strings,omitempty"` // want `change Go field name "strings" to "Strings" for tag "strings" in struct "Example"` + camelcaseexample *int `json:"camelCaseExample,omitempty"` // want `change Go field name "camelcaseexample" to "CamelCaseExample" for tag "camelCaseExample" in struct "Example"` + DollarRef string `json:"$ref"` // want `change Go field name "DollarRef" to "Ref" for tag "\$ref" in struct "Example"` +} + +type JSONFieldType struct { + ID string `json:"id,omitempty"` // want `change the \"ID\" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` + + Page string `url:"page,omitempty"` // want `change the "Page" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` + PerPage int `url:"per_page,omitempty"` // want `change the "PerPage" field type to "\*int" in the struct "JSONFieldType" because its tag uses "omitempty"` + Participating bool `url:"participating,omitempty"` // want `change the "Participating" field type to "\*bool" in the struct "JSONFieldType" because its tag uses "omitempty"` +} diff --git a/tools/structfield/testdata/src/no-warnings/main.go b/tools/structfield/testdata/src/no-warnings/main.go new file mode 100644 index 00000000000..79e923a96bc --- /dev/null +++ b/tools/structfield/testdata/src/no-warnings/main.go @@ -0,0 +1,33 @@ +// 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 + +import ( + "encoding/json" + "time" +) + +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 + Ref *string `json:"$ref,omitempty"` // Should not be flagged + Query string `json:"q"` // Should not be flagged due to exception +} + +type JSONFieldType struct { + ID *string `json:"id,omitempty"` // Should not be flagged + HookAttributes map[string]string `json:"hook_attributes,omitempty"` // Should not be flagged + Inputs json.RawMessage `json:"inputs,omitempty"` // Should not be flagged + Exception string `json:"exception,omitempty"` // Should not be flagged due to exception + Value any `json:"value,omitempty"` + + Page *string `url:"page,omitempty"` // Should not be flagged + PerPage *int `url:"per_page,omitempty"` // Should not be flagged + Labels []string `url:"labels,omitempty,comma"` // Should not be flagged + Since *time.Time `url:"since,omitempty"` // Should not be flagged + Fields []int64 `url:"fields,omitempty,comma"` // Should not be flagged +} From be99d05635474a490e625e4e8439780942cd8b7e Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Tue, 25 Nov 2025 15:42:33 +0200 Subject: [PATCH 2/5] sort linters in configs; update doc --- .custom-gcl.yml | 4 ++-- .golangci.yml | 8 ++++---- tools/structfield/structfield.go | 5 ++--- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.custom-gcl.yml b/.custom-gcl.yml index 2417cc0eb97..a0a0945e11a 100644 --- a/.custom-gcl.yml +++ b/.custom-gcl.yml @@ -2,7 +2,7 @@ version: v2.6.1 plugins: - module: "github.com/google/go-github/v79/tools/fmtpercentv" path: ./tools/fmtpercentv - - module: "github.com/google/go-github/v79/tools/structfield" - path: ./tools/structfield - module: "github.com/google/go-github/v79/tools/sliceofpointers" path: ./tools/sliceofpointers + - module: "github.com/google/go-github/v79/tools/structfield" + path: ./tools/structfield diff --git a/.golangci.yml b/.golangci.yml index 63ad5798cbc..8f9ce5afb4a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -152,6 +152,10 @@ linters: type: module description: Reports usage of %d or %s in format strings. original-url: github.com/google/go-github/v79/tools/fmtpercentv + sliceofpointers: + type: module + description: Reports usage of []*string and slices of structs without pointers. + original-url: github.com/google/go-github/v79/tools/sliceofpointers structfield: type: module description: Reports mismatches between Go field and JSON, URL tag names and types. @@ -404,10 +408,6 @@ linters: - UploadOptions.Label # TODO: Repositories - UploadOptions.Name # TODO: Repositories - UserListOptions.Since # TODO: Users - sliceofpointers: - type: module - description: Reports usage of []*string and slices of structs without pointers. - original-url: github.com/google/go-github/v79/tools/sliceofpointers exclusions: rules: - linters: diff --git a/tools/structfield/structfield.go b/tools/structfield/structfield.go index 151cabe178b..cbda827084f 100644 --- a/tools/structfield/structfield.go +++ b/tools/structfield/structfield.go @@ -9,8 +9,7 @@ // It honors idiomatic Go initialisms and handles the // special case of `Github` vs `GitHub` as agreed upon // by the original author of the repo. -// It also checks that fields with "omitempty" tags -// are pointer types, except for slices and maps. +// It also checks that fields with "omitempty" tags are reference types. package structfield import ( @@ -83,7 +82,7 @@ func (f *StructFieldPlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { Name: "structfield", Doc: `Reports mismatches between Go field and JSON or URL tag names and types. Note that the JSON or URL tag name is the source-of-truth and the Go field name needs to match it. -If the tag contains "omitempty", then the Go field must be a pointer type except slices and maps.`, +If the tag contains "omitempty", then the Go field must be a reference type.`, Run: func(pass *analysis.Pass) (any, error) { return run(pass, f.allowedTagNameExceptions, f.allowedTagTypeExceptions) }, From 14009b9a4a90295adf0b74bb361ba21ac9aca21f Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Tue, 25 Nov 2025 15:53:04 +0200 Subject: [PATCH 3/5] rename options --- .golangci.yml | 4 ++-- tools/structfield/structfield.go | 26 +++++++++++++------------- tools/structfield/structfield_test.go | 4 ++-- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8f9ce5afb4a..bace30e84e3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -161,7 +161,7 @@ linters: description: Reports mismatches between Go field and JSON, URL tag names and types. original-url: github.com/google/go-github/v79/tools/structfield settings: - allowed-tag-name-exceptions: + allowed-tag-names: - ActionsCacheUsageList.RepoCacheUsage # TODO: RepoCacheUsages ? - AuditEntry.ExternalIdentityNameID - AuditEntry.Timestamp @@ -225,7 +225,7 @@ linters: - WeeklyStats.Commits - WeeklyStats.Deletions - WeeklyStats.Week - allowed-tag-type-exceptions: + allowed-tag-types: - ActivityListStarredOptions.Direction # TODO: Activities - ActivityListStarredOptions.Sort # TODO: Activities - AddProjectItemOptions.ID # TODO: Projects diff --git a/tools/structfield/structfield.go b/tools/structfield/structfield.go index cbda827084f..0f20f920388 100644 --- a/tools/structfield/structfield.go +++ b/tools/structfield/structfield.go @@ -30,38 +30,38 @@ func init() { // StructFieldPlugin is a custom linter plugin for golangci-lint. type StructFieldPlugin struct { - allowedTagNameExceptions map[string]bool - allowedTagTypeExceptions map[string]bool + allowedTagNames map[string]bool + allowedTagTypes map[string]bool } // Settings is the configuration for the structfield linter. type Settings struct { - AllowedTagNameExceptions []string `json:"allowed-tag-name-exceptions" yaml:"allowed-tag-name-exceptions"` - AllowedTagTypeExceptions []string `json:"allowed-tag-type-exceptions" yaml:"allowed-tag-type-exceptions"` + AllowedTagNames []string `json:"allowed-tag-names" yaml:"allowed-tag-names"` + AllowedTagTypes []string `json:"allowed-tag-types" yaml:"allowed-tag-types"` } // New returns an analysis.Analyzer to use with golangci-lint. func New(cfg any) (register.LinterPlugin, error) { - allowedTagNameExceptions := map[string]bool{} - allowedTagTypeExceptions := map[string]bool{} + allowedTagNames := map[string]bool{} + allowedTagTypes := map[string]bool{} if cfg != nil { if settingsMap, ok := cfg.(map[string]any); ok { - if exceptionsRaw, ok := settingsMap["allowed-tag-name-exceptions"]; ok { + if exceptionsRaw, ok := settingsMap["allowed-tag-names"]; ok { if exceptionsList, ok := exceptionsRaw.([]any); ok { for _, item := range exceptionsList { if exception, ok := item.(string); ok { - allowedTagNameExceptions[exception] = true + allowedTagNames[exception] = true } } } } - if exceptionsRaw, ok := settingsMap["allowed-tag-type-exceptions"]; ok { + if exceptionsRaw, ok := settingsMap["allowed-tag-types"]; ok { if exceptionsList, ok := exceptionsRaw.([]any); ok { for _, item := range exceptionsList { if exception, ok := item.(string); ok { - allowedTagTypeExceptions[exception] = true + allowedTagTypes[exception] = true } } } @@ -70,8 +70,8 @@ func New(cfg any) (register.LinterPlugin, error) { } return &StructFieldPlugin{ - allowedTagNameExceptions: allowedTagNameExceptions, - allowedTagTypeExceptions: allowedTagTypeExceptions, + allowedTagNames: allowedTagNames, + allowedTagTypes: allowedTagTypes, }, nil } @@ -84,7 +84,7 @@ func (f *StructFieldPlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { Note that the JSON or URL tag name is the source-of-truth and the Go field name needs to match it. If the tag contains "omitempty", then the Go field must be a reference type.`, Run: func(pass *analysis.Pass) (any, error) { - return run(pass, f.allowedTagNameExceptions, f.allowedTagTypeExceptions) + return run(pass, f.allowedTagNames, f.allowedTagTypes) }, }, }, nil diff --git a/tools/structfield/structfield_test.go b/tools/structfield/structfield_test.go index 54ed61012ab..816bc3dc7fb 100644 --- a/tools/structfield/structfield_test.go +++ b/tools/structfield/structfield_test.go @@ -15,10 +15,10 @@ func TestRun(t *testing.T) { t.Parallel() testdata := analysistest.TestData() plugin, _ := New(map[string]any{ - "allowed-tag-name-exceptions": []any{ + "allowed-tag-names": []any{ "Example.Query", }, - "allowed-tag-type-exceptions": []any{ + "allowed-tag-types": []any{ "JSONFieldType.Exception", }, }) From a5979df9a5e05024e5b0b930ceaab7212fe871cf Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Tue, 25 Nov 2025 16:30:52 +0200 Subject: [PATCH 4/5] cover all cases from review --- tools/structfield/structfield.go | 160 ++++++++++++------ tools/structfield/structfield_test.go | 4 +- .../testdata/src/has-warnings/main.go | 34 ++-- .../testdata/src/no-warnings/main.go | 47 +++-- 4 files changed, 163 insertions(+), 82 deletions(-) diff --git a/tools/structfield/structfield.go b/tools/structfield/structfield.go index 0f20f920388..6e4a1d1aefc 100644 --- a/tools/structfield/structfield.go +++ b/tools/structfield/structfield.go @@ -16,6 +16,7 @@ import ( "fmt" "go/ast" "go/token" + "go/types" "reflect" "regexp" "strings" @@ -102,48 +103,26 @@ func run(pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions 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) + t, ok := n.(*ast.TypeSpec) + if !ok { + return true + } + structType, ok := t.Type.(*ast.StructType) + if !ok { + return true + } - jsonTagName, ok := structTag.Lookup("json") - if ok && jsonTagName != "-" { - if strings.Contains(jsonTagName, ",omitempty") { - checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypeExceptions) - jsonTagName = strings.ReplaceAll(jsonTagName, ",omitempty", "") - } - checkGoFieldName(structName, goField.Name, jsonTagName, goField.Pos(), pass, allowedTagNameExceptions) - } + // Check only exported + if !ast.IsExported(t.Name.Name) { + return true + } - urlTagName, ok := structTag.Lookup("url") - if ok && urlTagName != "-" { - if strings.Contains(urlTagName, ",omitempty") { - checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypeExceptions) - urlTagName = strings.ReplaceAll(urlTagName, ",omitempty", "") - } - urlTagName = strings.ReplaceAll(urlTagName, ",comma", "") - checkGoFieldName(structName, goField.Name, urlTagName, goField.Pos(), pass, allowedTagNameExceptions) - } + for _, field := range structType.Fields.List { + if field.Tag == nil || len(field.Names) == 0 { + continue } + + processStructField(t.Name.Name, field, pass, allowedTagNameExceptions, allowedTagTypeExceptions) } return true @@ -152,6 +131,33 @@ func run(pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions return nil, nil } +func processStructField(structName string, field *ast.Field, pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions map[string]bool) { + goField := field.Names[0] + tagValue := strings.Trim(field.Tag.Value, "`") + structTag := reflect.StructTag(tagValue) + + processTag(structName, goField, field, structTag, "json", pass, allowedTagNameExceptions, allowedTagTypeExceptions) + processTag(structName, goField, field, structTag, "url", pass, allowedTagNameExceptions, allowedTagTypeExceptions) +} + +func processTag(structName string, goField *ast.Ident, field *ast.Field, structTag reflect.StructTag, tagType string, pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions map[string]bool) { + tagName, ok := structTag.Lookup(tagType) + if !ok || tagName == "-" { + return + } + + if strings.Contains(tagName, ",omitempty") { + checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypeExceptions) + tagName = strings.ReplaceAll(tagName, ",omitempty", "") + } + + if tagType == "url" { + tagName = strings.ReplaceAll(tagName, ",comma", "") + } + + checkGoFieldName(structName, goField.Name, tagName, goField.Pos(), pass, allowedTagNameExceptions) +} + func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { fullName := structName + "." + goFieldName if allowedExceptions[fullName] { @@ -166,38 +172,84 @@ func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Po } func checkGoFieldType(structName, goFieldName string, field *ast.Field, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { - fullName := structName + "." + goFieldName - if allowedExceptions[fullName] { + if allowedExceptions[structName+"."+goFieldName] { return } - var skip bool - switch fieldType := field.Type.(type) { - case *ast.StarExpr, *ast.ArrayType, *ast.MapType: - skip = true + skipOmitempty := checkAndReportInvalidTypes(structName, goFieldName, field.Type, tokenPos, pass) + + if !skipOmitempty { + const msg = `change the %q field type to %q in the struct %q because its tag uses "omitempty"` + pass.Reportf(tokenPos, msg, goFieldName, "*"+exprToString(field.Type), structName) + } +} + +func checkAndReportInvalidTypes(structName, goFieldName string, fieldType ast.Expr, tokenPos token.Pos, pass *analysis.Pass) bool { + switch ft := fieldType.(type) { + case *ast.StarExpr: + // Check for *[]T where T is builtin - should be []T + if arrType, ok := ft.X.(*ast.ArrayType); ok { + if ident, ok := arrType.Elt.(*ast.Ident); ok && isBuiltinType(ident.Name) { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "[]"+ident.Name, structName) + } else { + checkStructArrayType(structName, goFieldName, arrType, tokenPos, pass) + } + } + // Check for *map - should be map + if _, ok := ft.X.(*ast.MapType); ok { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, exprToString(ft.X), structName) + } + return true + case *ast.MapType: + return true + case *ast.ArrayType: + checkStructArrayType(structName, goFieldName, ft, tokenPos, pass) + return true case *ast.SelectorExpr: - // check if type is json.RawMessage - if ident, ok := fieldType.X.(*ast.Ident); ok && ident.Name == "json" && fieldType.Sel.Name == "RawMessage" { - skip = true + // Check for json.RawMessage + if ident, ok := ft.X.(*ast.Ident); ok && ident.Name == "json" && ft.Sel.Name == "RawMessage" { + return true } case *ast.Ident: - // check if type is `any` - if fieldType.Name == "any" { - skip = true + // Check for `any` type + if ft.Name == "any" { + return true } } - if !skip { - const msg = `change the %q field type to %q in the struct %q because its tag uses "omitempty"` - pass.Reportf(tokenPos, msg, goFieldName, "*"+exprToString(field.Type), structName) + return false +} + +func checkStructArrayType(structName, goFieldName string, arrType *ast.ArrayType, tokenPos token.Pos, pass *analysis.Pass) { + if starExpr, ok := arrType.Elt.(*ast.StarExpr); ok { + if ident, ok := starExpr.X.(*ast.Ident); ok && isBuiltinType(ident.Name) { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "[]"+ident.Name, structName) + } + return + } + + if ident, ok := arrType.Elt.(*ast.Ident); ok && ident.Obj != nil { + if _, ok := ident.Obj.Decl.(*ast.TypeSpec).Type.(*ast.StructType); ok { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "[]*"+ident.Name, structName) + } } } +func isBuiltinType(typeName string) bool { + return types.Universe.Lookup(typeName) != nil +} + func exprToString(e ast.Expr) string { switch t := e.(type) { case *ast.Ident: return t.Name case *ast.SelectorExpr: return exprToString(t.X) + "." + t.Sel.Name + case *ast.MapType: + return "map[" + exprToString(t.Key) + "]" + exprToString(t.Value) default: return fmt.Sprintf("%T", e) } diff --git a/tools/structfield/structfield_test.go b/tools/structfield/structfield_test.go index 816bc3dc7fb..a192782313a 100644 --- a/tools/structfield/structfield_test.go +++ b/tools/structfield/structfield_test.go @@ -16,10 +16,12 @@ func TestRun(t *testing.T) { testdata := analysistest.TestData() plugin, _ := New(map[string]any{ "allowed-tag-names": []any{ - "Example.Query", + "JSONFieldName.Query", + "URLFieldName.Query", }, "allowed-tag-types": []any{ "JSONFieldType.Exception", + "URLFieldType.Exception", }, }) analyzers, _ := plugin.BuildAnalyzers() diff --git a/tools/structfield/testdata/src/has-warnings/main.go b/tools/structfield/testdata/src/has-warnings/main.go index 088e58f2702..8ddc1eaf14a 100644 --- a/tools/structfield/testdata/src/has-warnings/main.go +++ b/tools/structfield/testdata/src/has-warnings/main.go @@ -5,18 +5,32 @@ package main -type Example struct { - GitHubThing string `json:"github_thing"` // want `change Go field name "GitHubThing" to "GithubThing" for tag "github_thing" in struct "Example"` - Id *string `json:"id,omitempty"` // want `change Go field name "Id" to "ID" for tag "id" in struct "Example"` - strings *string `json:"strings,omitempty"` // want `change Go field name "strings" to "Strings" for tag "strings" in struct "Example"` - camelcaseexample *int `json:"camelCaseExample,omitempty"` // want `change Go field name "camelcaseexample" to "CamelCaseExample" for tag "camelCaseExample" in struct "Example"` - DollarRef string `json:"$ref"` // want `change Go field name "DollarRef" to "Ref" for tag "\$ref" in struct "Example"` +type JSONFieldName struct { + GitHubThing string `json:"github_thing"` // want `change Go field name "GitHubThing" to "GithubThing" for tag "github_thing" in struct "JSONFieldName"` + Id *string `json:"id,omitempty"` // want `change Go field name "Id" to "ID" for tag "id" in struct "JSONFieldName"` + strings *string `json:"strings,omitempty"` // want `change Go field name "strings" to "Strings" for tag "strings" in struct "JSONFieldName"` + camelcaseexample *int `json:"camelCaseExample,omitempty"` // want `change Go field name "camelcaseexample" to "CamelCaseExample" for tag "camelCaseExample" in struct "JSONFieldName"` + DollarRef string `json:"$ref"` // want `change Go field name "DollarRef" to "Ref" for tag "\$ref" in struct "JSONFieldName"` } type JSONFieldType struct { - ID string `json:"id,omitempty"` // want `change the \"ID\" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` + String string `json:"string,omitempty"` // want `change the "String" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` + SliceOfStringPointers []*string `json:"slice_of_string_pointers,omitempty"` // want `change the "SliceOfStringPointers" field type to "\[\]string" in the struct "JSONFieldType"` + PointerToSliceOfStrings *[]string `json:"pointer_to_slice_of_strings,omitempty"` // want `change the "PointerToSliceOfStrings" field type to "\[\]string" in the struct "JSONFieldType"` + SliceOfStructs []Struct `json:"slice_of_structs,omitempty"` // want `change the "SliceOfStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerToSliceOfStructs *[]Struct `json:"pointer_to_slice_of_structs,omitempty"` // want `change the "PointerToSliceOfStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerToMap *map[string]string `json:"pointer_to_map,omitempty"` // want `change the "PointerToMap" field type to "map\[string\]string" in the struct "JSONFieldType"` + SliceOfInts []*int `json:"slice_of_ints,omitempty"` // want `change the "SliceOfInts" field type to "\[\]int" in the struct "JSONFieldType"` +} + +type Struct struct{} + +type URLFieldName struct { + GitHubThing string `url:"github_thing"` // want `change Go field name "GitHubThing" to "GithubThing" for tag "github_thing" in struct "URLFieldName"` +} - Page string `url:"page,omitempty"` // want `change the "Page" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` - PerPage int `url:"per_page,omitempty"` // want `change the "PerPage" field type to "\*int" in the struct "JSONFieldType" because its tag uses "omitempty"` - Participating bool `url:"participating,omitempty"` // want `change the "Participating" field type to "\*bool" in the struct "JSONFieldType" because its tag uses "omitempty"` +type URLFieldType struct { + Page string `url:"page,omitempty"` // want `change the "Page" field type to "\*string" in the struct "URLFieldType" because its tag uses "omitempty"` + PerPage int `url:"per_page,omitempty"` // want `change the "PerPage" field type to "\*int" in the struct "URLFieldType" because its tag uses "omitempty"` + Participating bool `url:"participating,omitempty"` // want `change the "Participating" field type to "\*bool" in the struct "URLFieldType" because its tag uses "omitempty"` } diff --git a/tools/structfield/testdata/src/no-warnings/main.go b/tools/structfield/testdata/src/no-warnings/main.go index 79e923a96bc..fc0236f1cfe 100644 --- a/tools/structfield/testdata/src/no-warnings/main.go +++ b/tools/structfield/testdata/src/no-warnings/main.go @@ -10,24 +10,37 @@ import ( "time" ) -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 - Ref *string `json:"$ref,omitempty"` // Should not be flagged - Query string `json:"q"` // Should not be flagged due to exception +type JSONFieldName struct { + GithubThing string `json:"github_thing"` + ID *string `json:"id,omitempty"` + Strings *string `json:"strings,omitempty"` + Ref *string `json:"$ref,omitempty"` + Query string `json:"q"` } type JSONFieldType struct { - ID *string `json:"id,omitempty"` // Should not be flagged - HookAttributes map[string]string `json:"hook_attributes,omitempty"` // Should not be flagged - Inputs json.RawMessage `json:"inputs,omitempty"` // Should not be flagged - Exception string `json:"exception,omitempty"` // Should not be flagged due to exception - Value any `json:"value,omitempty"` - - Page *string `url:"page,omitempty"` // Should not be flagged - PerPage *int `url:"per_page,omitempty"` // Should not be flagged - Labels []string `url:"labels,omitempty,comma"` // Should not be flagged - Since *time.Time `url:"since,omitempty"` // Should not be flagged - Fields []int64 `url:"fields,omitempty,comma"` // Should not be flagged + WithoutTag string + + ID *string `json:"id,omitempty"` + HookAttributes map[string]string `json:"hook_attributes,omitempty"` + Inputs json.RawMessage `json:"inputs,omitempty"` + Exception string `json:"exception,omitempty"` + Value any `json:"value,omitempty"` + SliceOfPointerStructs []*Struct `json:"slice_of_pointer_structs,omitempty"` +} + +type URLFieldName struct { + ID *string `url:"id,omitempty"` + Query string `url:"q"` } + +type URLFieldType struct { + Page *string `url:"page,omitempty"` + PerPage *int `url:"per_page,omitempty"` + Labels []string `url:"labels,omitempty,comma"` + Since *time.Time `url:"since,omitempty"` + Fields []int64 `url:"fields,omitempty,comma"` + Exception string `url:"exception,omitempty"` +} + +type Struct struct{} From e83e366b7062435992cff77aab2177de40a2ca57 Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Tue, 25 Nov 2025 18:35:18 +0200 Subject: [PATCH 5/5] add exceptions --- .golangci.yml | 12 ++++++++ tools/structfield/structfield.go | 30 +++++++++++-------- .../testdata/src/has-warnings/main.go | 15 +++++----- 3 files changed, 38 insertions(+), 19 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index bace30e84e3..df503f41577 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -251,12 +251,16 @@ linters: - CredentialAuthorizationsListOptions.Login # TODO: Organizations - DependabotEncryptedSecret.SelectedRepositoryIDs # TODO: Dependabot - DependabotEncryptedSecret.Visibility # TODO: Dependabot + - DeploymentRequest.RequiredContexts # TODO: Deployments - DeploymentsListOptions.Environment # TODO: Repositories - DeploymentsListOptions.Ref # TODO: Repositories - DeploymentsListOptions.SHA # TODO: Repositories - DeploymentsListOptions.Task # TODO: Repositories - DiscussionCommentListOptions.Direction # TODO: Teams - DiscussionListOptions.Direction # TODO: Teams + - DismissalRestrictionsRequest.Apps # TODO: Repositories + - DismissalRestrictionsRequest.Teams # TODO: Repositories + - DismissalRestrictionsRequest.Users # TODO: Repositories - EncryptedSecret.SelectedRepositoryIDs # TODO: Actions - EncryptedSecret.Visibility # TODO: Actions - ErrorBlock.Reason # TODO: Common @@ -290,6 +294,11 @@ linters: - IssueListOptions.Since # TODO: Issues - IssueListOptions.Sort # TODO: Issues - IssueListOptions.State # TODO: Issues + - IssueRequest.Assignees # TODO: Issues + - IssueRequest.Labels # TODO: Issues + - License.Conditions # TODO: Licenses + - License.Limitations # TODO: Licenses + - License.Permissions # TODO: Licenses - ListCodespacesOptions.RepositoryID # TODO: Codespaces - ListCollaboratorsOptions.Affiliation # TODO: Repositories - ListCollaboratorsOptions.Permission # TODO: Repositories @@ -346,6 +355,7 @@ linters: - ListWorkflowRunsOptions.HeadSHA # TODO: Actions - ListWorkflowRunsOptions.Status # TODO: Actions - LockIssueOptions.LockReason # TODO: Issues + - MarketplacePlan.Bullets # TODO: Marketplaces - MilestoneListOptions.Direction # TODO: Issues - MilestoneListOptions.Sort # TODO: Issues - MilestoneListOptions.State # TODO: Issues @@ -388,6 +398,8 @@ linters: - RepositoryListOptions.Sort # TODO: Repositories - RepositoryListOptions.Type # TODO: Repositories - RepositoryListOptions.Visibility # TODO: Repositories + - RequiredStatusChecks.Checks # TODO: Repositories + - RequiredStatusChecks.Contexts # TODO: Repositories - SearchOptions.Order # TODO: Search - SearchOptions.Sort # TODO: Search - Secret.SelectedRepositoriesURL # TODO: Actions diff --git a/tools/structfield/structfield.go b/tools/structfield/structfield.go index 6e4a1d1aefc..19ac0db4de8 100644 --- a/tools/structfield/structfield.go +++ b/tools/structfield/structfield.go @@ -96,7 +96,7 @@ func (f *StructFieldPlugin) GetLoadMode() string { return register.LoadModeSyntax } -func run(pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions map[string]bool) (any, error) { +func run(pass *analysis.Pass, allowedTagNames, allowedTagTypes map[string]bool) (any, error) { for _, file := range pass.Files { ast.Inspect(file, func(n ast.Node) bool { if n == nil { @@ -122,7 +122,7 @@ func run(pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions continue } - processStructField(t.Name.Name, field, pass, allowedTagNameExceptions, allowedTagTypeExceptions) + processStructField(t.Name.Name, field, pass, allowedTagNames, allowedTagTypes) } return true @@ -131,23 +131,23 @@ func run(pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions return nil, nil } -func processStructField(structName string, field *ast.Field, pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions map[string]bool) { +func processStructField(structName string, field *ast.Field, pass *analysis.Pass, allowedTagNames, allowedTagTypes map[string]bool) { goField := field.Names[0] tagValue := strings.Trim(field.Tag.Value, "`") structTag := reflect.StructTag(tagValue) - processTag(structName, goField, field, structTag, "json", pass, allowedTagNameExceptions, allowedTagTypeExceptions) - processTag(structName, goField, field, structTag, "url", pass, allowedTagNameExceptions, allowedTagTypeExceptions) + processTag(structName, goField, field, structTag, "json", pass, allowedTagNames, allowedTagTypes) + processTag(structName, goField, field, structTag, "url", pass, allowedTagNames, allowedTagTypes) } -func processTag(structName string, goField *ast.Ident, field *ast.Field, structTag reflect.StructTag, tagType string, pass *analysis.Pass, allowedTagNameExceptions, allowedTagTypeExceptions map[string]bool) { +func processTag(structName string, goField *ast.Ident, field *ast.Field, structTag reflect.StructTag, tagType string, pass *analysis.Pass, allowedTagNames, allowedTagTypes map[string]bool) { tagName, ok := structTag.Lookup(tagType) if !ok || tagName == "-" { return } if strings.Contains(tagName, ",omitempty") { - checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypeExceptions) + checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypes) tagName = strings.ReplaceAll(tagName, ",omitempty", "") } @@ -155,12 +155,12 @@ func processTag(structName string, goField *ast.Ident, field *ast.Field, structT tagName = strings.ReplaceAll(tagName, ",comma", "") } - checkGoFieldName(structName, goField.Name, tagName, goField.Pos(), pass, allowedTagNameExceptions) + checkGoFieldName(structName, goField.Name, tagName, goField.Pos(), pass, allowedTagNames) } -func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { +func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Pos, pass *analysis.Pass, allowedNames map[string]bool) { fullName := structName + "." + goFieldName - if allowedExceptions[fullName] { + if allowedNames[fullName] { return } @@ -171,8 +171,8 @@ func checkGoFieldName(structName, goFieldName, tagName string, tokenPos token.Po } } -func checkGoFieldType(structName, goFieldName string, field *ast.Field, tokenPos token.Pos, pass *analysis.Pass, allowedExceptions map[string]bool) { - if allowedExceptions[structName+"."+goFieldName] { +func checkGoFieldType(structName, goFieldName string, field *ast.Field, tokenPos token.Pos, pass *analysis.Pass, allowedTypes map[string]bool) { + if allowedTypes[structName+"."+goFieldName] { return } @@ -192,6 +192,12 @@ func checkAndReportInvalidTypes(structName, goFieldName string, fieldType ast.Ex if ident, ok := arrType.Elt.(*ast.Ident); ok && isBuiltinType(ident.Name) { const msg = "change the %q field type to %q in the struct %q" pass.Reportf(tokenPos, msg, goFieldName, "[]"+ident.Name, structName) + } else if starExpr, ok := arrType.Elt.(*ast.StarExpr); ok { + // Check for *[]*T - should be []*T + if ident, ok := starExpr.X.(*ast.Ident); ok { + const msg = "change the %q field type to %q in the struct %q" + pass.Reportf(tokenPos, msg, goFieldName, "[]*"+ident.Name, structName) + } } else { checkStructArrayType(structName, goFieldName, arrType, tokenPos, pass) } diff --git a/tools/structfield/testdata/src/has-warnings/main.go b/tools/structfield/testdata/src/has-warnings/main.go index 8ddc1eaf14a..26d2704fc78 100644 --- a/tools/structfield/testdata/src/has-warnings/main.go +++ b/tools/structfield/testdata/src/has-warnings/main.go @@ -14,13 +14,14 @@ type JSONFieldName struct { } type JSONFieldType struct { - String string `json:"string,omitempty"` // want `change the "String" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` - SliceOfStringPointers []*string `json:"slice_of_string_pointers,omitempty"` // want `change the "SliceOfStringPointers" field type to "\[\]string" in the struct "JSONFieldType"` - PointerToSliceOfStrings *[]string `json:"pointer_to_slice_of_strings,omitempty"` // want `change the "PointerToSliceOfStrings" field type to "\[\]string" in the struct "JSONFieldType"` - SliceOfStructs []Struct `json:"slice_of_structs,omitempty"` // want `change the "SliceOfStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` - PointerToSliceOfStructs *[]Struct `json:"pointer_to_slice_of_structs,omitempty"` // want `change the "PointerToSliceOfStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` - PointerToMap *map[string]string `json:"pointer_to_map,omitempty"` // want `change the "PointerToMap" field type to "map\[string\]string" in the struct "JSONFieldType"` - SliceOfInts []*int `json:"slice_of_ints,omitempty"` // want `change the "SliceOfInts" field type to "\[\]int" in the struct "JSONFieldType"` + String string `json:"string,omitempty"` // want `change the "String" field type to "\*string" in the struct "JSONFieldType" because its tag uses "omitempty"` + SliceOfStringPointers []*string `json:"slice_of_string_pointers,omitempty"` // want `change the "SliceOfStringPointers" field type to "\[\]string" in the struct "JSONFieldType"` + PointerToSliceOfStrings *[]string `json:"pointer_to_slice_of_strings,omitempty"` // want `change the "PointerToSliceOfStrings" field type to "\[\]string" in the struct "JSONFieldType"` + SliceOfStructs []Struct `json:"slice_of_structs,omitempty"` // want `change the "SliceOfStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerToSliceOfStructs *[]Struct `json:"pointer_to_slice_of_structs,omitempty"` // want `change the "PointerToSliceOfStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerToSliceOfPointerStructs *[]*Struct `json:"pointer_to_slice_of_pointer_structs,omitempty"` // want `change the "PointerToSliceOfPointerStructs" field type to "\[\]\*Struct" in the struct "JSONFieldType"` + PointerToMap *map[string]string `json:"pointer_to_map,omitempty"` // want `change the "PointerToMap" field type to "map\[string\]string" in the struct "JSONFieldType"` + SliceOfInts []*int `json:"slice_of_ints,omitempty"` // want `change the "SliceOfInts" field type to "\[\]int" in the struct "JSONFieldType"` } type Struct struct{}