From e5913c17e9a94b8aafe6cf0c182f9142ab5d6b78 Mon Sep 17 00:00:00 2001
From: Zi Wang <mrziwang@google.com>
Date: Mon, 30 Oct 2023 14:48:37 -0700
Subject: [PATCH] Remove unnecessary used names before reporting

When property a.b.c is not used, (also there is no a.* or a.b.* used)
"a", "a.b" and "a.b.c" are all in unusedNames.
removeUnnecessaryUnusedNames only keeps the last "a.b.c" as the
real unused name.

Test: TestNonExistentPropertyInSoongConfigModule, unpack_test.go and CI

Bug: 171232169
Change-Id: I861fa6933e558b07694ee5ff40ef549117d115ff
---
 proptools/unpack.go      | 18 +++++++++
 proptools/unpack_test.go | 87 +++++++++++++++++++++++++++++++++++++++-
 2 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/proptools/unpack.go b/proptools/unpack.go
index e1d733c..e6b688e 100644
--- a/proptools/unpack.go
+++ b/proptools/unpack.go
@@ -108,6 +108,7 @@ func UnpackProperties(properties []*parser.Property, objects ...interface{}) (ma
 
 func (ctx *unpackContext) reportUnusedNames(unusedNames []string) []error {
 	sort.Strings(unusedNames)
+	unusedNames = removeUnnecessaryUnusedNames(unusedNames)
 	var lastReported string
 	for _, name := range unusedNames {
 		// if 'foo' has been reported, ignore 'foo\..*' and 'foo\[.*'
@@ -125,6 +126,23 @@ func (ctx *unpackContext) reportUnusedNames(unusedNames []string) []error {
 	return ctx.errs
 }
 
+// When property a.b.c is not used, (also there is no a.* or a.b.* used)
+// "a", "a.b" and "a.b.c" are all in unusedNames.
+// removeUnnecessaryUnusedNames only keeps the last "a.b.c" as the real unused
+// name.
+func removeUnnecessaryUnusedNames(names []string) []string {
+	if len(names) == 0 {
+		return names
+	}
+	var simplifiedNames []string
+	for index, name := range names {
+		if index == len(names)-1 || !strings.HasPrefix(names[index+1], name) {
+			simplifiedNames = append(simplifiedNames, name)
+		}
+	}
+	return simplifiedNames
+}
+
 func (ctx *unpackContext) buildPropertyMap(prefix string, properties []*parser.Property) bool {
 	nOldErrors := len(ctx.errs)
 	for _, property := range properties {
diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go
index 7e2751d..35aabf9 100644
--- a/proptools/unpack_test.go
+++ b/proptools/unpack_test.go
@@ -17,7 +17,6 @@ package proptools
 import (
 	"bytes"
 	"reflect"
-
 	"testing"
 
 	"github.com/google/blueprint/parser"
@@ -962,6 +961,37 @@ func TestUnpackErrors(t *testing.T) {
 				`<input>:3:16: can't assign string value to list property "map_list"`,
 			},
 		},
+		{
+			name: "non-existent property",
+			input: `
+				m {
+					foo: {
+						foo_prop1: true,
+						foo_prop2: false,
+						foo_prop3: true,
+					},
+					bar: {
+						bar_prop: false,
+					},
+					baz: true,
+					exist: false,
+				}
+			`,
+			output: []interface{}{
+				&struct {
+					Foo struct {
+						Foo_prop1 bool
+					}
+					Exist bool
+				}{},
+			},
+			errors: []string{
+				`<input>:5:16: unrecognized property "foo.foo_prop2"`,
+				`<input>:6:16: unrecognized property "foo.foo_prop3"`,
+				`<input>:9:15: unrecognized property "bar.bar_prop"`,
+				`<input>:11:9: unrecognized property "baz"`,
+			},
+		},
 	}
 
 	for _, testCase := range testCases {
@@ -1196,3 +1226,58 @@ func BenchmarkUnpackProperties(b *testing.B) {
 		run(b, props, bp)
 	})
 }
+
+func TestRemoveUnnecessaryUnusedNames(t *testing.T) {
+	testCases := []struct {
+		name   string
+		input  []string
+		output []string
+	}{
+		{
+			name:   "no unused names",
+			input:  []string{},
+			output: []string{},
+		},
+		{
+			name:   "only one unused name",
+			input:  []string{"a.b.c"},
+			output: []string{"a.b.c"},
+		},
+		{
+			name:   "unused names in a chain",
+			input:  []string{"a", "a.b", "a.b.c"},
+			output: []string{"a.b.c"},
+		},
+		{
+			name:   "unused names unrelated",
+			input:  []string{"a.b.c", "s.t", "x.y"},
+			output: []string{"a.b.c", "s.t", "x.y"},
+		},
+		{
+			name:   "unused names partially related one",
+			input:  []string{"a.b", "a.b.c", "a.b.d"},
+			output: []string{"a.b.c", "a.b.d"},
+		},
+		{
+			name:   "unused names partially related two",
+			input:  []string{"a", "a.b.c", "a.c"},
+			output: []string{"a.b.c", "a.c"},
+		},
+		{
+			name:   "unused names partially related three",
+			input:  []string{"a.b.c", "b.c", "c"},
+			output: []string{"a.b.c", "b.c", "c"},
+		},
+	}
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			simplifiedNames := removeUnnecessaryUnusedNames(testCase.input)
+			if !reflect.DeepEqual(simplifiedNames, testCase.output) {
+				t.Errorf("test case: %s", testCase.name)
+				t.Errorf("  input: %s", testCase.input)
+				t.Errorf("  expect: %s", testCase.output)
+				t.Errorf("  got: %s", simplifiedNames)
+			}
+		})
+	}
+}