fix: migrate 5 linters from syntactic stdlib-package matching to astutil.IsPkgSelector

docs/adr/43939-use-ispkgselector-for-stdlib-package-detection.md

@@ -0,0 +1,48 @@
+# ADR-43939: Use astutil.IsPkgSelector for Stdlib Package Detection in Linters
+
+**Date**: 2026-07-07
+**Status**: Draft
+**Deciders**: Unknown
+
+---
+
+### Context
+
+Five custom linters (`errstringmatch`, `fprintlnsprintf`, `osexitinlibrary`, `fileclosenotdeferred`, `contextcancelnotdeferred`) identified stdlib package calls using syntactic identifier name comparison: `ident.Name == "os"`, `ident.Name == "fmt"`, etc. This approach only inspects the AST identifier name, not the resolved import path, which produces two classes of defects:
+
+1. **False negatives**: aliased imports (e.g., `import xos "os"`) bypass the guard entirely because `xos != "os"`, causing the linter to miss real violations.
+2. **False positives**: local variables that shadow the package name (e.g., `var os fakeOS`) satisfy the string match even though `os` no longer refers to the stdlib package. Three of these linters are CI-enforced, making false positives build-blockers.
+
+### Decision
+
+We will replace every syntactic `ident.Name == "<pkg>"` guard in these five linters with `astutil.IsPkgSelector(pass, sel, "<pkg>")`, which resolves the selector's receiver through `pass.TypesInfo` to the actual imported package path. This requires threading `*analysis.Pass` into the affected private helper functions (`isFileOpenCall`, `isContextWithCancelCall`, `brittleErrStringFuncName`, `isFmtFunc`). Test data cases for aliased imports and local shadowing are added per linter to lock in the corrected behaviour.
+
+### Alternatives Considered
+
+#### Alternative 1: Retain Syntactic Matching and Document Limitations
+
+Continue using `ident.Name == "<pkg>"` and add a comment acknowledging that aliased imports are not detected and shadowed variables may produce false positives. This was rejected because the false-positive case is a concrete build-blocker for three CI-enforced linters — documenting a known defect does not eliminate the build failures it causes. The false-negative case also undermines the purpose of the linters, which is to catch real coding mistakes regardless of import style.
+
+#### Alternative 2: Resolve Aliases Manually via import-spec Iteration
+
+Iterate over the file's `ast.ImportSpec` nodes to build a map from local identifier to canonical package path, then check the local identifier's canonical path. This would fix false negatives for aliased imports. It was rejected because it reimplements logic that `pass.TypesInfo` already provides correctly, adds per-file bookkeeping to every linter run function, and still requires a custom lookup at each call site — more code and more opportunity for error than delegating to `astutil.IsPkgSelector`.
+
+### Consequences
+
+#### Positive
+- False-positive build failures caused by shadowed package names are eliminated for all three CI-enforced linters.
+- Aliased stdlib imports (e.g., `import ctx "context"`) are now correctly detected and flagged.
+- The fix uses the type system (`pass.TypesInfo`) as the authoritative source for package identity, consistent with how Go analysis tools are intended to work.
+- New testdata files provide regression coverage for both the alias case and the shadowing case.
+
+#### Negative
+- All five affected helper functions now require `*analysis.Pass` as an additional parameter, making their signatures more verbose and coupling them to the analysis pass context.
+- Callers cannot invoke the helpers outside an `analysis.Pass` run (e.g., in a standalone unit test that constructs AST nodes without a full type-checker pass), which reduces testability in isolation.
+
+#### Neutral
+- No change to the diagnostic messages or suggested fixes produced by these linters; only the package-identity predicate changes.
+- The pattern established here (`astutil.IsPkgSelector` instead of `ident.Name`) can serve as a guide for any future linter that needs to identify stdlib calls.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/contextcancelnotdeferred/contextcancelnotdeferred.go

