From 585cac22d9126fc094db3ba8963336d98836cee1 Mon Sep 17 00:00:00 2001
From: Liz Kammer <eakammer@google.com>
Date: Mon, 6 Jul 2020 09:12:57 -0700
Subject: [PATCH] Add array type property flags, equivalent to args

The property args is currently a string, but represents a collection of
arguments to metalava/doclava. This has resulted in variables being used
rather than default modules to share arguments.

Test: go test java_tests
Bug: 145644363
Change-Id: I670235e8f1cf19bfe8909049c4765637d0accd47
---
 java/droiddoc.go  | 73 ++++++++++++++++++++++++++++++-----------------
 java/java_test.go | 38 +++++++++++++++++++++++-
 2 files changed, 84 insertions(+), 27 deletions(-)

diff --git a/java/droiddoc.go b/java/droiddoc.go
index 99bfb6dc3..08865b666 100644
--- a/java/droiddoc.go
+++ b/java/droiddoc.go
@@ -112,13 +112,20 @@ type JavadocProperties struct {
 	// local files that are used within user customized droiddoc options.
 	Arg_files []string `android:"path"`
 
-	// user customized droiddoc args.
+	// user customized droiddoc args. Deprecated, use flags instead.
 	// Available variables for substitution:
 	//
 	//  $(location <label>): the path to the arg_files with name <label>
 	//  $$: a literal $
 	Args *string
 
+	// user customized droiddoc args. Not compatible with property args.
+	// Available variables for substitution:
+	//
+	//  $(location <label>): the path to the arg_files with name <label>
+	//  $$: a literal $
+	Flags []string
+
 	// names of the output files used in args that will be generated
 	Out []string
 
@@ -382,7 +389,7 @@ type Javadoc struct {
 	argFiles    android.Paths
 	implicits   android.Paths
 
-	args string
+	args []string
 
 	docZip      android.WritablePath
 	stubsSrcJar android.WritablePath
@@ -619,8 +626,8 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps {
 	}
 	srcFiles = filterHtml(srcFiles)
 
-	flags := j.collectAidlFlags(ctx, deps)
-	srcFiles = j.genSources(ctx, srcFiles, flags)
+	aidlFlags := j.collectAidlFlags(ctx, deps)
+	srcFiles = j.genSources(ctx, srcFiles, aidlFlags)
 
 	// srcs may depend on some genrule output.
 	j.srcJars = srcFiles.FilterByExt(".srcjar")
@@ -649,24 +656,38 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps {
 		}
 	}
 
-	var err error
-	j.args, err = android.Expand(String(j.properties.Args), func(name string) (string, error) {
-		if strings.HasPrefix(name, "location ") {
-			label := strings.TrimSpace(strings.TrimPrefix(name, "location "))
-			if paths, ok := argFilesMap[label]; ok {
-				return paths, nil
-			} else {
-				return "", fmt.Errorf("unknown location label %q, expecting one of %q",
-					label, strings.Join(argFileLabels, ", "))
-			}
-		} else if name == "genDir" {
-			return android.PathForModuleGen(ctx).String(), nil
-		}
-		return "", fmt.Errorf("unknown variable '$(%s)'", name)
-	})
+	var argsPropertyName string
+	flags := make([]string, 0)
+	if j.properties.Args != nil && j.properties.Flags != nil {
+		ctx.PropertyErrorf("args", "flags is set. Cannot set args")
+	} else if args := proptools.String(j.properties.Args); args != "" {
+		flags = append(flags, args)
+		argsPropertyName = "args"
+	} else {
+		flags = append(flags, j.properties.Flags...)
+		argsPropertyName = "flags"
+	}
 
-	if err != nil {
-		ctx.PropertyErrorf("args", "%s", err.Error())
+	for _, flag := range flags {
+		args, err := android.Expand(flag, func(name string) (string, error) {
+			if strings.HasPrefix(name, "location ") {
+				label := strings.TrimSpace(strings.TrimPrefix(name, "location "))
+				if paths, ok := argFilesMap[label]; ok {
+					return paths, nil
+				} else {
+					return "", fmt.Errorf("unknown location label %q, expecting one of %q",
+						label, strings.Join(argFileLabels, ", "))
+				}
+			} else if name == "genDir" {
+				return android.PathForModuleGen(ctx).String(), nil
+			}
+			return "", fmt.Errorf("unknown variable '$(%s)'", name)
+		})
+
+		if err != nil {
+			ctx.PropertyErrorf(argsPropertyName, "%s", err.Error())
+		}
+		j.args = append(j.args, args)
 	}
 
 	return deps
@@ -1010,7 +1031,7 @@ func (d *Droiddoc) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 
 	d.stubsFlags(ctx, cmd, stubsDir)
 
-	cmd.Flag(d.Javadoc.args).Implicits(d.Javadoc.argFiles)
+	cmd.Flag(strings.Join(d.Javadoc.args, " ")).Implicits(d.Javadoc.argFiles)
 
 	if d.properties.Compat_config != nil {
 		compatConfig := android.PathForModuleSrc(ctx, String(d.properties.Compat_config))
@@ -1327,7 +1348,7 @@ func (d *Droidstubs) annotationsFlags(ctx android.ModuleContext, cmd *android.Ru
 		cmd.Flag("--include-annotations")
 
 		validatingNullability :=
-			strings.Contains(d.Javadoc.args, "--validate-nullability-from-merged-stubs") ||
+			android.InList("--validate-nullability-from-merged-stubs", d.Javadoc.args) ||
 				String(d.properties.Validate_nullability_from_list) != ""
 
 		migratingNullability := String(d.properties.Previous_api) != ""
@@ -1539,14 +1560,14 @@ func (d *Droidstubs) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	d.apiLevelsAnnotationsFlags(ctx, cmd)
 	d.apiToXmlFlags(ctx, cmd)
 
-	if strings.Contains(d.Javadoc.args, "--generate-documentation") {
+	if android.InList("--generate-documentation", d.Javadoc.args) {
 		// Currently Metalava have the ability to invoke Javadoc in a seperate process.
 		// Pass "-nodocs" to suppress the Javadoc invocation when Metalava receives
 		// "--generate-documentation" arg. This is not needed when Metalava removes this feature.
-		d.Javadoc.args = d.Javadoc.args + " -nodocs "
+		d.Javadoc.args = append(d.Javadoc.args, "-nodocs")
 	}
 
-	cmd.Flag(d.Javadoc.args).Implicits(d.Javadoc.argFiles)
+	cmd.Flag(strings.Join(d.Javadoc.args, " ")).Implicits(d.Javadoc.argFiles)
 	for _, o := range d.Javadoc.properties.Out {
 		cmd.ImplicitOutput(android.PathForModuleGen(ctx, o))
 	}
diff --git a/java/java_test.go b/java/java_test.go
index fb003619f..db3f18740 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -1097,7 +1097,7 @@ func TestDroiddoc(t *testing.T) {
 		    ],
 		    proofread_file: "libcore-proofread.txt",
 		    todo_file: "libcore-docs-todo.html",
-		    args: "-offlinemode -title \"libcore\"",
+		    flags: ["-offlinemode -title \"libcore\""],
 		}
 		`,
 		map[string][]byte{
@@ -1124,6 +1124,42 @@ func TestDroiddoc(t *testing.T) {
 	}
 }
 
+func TestDroiddocArgsAndFlagsCausesError(t *testing.T) {
+	testJavaError(t, "flags is set. Cannot set args", `
+		droiddoc_exported_dir {
+		    name: "droiddoc-templates-sdk",
+		    path: ".",
+		}
+		filegroup {
+		    name: "bar-doc-aidl-srcs",
+		    srcs: ["bar-doc/IBar.aidl"],
+		    path: "bar-doc",
+		}
+		droiddoc {
+		    name: "bar-doc",
+		    srcs: [
+		        "bar-doc/a.java",
+		        "bar-doc/IFoo.aidl",
+		        ":bar-doc-aidl-srcs",
+		    ],
+		    exclude_srcs: [
+		        "bar-doc/b.java"
+		    ],
+		    custom_template: "droiddoc-templates-sdk",
+		    hdf: [
+		        "android.whichdoc offline",
+		    ],
+		    knowntags: [
+		        "bar-doc/known_oj_tags.txt",
+		    ],
+		    proofread_file: "libcore-proofread.txt",
+		    todo_file: "libcore-docs-todo.html",
+		    flags: ["-offlinemode -title \"libcore\""],
+		    args: "-offlinemode -title \"libcore\"",
+		}
+		`)
+}
+
 func TestDroidstubsWithSystemModules(t *testing.T) {
 	ctx, _ := testJava(t, `
 		droidstubs {