diff --git a/Makefile b/Makefile index c3b18f313..f930711a9 100644 --- a/Makefile +++ b/Makefile @@ -45,6 +45,7 @@ completions: bin/gh$(EXE) .PHONY: lint lint: golangci-lint run ./... + go run ./cmd/idtype-checker ./... # just convenience tasks around `go test` .PHONY: test diff --git a/cmd/idtype-checker/main.go b/cmd/idtype-checker/main.go new file mode 100644 index 000000000..b7cbf6f9e --- /dev/null +++ b/cmd/idtype-checker/main.go @@ -0,0 +1,16 @@ +// Command idtype-checker runs the idtype analyzer to flag struct fields +// representing GitHub database IDs that use int instead of int64. +// +// Usage: +// +// go run ./cmd/idtype-checker ./... +package main + +import ( + "github.com/cli/cli/v2/pkg/linter/idtype" + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { + singlechecker.Main(idtype.Analyzer) +} diff --git a/internal/codespaces/api/api.go b/internal/codespaces/api/api.go index 0d1eaf5b3..d85b6dc61 100644 --- a/internal/codespaces/api/api.go +++ b/internal/codespaces/api/api.go @@ -150,7 +150,7 @@ type RepositoryOwner struct { // Repository represents a GitHub repository. type Repository struct { - ID int `json:"id"` + ID int64 `json:"id"` FullName string `json:"full_name"` DefaultBranch string `json:"default_branch"` Owner RepositoryOwner `json:"owner"` @@ -602,7 +602,7 @@ type Machine struct { } // GetCodespacesMachines returns the codespaces machines for the given repo, branch and location. -func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*Machine, error) { +func (a *API) GetCodespacesMachines(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*Machine, error) { reqURL := fmt.Sprintf("%s/repositories/%d/codespaces/machines", a.githubAPI, repoID) req, err := http.NewRequest(http.MethodGet, reqURL, nil) if err != nil { @@ -642,7 +642,7 @@ func (a *API) GetCodespacesMachines(ctx context.Context, repoID int, branch, loc } // GetCodespacesPermissionsCheck returns a bool indicating whether the user has accepted permissions for the given repo and devcontainer path. -func (a *API) GetCodespacesPermissionsCheck(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { +func (a *API) GetCodespacesPermissionsCheck(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { reqURL := fmt.Sprintf("%s/repositories/%d/codespaces/permissions_check", a.githubAPI, repoID) req, err := http.NewRequest(http.MethodGet, reqURL, nil) if err != nil { @@ -804,7 +804,7 @@ func (a *API) GetCodespaceBillableOwner(ctx context.Context, nwo string) (*User, // CreateCodespaceParams are the required parameters for provisioning a Codespace. type CreateCodespaceParams struct { - RepositoryID int + RepositoryID int64 IdleTimeoutMinutes int RetentionPeriodMinutes *int Branch string @@ -855,7 +855,7 @@ func (a *API) CreateCodespace(ctx context.Context, params *CreateCodespaceParams } type startCreateRequest struct { - RepositoryID int `json:"repository_id"` + RepositoryID int64 `json:"repository_id"` IdleTimeoutMinutes int `json:"idle_timeout_minutes,omitempty"` RetentionPeriodMinutes *int `json:"retention_period_minutes,omitempty"` Ref string `json:"ref"` @@ -1009,7 +1009,7 @@ type DevContainerEntry struct { // ListDevContainers returns a list of valid devcontainer.json files for the repo. Pass a negative limit to request all pages from // the API until all devcontainer.json files have been fetched. -func (a *API) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []DevContainerEntry, err error) { +func (a *API) ListDevContainers(ctx context.Context, repoID int64, branch string, limit int) (devcontainers []DevContainerEntry, err error) { perPage := 100 if limit > 0 && limit < 100 { perPage = limit diff --git a/pkg/cmd/agent-task/capi/job.go b/pkg/cmd/agent-task/capi/job.go index a09338695..bfc887ce3 100644 --- a/pkg/cmd/agent-task/capi/job.go +++ b/pkg/cmd/agent-task/capi/job.go @@ -35,12 +35,12 @@ type Job struct { } type JobActor struct { - ID int `json:"id"` + ID int64 `json:"id"` Login string `json:"login"` } type JobPullRequest struct { - ID int `json:"id"` + ID int64 `json:"id"` Number int `json:"number"` BaseRef string `json:"base_ref,omitempty"` } diff --git a/pkg/cmd/cache/delete/delete.go b/pkg/cmd/cache/delete/delete.go index 9cb188008..d8b620f7b 100644 --- a/pkg/cmd/cache/delete/delete.go +++ b/pkg/cmd/cache/delete/delete.go @@ -147,7 +147,7 @@ func deleteRun(opts *DeleteOptions) error { } } for _, cache := range caches.ActionsCaches { - toDelete = append(toDelete, strconv.Itoa(cache.Id)) + toDelete = append(toDelete, strconv.FormatInt(cache.Id, 10)) } } else { toDelete = append(toDelete, opts.Identifier) diff --git a/pkg/cmd/cache/shared/shared.go b/pkg/cmd/cache/shared/shared.go index 3e8096396..a853b143f 100644 --- a/pkg/cmd/cache/shared/shared.go +++ b/pkg/cmd/cache/shared/shared.go @@ -22,7 +22,7 @@ var CacheFields = []string{ type Cache struct { CreatedAt time.Time `json:"created_at"` - Id int `json:"id"` + Id int64 `json:"id"` Key string `json:"key"` LastAccessedAt time.Time `json:"last_accessed_at"` Ref string `json:"ref"` diff --git a/pkg/cmd/codespace/common.go b/pkg/cmd/codespace/common.go index e56e6c0b8..a410b1a7d 100644 --- a/pkg/cmd/codespace/common.go +++ b/pkg/cmd/codespace/common.go @@ -76,10 +76,10 @@ type apiClient interface { CreateCodespace(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) EditCodespace(ctx context.Context, codespaceName string, params *api.EditCodespaceParams) (*api.Codespace, error) GetRepository(ctx context.Context, nwo string) (*api.Repository, error) - GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*api.Machine, error) - GetCodespacesPermissionsCheck(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) + GetCodespacesMachines(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*api.Machine, error) + GetCodespacesPermissionsCheck(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) GetCodespaceRepositoryContents(ctx context.Context, codespace *api.Codespace, path string) ([]byte, error) - ListDevContainers(ctx context.Context, repoID int, branch string, limit int) (devcontainers []api.DevContainerEntry, err error) + ListDevContainers(ctx context.Context, repoID int64, branch string, limit int) (devcontainers []api.DevContainerEntry, err error) GetCodespaceRepoSuggestions(ctx context.Context, partialSearch string, params api.RepoSearchParameters) ([]string, error) GetCodespaceBillableOwner(ctx context.Context, nwo string) (*api.User, error) HTTPClient() (*http.Client, error) diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index bd2eb4623..734509a30 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -511,7 +511,7 @@ func (a *App) showStatus(ctx context.Context, codespace *api.Codespace) error { } // getMachineName prompts the user to select the machine type, or validates the machine if non-empty. -func getMachineName(ctx context.Context, apiClient apiClient, prompter SurveyPrompter, repoID int, machine, branch, location string, devcontainerPath string) (string, error) { +func getMachineName(ctx context.Context, apiClient apiClient, prompter SurveyPrompter, repoID int64, machine, branch, location string, devcontainerPath string) (string, error) { machines, err := apiClient.GetCodespacesMachines(ctx, repoID, branch, location, devcontainerPath) if err != nil { return "", fmt.Errorf("error requesting machine instance types: %w", err) diff --git a/pkg/cmd/codespace/create_test.go b/pkg/cmd/codespace/create_test.go index c5f11bd2f..9a959f5da 100644 --- a/pkg/cmd/codespace/create_test.go +++ b/pkg/cmd/codespace/create_test.go @@ -164,7 +164,7 @@ func TestApp_Create(t *testing.T) { name: "create codespace with nonexistent machine results in error", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*api.Machine, error) { + GetCodespacesMachinesFunc: func(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*api.Machine, error) { return []*api.Machine{ { Name: "GIGA", @@ -208,7 +208,7 @@ func TestApp_Create(t *testing.T) { name: "create codespace with devcontainer path results in selecting the correct machine type", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*api.Machine, error) { + GetCodespacesMachinesFunc: func(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*api.Machine, error) { if devcontainerPath == "" { return []*api.Machine{ { @@ -270,7 +270,7 @@ func TestApp_Create(t *testing.T) { name: "create codespace with default branch with default devcontainer if no path provided and no devcontainer files exist in the repo", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + ListDevContainersFunc: func(ctx context.Context, repoID int64, branch string, limit int) ([]api.DevContainerEntry, error) { return []api.DevContainerEntry{}, nil }, CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { @@ -305,7 +305,7 @@ func TestApp_Create(t *testing.T) { name: "returns error when getting devcontainer paths fails", fields: fields{ apiClient: apiCreateDefaults(&apiClientMock{ - ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + ListDevContainersFunc: func(ctx context.Context, repoID int64, branch string, limit int) ([]api.DevContainerEntry, error) { return nil, fmt.Errorf("some error") }, }), @@ -793,7 +793,7 @@ func TestHandleAdditionalPermissions(t *testing.T) { CreateCodespaceFunc: func(ctx context.Context, params *api.CreateCodespaceParams) (*api.Codespace, error) { return nil, tt.createCodespaceErr }, - GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { + GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { if tt.pollForPermissionsErr != nil { return false, tt.pollForPermissionsErr } @@ -844,12 +844,12 @@ func apiCreateDefaults(c *apiClientMock) *apiClientMock { } } if c.ListDevContainersFunc == nil { - c.ListDevContainersFunc = func(ctx context.Context, repoID int, branch string, limit int) ([]api.DevContainerEntry, error) { + c.ListDevContainersFunc = func(ctx context.Context, repoID int64, branch string, limit int) ([]api.DevContainerEntry, error) { return []api.DevContainerEntry{{Path: ".devcontainer/devcontainer.json"}}, nil } } if c.GetCodespacesMachinesFunc == nil { - c.GetCodespacesMachinesFunc = func(ctx context.Context, repoID int, branch, location string, devcontainerPath string) ([]*api.Machine, error) { + c.GetCodespacesMachinesFunc = func(ctx context.Context, repoID int64, branch, location string, devcontainerPath string) ([]*api.Machine, error) { return []*api.Machine{ { Name: "GIGA", diff --git a/pkg/cmd/codespace/mock_api.go b/pkg/cmd/codespace/mock_api.go index e4de0ae7a..c96bcf118 100644 --- a/pkg/cmd/codespace/mock_api.go +++ b/pkg/cmd/codespace/mock_api.go @@ -38,10 +38,10 @@ import ( // GetCodespaceRepositoryContentsFunc: func(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) { // panic("mock out the GetCodespaceRepositoryContents method") // }, -// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { +// GetCodespacesMachinesFunc: func(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { // panic("mock out the GetCodespacesMachines method") // }, -// GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { +// GetCodespacesPermissionsCheckFunc: func(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { // panic("mock out the GetCodespacesPermissionsCheck method") // }, // GetOrgMemberCodespaceFunc: func(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) { @@ -59,7 +59,7 @@ import ( // ListCodespacesFunc: func(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) { // panic("mock out the ListCodespaces method") // }, -// ListDevContainersFunc: func(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { +// ListDevContainersFunc: func(ctx context.Context, repoID int64, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { // panic("mock out the ListDevContainers method") // }, // ServerURLFunc: func() string { @@ -100,10 +100,10 @@ type apiClientMock struct { GetCodespaceRepositoryContentsFunc func(ctx context.Context, codespace *codespacesAPI.Codespace, path string) ([]byte, error) // GetCodespacesMachinesFunc mocks the GetCodespacesMachines method. - GetCodespacesMachinesFunc func(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) + GetCodespacesMachinesFunc func(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) // GetCodespacesPermissionsCheckFunc mocks the GetCodespacesPermissionsCheck method. - GetCodespacesPermissionsCheckFunc func(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) + GetCodespacesPermissionsCheckFunc func(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) // GetOrgMemberCodespaceFunc mocks the GetOrgMemberCodespace method. GetOrgMemberCodespaceFunc func(ctx context.Context, orgName string, userName string, codespaceName string) (*codespacesAPI.Codespace, error) @@ -121,7 +121,7 @@ type apiClientMock struct { ListCodespacesFunc func(ctx context.Context, opts codespacesAPI.ListCodespacesOptions) ([]*codespacesAPI.Codespace, error) // ListDevContainersFunc mocks the ListDevContainers method. - ListDevContainersFunc func(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) + ListDevContainersFunc func(ctx context.Context, repoID int64, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) // ServerURLFunc mocks the ServerURL method. ServerURLFunc func() string @@ -200,7 +200,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // RepoID is the repoID argument value. - RepoID int + RepoID int64 // Branch is the branch argument value. Branch string // Location is the location argument value. @@ -213,7 +213,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // RepoID is the repoID argument value. - RepoID int + RepoID int64 // Branch is the branch argument value. Branch string // DevcontainerPath is the devcontainerPath argument value. @@ -257,7 +257,7 @@ type apiClientMock struct { // Ctx is the ctx argument value. Ctx context.Context // RepoID is the repoID argument value. - RepoID int + RepoID int64 // Branch is the branch argument value. Branch string // Limit is the limit argument value. @@ -582,13 +582,13 @@ func (mock *apiClientMock) GetCodespaceRepositoryContentsCalls() []struct { } // GetCodespacesMachines calls GetCodespacesMachinesFunc. -func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { +func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int64, branch string, location string, devcontainerPath string) ([]*codespacesAPI.Machine, error) { if mock.GetCodespacesMachinesFunc == nil { panic("apiClientMock.GetCodespacesMachinesFunc: method is nil but apiClient.GetCodespacesMachines was just called") } callInfo := struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Location string DevcontainerPath string @@ -611,14 +611,14 @@ func (mock *apiClientMock) GetCodespacesMachines(ctx context.Context, repoID int // len(mockedapiClient.GetCodespacesMachinesCalls()) func (mock *apiClientMock) GetCodespacesMachinesCalls() []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Location string DevcontainerPath string } { var calls []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Location string DevcontainerPath string @@ -630,13 +630,13 @@ func (mock *apiClientMock) GetCodespacesMachinesCalls() []struct { } // GetCodespacesPermissionsCheck calls GetCodespacesPermissionsCheckFunc. -func (mock *apiClientMock) GetCodespacesPermissionsCheck(ctx context.Context, repoID int, branch string, devcontainerPath string) (bool, error) { +func (mock *apiClientMock) GetCodespacesPermissionsCheck(ctx context.Context, repoID int64, branch string, devcontainerPath string) (bool, error) { if mock.GetCodespacesPermissionsCheckFunc == nil { panic("apiClientMock.GetCodespacesPermissionsCheckFunc: method is nil but apiClient.GetCodespacesPermissionsCheck was just called") } callInfo := struct { Ctx context.Context - RepoID int + RepoID int64 Branch string DevcontainerPath string }{ @@ -657,13 +657,13 @@ func (mock *apiClientMock) GetCodespacesPermissionsCheck(ctx context.Context, re // len(mockedapiClient.GetCodespacesPermissionsCheckCalls()) func (mock *apiClientMock) GetCodespacesPermissionsCheckCalls() []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string DevcontainerPath string } { var calls []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string DevcontainerPath string } @@ -849,13 +849,13 @@ func (mock *apiClientMock) ListCodespacesCalls() []struct { } // ListDevContainers calls ListDevContainersFunc. -func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { +func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int64, branch string, limit int) ([]codespacesAPI.DevContainerEntry, error) { if mock.ListDevContainersFunc == nil { panic("apiClientMock.ListDevContainersFunc: method is nil but apiClient.ListDevContainers was just called") } callInfo := struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Limit int }{ @@ -876,13 +876,13 @@ func (mock *apiClientMock) ListDevContainers(ctx context.Context, repoID int, br // len(mockedapiClient.ListDevContainersCalls()) func (mock *apiClientMock) ListDevContainersCalls() []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Limit int } { var calls []struct { Ctx context.Context - RepoID int + RepoID int64 Branch string Limit int } diff --git a/pkg/cmd/gpg-key/delete/delete.go b/pkg/cmd/gpg-key/delete/delete.go index d0fb5c0d6..7ca17de81 100644 --- a/pkg/cmd/gpg-key/delete/delete.go +++ b/pkg/cmd/gpg-key/delete/delete.go @@ -74,7 +74,7 @@ func deleteRun(opts *DeleteOptions) error { id := "" for _, gpgKey := range gpgKeys { if gpgKey.KeyID == opts.KeyID { - id = strconv.Itoa(gpgKey.ID) + id = strconv.FormatInt(gpgKey.ID, 10) break } } diff --git a/pkg/cmd/gpg-key/delete/http.go b/pkg/cmd/gpg-key/delete/http.go index cfb11b9c3..22b43133b 100644 --- a/pkg/cmd/gpg-key/delete/http.go +++ b/pkg/cmd/gpg-key/delete/http.go @@ -11,7 +11,7 @@ import ( ) type gpgKey struct { - ID int + ID int64 KeyID string `json:"key_id"` } diff --git a/pkg/cmd/repo/autolink/shared/autolink.go b/pkg/cmd/repo/autolink/shared/autolink.go index 28e975e7c..66db44e3d 100644 --- a/pkg/cmd/repo/autolink/shared/autolink.go +++ b/pkg/cmd/repo/autolink/shared/autolink.go @@ -3,7 +3,7 @@ package shared import "github.com/cli/cli/v2/pkg/cmdutil" type Autolink struct { - ID int `json:"id"` + ID int64 `json:"id"` IsAlphanumeric bool `json:"is_alphanumeric"` KeyPrefix string `json:"key_prefix"` URLTemplate string `json:"url_template"` diff --git a/pkg/cmd/repo/deploy-key/list/http.go b/pkg/cmd/repo/deploy-key/list/http.go index 4470cec04..134b128dc 100644 --- a/pkg/cmd/repo/deploy-key/list/http.go +++ b/pkg/cmd/repo/deploy-key/list/http.go @@ -13,7 +13,7 @@ import ( ) type deployKey struct { - ID int `json:"id"` + ID int64 `json:"id"` Key string `json:"key"` Title string `json:"title"` CreatedAt time.Time `json:"created_at"` diff --git a/pkg/cmd/repo/deploy-key/list/list.go b/pkg/cmd/repo/deploy-key/list/list.go index 79d00c912..b0475ddf2 100644 --- a/pkg/cmd/repo/deploy-key/list/list.go +++ b/pkg/cmd/repo/deploy-key/list/list.go @@ -81,7 +81,7 @@ func listRun(opts *ListOptions) error { now := time.Now() for _, deployKey := range deployKeys { - sshID := strconv.Itoa(deployKey.ID) + sshID := strconv.FormatInt(deployKey.ID, 10) t.AddField(sshID) t.AddField(deployKey.Title) sshType := "read-only" diff --git a/pkg/cmd/ruleset/list/list.go b/pkg/cmd/ruleset/list/list.go index 8c79b5d96..1f3aabb4b 100644 --- a/pkg/cmd/ruleset/list/list.go +++ b/pkg/cmd/ruleset/list/list.go @@ -164,7 +164,7 @@ func listRun(opts *ListOptions) error { tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "NAME", "SOURCE", "STATUS", "RULES")) for _, rs := range result.Rulesets { - tp.AddField(strconv.Itoa(rs.DatabaseId), tableprinter.WithColor(cs.Cyan)) + tp.AddField(strconv.FormatInt(rs.DatabaseId, 10), tableprinter.WithColor(cs.Cyan)) tp.AddField(rs.Name, tableprinter.WithColor(cs.Bold)) tp.AddField(shared.RulesetSource(rs)) tp.AddField(strings.ToLower(rs.Enforcement)) diff --git a/pkg/cmd/ruleset/shared/shared.go b/pkg/cmd/ruleset/shared/shared.go index 536e8a5a9..e74c66221 100644 --- a/pkg/cmd/ruleset/shared/shared.go +++ b/pkg/cmd/ruleset/shared/shared.go @@ -10,7 +10,7 @@ import ( ) type RulesetGraphQL struct { - DatabaseId int + DatabaseId int64 Name string Target string Enforcement string @@ -24,13 +24,13 @@ type RulesetGraphQL struct { } type RulesetREST struct { - Id int + Id int64 Name string Target string Enforcement string CurrentUserCanBypass string `json:"current_user_can_bypass"` BypassActors []struct { - ActorId int `json:"actor_id"` + ActorId int64 `json:"actor_id"` ActorType string `json:"actor_type"` BypassMode string `json:"bypass_mode"` } `json:"bypass_actors"` @@ -50,7 +50,7 @@ type RulesetRule struct { Parameters map[string]interface{} RulesetSourceType string `json:"ruleset_source_type"` RulesetSource string `json:"ruleset_source"` - RulesetId int `json:"ruleset_id"` + RulesetId int64 `json:"ruleset_id"` } // Returns the source of the ruleset in the format "owner/name (repo)" or "owner (org)" diff --git a/pkg/cmd/ruleset/view/view.go b/pkg/cmd/ruleset/view/view.go index b535a1033..8cc43927c 100644 --- a/pkg/cmd/ruleset/view/view.go +++ b/pkg/cmd/ruleset/view/view.go @@ -150,7 +150,7 @@ func viewRun(opts *ViewOptions) error { } if rs != nil { - opts.ID = strconv.Itoa(rs.DatabaseId) + opts.ID = strconv.FormatInt(rs.DatabaseId, 10) // can't get a ruleset lower in the chain than what was queried, so no need to handle repos here if rs.Source.TypeName == "Organization" { @@ -185,7 +185,7 @@ func viewRun(opts *ViewOptions) error { } fmt.Fprintf(w, "\n%s\n", cs.Bold(rs.Name)) - fmt.Fprintf(w, "ID: %s\n", cs.Cyan(strconv.Itoa(rs.Id))) + fmt.Fprintf(w, "ID: %s\n", cs.Cyan(strconv.FormatInt(rs.Id, 10))) fmt.Fprintf(w, "Source: %s (%s)\n", rs.Source, rs.SourceType) fmt.Fprint(w, "Enforcement: ") @@ -262,7 +262,7 @@ func selectRulesetID(rsList *shared.RulesetList, p prompter.Prompter, cs *iostre for i, rs := range rsList.Rulesets { s := fmt.Sprintf( "%s: %s | %s | contains %s | configured in %s", - cs.Cyan(strconv.Itoa(rs.DatabaseId)), + cs.Cyan(strconv.FormatInt(rs.DatabaseId, 10)), rs.Name, strings.ToLower(rs.Enforcement), text.Pluralize(rs.Rules.TotalCount, "rule"), diff --git a/pkg/cmd/skills/publish/publish.go b/pkg/cmd/skills/publish/publish.go index c53ea0b6b..f5c3f990b 100644 --- a/pkg/cmd/skills/publish/publish.go +++ b/pkg/cmd/skills/publish/publish.go @@ -64,7 +64,7 @@ type tagEntry struct { // rulesetsResponse is a single ruleset from the rulesets API. type rulesetsResponse struct { - ID int `json:"id"` + ID int64 `json:"id"` Name string `json:"name"` Target string `json:"target"` Enforcement string `json:"enforcement"` diff --git a/pkg/cmd/ssh-key/list/list.go b/pkg/cmd/ssh-key/list/list.go index f69e57ea3..b92f4274c 100644 --- a/pkg/cmd/ssh-key/list/list.go +++ b/pkg/cmd/ssh-key/list/list.go @@ -83,7 +83,7 @@ func listRun(opts *ListOptions) error { now := time.Now() for _, sshKey := range sshKeys { - id := strconv.Itoa(sshKey.ID) + id := strconv.FormatInt(sshKey.ID, 10) if t.IsTTY() { t.AddField(sshKey.Title) t.AddField(id) diff --git a/pkg/cmd/ssh-key/shared/user_keys.go b/pkg/cmd/ssh-key/shared/user_keys.go index 035d00244..6a3d286ab 100644 --- a/pkg/cmd/ssh-key/shared/user_keys.go +++ b/pkg/cmd/ssh-key/shared/user_keys.go @@ -17,7 +17,7 @@ const ( ) type sshKey struct { - ID int + ID int64 Key string Title string Type string diff --git a/pkg/linter/idtype/analyzer.go b/pkg/linter/idtype/analyzer.go new file mode 100644 index 000000000..fcf659daa --- /dev/null +++ b/pkg/linter/idtype/analyzer.go @@ -0,0 +1,119 @@ +// Package idtype provides a go/analysis analyzer that flags struct fields +// representing GitHub database IDs that use int instead of int64. +// +// GitHub database IDs are internally 64-bit, and the REST API OpenAPI spec +// declares many of them with format: int64. Using Go's int (which is +// platform-dependent) risks overflow on 32-bit architectures. +package idtype + +import ( + "go/ast" + "go/token" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +var Analyzer = &analysis.Analyzer{ + Name: "idtype", + Doc: "checks that struct fields representing GitHub database IDs use int64 instead of int", + Run: run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, +} + +func run(pass *analysis.Pass) (interface{}, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + + nodeFilter := []ast.Node{ + (*ast.StructType)(nil), + } + + insp.Preorder(nodeFilter, func(n ast.Node) { + st := n.(*ast.StructType) + for _, field := range st.Fields.List { + if !isIntType(field.Type) { + continue + } + for _, name := range field.Names { + if isIDField(name.Name, field.Tag) { + pass.Reportf(field.Pos(), "struct field %s looks like a GitHub database ID but uses int; use int64 instead", name.Name) + } + } + } + }) + + return nil, nil +} + +// isIntType checks if the type expression is the bare "int" type. +func isIntType(expr ast.Expr) bool { + ident, ok := expr.(*ast.Ident) + return ok && ident.Name == "int" +} + +// isIDField returns true if the field name or JSON tag suggests this is a +// database ID. It checks: +// - Field name is exactly "ID" or "Id" +// - Field name ends with "ID" or "Id" (e.g. DatabaseId, RepositoryID, ActorId) +// - JSON tag is exactly "id" or ends with "_id" +func isIDField(fieldName string, tag *ast.BasicLit) bool { + if matchesIDName(fieldName) { + return true + } + if tag != nil && tag.Kind == token.STRING { + jsonTag := extractJSONTag(tag.Value) + if matchesIDTag(jsonTag) { + return true + } + } + return false +} + +// matchesIDName checks if a Go field name looks like a database ID field. +func matchesIDName(name string) bool { + if name == "ID" || name == "Id" { + return true + } + if strings.HasSuffix(name, "ID") || strings.HasSuffix(name, "Id") { + return true + } + return false +} + +// matchesIDTag checks if a JSON tag value looks like a database ID field. +func matchesIDTag(tag string) bool { + if tag == "" { + return false + } + if tag == "id" { + return true + } + if strings.HasSuffix(tag, "_id") { + return true + } + return false +} + +// extractJSONTag extracts the JSON field name from a struct tag literal. +// For example, given `json:"repository_id,omitempty"`, it returns "repository_id". +func extractJSONTag(rawTag string) string { + // Remove surrounding backticks or quotes + tag := strings.Trim(rawTag, "`\"") + + const prefix = `json:"` + idx := strings.Index(tag, prefix) + if idx < 0 { + return "" + } + tag = tag[idx+len(prefix):] + if end := strings.Index(tag, `"`); end >= 0 { + tag = tag[:end] + } + // Strip options like omitempty + if comma := strings.Index(tag, ","); comma >= 0 { + tag = tag[:comma] + } + return tag +} diff --git a/pkg/linter/idtype/analyzer_test.go b/pkg/linter/idtype/analyzer_test.go new file mode 100644 index 000000000..122406855 --- /dev/null +++ b/pkg/linter/idtype/analyzer_test.go @@ -0,0 +1,12 @@ +package idtype + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "example") +} diff --git a/pkg/linter/idtype/testdata/src/example/example.go b/pkg/linter/idtype/testdata/src/example/example.go new file mode 100644 index 000000000..de62dee5f --- /dev/null +++ b/pkg/linter/idtype/testdata/src/example/example.go @@ -0,0 +1,66 @@ +package example + +// Should be flagged - int fields with ID-like names or tags. + +type Repository struct { + ID int `json:"id"` // want `struct field ID looks like a GitHub database ID but uses int; use int64 instead` + Name string `json:"name"` +} + +type Cache struct { + Id int `json:"id"` // want `struct field Id looks like a GitHub database ID but uses int; use int64 instead` + Key string `json:"key"` +} + +type RulesetRule struct { + Type string + RulesetId int `json:"ruleset_id"` // want `struct field RulesetId looks like a GitHub database ID but uses int; use int64 instead` +} + +type Autolink struct { + ID int `json:"id"` // want `struct field ID looks like a GitHub database ID but uses int; use int64 instead` +} + +type DeployKey struct { + ID int // want `struct field ID looks like a GitHub database ID but uses int; use int64 instead` +} + +type Actor struct { + ActorId int `json:"actor_id"` // want `struct field ActorId looks like a GitHub database ID but uses int; use int64 instead` +} + +type WithDatabaseID struct { + DatabaseId int // want `struct field DatabaseId looks like a GitHub database ID but uses int; use int64 instead` +} + +type RepositoryIDField struct { + RepositoryID int `json:"repository_id"` // want `struct field RepositoryID looks like a GitHub database ID but uses int; use int64 instead` +} + +// Should NOT be flagged - correct types or non-ID fields. + +type CorrectRepository struct { + ID int64 `json:"id"` + Name string `json:"name"` +} + +type CorrectRule struct { + RulesetId int64 `json:"ruleset_id"` +} + +type NotAnID struct { + Count int `json:"count"` + Timeout int `json:"idle_timeout_minutes"` + Name string `json:"name"` + Description string +} + +type StringID struct { + ID string `json:"id"` +} + +type NumberField struct { + TotalCount int + Number int `json:"number"` + Port int `json:"port"` +}