Improve linting for defer Close type issues

[willmurphyscode]

This issue is a follow up to prevent #2819 from happening again.

Syft has a method that should be called to clean up after file IO:

syft/internal/err_helper.go

Line 13 in 0774ad1

func CloseAndLogError(closer io.Closer, location string) {

Ideally, we'd like to enforce via linting:

• Files are closed with with internal.CloseAndLogError - chore: use ruleguard to test for missing defer statements #2837
• The call to internal.CloseAndLogError must be called with defer - chore: use ruleguard to test for missing defer statements #2837
• Defer must not be called in a loop - chore: enable go-critic deferInLoop lint #2825

That way, every time someone asks a file resolver for a ReadCloser, it would be guaranteed not to leak the handle.

[willmurphyscode]

Here's a quick implementation that could be wired up to golangci-lint. It hard-codes the name of the function that must defer for now, but that could be passed via config.

package analyzer

import (
	"go/ast"
	"golang.org/x/tools/go/analysis"
	"golang.org/x/tools/go/analysis/passes/inspect"
	"golang.org/x/tools/go/ast/inspector"
)

func run(pass *analysis.Pass) (any, error) {
	insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
	nodeFilter := []ast.Node{
		(*ast.ExprStmt)(nil),
		(*ast.DeferStmt)(nil),
	}
	insp.Preorder(nodeFilter, func(node ast.Node) {
		// if we have a *ast.ExprStmt that calls internal.CloseAndLogError, report a problem.
		// (if the function is correctly called in a defer statement, the node will have type *ast.DeferStmt)
		switch t := node.(type) {
		case *ast.ExprStmt:
			if !isExprStmtAllowed(t, pass) {
				pass.Reportf(t.Pos(), "internal.CloseAndLogError must be called in defer")
			}
		}
	})
	return nil, nil
}

func isExprStmtAllowed(e *ast.ExprStmt, pass *analysis.Pass) bool {
	call, ok := e.X.(*ast.CallExpr)
	if !ok {
		return true
	}
	sel, ok := call.Fun.(*ast.SelectorExpr)
	if !ok {
		return true
	}

	obj := pass.TypesInfo.Uses[sel.Sel]
	if obj == nil {
		return true
	}
	pkg := obj.Pkg()
	if pkg == nil {
		return true
	}

	if pkg.Path() == "github.com/anchore/syft/internal" && sel.Sel.Name == "CloseAndLogError" {
		return false
	}

	return true
}

func NewAnalyzer() *analysis.Analyzer {
	analyzer := analysis.Analyzer{
		Name:     "mustdefer",
		Doc:      "functions whose doc comemnt includes `*mustdefer*` must be invoked with the defer keyword",
		Run:      run,
		Requires: []*analysis.Analyzer{inspect.Analyzer},
	}
	return &analyzer
}

With a main function like this:

func main() {
	singlechecker.Main(analyzer.NewAnalyzer())
}

This is invoked like this:

$ ../investigations/golangci-custom-lint/mustdefer/main ./...
/Users/willmurphy/work/syft-clean/syft/linux/identify_release.go:74:4: internal.CloseAndLogError must be called in defer
/Users/willmurphy/work/syft-clean/syft/pkg/cataloger/generic/cataloger.go:132:3: internal.CloseAndLogError must be called in defer
/Users/willmurphy/work/syft-clean/syft/pkg/cataloger/golang/parse_go_binary.go:70:2: internal.CloseAndLogError must be called in defer
/Users/willmurphy/work/syft-clean/syft/pkg/cataloger/java/graalvm_native_image_cataloger.go:595:3: internal.CloseAndLogError must be called in defer

[willmurphyscode]

Fixed by #2837