@@ -81,7 +81,7 @@ func inspectCancelNode(pass *analysis.Pass, cancelVars map[types.Object]*cancelV
 	if assign, ok := node.(*ast.AssignStmt); ok {
 		for i, rhs := range assign.Rhs {
 			call, ok := rhs.(*ast.CallExpr)
-			if !ok || !isContextWithCancelCall(call) {
+			if !ok || !isContextWithCancelCall(pass, call) {
 				continue
 			}
 			if len(assign.Rhs) == 1 && i == 0 && len(assign.Lhs) >= 2 {
@@ -131,13 +131,12 @@ type cancelVarState struct {
 	hasDirectCancel bool
 }
 
-func isContextWithCancelCall(call *ast.CallExpr) bool {
+func isContextWithCancelCall(pass *analysis.Pass, call *ast.CallExpr) bool {
 	sel, ok := call.Fun.(*ast.SelectorExpr)
 	if !ok {
 		return false
 	}
-	pkgIdent, ok := sel.X.(*ast.Ident)
-	if !ok || pkgIdent.Name != "context" {
+	if !astutil.IsPkgSelector(pass, sel, "context") {
 		return false
 	}
 	switch sel.Sel.Name {

pkg/linters/contextcancelnotdeferred/testdata/src/contextcancelnotdeferred/contextcancelnotdeferred_alias.go

@@ -0,0 +1,22 @@
+package contextcancelnotdeferred
+
+import (
+	ctx "context"
+	"time"
+)
+
+// flagged: aliased context import still resolves to context package
+func BadWithCancelAliased(parent ctx.Context) error {
+	c, cancel := ctx.WithCancel(parent) // want `context cancel function should be deferred immediately after context.WithCancel/WithTimeout/WithDeadline`
+	_ = c
+	cancel()
+	return nil
+}
+
+// not flagged: defer used correctly with aliased import
+func GoodWithTimeoutAliased(parent ctx.Context) error {
+	c, cancel := ctx.WithTimeout(parent, time.Second)
+	defer cancel()
+	_ = c
+	return nil
+}

pkg/linters/errstringmatch/errstringmatch.go

@@ -59,7 +59,7 @@ func run(pass *analysis.Pass) (any, error) {
 		}
 
 		// Match strings.<BrittleFunc>(X, Y)
-		funcName, matched := brittleErrStringFuncName(outer)
+		funcName, matched := brittleErrStringFuncName(pass, outer)
 		if !matched {
 			return
 		}
@@ -88,16 +88,12 @@ func run(pass *analysis.Pass) (any, error) {
 
 // brittleErrStringFuncName returns the matched strings function name and true
 // when call is a strings.<BrittleFunc>(...) call expression.
-func brittleErrStringFuncName(call *ast.CallExpr) (string, bool) {
+func brittleErrStringFuncName(pass *analysis.Pass, call *ast.CallExpr) (string, bool) {
 	sel, ok := call.Fun.(*ast.SelectorExpr)
 	if !ok {
 		return "", false
 	}
-	ident, ok := sel.X.(*ast.Ident)
-	if !ok {
-		return "", false
-	}
-	if ident.Name != "strings" {
+	if !astutil.IsPkgSelector(pass, sel, "strings") {
 		return "", false
 	}
 	if brittleErrStringFuncs[sel.Sel.Name] {

pkg/linters/errstringmatch/testdata/src/errstringmatch/errstringmatch_alias.go

@@ -0,0 +1,20 @@
+package errstringmatch
+
+import (
+	str "strings"
+)
+
+// flagged: aliased strings import still resolves to strings package
+func checkErrorAliased(err error) bool {
+	return str.Contains(err.Error(), "not found") // want `avoid strings\.Contains\(err\.Error\(\)`
+}
+
+// not flagged: shadowing local named "strings" — not the stdlib package
+type fakeStrings struct{}
+
+func (fakeStrings) Contains(s, substr string) bool { return false }
+
+func checkShadowedStrings(err error) bool {
+	var strings fakeStrings
+	return strings.Contains(err.Error(), "prefix")
+}

pkg/linters/fileclosenotdeferred/fileclosenotdeferred.go

@@ -121,7 +121,7 @@ func analyzeASTNodeForFileClosePatterns(pass *analysis.Pass, fileVars map[types.
 func trackFileOpenAssignment(pass *analysis.Pass, fileVars map[types.Object]*fileVarState, assign *ast.AssignStmt, noLintLinesByFile map[string]map[int]struct{}) {
 	for i, rhs := range assign.Rhs {
 		call, ok := rhs.(*ast.CallExpr)
-		if !ok || !isFileOpenCall(call) {
+		if !ok || !isFileOpenCall(pass, call) {
 			continue
 		}
 		if i >= len(assign.Lhs) {
@@ -165,13 +165,12 @@ type fileVarState struct {
 }
 
 // isFileOpenCall returns true if the call is os.Open, os.Create, or os.OpenFile
-func isFileOpenCall(call *ast.CallExpr) bool {
+func isFileOpenCall(pass *analysis.Pass, call *ast.CallExpr) bool {
 	sel, ok := call.Fun.(*ast.SelectorExpr)
 	if !ok {
 		return false
 	}
-	ident, ok := sel.X.(*ast.Ident)
-	if !ok || ident.Name != "os" {
+	if !astutil.IsPkgSelector(pass, sel, "os") {
 		return false
 	}
 	return sel.Sel.Name == "Open" || sel.Sel.Name == "Create" || sel.Sel.Name == "OpenFile"

pkg/linters/fileclosenotdeferred/testdata/src/fileclosenotdeferred/fileclosenotdeferred_alias.go

@@ -0,0 +1,23 @@
+package fileclosenotdeferred
+
+import xos "os"
+
+// flagged: aliased os import still resolves to os package
+func ReadFileManualCloseAliased() error {
+	file, err := xos.Open("test.txt") // want `file Close\(\) should be deferred immediately after successful open to prevent resource leaks`
+	if err != nil {
+		return err
+	}
+	file.Close()
+	return nil
+}
+
+// not flagged: defer used correctly with aliased import
+func ReadFileDeferCloseAliased() error {
+	file, err := xos.Open("test.txt")
+	if err != nil {
+		return err
+	}
+	defer file.Close()
+	return nil
+}

pkg/linters/fprintlnsprintf/fprintlnsprintf.go

@@ -41,7 +41,7 @@ func run(pass *analysis.Pass) (any, error) {
 		}
 
 		// Check if this is exactly fmt.Fprintln(w, fmt.Sprintf(...)).
-		if !isFmtFunc(call, "Fprintln") {
+		if !isFmtFunc(pass, call, "Fprintln") {
 			return
 		}
 		if len(call.Args) != 2 {
@@ -59,7 +59,7 @@ func run(pass *analysis.Pass) (any, error) {
 		if !ok {
 			return
 		}
-		if !isFmtFunc(printedArg, "Sprintf") {
+		if !isFmtFunc(pass, printedArg, "Sprintf") {
 			return
 		}
 		if nolint.HasDirective(pos, noLintLinesByFile) {
@@ -120,14 +120,10 @@ func buildFprintfFix(call *ast.CallExpr, sprintfCall *ast.CallExpr) []analysis.S
 }
 
 // isFmtFunc returns true if call is a call to fmt.<name>.
-func isFmtFunc(call *ast.CallExpr, name string) bool {
+func isFmtFunc(pass *analysis.Pass, call *ast.CallExpr, name string) bool {
 	sel, ok := call.Fun.(*ast.SelectorExpr)
 	if !ok {
 		return false
 	}
-	ident, ok := sel.X.(*ast.Ident)
-	if !ok {
-		return false
-	}
-	return ident.Name == "fmt" && sel.Sel.Name == name
+	return astutil.IsPkgSelector(pass, sel, "fmt") && sel.Sel.Name == name
 }

pkg/linters/fprintlnsprintf/testdata/src/fprintlnsprintf/fprintlnsprintf_alias.go

@@ -0,0 +1,22 @@
+package fprintlnsprintf
+
+import (
+	xfmt "fmt"
+	"os"
+)
+
+// flagged: aliased fmt import still resolves to fmt package
+func flaggedAliased(name string) {
+	xfmt.Fprintln(os.Stderr, xfmt.Sprintf("hello %s", name)) // want "use fmt.Fprintf"
+}
+
+// not flagged: shadowing local named "fmt" — not the stdlib package
+type fakeFmt struct{}
+
+func (fakeFmt) Fprintln(_ interface{}, args ...interface{}) {}
+func (fakeFmt) Sprintf(format string, args ...interface{}) string { return "" }
+
+func notFlaggedShadowedFmt(name string) {
+	var fmt fakeFmt
+	fmt.Fprintln(os.Stderr, fmt.Sprintf("hello %s", name))
+}

pkg/linters/fprintlnsprintf/testdata/src/fprintlnsprintf/fprintlnsprintf_alias.go.golden

@@ -0,0 +1,22 @@
+package fprintlnsprintf
+
+import (
+	xfmt "fmt"
+	"os"
+)
+
+// flagged: aliased fmt import still resolves to fmt package
+func flaggedAliased(name string) {
+	xfmt.Fprintf(os.Stderr, "hello %s\n", name) // want "use fmt.Fprintf"
+}
+
+// not flagged: shadowing local named "fmt" — not the stdlib package
+type fakeFmt struct{}
+
+func (fakeFmt) Fprintln(_ interface{}, args ...interface{}) {}
+func (fakeFmt) Sprintf(format string, args ...interface{}) string { return "" }
+
+func notFlaggedShadowedFmt(name string) {
+	var fmt fakeFmt
+	fmt.Fprintln(os.Stderr, fmt.Sprintf("hello %s", name))
+}

pkg/linters/osexitinlibrary/osexitinlibrary.go

@@ -52,11 +52,7 @@ func run(pass *analysis.Pass) (any, error) {
 		if !ok {
 			return
 		}
-		ident, ok := sel.X.(*ast.Ident)
-		if !ok {
-			return
-		}
-		if ident.Name == "os" && sel.Sel.Name == "Exit" {
+		if astutil.IsPkgSelector(pass, sel, "os") && sel.Sel.Name == "Exit" {
 			position := pass.Fset.PositionFor(call.Pos(), false)
 			if nolint.HasDirective(position, noLintLinesByFile) {
 				return

pkg/linters/osexitinlibrary/testdata/src/osexitinlibrary/osexitinlibrary_alias.go

@@ -0,0 +1,18 @@
+package osexitinlibrary
+
+import xos "os"
+
+// flagged: aliased os import still resolves to os package
+func stopProcessAliased() {
+	xos.Exit(1) // want `os.Exit called in library package`
+}
+
+// not flagged: shadowing local named "os" — not the stdlib package
+type fakeOS struct{}
+
+func (fakeOS) Exit(code int) {}
+
+func notFlaggedShadowedOS() {
+	var os fakeOS
+	os.Exit(1)
+}