Skip to content

Commit 1105e1a

Browse files
tmzanetmzane
authored
fix: do not report types implementing (Un)Marshaler (#67)
1 parent 11f0e6c commit 1105e1a

File tree

3 files changed

+265
-26
lines changed

3 files changed

+265
-26
lines changed

‎builtins.go

+132-22
Original file line numberDiff line numberDiff line change
@@ -3,34 +3,144 @@ package musttag
33
// builtins is a set of functions supported out of the box.
44
var builtins = []Func{
55
// https://pkg.go.dev/encoding/json
6-
{Name: "encoding/json.Marshal", Tag: "json", ArgPos: 0},
7-
{Name: "encoding/json.MarshalIndent", Tag: "json", ArgPos: 0},
8-
{Name: "encoding/json.Unmarshal", Tag: "json", ArgPos: 1},
9-
{Name: "(*encoding/json.Encoder).Encode", Tag: "json", ArgPos: 0},
10-
{Name: "(*encoding/json.Decoder).Decode", Tag: "json", ArgPos: 0},
6+
{
7+
Name: "encoding/json.Marshal",
8+
Tag: "json",
9+
ArgPos: 0,
10+
ifaceWhitelist: []string{"encoding/json.Marshaler", "encoding.TextMarshaler"},
11+
},
12+
{
13+
Name: "encoding/json.MarshalIndent",
14+
Tag: "json",
15+
ArgPos: 0,
16+
ifaceWhitelist: []string{"encoding/json.Marshaler", "encoding.TextMarshaler"},
17+
},
18+
{
19+
Name: "encoding/json.Unmarshal",
20+
Tag: "json",
21+
ArgPos: 1,
22+
ifaceWhitelist: []string{"encoding/json.Unmarshaler", "encoding.TextUnmarshaler"},
23+
},
24+
{
25+
Name: "(*encoding/json.Encoder).Encode",
26+
Tag: "json",
27+
ArgPos: 0,
28+
ifaceWhitelist: []string{"encoding/json.Marshaler", "encoding.TextMarshaler"},
29+
},
30+
{
31+
Name: "(*encoding/json.Decoder).Decode",
32+
Tag: "json",
33+
ArgPos: 0,
34+
ifaceWhitelist: []string{"encoding/json.Unmarshaler", "encoding.TextUnmarshaler"},
35+
},
1136

1237
// https://pkg.go.dev/encoding/xml
13-
{Name: "encoding/xml.Marshal", Tag: "xml", ArgPos: 0},
14-
{Name: "encoding/xml.MarshalIndent", Tag: "xml", ArgPos: 0},
15-
{Name: "encoding/xml.Unmarshal", Tag: "xml", ArgPos: 1},
16-
{Name: "(*encoding/xml.Encoder).Encode", Tag: "xml", ArgPos: 0},
17-
{Name: "(*encoding/xml.Decoder).Decode", Tag: "xml", ArgPos: 0},
18-
{Name: "(*encoding/xml.Encoder).EncodeElement", Tag: "xml", ArgPos: 0},
19-
{Name: "(*encoding/xml.Decoder).DecodeElement", Tag: "xml", ArgPos: 0},
38+
{
39+
Name: "encoding/xml.Marshal",
40+
Tag: "xml",
41+
ArgPos: 0,
42+
ifaceWhitelist: []string{"encoding/xml.Marshaler", "encoding.TextMarshaler"},
43+
},
44+
{
45+
Name: "encoding/xml.MarshalIndent",
46+
Tag: "xml",
47+
ArgPos: 0,
48+
ifaceWhitelist: []string{"encoding/xml.Marshaler", "encoding.TextMarshaler"},
49+
},
50+
{
51+
Name: "encoding/xml.Unmarshal",
52+
Tag: "xml",
53+
ArgPos: 1,
54+
ifaceWhitelist: []string{"encoding/xml.Unmarshaler", "encoding.TextUnmarshaler"},
55+
},
56+
{
57+
Name: "(*encoding/xml.Encoder).Encode",
58+
Tag: "xml",
59+
ArgPos: 0,
60+
ifaceWhitelist: []string{"encoding/xml.Marshaler", "encoding.TextMarshaler"},
61+
},
62+
{
63+
Name: "(*encoding/xml.Decoder).Decode",
64+
Tag: "xml",
65+
ArgPos: 0,
66+
ifaceWhitelist: []string{"encoding/xml.Unmarshaler", "encoding.TextUnmarshaler"},
67+
},
68+
{
69+
Name: "(*encoding/xml.Encoder).EncodeElement",
70+
Tag: "xml",
71+
ArgPos: 0,
72+
ifaceWhitelist: []string{"encoding/xml.Marshaler", "encoding.TextMarshaler"},
73+
},
74+
{
75+
Name: "(*encoding/xml.Decoder).DecodeElement",
76+
Tag: "xml",
77+
ArgPos: 0,
78+
ifaceWhitelist: []string{"encoding/xml.Unmarshaler", "encoding.TextUnmarshaler"},
79+
},
2080

2181
// https://github.com/go-yaml/yaml
22-
{Name: "gopkg.in/yaml.v3.Marshal", Tag: "yaml", ArgPos: 0},
23-
{Name: "gopkg.in/yaml.v3.Unmarshal", Tag: "yaml", ArgPos: 1},
24-
{Name: "(*gopkg.in/yaml.v3.Encoder).Encode", Tag: "yaml", ArgPos: 0},
25-
{Name: "(*gopkg.in/yaml.v3.Decoder).Decode", Tag: "yaml", ArgPos: 0},
82+
{
83+
Name: "gopkg.in/yaml.v3.Marshal",
84+
Tag: "yaml",
85+
ArgPos: 0,
86+
ifaceWhitelist: []string{"gopkg.in/yaml.v3.Marshaler"},
87+
},
88+
{
89+
Name: "gopkg.in/yaml.v3.Unmarshal",
90+
Tag: "yaml",
91+
ArgPos: 1,
92+
ifaceWhitelist: []string{"gopkg.in/yaml.v3.Unmarshaler"},
93+
},
94+
{
95+
Name: "(*gopkg.in/yaml.v3.Encoder).Encode",
96+
Tag: "yaml",
97+
ArgPos: 0,
98+
ifaceWhitelist: []string{"gopkg.in/yaml.v3.Marshaler"},
99+
},
100+
{
101+
Name: "(*gopkg.in/yaml.v3.Decoder).Decode",
102+
Tag: "yaml",
103+
ArgPos: 0,
104+
ifaceWhitelist: []string{"gopkg.in/yaml.v3.Unmarshaler"},
105+
},
26106

27107
// https://github.com/BurntSushi/toml
28-
{Name: "github.com/BurntSushi/toml.Unmarshal", Tag: "toml", ArgPos: 1},
29-
{Name: "github.com/BurntSushi/toml.Decode", Tag: "toml", ArgPos: 1},
30-
{Name: "github.com/BurntSushi/toml.DecodeFS", Tag: "toml", ArgPos: 2},
31-
{Name: "github.com/BurntSushi/toml.DecodeFile", Tag: "toml", ArgPos: 1},
32-
{Name: "(*github.com/BurntSushi/toml.Encoder).Encode", Tag: "toml", ArgPos: 0},
33-
{Name: "(*github.com/BurntSushi/toml.Decoder).Decode", Tag: "toml", ArgPos: 0},
108+
{
109+
Name: "github.com/BurntSushi/toml.Unmarshal",
110+
Tag: "toml",
111+
ArgPos: 1,
112+
ifaceWhitelist: []string{"github.com/BurntSushi/toml.Unmarshaler", "encoding.TextUnmarshaler"},
113+
},
114+
{
115+
Name: "github.com/BurntSushi/toml.Decode",
116+
Tag: "toml",
117+
ArgPos: 1,
118+
ifaceWhitelist: []string{"github.com/BurntSushi/toml.Unmarshaler", "encoding.TextUnmarshaler"},
119+
},
120+
{
121+
Name: "github.com/BurntSushi/toml.DecodeFS",
122+
Tag: "toml",
123+
ArgPos: 2,
124+
ifaceWhitelist: []string{"github.com/BurntSushi/toml.Unmarshaler", "encoding.TextUnmarshaler"},
125+
},
126+
{
127+
Name: "github.com/BurntSushi/toml.DecodeFile",
128+
Tag: "toml",
129+
ArgPos: 1,
130+
ifaceWhitelist: []string{"github.com/BurntSushi/toml.Unmarshaler", "encoding.TextUnmarshaler"},
131+
},
132+
{
133+
Name: "(*github.com/BurntSushi/toml.Encoder).Encode",
134+
Tag: "toml",
135+
ArgPos: 0,
136+
ifaceWhitelist: []string{"encoding.TextMarshaler"},
137+
},
138+
{
139+
Name: "(*github.com/BurntSushi/toml.Decoder).Decode",
140+
Tag: "toml",
141+
ArgPos: 0,
142+
ifaceWhitelist: []string{"github.com/BurntSushi/toml.Unmarshaler", "encoding.TextUnmarshaler"},
143+
},
34144

35145
// https://github.com/mitchellh/mapstructure
36146
{Name: "github.com/mitchellh/mapstructure.Decode", Tag: "mapstructure", ArgPos: 1},

‎musttag.go

+63-4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ type Func struct {
2424
Name string // Name is the full name of the function, including the package.
2525
Tag string // Tag is the struct tag whose presence should be ensured.
2626
ArgPos int // ArgPos is the position of the argument to check.
27+
28+
// a list of interface names (including the package);
29+
// if at least one is implemented by the argument, no check is performed.
30+
ifaceWhitelist []string
2731
}
2832

2933
func (fn Func) shortName() string {
@@ -93,7 +97,7 @@ var report = func(pass *analysis.Pass, st *structType, fn Func, fnPos token.Posi
9397
pass.Reportf(st.Pos, format, st.Name, fn.Tag, fn.shortName(), fnPos)
9498
}
9599

96-
var cleanFullName = regexp.MustCompile(`([^*/(]+/vendor/)`)
100+
var trimVendor = regexp.MustCompile(`([^*/(]+/vendor/)`)
97101

98102
// run starts the analysis.
99103
func run(pass *analysis.Pass, mainModule string, funcs map[string]Func) (any, error) {
@@ -117,7 +121,7 @@ func run(pass *analysis.Pass, mainModule string, funcs map[string]Func) (any, er
117121
return // not a static call.
118122
}
119123

120-
name := cleanFullName.ReplaceAllString(callee.FullName(), "")
124+
name := trimVendor.ReplaceAllString(callee.FullName(), "")
121125
fn, ok := funcs[name]
122126
if !ok {
123127
return // the function is not supported.
@@ -144,13 +148,21 @@ func run(pass *analysis.Pass, mainModule string, funcs map[string]Func) (any, er
144148
initialPos = arg.Pos()
145149
}
146150

151+
typ := pass.TypesInfo.TypeOf(arg)
152+
if typ == nil {
153+
return // no type info found.
154+
}
155+
156+
if implementsInterface(typ, fn.ifaceWhitelist, pass.Pkg.Imports()) {
157+
return // the type implements a Marshaler interface, nothing to check; see issue #64.
158+
}
159+
147160
checker := checker{
148161
mainModule: mainModule,
149162
seenTypes: make(map[string]struct{}),
150163
}
151164

152-
t := pass.TypesInfo.TypeOf(arg)
153-
st, ok := checker.parseStructType(t, initialPos)
165+
st, ok := checker.parseStructType(typ, initialPos)
154166
if !ok {
155167
return // not a struct argument.
156168
}
@@ -257,3 +269,50 @@ func (c *checker) checkStructType(st *structType, tag string) (*structType, bool
257269

258270
return nil, true
259271
}
272+
273+
func implementsInterface(typ types.Type, ifaces []string, imports []*types.Package) bool {
274+
findScope := func(pkgName string) (*types.Scope, bool) {
275+
// fast path: check direct imports (e.g. looking for "encoding/json.Marshaler").
276+
for _, direct := range imports {
277+
if pkgName == trimVendor.ReplaceAllString(direct.Path(), "") {
278+
return direct.Scope(), true
279+
}
280+
}
281+
// slow path: check indirect imports (e.g. looking for "encoding.TextMarshaler").
282+
for _, direct := range imports {
283+
for _, indirect := range direct.Imports() {
284+
if pkgName == trimVendor.ReplaceAllString(indirect.Path(), "") {
285+
return indirect.Scope(), true
286+
}
287+
}
288+
}
289+
return nil, false
290+
}
291+
292+
for _, ifacePath := range ifaces {
293+
// "encoding/json.Marshaler" -> "encoding/json" + "Marshaler"
294+
idx := strings.LastIndex(ifacePath, ".")
295+
if idx == -1 {
296+
continue
297+
}
298+
pkgName, ifaceName := ifacePath[:idx], ifacePath[idx+1:]
299+
300+
scope, ok := findScope(pkgName)
301+
if !ok {
302+
continue
303+
}
304+
obj := scope.Lookup(ifaceName)
305+
if obj == nil {
306+
continue
307+
}
308+
iface, ok := obj.Type().Underlying().(*types.Interface)
309+
if !ok {
310+
continue
311+
}
312+
if types.Implements(typ, iface) {
313+
return true
314+
}
315+
}
316+
317+
return false
318+
}

‎testdata/src/builtins/builtins.go

+70
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,44 @@ type User struct { /* want
7676
Email string `json:"email" xml:"email" yaml:"email" toml:"email" mapstructure:"email" db:"email" custom:"email"`
7777
}
7878

79+
// TODO: Unmarshaler should be implemented using pointer semantics.
80+
81+
type TextMarshaler struct{ NoTag string }
82+
83+
func (TextMarshaler) MarshalText() ([]byte, error) { return nil, nil }
84+
func (TextMarshaler) UnmarshalText([]byte) error { return nil }
85+
86+
type Marshaler struct{ NoTag string }
87+
88+
func (Marshaler) MarshalJSON() ([]byte, error) { return nil, nil }
89+
func (Marshaler) UnmarshalJSON([]byte) error { return nil }
90+
func (Marshaler) MarshalXML(e *xml.Encoder, start xml.StartElement) error { return nil }
91+
func (Marshaler) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error { return nil }
92+
func (Marshaler) MarshalYAML() (any, error) { return nil, nil }
93+
func (Marshaler) UnmarshalYAML(*yaml.Node) error { return nil }
94+
func (Marshaler) UnmarshalTOML(any) error { return nil }
95+
7996
func testJSON() {
8097
var user User
8198
json.Marshal(user)
8299
json.MarshalIndent(user, "", "")
83100
json.Unmarshal(nil, &user)
84101
json.NewEncoder(nil).Encode(user)
85102
json.NewDecoder(nil).Decode(&user)
103+
104+
var m Marshaler
105+
json.Marshal(m)
106+
json.MarshalIndent(m, "", "")
107+
json.Unmarshal(nil, &m)
108+
json.NewEncoder(nil).Encode(m)
109+
json.NewDecoder(nil).Decode(&m)
110+
111+
var tm TextMarshaler
112+
json.Marshal(tm)
113+
json.MarshalIndent(tm, "", "")
114+
json.Unmarshal(nil, &tm)
115+
json.NewEncoder(nil).Encode(tm)
116+
json.NewDecoder(nil).Decode(&tm)
86117
}
87118

88119
func testXML() {
@@ -94,6 +125,24 @@ func testXML() {
94125
xml.NewDecoder(nil).Decode(&user)
95126
xml.NewEncoder(nil).EncodeElement(user, xml.StartElement{})
96127
xml.NewDecoder(nil).DecodeElement(&user, &xml.StartElement{})
128+
129+
var m Marshaler
130+
xml.Marshal(m)
131+
xml.MarshalIndent(m, "", "")
132+
xml.Unmarshal(nil, &m)
133+
xml.NewEncoder(nil).Encode(m)
134+
xml.NewDecoder(nil).Decode(&m)
135+
xml.NewEncoder(nil).EncodeElement(m, xml.StartElement{})
136+
xml.NewDecoder(nil).DecodeElement(&m, &xml.StartElement{})
137+
138+
var tm TextMarshaler
139+
xml.Marshal(tm)
140+
xml.MarshalIndent(tm, "", "")
141+
xml.Unmarshal(nil, &tm)
142+
xml.NewEncoder(nil).Encode(tm)
143+
xml.NewDecoder(nil).Decode(&tm)
144+
xml.NewEncoder(nil).EncodeElement(tm, xml.StartElement{})
145+
xml.NewDecoder(nil).DecodeElement(&tm, &xml.StartElement{})
97146
}
98147

99148
func testYAML() {
@@ -102,6 +151,12 @@ func testYAML() {
102151
yaml.Unmarshal(nil, &user)
103152
yaml.NewEncoder(nil).Encode(user)
104153
yaml.NewDecoder(nil).Decode(&user)
154+
155+
var m Marshaler
156+
yaml.Marshal(m)
157+
yaml.Unmarshal(nil, &m)
158+
yaml.NewEncoder(nil).Encode(m)
159+
yaml.NewDecoder(nil).Decode(&m)
105160
}
106161

107162
func testTOML() {
@@ -112,6 +167,21 @@ func testTOML() {
112167
toml.DecodeFile("", &user)
113168
toml.NewEncoder(nil).Encode(user)
114169
toml.NewDecoder(nil).Decode(&user)
170+
171+
var m Marshaler
172+
toml.Unmarshal(nil, &m)
173+
toml.Decode("", &m)
174+
toml.DecodeFS(nil, "", &m)
175+
toml.DecodeFile("", &m)
176+
toml.NewDecoder(nil).Decode(&m)
177+
178+
var tm TextMarshaler
179+
toml.Unmarshal(nil, &tm)
180+
toml.Decode("", &tm)
181+
toml.DecodeFS(nil, "", &tm)
182+
toml.DecodeFile("", &tm)
183+
toml.NewEncoder(nil).Encode(tm)
184+
toml.NewDecoder(nil).Decode(&tm)
115185
}
116186

117187
func testMapstructure() {

0 commit comments

Comments
 (0)