From 4603b21da14337507132b3e8d38689e00eb4e9c4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:06:13 +0000 Subject: [PATCH 1/2] Initial plan From 60c7749c36d89c3da1149cdd86beea00f8d3928e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:15:08 +0000 Subject: [PATCH 2/2] Add full-repo audit mode to errormessage linter Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- Makefile | 11 ++++++ pkg/linters/README.md | 4 +-- pkg/linters/errormessage/errormessage.go | 34 +++++++++++++++++-- pkg/linters/errormessage/errormessage_test.go | 22 ++++++++++++ 4 files changed, 67 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 04df77d164d..b3e6675b343 100644 --- a/Makefile +++ b/Makefile @@ -1023,6 +1023,16 @@ lint-cjs: fmt-check-cjs validate-cjs-syntax check-node-version lint-json: fmt-check-json @echo "✓ JSON formatting validated" +# Full-repository error-message audit (non-blocking report). +# Reports pre-existing error-message violations across the repo so the debt can +# be tracked as a metric over time. Always exits 0. +.PHONY: lint-error-messages-report +lint-error-messages-report: + @echo "Building custom linters..." + @env -u GOOS -u GOARCH go build -o /tmp/gh-aw-linters ./cmd/linters + @echo "Auditing error messages across $(LINTER_PACKAGES) (non-blocking)..." + @/tmp/gh-aw-linters -errormessage -errormessage.full-repo $(LINTER_PACKAGES) || true + # Lint error messages for quality compliance .PHONY: lint-errors lint-errors: @@ -1399,6 +1409,7 @@ help: @echo " validate-cjs-syntax - Syntax-check all non-test .cjs files (catches module-load SyntaxErrors)" @echo " lint-json - Lint JSON files in pkg directory (excluding actions/setup/js)" @echo " lint-errors - Lint error messages for quality compliance" + @echo " lint-error-messages-report - Non-blocking full-repo error message audit" @echo " validate-otel-contract - Validate the gh-aw OpenTelemetry compatibility contract" @echo " lint-action-sh - Lint action shell scripts for python/python3 invocations" @echo " shellcheck-setup-sh - Run shellcheck on actions/setup/sh scripts" diff --git a/pkg/linters/README.md b/pkg/linters/README.md index 60fe1ff1a20..30f7ea61b4d 100644 --- a/pkg/linters/README.md +++ b/pkg/linters/README.md @@ -15,7 +15,7 @@ This package currently provides custom Go analyzers in the following subpackages - `deferinloop` — reports `defer` statements placed directly inside `for`/`range` loop bodies, which execute when the enclosing function returns rather than each iteration and can cause resource leaks. - `errorfwrapv` — reports `fmt.Errorf` calls that pass error arguments without `%w` wrapping. - `excessivefuncparams` — reports function declarations that exceed a configurable parameter-count threshold. -- `errormessage` — reports non-actionable error-message patterns in changed files. +- `errormessage` — reports non-actionable error-message patterns in changed files; pass `-errormessage.full-repo` (or `-errormessage.changed-files=all`) to audit the whole repository. - `errortypeassertion` — reports type assertions from `error` to concrete types and recommends `errors.As`. - `errstringmatch` — reports `strings.Contains(err.Error(), "...")` patterns and recommends `errors.Is` / `errors.As`. - `fileclosenotdeferred` — reports non-deferred file `Close()` calls that can leak resources. @@ -85,7 +85,7 @@ This package currently provides custom Go analyzers in the following subpackages | `deferinloop` | Custom `go/analysis` analyzer that flags `defer` statements inside `for`/`range` loop bodies that execute when the enclosing function returns rather than each iteration | | `errorfwrapv` | Custom `go/analysis` analyzer that flags `fmt.Errorf` calls that pass error arguments without `%w` wrapping | | `excessivefuncparams` | Custom `go/analysis` analyzer that flags function declarations with too many positional parameters | -| `errormessage` | Custom `go/analysis` analyzer that flags non-actionable error message patterns in changed files | +| `errormessage` | Custom `go/analysis` analyzer that flags non-actionable error message patterns in changed files (or all files with `-errormessage.full-repo`) | | `errortypeassertion` | Custom `go/analysis` analyzer that flags type assertions from `error` to concrete types and recommends `errors.As` | | `errstringmatch` | Custom `go/analysis` analyzer that flags brittle `strings.Contains(err.Error(), "...")` checks | | `execcommandwithoutcontext` | Custom `go/analysis` analyzer that flags `exec.Command(...)` calls that should use `exec.CommandContext(...)` in context-receiving functions | diff --git a/pkg/linters/errormessage/errormessage.go b/pkg/linters/errormessage/errormessage.go index 2557a55979f..f664fb4c805 100644 --- a/pkg/linters/errormessage/errormessage.go +++ b/pkg/linters/errormessage/errormessage.go @@ -24,22 +24,35 @@ var ( // changedFilesCSV allows CI to scope linting to changed files only, // preventing legacy violations from blocking incremental adoption. changedFilesCSV string + // fullRepo enables auditing every analyzed file instead of only the + // changed ones, so pre-existing violations can be tracked as a metric. + fullRepo bool ) +// fullRepoSentinel is the -changed-files value that enables full-repository +// auditing, equivalent to passing -full-repo. +const fullRepoSentinel = "all" + // Analyzer is the errormessage analysis pass. var Analyzer = &analysis.Analyzer{ Name: "errormessage", - Doc: "reports non-actionable error message patterns in changed files", + Doc: "reports non-actionable error message patterns in changed files (or all files with -full-repo)", URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/errormessage", Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer}, Run: run, } func init() { - Analyzer.Flags.StringVar(&changedFilesCSV, "changed-files", "", "comma-separated list of changed file paths to lint (when empty, analyzer is a no-op)") + Analyzer.Flags.StringVar(&changedFilesCSV, "changed-files", "", "comma-separated list of changed file paths to lint (when empty, analyzer is a no-op; use \"all\" to audit every file)") + Analyzer.Flags.BoolVar(&fullRepo, "full-repo", false, "audit every analyzed file instead of only the changed ones") } func run(pass *analysis.Pass) (any, error) { + if fullRepo || isFullRepoSentinel(changedFilesCSV) { + pkgLog.Printf("analyzing package %s in full-repo mode", pass.Pkg.Path()) + return runOnFiles(pass, nil) + } + changed := parseChangedFiles(changedFilesCSV) if len(changed) == 0 { pkgLog.Printf("no changed files provided for %s, skipping", pass.Pkg.Path()) @@ -47,6 +60,12 @@ func run(pass *analysis.Pass) (any, error) { } pkgLog.Printf("analyzing package %s (%d changed files)", pass.Pkg.Path(), len(changed)) + return runOnFiles(pass, changed) +} + +// runOnFiles analyzes the package. When changed is nil every file is checked +// (full-repo audit mode); otherwise only files present in changed are checked. +func runOnFiles(pass *analysis.Pass, changed map[string]struct{}) (any, error) { insp, err := astutil.Inspector(pass) if err != nil { return nil, err @@ -104,7 +123,18 @@ func parseChangedFiles(csv string) map[string]struct{} { return changed } +// isFullRepoSentinel reports whether the -changed-files value requests a +// full-repository audit. +func isFullRepoSentinel(csv string) bool { + return strings.EqualFold(strings.TrimSpace(csv), fullRepoSentinel) +} + +// shouldCheckFile reports whether filename is in scope. A nil changed set means +// full-repo audit mode, where every file is in scope. func shouldCheckFile(filename string, changed map[string]struct{}) bool { + if changed == nil { + return true + } path := filepath.ToSlash(filename) for changedPath := range changed { if path == changedPath || strings.HasSuffix(path, "/"+changedPath) { diff --git a/pkg/linters/errormessage/errormessage_test.go b/pkg/linters/errormessage/errormessage_test.go index 7d928b43b0c..7bde75c9bee 100644 --- a/pkg/linters/errormessage/errormessage_test.go +++ b/pkg/linters/errormessage/errormessage_test.go @@ -22,3 +22,25 @@ func TestErrorMessage(t *testing.T) { analysistest.Run(t, analysistest.TestData(), errormessage.Analyzer, "errormessage") } + +func TestErrorMessageFullRepoFlag(t *testing.T) { + if err := errormessage.Analyzer.Flags.Set("full-repo", "true"); err != nil { + t.Fatalf("failed to set full-repo flag: %v", err) + } + t.Cleanup(func() { + _ = errormessage.Analyzer.Flags.Set("full-repo", "false") + }) + + analysistest.Run(t, analysistest.TestData(), errormessage.Analyzer, "errormessage") +} + +func TestErrorMessageFullRepoSentinel(t *testing.T) { + if err := errormessage.Analyzer.Flags.Set("changed-files", "all"); err != nil { + t.Fatalf("failed to set changed-files flag: %v", err) + } + t.Cleanup(func() { + _ = errormessage.Analyzer.Flags.Set("changed-files", "") + }) + + analysistest.Run(t, analysistest.TestData(), errormessage.Analyzer, "errormessage") +}