diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..8230ad5 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,56 @@ +version: "2" + +linters: + default: none + + enable: + # Core: catch real bugs + - errcheck # unchecked errors + - govet # suspicious constructs + - ineffassign # wasted assignments + - staticcheck # comprehensive bug detection + - unused # dead code + + # Error handling + - errorlint # errors.As / %w best practices + + # Security + - gosec # security issues + + # Typos + - misspell # spelling mistakes in identifiers + + settings: + gosec: + excludes: + - G104 # errcheck already handles unchecked errors + - G304 # file inclusion by variable is expected for CLI tools + + exclusions: + paths: + - vendor/ + - npm/ + - skills/ + - docs/ + rules: + # Idiomatic Go: defer Close() error is intentionally ignored + - linters: [errcheck] + text: "Error return value of .*(resp\\.Body\\.Close|file\\.Close).*is not checked" + # Output formatting: fmt.Fprint* errors are low-value + - linters: [errcheck] + text: "Error return value of `fmt\\.Fprintf?" + # Test helpers: FlagSet.Set is setup code + - linters: [errcheck] + text: "Error return value of .*FlagSet.*\\.Set" + # Best-effort output rendering + - linters: [errcheck] + path: render\.go$ + # errorlint: type assertions are fine in tests + - linters: [errorlint] + path: _test\.go$ + # gosec: tests are not attack surface + - linters: [gosec] + path: _test\.go$ + # apiInt: intentional uint64->int truncation for API response parsing + - linters: [gosec] + text: "G115: integer overflow conversion uint64 -> int" diff --git a/Makefile b/Makefile index 6a9db7f..8c6702d 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ BINARY := gitlink-cli VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev") LDFLAGS := -s -w -X '$(MODULE)/cmd.Version=$(VERSION)' -.PHONY: build install clean test check vet fmt cover +.PHONY: build install clean test check vet fmt cover lint build: go build -ldflags "$(LDFLAGS)" -o $(BINARY) . @@ -32,7 +32,10 @@ cover: go test -coverprofile=coverage.out ./... go tool cover -func=coverage.out -check: fmt vet test +lint: + golangci-lint run ./... + +check: fmt vet lint test @echo "All checks passed." hooks: diff --git a/cmd/api/api.go b/cmd/api/api.go index d13cf52..a8f1f2d 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -2,6 +2,7 @@ package api import ( "encoding/json" + "errors" "fmt" "io" "net/url" @@ -68,7 +69,8 @@ func runAPI(c *cobra.Command, args []string) error { env, err := cli.Do(method, path, body, query) if err != nil { - if apiErr, ok := err.(*client.APIError); ok { + var apiErr *client.APIError + if errors.As(err, &apiErr) { errEnv := output.ErrorEnvelope(apiErr.Code, apiErr.Message, "") return output.Print(errEnv, resolveFormat()) } diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index ed9dc22..e9af757 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -51,7 +51,7 @@ func loginWithPassword() error { username = strings.TrimSpace(username) fmt.Print("Password: ") - passwordBytes, err := term.ReadPassword(int(syscall.Stdin)) + passwordBytes, err := term.ReadPassword(syscall.Stdin) if err != nil { return fmt.Errorf("failed to read password: %w", err) } diff --git a/internal/auth/login.go b/internal/auth/login.go index 4b035e7..07e512c 100644 --- a/internal/auth/login.go +++ b/internal/auth/login.go @@ -102,7 +102,7 @@ func Login(username, password string) (*LoginResult, error) { if _, verifyErr := GetCurrentUser(); verifyErr != nil { // Clean up the bad token _ = DeleteToken() - return nil, fmt.Errorf("login failed: credentials not accepted by API (%v)", verifyErr) + return nil, fmt.Errorf("login failed: credentials not accepted by API (%w)", verifyErr) } return &result, nil diff --git a/internal/client/client.go b/internal/client/client.go index 831caa7..c001788 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -56,7 +56,7 @@ func (c *Client) Do(method, path string, body interface{}, query url.Values) (*o path += ".json" } fullURL := c.BaseURL + path - if query != nil && len(query) > 0 { + if len(query) > 0 { sep := "?" if strings.Contains(fullURL, "?") { sep = "&" diff --git a/scripts/pre-commit b/scripts/pre-commit index 0542568..9bb97aa 100755 --- a/scripts/pre-commit +++ b/scripts/pre-commit @@ -1,5 +1,5 @@ #!/bin/sh -# Pre-commit hook: run fmt + vet + test before every commit. +# Pre-commit hook: run fmt + vet + lint + test before every commit. # Install: make hooks set -e @@ -18,6 +18,10 @@ echo "=== go vet ===" go vet ./... echo " OK" +echo "=== golangci-lint ===" +golangci-lint run ./... +echo " OK" + echo "=== go test ===" go test -race ./... echo " OK" diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 056b595..03f67e2 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -1,6 +1,7 @@ package common import ( + "fmt" "strconv" "github.com/spf13/cobra" @@ -49,7 +50,9 @@ func MountShortcut(parent *cobra.Command, s *Shortcut) { cmd.Flags().String(f.Name, f.Default, f.Usage) } if f.Required { - cmd.MarkFlagRequired(f.Name) + if err := cmd.MarkFlagRequired(f.Name); err != nil { + panic(fmt.Sprintf("failed to mark flag %s as required: %v", f.Name, err)) + } } } diff --git a/shortcuts/workflow/api_types.go b/shortcuts/workflow/api_types.go index 3a0bf3e..4a4fe53 100644 --- a/shortcuts/workflow/api_types.go +++ b/shortcuts/workflow/api_types.go @@ -59,7 +59,7 @@ func normalizeAPIData(data interface{}) (interface{}, error) { return nil, nil } var decoded interface{} - if err := json.Unmarshal([]byte(trimmed), &decoded); err != nil { + if json.Unmarshal([]byte(trimmed), &decoded) != nil { return v, nil } return decoded, nil @@ -133,6 +133,8 @@ func apiString(v interface{}) string { switch value := v.(type) { case string: return value + case json.Number: + return value.String() case fmt.Stringer: return value.String() case float64: @@ -149,8 +151,6 @@ func apiString(v interface{}) string { return strconv.FormatUint(value, 10) case uint32: return strconv.FormatUint(uint64(value), 10) - case json.Number: - return value.String() case bool: return strconv.FormatBool(value) default: diff --git a/shortcuts/workflow/health_fetch.go b/shortcuts/workflow/health_fetch.go index 0efbe75..62066c5 100644 --- a/shortcuts/workflow/health_fetch.go +++ b/shortcuts/workflow/health_fetch.go @@ -36,7 +36,7 @@ func FetchHealthInput(ctx *common.RuntimeContext, opts HealthFetchOptions) (Heal } else { input.OpenIssues = len(issues) input.StaleIssues = countStaleItems(issues, staleDays) - input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(issues, input.RecentActivityDays)) + input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(issues)) } if prs, err := fetchAllListItems(ctx, workflowRepoPath(owner, repo)+"/pulls", issueListQuery("open"), 100); err != nil { @@ -44,7 +44,7 @@ func FetchHealthInput(ctx *common.RuntimeContext, opts HealthFetchOptions) (Heal } else { input.OpenPRs = len(prs) input.StalePRs = countStaleItems(prs, staleDays) - input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(prs, input.RecentActivityDays)) + input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(prs)) } if opts.IncludeRelease { @@ -54,7 +54,7 @@ func FetchHealthInput(ctx *common.RuntimeContext, opts HealthFetchOptions) (Heal } else { input.ReleaseKnown = true input.HasRecentRelease = len(releases) > 0 - input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(releases, input.RecentActivityDays)) + input.RecentActivityKnown, input.RecentActivityDays, input = updateRecentActivity(input, latestTimeFromItems(releases)) } } @@ -152,7 +152,7 @@ func applyAgentReadinessEstimate(input *HealthInput) { score++ } input.AgentReadinessKnown = true - input.AgentReadinessScore = clampInt(score, 0, 10) + input.AgentReadinessScore = clampInt(score, 10) } func countStaleItems(items []map[string]interface{}, staleDays int) int { @@ -187,7 +187,7 @@ func itemActivityTime(item map[string]interface{}) time.Time { ) } -func latestTimeFromItems(items []map[string]interface{}, currentDays int) time.Time { +func latestTimeFromItems(items []map[string]interface{}) time.Time { latest := time.Time{} for _, item := range items { latest = apiLatestTime(latest, itemActivityTime(item)) @@ -280,10 +280,7 @@ func buildPassing(item map[string]interface{}) bool { } } } - if apiBool(item["success"]) { - return true - } - return false + return apiBool(item["success"]) } func uniqueScoringNotes(notes []ScoringNote) []ScoringNote { diff --git a/shortcuts/workflow/health_score.go b/shortcuts/workflow/health_score.go index 771d6dd..97ab1ac 100644 --- a/shortcuts/workflow/health_score.go +++ b/shortcuts/workflow/health_score.go @@ -3,7 +3,7 @@ package workflow func ScoreHealth(input HealthInput, lang string) HealthResult { lang = normalizeLang(lang) - metrics := []HealthMetric{} + metrics := make([]HealthMetric, 0, 8) notes := []ScoringNote{} recommendations := []string{} @@ -73,7 +73,7 @@ func ScoreHealth(input HealthInput, lang string) HealthResult { healthScore := 0 if maxTotal > 0 { - healthScore = clampInt(total*100/maxTotal, 0, 100) + healthScore = clampInt(total*100/maxTotal, 100) } if len(recommendations) == 0 { recommendations = append(recommendations, message(lang, "rec_maintain")) @@ -99,7 +99,7 @@ func scoreIssueBacklog(input HealthInput, lang string) HealthMetric { } else if input.OpenIssues > 10 { score -= 2 } - score = clampInt(score, 0, 20) + score = clampInt(score, 20) status := "good" reason := message(lang, "health_issue_backlog_good") @@ -120,7 +120,7 @@ func scorePRBacklog(input HealthInput, lang string) HealthMetric { } else if input.OpenPRs > 5 { score -= 2 } - score = clampInt(score, 0, 20) + score = clampInt(score, 20) status := "good" reason := message(lang, "health_pr_backlog_good") @@ -195,7 +195,7 @@ func scoreAgentReadiness(input HealthInput, lang string) (HealthMetric, ScoringN if !input.AgentReadinessKnown { return HealthMetric{Name: "agent_readiness", Status: "unknown", Score: 5, MaxScore: 10, Reason: message(lang, "health_agent_unknown")}, ScoringNote{Metric: "agent_readiness", Note: message(lang, "health_agent_unknown")} } - score := clampInt(input.AgentReadinessScore, 0, 10) + score := clampInt(input.AgentReadinessScore, 10) status := "attention" if score >= 8 { status = "good" @@ -228,9 +228,9 @@ func riskLevel(score int) string { } } -func clampInt(value int, minValue int, maxValue int) int { - if value < minValue { - return minValue +func clampInt(value, maxValue int) int { + if value < 0 { + return 0 } if value > maxValue { return maxValue diff --git a/shortcuts/workflow/repo_report.go b/shortcuts/workflow/repo_report.go index d120f28..4043be6 100644 --- a/shortcuts/workflow/repo_report.go +++ b/shortcuts/workflow/repo_report.go @@ -264,7 +264,7 @@ func computeRepoReportScore(baseScore int, hasHealth bool, issueSummary RepoIssu if issueSummary.Total == 0 && prSummary.Total == 0 && !hasHealth { score = 50 } - return clampInt(score, 0, 100) + return clampInt(score, 100) } func hasSecurityP0(results []TriageResult) bool { diff --git a/shortcuts/workflow/triage_rules.go b/shortcuts/workflow/triage_rules.go index 2ecaf5a..ff204bc 100644 --- a/shortcuts/workflow/triage_rules.go +++ b/shortcuts/workflow/triage_rules.go @@ -2,7 +2,6 @@ package workflow import ( "fmt" - "sort" "strings" ) @@ -78,7 +77,9 @@ func AnalyzeIssue(input IssueInput, lang string) TriageResult { } func normalizeIssueText(input IssueInput) string { - parts := []string{input.Title, input.Body} + parts := make([]string, 2, 2+len(input.Labels)) + parts[0] = input.Title + parts[1] = input.Body parts = append(parts, input.Labels...) return strings.ToLower(strings.Join(parts, "\n")) } @@ -158,7 +159,7 @@ func calculateConfidence(detectedType string, scores map[string]int, matchedRule if detectedType == IssueTypeSecurity || detectedType == IssueTypeBug || detectedType == IssueTypeCI { confidence += 10 } - return clampInt(confidence, 0, 100) + return clampInt(confidence, 100) } func detectMissingInformation(text string, detectedType string, lang string) []string { @@ -298,9 +299,3 @@ func uniqueStrings(values []string) []string { } return unique } - -func sortedStrings(values []string) []string { - copied := append([]string(nil), values...) - sort.Strings(copied) - return copied -}