diff --git a/cmd/linters/main.go b/cmd/linters/main.go index 9c6fff9d628..f288339af12 100644 --- a/cmd/linters/main.go +++ b/cmd/linters/main.go @@ -23,6 +23,7 @@ import ( "github.com/github/gh-aw/pkg/linters/excessivefuncparams" "github.com/github/gh-aw/pkg/linters/fileclosenotdeferred" "github.com/github/gh-aw/pkg/linters/fprintlnsprintf" + "github.com/github/gh-aw/pkg/linters/jsonmarshalignoredeerror" "github.com/github/gh-aw/pkg/linters/largefunc" "github.com/github/gh-aw/pkg/linters/manualmutexunlock" "github.com/github/gh-aw/pkg/linters/osexitinlibrary" @@ -53,6 +54,7 @@ func main() { regexpcompileinfunction.Analyzer, ssljson.Analyzer, strconvparseignorederror.Analyzer, + jsonmarshalignoredeerror.Analyzer, uncheckedtypeassertion.Analyzer, ) } diff --git a/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go b/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go new file mode 100644 index 00000000000..7ae82078b13 --- /dev/null +++ b/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go @@ -0,0 +1,79 @@ +// Package jsonmarshalignoredeerror implements a Go analysis linter that flags +// json.Marshal and json.Unmarshal calls where the error return is discarded with _. +package jsonmarshalignoredeerror + +import ( + "go/ast" + "go/types" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/inspector" +) + +// Analyzer is the json-marshal-ignored-error analysis pass. +var Analyzer = &analysis.Analyzer{ + Name: "jsonmarshalignoredeerror", + Doc: "reports json.Marshal and json.Unmarshal calls where the error return is discarded with _", + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/jsonmarshalignoredeerror", + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) + nodeFilter := []ast.Node{(*ast.AssignStmt)(nil)} + insp.Preorder(nodeFilter, func(n ast.Node) { + assign, ok := n.(*ast.AssignStmt) + if !ok { + return + } + + // Pattern: val, _ := json.Marshal(x) — 2 lhs, 1 rhs, Lhs[1] is blank + if len(assign.Lhs) == 2 && len(assign.Rhs) == 1 { + blank, ok := assign.Lhs[1].(*ast.Ident) + if ok && blank.Name == "_" { + call, ok := assign.Rhs[0].(*ast.CallExpr) + if ok { + if isJSONFunc(pass, call, "Marshal") { + pass.ReportRangef(call, "error return from json.Marshal is discarded; marshal failures produce nil bytes silently") + } + } + } + } + + // Pattern: _ = json.Unmarshal(data, &v) — 1 lhs, 1 rhs, Lhs[0] is blank + if len(assign.Lhs) == 1 && len(assign.Rhs) == 1 { + blank, ok := assign.Lhs[0].(*ast.Ident) + if ok && blank.Name == "_" { + call, ok := assign.Rhs[0].(*ast.CallExpr) + if ok { + if isJSONFunc(pass, call, "Unmarshal") { + pass.ReportRangef(call, "error return from json.Unmarshal is discarded; unmarshal failures leave the target value in a partial state") + } + } + } + } + }) + return nil, nil +} + +func isJSONFunc(pass *analysis.Pass, call *ast.CallExpr, name string) bool { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + if sel.Sel.Name != name { + return false + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return false + } + obj := pass.TypesInfo.Uses[ident] + pkgName, ok := obj.(*types.PkgName) + if !ok { + return false + } + return pkgName.Imported().Path() == "encoding/json" +} diff --git a/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror_test.go b/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror_test.go new file mode 100644 index 00000000000..03aca55b769 --- /dev/null +++ b/pkg/linters/jsonmarshalignoredeerror/jsonmarshalignoredeerror_test.go @@ -0,0 +1,16 @@ +//go:build !integration + +package jsonmarshalignoredeerror_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/jsonmarshalignoredeerror" +) + +func TestAnalyzer(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, jsonmarshalignoredeerror.Analyzer, "jsonmarshalignoredeerror") +} diff --git a/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go b/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go new file mode 100644 index 00000000000..0d8532b5da4 --- /dev/null +++ b/pkg/linters/jsonmarshalignoredeerror/testdata/src/jsonmarshalignoredeerror/jsonmarshalignoredeerror.go @@ -0,0 +1,29 @@ +package jsonmarshalignoredeerror + +import "encoding/json" + +type Foo struct{ X int } + +func Bad() { + f := Foo{X: 1} + val, _ := json.Marshal(f) // want `error return from json\.Marshal is discarded` + _ = val + + var f2 Foo + _ = json.Unmarshal([]byte(`{}`), &f2) // want `error return from json\.Unmarshal is discarded` +} + +func Good() error { + f := Foo{X: 1} + val, err := json.Marshal(f) + if err != nil { + return err + } + _ = val + + var f2 Foo + if err := json.Unmarshal([]byte(`{}`), &f2); err != nil { + return err + } + return nil +}