diff --git a/docs/adr/43649-consolidate-linter-astutil-helpers.md b/docs/adr/43649-consolidate-linter-astutil-helpers.md new file mode 100644 index 00000000000..87be172fb60 --- /dev/null +++ b/docs/adr/43649-consolidate-linter-astutil-helpers.md @@ -0,0 +1,46 @@ +# ADR-43649: Consolidate Duplicated AST/Context Helpers into internal/astutil + +**Date**: 2026-07-06 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The `pkg/linters` suite contained multiple independent linter packages, each with its own private copies of the same small helper functions: `enclosingFuncType` (extract `*ast.FuncType` from a `FuncDecl` or `FuncLit`), `contextContextType` (resolve `context.Context` via type-checker imports), `contextParamName` (find the first `context.Context` parameter name in a function signature), `calledOSFunc` (check whether a call targets a named function in the `os` package), and `flipOp` / `flipComparisonOp` (flip comparison operators for normalization). At least eight analyzer packages (`execcommandwithoutcontext`, `httpnoctx`, `timesleepnocontext`, `ctxbackground`, `osgetenvlibrary`, `ossetenvlibrary`, `lenstringzero`, `stringsindexcontains`) maintained independent copies, creating a realistic drift risk: a correctness fix or a nil-guard addition applied to one copy could silently leave the others broken. + +### Decision + +We will centralize all shared AST and type-checker utility helpers into the existing `pkg/linters/internal/astutil` package and delete per-package copies. The consolidated helpers are exported as `EnclosingFuncType`, `ContextContextType`, `ContextParamName`, `CalledOSFunc` (redesigned with a variadic `allowedNames` filter for cleaner call sites), and `FlipComparisonOp`. Each affected linter is rewired to import and call the shared versions, and unit tests covering the new functions are added to `astutil_test.go`. The `internal` placement restricts access to packages within `pkg/linters/...`, which is the correct scope. + +### Alternatives Considered + +#### Alternative 1: Keep Per-Package Copies with No Shared Abstraction + +Each analyzer would continue to own its local private helper. This is the status quo and requires zero refactoring effort. It was rejected because identical helper functions across eight-plus packages are a demonstrated maintenance hazard: divergent nil-guards and differing function-name filters were already present across packages, confirming that drift was occurring. There is no mechanism to enforce consistency without a single source of truth. + +#### Alternative 2: Code-Generate Per-Package Helpers from a Template + +A generator (e.g., using `go generate`) could stamp identical helper bodies into each package from a shared template, keeping copies in sync while preserving package-private visibility. This eliminates the behavioral drift risk without changing import paths. It was rejected because it introduces generator tooling complexity, requires each package's generated file to be regenerated whenever the template changes, and provides no advantage over a shared package for helpers that carry no package-specific state. The `internal` package approach is simpler and idiomatic in the Go standard library and `x/tools` ecosystem. + +### Consequences + +#### Positive +- Single source of truth for shared helpers: a bug fix or a nil-guard addition needs to be made only once and takes effect across all linters immediately. +- `CalledOSFunc` gains a variadic `allowedNames` parameter, making call sites more readable (`astutil.CalledOSFunc(pass, call, "Getenv", "LookupEnv")` vs. a hardcoded name comparison inside a local copy). +- New linter authors can discover and reuse existing utilities rather than copy-pasting, lowering the per-linter authoring cost. +- Expanded unit test coverage for `astutil` provides a regression harness for the shared utilities independent of any individual linter's test suite. + +#### Negative +- The `internal` package boundary limits these helpers to packages under `pkg/linters/...`; code outside that tree (e.g., a future top-level analysis driver) cannot import them without restructuring. +- Callers now take an indirect dependency on `pkg/linters/internal/astutil` — a change to a helper's signature requires updating every consumer in the same commit, rather than each package being independently evolvable. + +#### Neutral +- No behavioral change is intended; existing linter diagnostics and suggested fixes should be identical before and after the migration. +- The `astutil` package already existed and was in use; this PR extends it rather than introducing a new dependency. +- Test coverage for the consolidated helpers lives in `astutil_test.go`, separate from individual linter testdata directories. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/linters/ctxbackground/ctxbackground.go b/pkg/linters/ctxbackground/ctxbackground.go index 792c998abeb..65802084c33 100644 --- a/pkg/linters/ctxbackground/ctxbackground.go +++ b/pkg/linters/ctxbackground/ctxbackground.go @@ -46,16 +46,11 @@ func run(pass *analysis.Pass) (any, error) { } for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { - var ftype *ast.FuncType - switch fn := encl.Node().(type) { - case *ast.FuncDecl: - ftype = fn.Type - case *ast.FuncLit: - ftype = fn.Type - default: + ftype := astutil.EnclosingFuncType(encl.Node()) + if ftype == nil { continue } - ctxParamName, ok := contextParamName(pass, ftype) + ctxParamName, ok := astutil.ContextParamName(pass, ftype) if !ok { break } @@ -103,44 +98,3 @@ func isContextBackgroundCall(pass *analysis.Pass, call *ast.CallExpr) bool { } return pkgName.Imported().Path() == "context" } - -// contextParamName returns the first non-blank context.Context parameter name. -func contextParamName(pass *analysis.Pass, ftype *ast.FuncType) (string, bool) { - if ftype == nil || ftype.Params == nil { - return "", false - } - ctxType := contextType(pass) - if ctxType == nil { - return "", false - } - for _, field := range ftype.Params.List { - t := pass.TypesInfo.TypeOf(field.Type) - if t == nil { - continue - } - if !types.Identical(t, ctxType) { - continue - } - // At least one name must not be blank. - for _, name := range field.Names { - if name.Name != "_" { - return name.Name, true - } - } - } - return "", false -} - -// contextType returns the types.Type for context.Context, or nil if the -// package is not imported. -func contextType(pass *analysis.Pass) types.Type { - for _, pkg := range pass.Pkg.Imports() { - if pkg.Path() == "context" { - obj := pkg.Scope().Lookup("Context") - if obj != nil { - return obj.Type() - } - } - } - return nil -} diff --git a/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go b/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go index 6299120a37a..7017f00b30b 100644 --- a/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go +++ b/pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go @@ -52,11 +52,11 @@ func run(pass *analysis.Pass) (any, error) { } for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { - funcType := enclosingFuncType(encl.Node()) + funcType := astutil.EnclosingFuncType(encl.Node()) if funcType == nil { continue } - ctxParamName, hasCtx := contextParamName(pass, funcType) + ctxParamName, hasCtx := astutil.ContextParamName(pass, funcType) if !hasCtx { continue } @@ -113,52 +113,3 @@ func execCommandSelector(pass *analysis.Pass, call *ast.CallExpr) (*ast.Selector } return sel, true } - -// contextParamName returns the name of the first context.Context parameter -// in fn, and true, or "", false if none exists. -func contextParamName(pass *analysis.Pass, fn *ast.FuncType) (string, bool) { - if fn == nil || fn.Params == nil { - return "", false - } - ctxType := contextContextType(pass) - if ctxType == nil { - return "", false - } - for _, field := range fn.Params.List { - t := pass.TypesInfo.TypeOf(field.Type) - if t == nil || !types.Identical(t, ctxType) { - continue - } - for _, name := range field.Names { - if name.Name != "_" { - return name.Name, true - } - } - } - return "", false -} - -func enclosingFuncType(node ast.Node) *ast.FuncType { - switch fn := node.(type) { - case *ast.FuncDecl: - return fn.Type - case *ast.FuncLit: - return fn.Type - default: - return nil - } -} - -// contextContextType returns the types.Type for context.Context, or nil if -// the context package is not imported. -func contextContextType(pass *analysis.Pass) types.Type { - for _, pkg := range pass.Pkg.Imports() { - if pkg.Path() == "context" { - obj := pkg.Scope().Lookup("Context") - if obj != nil { - return obj.Type() - } - } - } - return nil -} diff --git a/pkg/linters/httpnoctx/httpnoctx.go b/pkg/linters/httpnoctx/httpnoctx.go index 5d599f0c42a..bdd166f4ed3 100644 --- a/pkg/linters/httpnoctx/httpnoctx.go +++ b/pkg/linters/httpnoctx/httpnoctx.go @@ -44,7 +44,6 @@ func run(pass *analysis.Pass) (any, error) { return nil, err } noLintLinesByFile := nolint.BuildLineIndex(pass, "httpnoctx") - ctxType := contextContextType(pass) for cursor := range insp.Root().Preorder((*ast.CallExpr)(nil)) { call, ok := cursor.Node().(*ast.CallExpr) @@ -82,7 +81,7 @@ func run(pass *analysis.Pass) (any, error) { } } - if sel.Sel.Name == "NewRequest" && isHTTPPackage(pass, sel.X) && hasContextInEnclosingFunc(pass, cursor, ctxType) { + if sel.Sel.Name == "NewRequest" && isHTTPPackage(pass, sel.X) && hasContextInEnclosingFunc(pass, cursor) { pass.ReportRangef(call, "http.NewRequest does not propagate context; use http.NewRequestWithContext when context.Context is in scope", ) @@ -134,56 +133,20 @@ func isHTTPPackage(pass *analysis.Pass, expr ast.Expr) bool { return pkgName.Imported().Path() == "net/http" } -func hasContextInEnclosingFunc(pass *analysis.Pass, cursor inspector.Cursor, ctxType types.Type) bool { - if ctxType == nil { - return false - } - +func hasContextInEnclosingFunc(pass *analysis.Pass, cursor inspector.Cursor) bool { for enclosing := range cursor.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { - fnType := enclosingFuncType(enclosing.Node()) + fnType := astutil.EnclosingFuncType(enclosing.Node()) if fnType == nil || fnType.Params == nil { continue } - - for _, field := range fnType.Params.List { - t := pass.TypesInfo.TypeOf(field.Type) - if t == nil || !types.Identical(t, ctxType) { - continue - } - for _, name := range field.Names { - if name.Name != "_" { - return true - } - } + if _, ok := astutil.ContextParamName(pass, fnType); ok { + return true } } return false } -func enclosingFuncType(node ast.Node) *ast.FuncType { - switch fn := node.(type) { - case *ast.FuncDecl: - return fn.Type - case *ast.FuncLit: - return fn.Type - default: - return nil - } -} - -func contextContextType(pass *analysis.Pass) types.Type { - for _, pkg := range pass.Pkg.Imports() { - if pkg.Path() != "context" { - continue - } - if obj := pkg.Scope().Lookup("Context"); obj != nil { - return obj.Type() - } - } - return nil -} - func isHTTPDefaultClient(pass *analysis.Pass, expr ast.Expr) bool { sel, ok := expr.(*ast.SelectorExpr) if !ok || sel.Sel.Name != "DefaultClient" { diff --git a/pkg/linters/internal/astutil/astutil.go b/pkg/linters/internal/astutil/astutil.go index d2fe94ce111..4e275d2ae8c 100644 --- a/pkg/linters/internal/astutil/astutil.go +++ b/pkg/linters/internal/astutil/astutil.go @@ -8,6 +8,7 @@ import ( "go/printer" "go/token" "go/types" + "slices" "golang.org/x/tools/go/analysis" "golang.org/x/tools/go/analysis/passes/inspect" @@ -48,6 +49,59 @@ func IsStringLiteral(expr ast.Expr) bool { return ok && lit.Kind == token.STRING } +// EnclosingFuncType extracts a function type from a FuncDecl or FuncLit node. +func EnclosingFuncType(node ast.Node) *ast.FuncType { + switch fn := node.(type) { + case *ast.FuncDecl: + return fn.Type + case *ast.FuncLit: + return fn.Type + default: + return nil + } +} + +// ContextContextType returns the types.Type for context.Context, or nil if +// the context package is not imported. +func ContextContextType(pass *analysis.Pass) types.Type { + if pass == nil || pass.Pkg == nil { + return nil + } + for _, pkg := range pass.Pkg.Imports() { + if pkg.Path() == "context" { + obj := pkg.Scope().Lookup("Context") + if obj != nil { + return obj.Type() + } + } + } + return nil +} + +// ContextParamName returns the name of the first context.Context parameter in +// fn, and true, or "", false if none exists. +func ContextParamName(pass *analysis.Pass, fn *ast.FuncType) (string, bool) { + if pass == nil || pass.TypesInfo == nil || fn == nil || fn.Params == nil { + return "", false + } + ctxType := ContextContextType(pass) + if ctxType == nil { + return "", false + } + for _, field := range fn.Params.List { + t := pass.TypesInfo.TypeOf(field.Type) + if t == nil || !types.Identical(t, ctxType) { + continue + } + for _, name := range field.Names { + if name.Name != "_" { + return name.Name, true + } + } + } + return "", false +} + // IsFmtErrorf reports whether call is a call to fmt.Errorf (including aliases). func IsFmtErrorf(pass *analysis.Pass, call *ast.CallExpr) bool { sel, ok := call.Fun.(*ast.SelectorExpr) @@ -72,6 +126,36 @@ func IsFmtErrorf(pass *analysis.Pass, call *ast.CallExpr) bool { return pkgName.Imported().Path() == "fmt" } +// CalledOSFunc reports whether call resolves to a function in package os. If +// allowedNames are provided, the function name must match one of them. +func CalledOSFunc(pass *analysis.Pass, call *ast.CallExpr, allowedNames ...string) (*types.Func, bool) { + if pass == nil || pass.TypesInfo == nil || call == nil { + return nil, false + } + + var obj types.Object + switch fun := call.Fun.(type) { + case *ast.SelectorExpr: + obj = pass.TypesInfo.Uses[fun.Sel] + case *ast.Ident: + obj = pass.TypesInfo.Uses[fun] + default: + return nil, false + } + + fn, ok := obj.(*types.Func) + if !ok || fn.Pkg() == nil || fn.Pkg().Path() != "os" { + return nil, false + } + if len(allowedNames) == 0 { + return fn, true + } + if slices.Contains(allowedNames, fn.Name()) { + return fn, true + } + return nil, false +} + // IsPkgSelector reports whether sel is a selector on an imported package with // the given import path. func IsPkgSelector(pass *analysis.Pass, sel *ast.SelectorExpr, pkgPath string) bool { @@ -93,6 +177,23 @@ func IsPkgSelector(pass *analysis.Pass, sel *ast.SelectorExpr, pkgPath string) b return pkgName.Imported().Path() == pkgPath } +// FlipComparisonOp returns the comparison operator with left and right +// operands swapped. +func FlipComparisonOp(op token.Token) token.Token { + switch op { + case token.LSS: + return token.GTR + case token.GTR: + return token.LSS + case token.LEQ: + return token.GEQ + case token.GEQ: + return token.LEQ + default: + return op + } +} + // Inspector extracts the *inspector.Inspector from pass.ResultOf. // It returns an error if the result has an unexpected type. func Inspector(pass *analysis.Pass) (*inspector.Inspector, error) { diff --git a/pkg/linters/internal/astutil/astutil_test.go b/pkg/linters/internal/astutil/astutil_test.go index ee7083b47cd..b3c1f22eb31 100644 --- a/pkg/linters/internal/astutil/astutil_test.go +++ b/pkg/linters/internal/astutil/astutil_test.go @@ -1,3 +1,5 @@ +//go:build !integration + package astutil import ( @@ -149,3 +151,144 @@ func TestIsPkgSelector(t *testing.T) { }) } } + +func TestEnclosingFuncType(t *testing.T) { + t.Parallel() + + funcDecl := &ast.FuncDecl{Type: &ast.FuncType{}} + if got := EnclosingFuncType(funcDecl); got != funcDecl.Type { + t.Fatalf("EnclosingFuncType(FuncDecl) = %#v, want %#v", got, funcDecl.Type) + } + + funcLit := &ast.FuncLit{Type: &ast.FuncType{}} + if got := EnclosingFuncType(funcLit); got != funcLit.Type { + t.Fatalf("EnclosingFuncType(FuncLit) = %#v, want %#v", got, funcLit.Type) + } + + if got := EnclosingFuncType(ast.NewIdent("x")); got != nil { + t.Fatalf("EnclosingFuncType(non-func) = %#v, want nil", got) + } +} + +func TestContextHelpers(t *testing.T) { + t.Parallel() + + ctxPkg := types.NewPackage("context", "context") + ctxIface := types.NewInterfaceType(nil, nil) + ctxIface.Complete() + ctxType := types.NewTypeName(token.NoPos, ctxPkg, "Context", ctxIface) + ctxPkg.Scope().Insert(ctxType) + + makePassWithFuncType := func(includeContextImport bool, paramName string) (*analysis.Pass, *ast.FuncType) { + pkg := types.NewPackage("example.com/p", "p") + if includeContextImport { + pkg.SetImports([]*types.Package{ctxPkg}) + } + ctxIdent := ast.NewIdent("Context") + fnType := &ast.FuncType{ + Params: &ast.FieldList{ + List: []*ast.Field{{ + Names: []*ast.Ident{ast.NewIdent(paramName)}, + Type: ctxIdent, + }}, + }, + } + pass := &analysis.Pass{ + Pkg: pkg, + TypesInfo: &types.Info{ + Types: map[ast.Expr]types.TypeAndValue{ + ctxIdent: {Type: ctxType.Type()}, + }, + }, + } + return pass, fnType + } + + passWithContext, fnTypeWithContext := makePassWithFuncType(true, "ctx") + if got := ContextContextType(passWithContext); got == nil { + t.Fatal("ContextContextType() = nil, want context.Context type") + } + name, ok := ContextParamName(passWithContext, fnTypeWithContext) + if !ok || name != "ctx" { + t.Fatalf("ContextParamName() = (%q, %v), want (%q, true)", name, ok, "ctx") + } + + // blank identifier: a context param named "_" should not be found. + passWithBlank, fnTypeWithBlank := makePassWithFuncType(true, "_") + if _, ok := ContextParamName(passWithBlank, fnTypeWithBlank); ok { + t.Fatal("ContextParamName() = ok=true for blank-identifier param, want false") + } + + passWithoutContext, fnTypeWithoutContext := makePassWithFuncType(false, "ctx") + if got := ContextContextType(passWithoutContext); got != nil { + t.Fatalf("ContextContextType() = %#v, want nil without context import", got) + } + if _, ok := ContextParamName(passWithoutContext, fnTypeWithoutContext); ok { + t.Fatal("ContextParamName() = ok=true, want false without context import") + } +} + +func TestCalledOSFunc(t *testing.T) { + t.Parallel() + + sig := types.NewSignatureType(nil, nil, nil, nil, nil, false) + osPkg := types.NewPackage("os", "os") + osFunc := types.NewFunc(token.NoPos, osPkg, "Getenv", sig) + otherPkg := types.NewPackage("example.com/p", "p") + otherFunc := types.NewFunc(token.NoPos, otherPkg, "Getenv", sig) + + selIdent := ast.NewIdent("Getenv") + pass := &analysis.Pass{ + TypesInfo: &types.Info{ + Uses: map[*ast.Ident]types.Object{ + selIdent: osFunc, + }, + }, + } + call := &ast.CallExpr{Fun: &ast.SelectorExpr{X: ast.NewIdent("os"), Sel: selIdent}} + + if fn, ok := CalledOSFunc(pass, call, "Getenv", "LookupEnv"); !ok || fn != osFunc { + t.Fatalf("CalledOSFunc() = (%#v, %v), want (%#v, true)", fn, ok, osFunc) + } + if _, ok := CalledOSFunc(pass, call, "Setenv"); ok { + t.Fatal("CalledOSFunc() = ok=true for non-allowed name, want false") + } + + pass.TypesInfo.Uses[selIdent] = otherFunc + if _, ok := CalledOSFunc(pass, call); ok { + t.Fatal("CalledOSFunc() = ok=true for non-os package, want false") + } + + // direct *ast.Ident call (e.g. via dot-import): CalledOSFunc resolves Uses on the Ident. + directIdent := ast.NewIdent("Getenv") + pass.TypesInfo.Uses[directIdent] = osFunc + directCall := &ast.CallExpr{Fun: directIdent} + if fn, ok := CalledOSFunc(pass, directCall, "Getenv"); !ok || fn != osFunc { + t.Fatalf("CalledOSFunc() direct ident = (%#v, %v), want (%#v, true)", fn, ok, osFunc) + } +} + +func TestFlipComparisonOp(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in token.Token + want token.Token + }{ + {name: "less", in: token.LSS, want: token.GTR}, + {name: "greater", in: token.GTR, want: token.LSS}, + {name: "leq", in: token.LEQ, want: token.GEQ}, + {name: "geq", in: token.GEQ, want: token.LEQ}, + {name: "equal unchanged", in: token.EQL, want: token.EQL}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := FlipComparisonOp(tt.in); got != tt.want { + t.Fatalf("FlipComparisonOp(%v) = %v, want %v", tt.in, got, tt.want) + } + }) + } +} diff --git a/pkg/linters/lenstringzero/lenstringzero.go b/pkg/linters/lenstringzero/lenstringzero.go index 333e0037833..d3272ceb336 100644 --- a/pkg/linters/lenstringzero/lenstringzero.go +++ b/pkg/linters/lenstringzero/lenstringzero.go @@ -124,20 +124,20 @@ func matchLenLiteralExpr(pass *analysis.Pass, expr *ast.BinaryExpr, aliases map[ // Flip the operator so the normalized form has len on the left. if isLenCall(expr.Y) { if isIntZero(expr.X) { - return lenCallArg(expr.Y), true, flipOp(op), 0, true + return lenCallArg(expr.Y), true, astutil.FlipComparisonOp(op), 0, true } if isIntOne(expr.X) { - return lenCallArg(expr.Y), true, flipOp(op), 1, true + return lenCallArg(expr.Y), true, astutil.FlipComparisonOp(op), 1, true } } if isIntZero(expr.X) { if arg, ok2 := lenAliasArg(pass, expr.Y, aliases); ok2 { - return arg, false, flipOp(op), 0, true + return arg, false, astutil.FlipComparisonOp(op), 0, true } } if isIntOne(expr.X) { if arg, ok2 := lenAliasArg(pass, expr.Y, aliases); ok2 { - return arg, false, flipOp(op), 1, true + return arg, false, astutil.FlipComparisonOp(op), 1, true } } @@ -206,23 +206,6 @@ func buildLenStringFix(pass *analysis.Pass, expr *ast.BinaryExpr, lenArg ast.Exp }} } -// flipOp returns the comparison operator adjusted for swapping left and right operands. -// For example, when converting "0 < len(s)" to the normalized "len(s) > 0", LSS becomes GTR. -func flipOp(op token.Token) token.Token { - switch op { - case token.LSS: - return token.GTR - case token.GTR: - return token.LSS - case token.LEQ: - return token.GEQ - case token.GEQ: - return token.LEQ - default: - return op - } -} - func isLenCall(expr ast.Expr) bool { call, ok := expr.(*ast.CallExpr) if !ok || len(call.Args) != 1 { diff --git a/pkg/linters/osgetenvlibrary/osgetenvlibrary.go b/pkg/linters/osgetenvlibrary/osgetenvlibrary.go index ff3a3a4a9a8..84f6b6204dc 100644 --- a/pkg/linters/osgetenvlibrary/osgetenvlibrary.go +++ b/pkg/linters/osgetenvlibrary/osgetenvlibrary.go @@ -4,7 +4,6 @@ package osgetenvlibrary import ( "go/ast" - "go/types" "strings" "golang.org/x/tools/go/analysis" @@ -50,7 +49,7 @@ func run(pass *analysis.Pass) (any, error) { return } - fn, ok := calledOSFunc(pass, call) + fn, ok := astutil.CalledOSFunc(pass, call, "Getenv", "LookupEnv") if !ok { return } @@ -68,24 +67,3 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } - -func calledOSFunc(pass *analysis.Pass, call *ast.CallExpr) (*types.Func, bool) { - var obj types.Object - switch fun := call.Fun.(type) { - case *ast.SelectorExpr: - obj = pass.TypesInfo.Uses[fun.Sel] - case *ast.Ident: - obj = pass.TypesInfo.Uses[fun] - default: - return nil, false - } - - fn, ok := obj.(*types.Func) - if !ok || fn.Pkg() == nil || fn.Pkg().Path() != "os" { - return nil, false - } - if fn.Name() != "Getenv" && fn.Name() != "LookupEnv" { - return nil, false - } - return fn, true -} diff --git a/pkg/linters/ossetenvlibrary/ossetenvlibrary.go b/pkg/linters/ossetenvlibrary/ossetenvlibrary.go index 95d845ad1b8..e7ce83937a1 100644 --- a/pkg/linters/ossetenvlibrary/ossetenvlibrary.go +++ b/pkg/linters/ossetenvlibrary/ossetenvlibrary.go @@ -4,7 +4,6 @@ package ossetenvlibrary import ( "go/ast" - "go/types" "strings" "golang.org/x/tools/go/analysis" @@ -50,7 +49,7 @@ func run(pass *analysis.Pass) (any, error) { return } - fn, ok := calledOSFunc(pass, call) + fn, ok := astutil.CalledOSFunc(pass, call, "Setenv", "Unsetenv") if !ok { return } @@ -68,24 +67,3 @@ func run(pass *analysis.Pass) (any, error) { return nil, nil } - -func calledOSFunc(pass *analysis.Pass, call *ast.CallExpr) (*types.Func, bool) { - var obj types.Object - switch fun := call.Fun.(type) { - case *ast.SelectorExpr: - obj = pass.TypesInfo.Uses[fun.Sel] - case *ast.Ident: - obj = pass.TypesInfo.Uses[fun] - default: - return nil, false - } - - fn, ok := obj.(*types.Func) - if !ok || fn.Pkg() == nil || fn.Pkg().Path() != "os" { - return nil, false - } - if fn.Name() != "Setenv" && fn.Name() != "Unsetenv" { - return nil, false - } - return fn, true -} diff --git a/pkg/linters/stringsindexcontains/stringsindexcontains.go b/pkg/linters/stringsindexcontains/stringsindexcontains.go index 7304246c6e4..7001e5ae36d 100644 --- a/pkg/linters/stringsindexcontains/stringsindexcontains.go +++ b/pkg/linters/stringsindexcontains/stringsindexcontains.go @@ -117,7 +117,7 @@ func matchIndexComparison(pass *analysis.Pass, expr *ast.BinaryExpr) (call *ast. op := expr.Op if flipped { - op = flipOp(op) + op = astutil.FlipComparisonOp(op) } litVal, ok := constIntValue(pass, right) @@ -197,22 +197,6 @@ func constIntValue(pass *analysis.Pass, expr ast.Expr) (int64, bool) { return v, exact } -// flipOp returns the comparison operator with left and right operands swapped. -func flipOp(op token.Token) token.Token { - switch op { - case token.LSS: - return token.GTR - case token.GTR: - return token.LSS - case token.LEQ: - return token.GEQ - case token.GEQ: - return token.LEQ - default: - return op - } -} - // indexPkgText returns the package selector text (e.g., "strings") from a strings.Index call. func indexPkgText(pass *analysis.Pass, call *ast.CallExpr) string { sel, ok := call.Fun.(*ast.SelectorExpr) diff --git a/pkg/linters/timesleepnocontext/timesleepnocontext.go b/pkg/linters/timesleepnocontext/timesleepnocontext.go index aca6183807d..177118eb76e 100644 --- a/pkg/linters/timesleepnocontext/timesleepnocontext.go +++ b/pkg/linters/timesleepnocontext/timesleepnocontext.go @@ -53,11 +53,11 @@ func run(pass *analysis.Pass) (any, error) { for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) { funcNode := encl.Node() - funcType := enclosingFuncType(funcNode) + funcType := astutil.EnclosingFuncType(funcNode) if funcType == nil { continue } - ctxParamName, hasCtx := contextParamName(pass, funcType) + ctxParamName, hasCtx := astutil.ContextParamName(pass, funcType) if !hasCtx { if _, isFuncLit := funcNode.(*ast.FuncLit); isFuncLit && !isGoOrDeferClosure(encl) { break @@ -141,52 +141,3 @@ func isTimeSleepCall(pass *analysis.Pass, call *ast.CallExpr) bool { } return pkgName.Imported().Path() == "time" } - -func enclosingFuncType(node ast.Node) *ast.FuncType { - switch fn := node.(type) { - case *ast.FuncDecl: - return fn.Type - case *ast.FuncLit: - return fn.Type - default: - return nil - } -} - -// contextParamName returns the name of the first context.Context parameter -// in fn, and true, or "", false if none exists. -func contextParamName(pass *analysis.Pass, fn *ast.FuncType) (string, bool) { - if fn == nil || fn.Params == nil { - return "", false - } - ctxType := contextContextType(pass) - if ctxType == nil { - return "", false - } - for _, field := range fn.Params.List { - t := pass.TypesInfo.TypeOf(field.Type) - if t == nil || !types.Identical(t, ctxType) { - continue - } - for _, name := range field.Names { - if name.Name != "_" { - return name.Name, true - } - } - } - return "", false -} - -// contextContextType returns the types.Type for context.Context, or nil if -// the context package is not imported. -func contextContextType(pass *analysis.Pass) types.Type { - for _, pkg := range pass.Pkg.Imports() { - if pkg.Path() == "context" { - obj := pkg.Scope().Lookup("Context") - if obj != nil { - return obj.Type() - } - } - } - return nil -}