Add globwalkignorederror linter: flag discarded errors from filepath.Glob/os.ReadDir

pkg/linters/README.md

@@ -22,6 +22,7 @@ This package currently provides custom Go analyzers in the following subpackages
 - `execcommandwithoutcontext` — reports `exec.Command(...)` calls inside functions that already receive `context.Context` and should use `exec.CommandContext(...)`.
 - `fmterrorfnoverbs` — reports `fmt.Errorf` calls whose format string contains no verbs, recommending `errors.New` instead.
 - `fprintlnsprintf` — reports `fmt.Fprintln(..., fmt.Sprintf(...))` patterns and recommends direct formatting calls.
+- `globwalkignorederror` — reports `filepath.Glob` and `os.ReadDir` calls where the error return is discarded with `_`.
 - `goroutinemissingrecover` — reports goroutines started via a function literal whose body does not install a top-level `defer func() { recover() }()` guard.
 - `hardcodedfilepath` — reports hard-coded file path string literals that match known path constants or should be extracted into named constants; also annotates paths that appear in log/print calls.
 - `httpnoctx` — reports HTTP client and package-level HTTP calls that do not accept a `context.Context`.
@@ -92,6 +93,7 @@ This package currently provides custom Go analyzers in the following subpackages
 | `fileclosenotdeferred` | Custom `go/analysis` analyzer that flags file `Close()` calls that are not deferred immediately |
 | `fmterrorfnoverbs` | Custom `go/analysis` analyzer that flags `fmt.Errorf` calls with no format verbs, recommending `errors.New` |
 | `fprintlnsprintf` | Custom `go/analysis` analyzer that flags `fmt.Fprintln(..., fmt.Sprintf(...))` patterns |
+| `globwalkignorederror` | Custom `go/analysis` analyzer that flags `filepath.Glob` and `os.ReadDir` calls where the error return is discarded with `_` |
 | `goroutinemissingrecover` | Custom `go/analysis` analyzer that flags goroutines started via a function literal that do not install a top-level defer/recover guard |
 | `hardcodedfilepath` | Custom `go/analysis` analyzer that flags hard-coded file path string literals that match known path constants or should be extracted as named constants; annotates paths in log/print calls |
 | `httpnoctx` | Custom `go/analysis` analyzer that flags HTTP calls that do not accept a `context.Context` |
@@ -230,6 +232,7 @@ _ = trimleftright.Analyzer
 - `github.com/github/gh-aw/pkg/linters/fileclosenotdeferred` — file-close-not-deferred analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/fmterrorfnoverbs` — fmt-errorf-no-verbs analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/fprintlnsprintf` — fprintln-sprintf analyzer subpackage
+- `github.com/github/gh-aw/pkg/linters/globwalkignorederror` — glob-walk-ignored-error analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/hardcodedfilepath` — hard-coded-file-path analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/httpnoctx` — HTTP-no-context analyzer subpackage
 - `github.com/github/gh-aw/pkg/linters/httprespbodyclose` — HTTP-response-body-close analyzer subpackage

pkg/linters/doc.go

@@ -1,6 +1,6 @@
 // Package linters is a namespace for gh-aw's custom Go analysis linters.
 //
-// All 63 active analyzers:
+// All 64 active analyzers:
 //
 //   - appendbytestring — flags append(b, []byte(s)...) calls where s is a string that can be simplified to append(b, s...)
 //   - appendoneelement — flags append(s, []T{x}...) calls where a single-element slice literal is spread and can be simplified to append(s, x)
@@ -18,6 +18,7 @@
 //   - fileclosenotdeferred — flags file Close() calls that are not deferred
 //   - fmterrorfnoverbs — flags fmt.Errorf calls with no format verbs, recommending errors.New
 //   - fprintlnsprintf — flags fmt.Fprintln(..., fmt.Sprintf(...)) patterns
+//   - globwalkignorederror — flags filepath.Glob and os.ReadDir calls where the error return is discarded with _
 //   - goroutinemissingrecover — flags goroutines started via a function literal whose body does not install a top-level defer/recover guard
 //   - hardcodedfilepath — flags hard-coded file path string literals that match known path constants or should be extracted as named constants
 //   - httpnoctx — flags HTTP calls that do not accept a context.Context

pkg/linters/globwalkignorederror/globwalkignorederror.go

