SA1026: False positive when func json.Marshaler used as a field

[Merovius]

The SA1026 check (to check if a func or chan is being marshalled) gives a false positive, when a json.Marshaler is used as a field:

package main

import (
	"encoding/json"
	"fmt"
)

type T func()

func (*T) MarshalJSON() ([]byte, error) {
	return []byte(`"foobar"`), nil
}

type S struct {
	T T
}

func main() {
	b, err := json.Marshal(new(S))
	fmt.Println(string(b), err)
}

When running staticcheck . on this, it outputs:

x.go:19:25: trying to marshal chan or func value, field *x.S.T (SA1026)

But running this program outputs:

{"T":"foobar"} <nil>

showing that this works as expected.

I assume the cause here is that staticcheck looks at the method set of T, not of *T, as that's the type of the field. If I change the method receiver to T, the error message disappears.

---

staticcheck version: staticcheck 2021.1.1 (v0.2.1)

'staticcheck -debug.version'

staticcheck 2021.1.1 (v0.2.1)

Compiled with Go version: go1.17.1
Main module:
	honnef.co/go/tools@v0.2.1 (sum: h1:/EPr//+UMMXwMTkXvCCoaJDq8cpjMO80Ou+L4PDo2mY=)
Dependencies:
	github.com/BurntSushi/toml@v0.3.1 (sum: h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=)
	golang.org/x/mod@v0.3.0 (sum: h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=)
	golang.org/x/sys@v0.0.0-20210119212857-b64e53b001e4 (sum: h1:myAQVi0cGEoqQVR5POX+8RR2mrocKqNN1hmeMqhX27k=)
	golang.org/x/tools@v0.1.0 (sum: h1:po9/4sTYwZU9lPhi1tOrb4hCv3qrhiQ77LZfGa2OjwY=)
	golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 (sum: h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=)

go version: go version go1.17.1 linux/amd64

Output of 'go env'

GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/mero/.cache/go-build"
GOENV="/home/mero/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/mero/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/mero"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/mero/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/mero/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.17.1"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/mero/tmp/x/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build874443015=/tmp/go-build -gno-record-gcc-switches"

[dominikh]

Interesting. I didn't expect this to work, but apparently the fact you're marshaling *S and not S means that its fields are also addressable.

[dominikh]

Further investigation revealed that the check, as implemented, is very naive and doesn't implement addressability, nesting, or the various rules of encoding/json that cause fields to be omitted (such as embedded fields shadowing non-embedded fields).

For the fix we will maintain a modified copy of encoding/json that uses go/types instead of reflect. After removing all the dynamic code that is responsible for actually producing JSON, we are left with ~450 lines of type checking that matches encoding/json exactly.

We may have to do something similar for encoding/xml.