Expand dist property checks to cover dists

Previously, only the dist property's nested properties were checked for
correctness. This change also checks the dists property's nested dist
structures and adds some tests to verify that the checks are run and
correctly report the location of the incorrect property even when it is
within a slice of dist structs.

Test: m nothing
Bug: 174226317
Change-Id: If5a19360e1e4c98ee3b5afc813e35349d1fc6f6f

2 files changed, 77 insertions, 16 deletions

diff --git a/android/module.go b/android/module.go
index bd60829f1..0716b5dac 100644
--- a/android/module.go
+++ b/android/module.go
@@ -1579,22 +1579,9 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext)
 	ctx.Variable(pctx, "moduleDescSuffix", s)
 
 	// Some common property checks for properties that will be used later in androidmk.go
-	if m.distProperties.Dist.Dest != nil {
-		_, err := validateSafePath(*m.distProperties.Dist.Dest)
-		if err != nil {
-			ctx.PropertyErrorf("dist.dest", "%s", err.Error())
-		}
-	}
-	if m.distProperties.Dist.Dir != nil {
-		_, err := validateSafePath(*m.distProperties.Dist.Dir)
-		if err != nil {
-			ctx.PropertyErrorf("dist.dir", "%s", err.Error())
-		}
-	}
-	if m.distProperties.Dist.Suffix != nil {
-		if strings.Contains(*m.distProperties.Dist.Suffix, "/") {
-			ctx.PropertyErrorf("dist.suffix", "Suffix may not contain a '/' character.")
-		}
+	checkDistProperties(ctx, "dist", &m.distProperties.Dist)
+	for i, _ := range m.distProperties.Dists {
+		checkDistProperties(ctx, fmt.Sprintf("dists[%d]", i), &m.distProperties.Dists[i])
 	}
 
 	if m.Enabled() {
@@ -1659,6 +1646,32 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext)
 	m.variables = ctx.variables
 }
 
+// Check the supplied dist structure to make sure that it is valid.
+//
+// property - the base property, e.g. dist or dists[1], which is combined with the
+// name of the nested property to produce the full property, e.g. dist.dest or
+// dists[1].dir.
+func checkDistProperties(ctx *moduleContext, property string, dist *Dist) {
+	if dist.Dest != nil {
+		_, err := validateSafePath(*dist.Dest)
+		if err != nil {
+			ctx.PropertyErrorf(property+".dest", "%s", err.Error())
+		}
+	}
+	if dist.Dir != nil {
+		_, err := validateSafePath(*dist.Dir)
+		if err != nil {
+			ctx.PropertyErrorf(property+".dir", "%s", err.Error())
+		}
+	}
+	if dist.Suffix != nil {
+		if strings.Contains(*dist.Suffix, "/") {
+			ctx.PropertyErrorf(property+".suffix", "Suffix may not contain a '/' character.")
+		}
+	}
+
+}
+
 type earlyModuleContext struct {
 	blueprint.EarlyModuleContext
 
diff --git a/android/module_test.go b/android/module_test.go
index 6cc18132a..e3cc61332 100644
--- a/android/module_test.go
+++ b/android/module_test.go
@@ -232,3 +232,51 @@ func TestValidateIncorrectBuildParams(t *testing.T) {
 		t.Errorf("Expected build params to fail validation: %+v", bparams)
 	}
 }
+
+func TestDistErrorChecking(t *testing.T) {
+	bp := `
+		deps {
+			name: "foo",
+      dist: {
+        dest: "../invalid-dest",
+        dir: "../invalid-dir",
+        suffix: "invalid/suffix",
+      },
+      dists: [
+        {
+          dest: "../invalid-dest0",
+          dir: "../invalid-dir0",
+          suffix: "invalid/suffix0",
+        },
+        {
+          dest: "../invalid-dest1",
+          dir: "../invalid-dir1",
+          suffix: "invalid/suffix1",
+        },
+      ],
+ 		}
+	`
+
+	config := TestConfig(buildDir, nil, bp, nil)
+
+	ctx := NewTestContext(config)
+	ctx.RegisterModuleType("deps", depsModuleFactory)
+	ctx.Register()
+
+	_, errs := ctx.ParseFileList(".", []string{"Android.bp"})
+	FailIfErrored(t, errs)
+	_, errs = ctx.PrepareBuildActions(config)
+
+	expectedErrs := []string{
+		"\\QAndroid.bp:5:13: module \"foo\": dist.dest: Path is outside directory: ../invalid-dest\\E",
+		"\\QAndroid.bp:6:12: module \"foo\": dist.dir: Path is outside directory: ../invalid-dir\\E",
+		"\\QAndroid.bp:7:15: module \"foo\": dist.suffix: Suffix may not contain a '/' character.\\E",
+		"\\QAndroid.bp:11:15: module \"foo\": dists[0].dest: Path is outside directory: ../invalid-dest0\\E",
+		"\\QAndroid.bp:12:14: module \"foo\": dists[0].dir: Path is outside directory: ../invalid-dir0\\E",
+		"\\QAndroid.bp:13:17: module \"foo\": dists[0].suffix: Suffix may not contain a '/' character.\\E",
+		"\\QAndroid.bp:16:15: module \"foo\": dists[1].dest: Path is outside directory: ../invalid-dest1\\E",
+		"\\QAndroid.bp:17:14: module \"foo\": dists[1].dir: Path is outside directory: ../invalid-dir1\\E",
+		"\\QAndroid.bp:18:17: module \"foo\": dists[1].suffix: Suffix may not contain a '/' character.\\E",
+	}
+	CheckErrorsAgainstExpectations(t, errs, expectedErrs)
+}