@@ -0,0 +1,86 @@
+// Package globwalkignorederror implements a Go analysis linter that flags
+// filepath.Glob and os.ReadDir calls where the error return is discarded
+// with _.
+package globwalkignorederror
+
+import (
+	"go/ast"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+
+	"github.com/github/gh-aw/pkg/linters/internal/analyzerutil"
+	"github.com/github/gh-aw/pkg/linters/internal/filecheck"
+	"github.com/github/gh-aw/pkg/linters/internal/nolint"
+)
+
+// Analyzer is the glob-walk-ignored-error analysis pass.
+var Analyzer = analyzerutil.New("globwalkignorederror", "reports filepath.Glob and os.ReadDir calls where the error return is discarded with _", run)
+
+// checkedFuncs maps package import path to the set of function names within
+// that package whose discarded error return should be flagged.
+var checkedFuncs = map[string]map[string]bool{
+	"path/filepath": {"Glob": true},
+	"os":            {"ReadDir": true},
+}
+
+func run(pass *analysis.Pass) (any, error) {
+	nolintIndex, err := nolint.Index(pass)
+	if err != nil {
+		return nil, err
+	}
+	generatedFiles, err := filecheck.Index(pass)
+	if err != nil {
+		return nil, err
+	}
+
+	nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)}
+	return analyzerutil.Preorder(pass, nodeFilter, func(n ast.Node) {
+		analyzeGlobWalkAssign(pass, n, generatedFiles, nolintIndex)
+	})
+}
+
+// analyzeGlobWalkAssign checks whether an assignment discards the error
+// return from filepath.Glob or os.ReadDir and reports a diagnostic if so.
+func analyzeGlobWalkAssign(pass *analysis.Pass, n ast.Node, generatedFiles filecheck.GeneratedIndex, nolintIndex nolint.DirectiveIndex) {
+	assign, ok := n.(*ast.AssignStmt)
+	if !ok {
+		return
+	}
+	if len(assign.Lhs) != 2 || len(assign.Rhs) != 1 {
+		return
+	}
+	blank, ok := assign.Lhs[1].(*ast.Ident)
+	if !ok || blank.Name != "_" {
+		return
+	}
+	call, ok := assign.Rhs[0].(*ast.CallExpr)
+	if !ok {
+		return
+	}
+	sel, ok := call.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return
+	}
+	ident, ok := sel.X.(*ast.Ident)
+	if !ok {
+		return
+	}
+	obj := pass.TypesInfo.Uses[ident]
+	pkgName, ok := obj.(*types.PkgName)
+	if !ok {
+		return
+	}
+	funcs, ok := checkedFuncs[pkgName.Imported().Path()]
+	if !ok || !funcs[sel.Sel.Name] {
+		return
+	}
+	position := pass.Fset.PositionFor(call.Pos(), false)
+	if filecheck.ShouldSkipFilename(position.Filename, generatedFiles) {
+		return
+	}
+	if nolint.HasDirectiveForLinter(position, nolintIndex, "globwalkignorederror") {
+		return
+	}
+	pass.ReportRangef(call, "error return from %s.%s is discarded; malformed patterns or unreadable directories silently produce an empty result", pkgName.Imported().Name(), sel.Sel.Name)
+}

pkg/linters/globwalkignorederror/globwalkignorederror_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package globwalkignorederror_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/globwalkignorederror"
+)
+
+func TestAnalyzer(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, globwalkignorederror.Analyzer, "globwalkignorederror")
+}

pkg/linters/globwalkignorederror/testdata/src/globwalkignorederror/globwalkignorederror.go

@@ -0,0 +1,35 @@
+package globwalkignorederror
+
+import (
+	"os"
+	"path/filepath"
+)
+
+func bad() {
+	files, _ := filepath.Glob("*.go") // want `error return from filepath\.Glob is discarded`
+	_ = files
+	entries, _ := os.ReadDir(".") // want `error return from os\.ReadDir is discarded`
+	_ = entries
+}
+
+func good() {
+	files, err := filepath.Glob("*.go")
+	if err != nil {
+		return
+	}
+	_ = files
+
+	entries, err2 := os.ReadDir(".")
+	if err2 != nil {
+		return
+	}
+	_ = entries
+}
+
+func suppressed() {
+	//nolint:globwalkignorederror
+	files, _ := filepath.Glob("*.go")
+	_ = files
+	entries, _ := os.ReadDir(".") //nolint:globwalkignorederror
+	_ = entries
+}

