diff --git a/.custom-gcl.yml b/.custom-gcl.yml new file mode 100644 index 000000000..e6b04f34f --- /dev/null +++ b/.custom-gcl.yml @@ -0,0 +1,6 @@ +version: v1.57.0 +destination: ./.tool/ +plugins: + - module: 'github.com/anchore/syft' + import: 'github.com/anchore/syft/test/linters/ensuredefer' + path: . diff --git a/.golangci.yaml b/.golangci.yaml index efe5fa2ec..5ded6d9ee 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -14,6 +14,7 @@ linters: - bodyclose - dogsled - dupl + - ensuredefer - errcheck - exportloopref - funlen @@ -52,6 +53,13 @@ linters-settings: gocritic: enabled-checks: - deferInLoop + custom: + ensuredefer: + type: "module" + description: Ensure that all important closers are called with defer + settings: + symbols: + - github.com/anchore/syft/internal.CloseAndLogError output: uniq-by-line: false run: diff --git a/Taskfile.yaml b/Taskfile.yaml index 61312f40e..0f0b95fd0 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -150,8 +150,10 @@ tasks: # ensure there are no files with ":" in it (a known back case in the go ecosystem) - cmd: 'test -z "{{ .BAD_FILE_NAMES }}" || (echo "files with bad names: [{{ .BAD_FILE_NAMES }}]"; exit 1)' silent: true + # build custom linter executable + - "{{ .TOOL_DIR }}/golangci-lint custom -v" # run linting - - "{{ .TOOL_DIR }}/golangci-lint run --tests=false" + - "{{ .TOOL_DIR }}/custom-gcl run --tests=false" check-licenses: diff --git a/go.mod b/go.mod index 606ca115c..a94762217 100644 --- a/go.mod +++ b/go.mod @@ -83,6 +83,11 @@ require ( modernc.org/sqlite v1.29.8 ) +require ( + github.com/golangci/plugin-module-register v0.1.1 + golang.org/x/tools v0.19.0 +) + require ( dario.cat/mergo v1.0.0 // indirect github.com/AdaLogics/go-fuzz-headers v0.0.0-20230811130428-ced1acdcaa24 // indirect @@ -224,7 +229,6 @@ require ( golang.org/x/sys v0.19.0 // indirect golang.org/x/term v0.19.0 // indirect golang.org/x/text v0.14.0 // indirect - golang.org/x/tools v0.19.0 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect google.golang.org/genproto v0.0.0-20231106174013-bbf56f31fb17 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231120223509-83a465c0220f // indirect diff --git a/go.sum b/go.sum index ca4ce00ec..28d7f70df 100644 --- a/go.sum +++ b/go.sum @@ -357,6 +357,8 @@ github.com/golang/snappy v0.0.2/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEW github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM= github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/golangci/plugin-module-register v0.1.1 h1:TCmesur25LnyJkpsVrupv1Cdzo+2f7zX0H6Jkw1Ol6c= +github.com/golangci/plugin-module-register v0.1.1/go.mod h1:TTpqoB6KkwOJMV8u7+NyXMrkwwESJLOkfl9TxR1DGFc= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= diff --git a/test/linters/ensuredefer/analyzer.go b/test/linters/ensuredefer/analyzer.go new file mode 100644 index 000000000..a3187dae6 --- /dev/null +++ b/test/linters/ensuredefer/analyzer.go @@ -0,0 +1,85 @@ +package ensuredefer + +import ( + "github.com/golangci/plugin-module-register/register" + "go/ast" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +func init() { + register.Plugin("ensuredefer", func(conf any) (register.LinterPlugin, error) { + return &analyzerPlugin{}, nil + }) +} + +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 block will have + 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: "ensuredefer", + Doc: "enforce that specified functions are called in defer statements", + Run: run, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + } + return &analyzer +} + +var analyzerInstance = NewAnalyzer() + +type analyzerPlugin struct{} + +func (p *analyzerPlugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { + return []*analysis.Analyzer{ + analyzerInstance, + }, nil +} + +func (p *analyzerPlugin) GetLoadMode() string { + //TODO: what does this do + return register.LoadModeSyntax +} diff --git a/test/linters/ensuredefer/main.go b/test/linters/ensuredefer/main.go new file mode 100644 index 000000000..86f2b114f --- /dev/null +++ b/test/linters/ensuredefer/main.go @@ -0,0 +1,9 @@ +package ensuredefer + +import ( + "golang.org/x/tools/go/analysis/singlechecker" +) + +func main() { + singlechecker.Main(NewAnalyzer()) +}