From b6dee63b32cd0273cdabfe469130956f6123e576 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Wed, 5 May 2021 22:47:25 +0100
Subject: [PATCH] improve handling of reflect on foreign unnamed types

If a package A imports package B, and uses reflect.TypeOf on an unnamed
struct type B.T (such as an alias), we don't want to record B.T's fields
as "do not obfuscate". This is for the same reason that we don't if B.T
is a named struct type: the detection only works for the package
defining the type, as otherwise it's inconsistent.

We failed to handle this case well, because we assumed all struct types
would be under some named type. This is not the case for type aliases.

Fortunately, struct fields are named, and as such they are objects.
Check their package too, just like we do for named types.

Fixes another build error when obfuscating the protobuf module.
We add a simplified version of the example above as a test case,
which originated from debugging the protobuf build failure.
---
 main.go                      | 12 ++++++++++--
 testdata/scripts/reflect.txt | 23 +++++++++++++++++++++--
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/main.go b/main.go
index 43bc32e..8f75048 100644
--- a/main.go
+++ b/main.go
@@ -673,7 +673,7 @@ func transformCompile(args []string) ([]string, error) {
 			}
 
 			debugFilePath := filepath.Join(pkgDebugDir, name)
-			if err := os.WriteFile(debugFilePath, src, 0666); err != nil {
+			if err := os.WriteFile(debugFilePath, src, 0o666); err != nil {
 				return nil, err
 			}
 		}
@@ -972,7 +972,8 @@ func (tf *transformer) recordReflectArgs(files []*ast.File) {
 
 		if fnType.Pkg().Path() == "reflect" && (fnType.Name() == "TypeOf" || fnType.Name() == "ValueOf") {
 			for _, arg := range call.Args {
-				tf.recordIgnore(tf.info.TypeOf(arg), false)
+				argType := tf.info.TypeOf(arg)
+				tf.recordIgnore(argType, false)
 			}
 		}
 		return true
@@ -1301,6 +1302,13 @@ func (tf *transformer) recordIgnore(t types.Type, allPkgs bool) {
 		for i := 0; i < t.NumFields(); i++ {
 			field := t.Field(i)
 
+			// This check is similar to the one in *types.Named.
+			// It's necessary for unnamed struct types,
+			// as they aren't named but still have named fields.
+			if !allPkgs && field.Pkg() != tf.pkg {
+				return // not from the current package
+			}
+
 			// Record the field itself, too.
 			tf.ignoreObjects[field] = true
 
diff --git a/testdata/scripts/reflect.txt b/testdata/scripts/reflect.txt
index bb50515..ecfbadf 100644
--- a/testdata/scripts/reflect.txt
+++ b/testdata/scripts/reflect.txt
@@ -51,7 +51,7 @@ func main() {
 
 	// Use of a common library using reflect, encoding/json.
 	// TODO: remove the TypeOf hint once we detect json.Marshal.
-	var _ = reflect.TypeOf(EncodingT{})
+	_ = reflect.TypeOf(EncodingT{})
 	enc, _ := json.Marshal(EncodingT{Foo: 3})
 	println(string(enc))
 
@@ -60,10 +60,24 @@ func main() {
 	outer.InnerField = 3
 	enc, _ = json.Marshal(outer)
 	println(string(enc))
+
+	// An edge case; the struct type is defined in a different package.
+	// Note that the struct type is unnamed, but it still has named fields.
+	// We only use reflection on it here, not the declaring package.
+	// As such, we should obfuscate the field name.
+	// Simply using the field name here used to cause build failures.
+	_ = reflect.TypeOf(importedpkg.UnnamedWithDownstreamReflect{})
+	fmt.Printf("%v\n", importedpkg.UnnamedWithDownstreamReflect{
+		ExportedField: "foo",
+	})
 }
 
 func printfWithoutPackage(format string, v interface{}) {
-	fmt.Print(strings.Split(fmt.Sprintf(format, v), ".")[1])
+	s := fmt.Sprintf(format, v)
+	if i := strings.IndexByte(s, '.'); i > 0 {
+		s = s[i+1:]
+	}
+	fmt.Print(s)
 }
 
 type EncodingT struct {
@@ -128,6 +142,10 @@ type EmbeddingInner struct {
 	InnerField int
 }
 
+type UnnamedWithDownstreamReflect = struct {
+	ExportedField string
+}
+
 -- main.stdout --
 9000
 {5 0}
@@ -135,3 +153,4 @@ ReflectTypeOf
 ReflectTypeOfIndirect
 ReflectValueOf{ExportedField:"abc", unexportedField:""}
 [method: abc]
+{foo}