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

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 {
+	checkDistProperties(ctx, "dist", &m.distProperties.Dist)
-		_, err := validateSafePath(*m.distProperties.Dist.Dest)
+	for i, _ := range m.distProperties.Dists {
-		if err != nil {
+		checkDistProperties(ctx, fmt.Sprintf("dists[%d]", i), &m.distProperties.Dists[i])
-			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.")
-		}
 	}
 
 	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
 

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)
+}