From 3eaecda7e6da6ba5cb634b05bf056370400e3625 Mon Sep 17 00:00:00 2001
From: Florian Forster <fforster@gitlab.com>
Date: Mon, 25 Mar 2024 15:45:36 +0100
Subject: [PATCH] feat: Check whether nested types implement a valid interface.

Previously, only the top-level type was checked whether it implemented one of
the provided interfaces. Nested types were not checked.

This leads to false positives, for example when one field is of a struct type
implementing `json.Marshaller`.
---
 musttag.go                  | 53 ++++++++++++++++++++-----------------
 testdata/src/tests/tests.go | 19 +++++++++++++
 2 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/musttag.go b/musttag.go
index d791177..55edbfb 100644
--- a/musttag.go
+++ b/musttag.go
@@ -119,22 +119,14 @@ func run(pass *analysis.Pass, mainModule string, funcs map[string]Func) (_ any,
 			return // no type info found.
 		}
 
-		// TODO: check nested structs too.
-		if implementsInterface(typ, fn.ifaceWhitelist, pass.Pkg.Imports()) {
-			return // the type implements a Marshaler interface; see issue #64.
-		}
-
 		checker := checker{
-			mainModule: mainModule,
-			seenTypes:  make(map[string]struct{}),
-		}
-
-		styp, ok := checker.parseStruct(typ)
-		if !ok {
-			return // not a struct.
+			mainModule:     mainModule,
+			seenTypes:      make(map[string]struct{}),
+			ifaceWhitelist: fn.ifaceWhitelist,
+			imports:        pass.Pkg.Imports(),
 		}
 
-		if valid := checker.checkStruct(styp, fn.Tag); valid {
+		if valid := checker.checkType(typ, fn.Tag); valid {
 			return // nothing to report.
 		}
 
@@ -145,8 +137,28 @@ func run(pass *analysis.Pass, mainModule string, funcs map[string]Func) (_ any,
 }
 
 type checker struct {
-	mainModule string
-	seenTypes  map[string]struct{}
+	mainModule     string
+	seenTypes      map[string]struct{}
+	ifaceWhitelist []string
+	imports        []*types.Package
+}
+
+func (c *checker) checkType(typ types.Type, tag string) bool {
+	if _, ok := c.seenTypes[typ.String()]; ok {
+		return true // already checked.
+	}
+	c.seenTypes[typ.String()] = struct{}{}
+
+	if implementsInterface(typ, c.ifaceWhitelist, c.imports) {
+		return true // the type implements a Marshaler interface; see issue #64.
+	}
+
+	styp, ok := c.parseStruct(typ)
+	if !ok {
+		return true // not a struct.
+	}
+
+	return c.checkStruct(styp, tag)
 }
 
 func (c *checker) parseStruct(typ types.Type) (*types.Struct, bool) {
@@ -186,8 +198,6 @@ func (c *checker) parseStruct(typ types.Type) (*types.Struct, bool) {
 }
 
 func (c *checker) checkStruct(styp *types.Struct, tag string) (valid bool) {
-	c.seenTypes[styp.String()] = struct{}{}
-
 	for i := 0; i < styp.NumFields(); i++ {
 		field := styp.Field(i)
 		if !field.Exported() {
@@ -201,14 +211,7 @@ func (c *checker) checkStruct(styp *types.Struct, tag string) (valid bool) {
 			}
 		}
 
-		nested, ok := c.parseStruct(field.Type())
-		if !ok {
-			continue
-		}
-		if _, ok := c.seenTypes[nested.String()]; ok {
-			continue
-		}
-		if valid := c.checkStruct(nested, tag); !valid {
+		if valid := c.checkType(field.Type(), tag); !valid {
 			return false
 		}
 	}
diff --git a/testdata/src/tests/tests.go b/testdata/src/tests/tests.go
index bd869be..708b405 100644
--- a/testdata/src/tests/tests.go
+++ b/testdata/src/tests/tests.go
@@ -130,3 +130,22 @@ func shouldBeIgnored() {
 	json.Marshal(0)   // a non-struct argument.
 	json.Marshal(nil) // nil argument, see issue #20.
 }
+
+type WithInterface struct {
+	NoTag string
+}
+
+func (w WithInterface) MarshalJSON() ([]byte, error) {
+	return json.Marshal(w.NoTag)
+}
+
+func nestedTypeWithInterface() {
+	type Foo struct {
+		Nested WithInterface `json:"nested"`
+	}
+	var foo Foo
+	json.Marshal(foo)    // no error
+	json.Marshal(&foo)   // no error
+	json.Marshal(Foo{})  // no error
+	json.Marshal(&Foo{}) // no error
+}