pkg/linters/registry.go

@@ -19,6 +19,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/fileclosenotdeferred"
 	"github.com/github/gh-aw/pkg/linters/fmterrorfnoverbs"
 	"github.com/github/gh-aw/pkg/linters/fprintlnsprintf"
+	"github.com/github/gh-aw/pkg/linters/globwalkignorederror"
 	"github.com/github/gh-aw/pkg/linters/goroutinemissingrecover"
 	"github.com/github/gh-aw/pkg/linters/hardcodedfilepath"
 	"github.com/github/gh-aw/pkg/linters/httpnoctx"
@@ -94,6 +95,7 @@ var allAnalyzers = []*analysis.Analyzer{
 	excessivefuncparams.Analyzer,
 	fileclosenotdeferred.Analyzer,
 	fmterrorfnoverbs.Analyzer,
+	globwalkignorederror.Analyzer,
 	goroutinemissingrecover.Analyzer,
 	hardcodedfilepath.Analyzer,
 	httpnoctx.Analyzer,

pkg/linters/spec_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/fileclosenotdeferred"
 	"github.com/github/gh-aw/pkg/linters/fmterrorfnoverbs"
 	"github.com/github/gh-aw/pkg/linters/fprintlnsprintf"
+	"github.com/github/gh-aw/pkg/linters/globwalkignorederror"
 	"github.com/github/gh-aw/pkg/linters/goroutinemissingrecover"
 	"github.com/github/gh-aw/pkg/linters/hardcodedfilepath"
 	"github.com/github/gh-aw/pkg/linters/httpnoctx"
@@ -89,15 +90,15 @@ type docAnalyzer struct {
 }
 
 // documentedAnalyzers returns the analyzer subpackages documented in the README
-// "Public API > Subpackages" table. The README documents 62 analyzers
+// "Public API > Subpackages" table. The README documents 64 analyzers
 // subpackages (the non-analyzer `internal` helper subpackage is excluded because
 // it exposes no Analyzer).
 //
 // Spec (README "Public API > Subpackages"):
 //
 //	appendbytestring, appendoneelement, bytesbufferstring, bytescomparestring, contextcancelnotdeferred, ctxbackground, deferinloop, errorfwrapv, excessivefuncparams, errormessage,
 //	errortypeassertion, errstringmatch, execcommandwithoutcontext, fileclosenotdeferred, fmterrorfnoverbs, fprintlnsprintf,
-//	goroutinemissingrecover, hardcodedfilepath, httpnoctx, httprespbodyclose, httpstatuscode, ioutildeprecated, jsonmarshalignoredeerror, largefunc, lenstringsplit, lenstringzero,
+//	globwalkignorederror, goroutinemissingrecover, hardcodedfilepath, httpnoctx, httprespbodyclose, httpstatuscode, ioutildeprecated, jsonmarshalignoredeerror, largefunc, lenstringsplit, lenstringzero,
 //	logfatallibrary, manualmutexunlock, mapclearloop, mapdeletecheck, nilctxpassed, osexitinlibrary, osgetenvlibrary, ossetenvlibrary, panic-in-library-code, rawloginlib,
 //	regexpcompileinfunction, regexpdynamicpattern, seenmapbool, sortslice, sprintferrdot, sprintferrorsnew, sprintfbool, sprintfint, ssljson,
 //	strconvparseignorederror, stringbytesroundtrip, stringreplaceminusone, stringsconcatloop, stringscountcontains, stringsindexcontains, stringsindexhasprefix, stringsjoinone, timeafterleak, timesleepnocontext, timenowsub,
@@ -120,6 +121,7 @@ func documentedAnalyzers() []docAnalyzer {
 		{"fileclosenotdeferred", fileclosenotdeferred.Analyzer},
 		{"fmterrorfnoverbs", fmterrorfnoverbs.Analyzer},
 		{"fprintlnsprintf", fprintlnsprintf.Analyzer},
+		{"globwalkignorederror", globwalkignorederror.Analyzer},
 		{"goroutinemissingrecover", goroutinemissingrecover.Analyzer},
 		{"hardcodedfilepath", hardcodedfilepath.Analyzer},
 		{"httpnoctx", httpnoctx.Analyzer},