SA5008: support easyjson's `intern` struct tag

[dominikh]

easyjson adds their own intern option, see https://github.com/mailru/easyjson#string-interning – staticcheck currently flags this: bar.go:5:15: unknown JSON option "intern" (SA5008)

Marking as needs-decision because I don't know how I feel about people adding their own options to established struct tags.

Found via https://github.com/golang/go/issues/41769#issuecomment-703206704

[ainar-g]

As a counterpoint, if I were to remove package easyjson or copy a struct originally intended for use with package easyjson somewhere where I am going to use encoding/json, I would really want staticcheck to report those options as unknown.

[dominikh]

We can tweak the rate of false positives and negatives by using several heuristics. We can require that easyjson actually be imported in the file that defines a struct with the intern option. We can look at the concrete type of a value flowing into encoding/json methods and flag the incompatible tags. A combination of the two probably avoids most false positives and negatives.

[zikaeroh]

That might not be enough; easyjson outputs to a different file. You don't need to import easyjson in the file where the structs are defined to use it (in fact, you may never import it at all other than in the generated code, since its intent is to speed up JSON transparently). You might to get something like easyjson.RawMessage, but I'm guessing that's rare.

Note that easyjson actually has another directive, nocopy: https://github.com/mailru/easyjson#structure-json-tag-options

Maybe easyjson just needs to be convinced not to add its own stuff to json:"..." and use easyjson:"..." instead; they are tags for the code gen after all... 🙂

[dominikh]

Hm. We could try to detect the generated marshaling methods. If a struct implements easyjson.Marshaler then we allow use of the easyjson struct tags. At least we'd stop complaining once code generation has completed.

Maybe easyjson just needs to be convinced not to add its own stuff to json:"..." and use easyjson:"..." instead; they are tags for the code gen after all...

I agree that they really shouldn't be doing this. But I won't be the one pushing them to change their code.

[zikaeroh]

That seems reasonable. If it's easier, you could just cheat and look for the MarshalEasyJSON method by name (rather than actually checking for the right interface implementation). But my knowledge of the Analyzer system isn't that great.

[mxv2]

Hi.

I had that issue. And I got an idea. Let's allow to define user's list of valid struct tags additional to the basic list. Maybe it's resolved issue?

[dominikh]

Let's allow to define user's list of valid struct tags additional to the basic list.

I don't want to encourage libraries to hijack the json namespace, nor do I want users to have to configure this manually.

[yuyang0914]

My repo uses the go-zero library, which also customizes the json tag.

Here is an example using go-zero： https://github.com/zeromicro/zero-examples/blob/939810522e1d5eb41c5beca8bb1fe14ee2a7d5a1/http/signature/server/server.go#L19

When I use staticcheck to check code, I always get the message unknown JSON option "optional" (SA5008). Do @dominikh have any suggestions?