Add type hints to mk2rbc

Type hints have the format #RBC# type_hint MY_VAR list
and must be specified at the top of the Makefile. Setting
one will cause that variable to have that type for the
remainder of the Makefile. This can be used where mk2rbc's
type inference detects the wrong type and it must be
manually changed.

Bug: 224601891
Test: go test
Change-Id: I6db2c50056d0298227e1d2801a522adf8bbd1df8

mk2rbc/mk2rbc.go

@@ -410,6 +410,8 @@ type parseContext struct {
 	dependentModules map[string]*moduleInfo
 	soongNamespaces  map[string]map[string]bool
 	includeTops      []string
+	typeHints        map[string]starlarkType
+	atTopOfMakefile  bool
 }
 
 func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
@@ -453,6 +455,8 @@ func newParseContext(ss *StarlarkScript, nodes []mkparser.Node) *parseContext {
 		dependentModules: make(map[string]*moduleInfo),
 		soongNamespaces:  make(map[string]map[string]bool),
 		includeTops:      []string{},
+		typeHints:        make(map[string]starlarkType),
+		atTopOfMakefile:  true,
 	}
 	ctx.pushVarAssignments()
 	for _, item := range predefined {
@@ -1687,7 +1691,8 @@ func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) []starlarkNod
 	// Clear the includeTops after each non-comment statement
 	// so that include annotations placed on certain statements don't apply
 	// globally for the rest of the makefile was well.
-	if _, wasComment := node.(*mkparser.Comment); !wasComment && len(ctx.includeTops) > 0 {
+	if _, wasComment := node.(*mkparser.Comment); !wasComment {
+		ctx.atTopOfMakefile = false
 		ctx.includeTops = []string{}
 	}
 
@@ -1697,6 +1702,12 @@ func (ctx *parseContext) handleSimpleStatement(node mkparser.Node) []starlarkNod
 	return result
 }
 
+// The types allowed in a type_hint
+var typeHintMap = map[string]starlarkType{
+	"string": starlarkTypeString,
+	"list":   starlarkTypeList,
+}
+
 // Processes annotation. An annotation is a comment that starts with #RBC# and provides
 // a conversion hint -- say, where to look for the dynamically calculated inherit/include
 // paths. Returns true if the comment was a successfully-handled annotation.
@@ -1721,6 +1732,35 @@ func (ctx *parseContext) maybeHandleAnnotation(cnode *mkparser.Comment) (starlar
 		}
 		ctx.includeTops = append(ctx.includeTops, p)
 		return nil, true
+	} else if p, ok := maybeTrim(annotation, "type_hint"); ok {
+		// Type hints must come at the beginning the file, to avoid confusion
+		// if a type hint was specified later and thus only takes effect for half
+		// of the file.
+		if !ctx.atTopOfMakefile {
+			return ctx.newBadNode(cnode, "type_hint annotations must come before the first Makefile statement"), true
+		}
+
+		parts := strings.Fields(p)
+		if len(parts) <= 1 {
+			return ctx.newBadNode(cnode, "Invalid type_hint annotation: %s. Must be a variable type followed by a list of variables of that type", p), true
+		}
+
+		var varType starlarkType
+		if varType, ok = typeHintMap[parts[0]]; !ok {
+			varType = starlarkTypeUnknown
+		}
+		if varType == starlarkTypeUnknown {
+			return ctx.newBadNode(cnode, "Invalid type_hint annotation. Only list/string types are accepted, found %s", parts[0]), true
+		}
+
+		for _, name := range parts[1:] {
+			// Don't allow duplicate type hints
+			if _, ok := ctx.typeHints[name]; ok {
+				return ctx.newBadNode(cnode, "Duplicate type hint for variable %s", name), true
+			}
+			ctx.typeHints[name] = varType
+		}
+		return nil, true
 	}
 	return ctx.newBadNode(cnode, "unsupported annotation %s", cnode.Comment), true
 }

mk2rbc/mk2rbc_test.go

@@ -1367,6 +1367,53 @@ def init(g, handle):
     pass
   if int("100%s" % g.get("MY_VAR", "")) >= 10:
     pass
+`,
+	},
+	{
+		desc:   "Type hints",
+		mkname: "product.mk",
+		in: `
+# Test type hints
+#RBC# type_hint list MY_VAR MY_VAR_2
+# Unsupported type
+#RBC# type_hint bool MY_VAR_3
+# Invalid syntax
+#RBC# type_hint list
+# Duplicated variable
+#RBC# type_hint list MY_VAR_2
+#RBC# type_hint list my-local-var-with-dashes
+
+MY_VAR := foo
+MY_VAR_UNHINTED := foo
+
+# Vars set after other statements still get the hint
+MY_VAR_2 := foo
+
+# You can't specify a type hint after the first statement
+#RBC# type_hint list MY_VAR_4
+MY_VAR_4 := foo
+
+my-local-var-with-dashes := foo
+`,
+		expected: `# Test type hints
+# Unsupported type
+load("//build/make/core:product_config.rbc", "rblf")
+
+def init(g, handle):
+  cfg = rblf.cfg(handle)
+  rblf.mk2rbc_error("product.mk:5", "Invalid type_hint annotation. Only list/string types are accepted, found bool")
+  # Invalid syntax
+  rblf.mk2rbc_error("product.mk:7", "Invalid type_hint annotation: list. Must be a variable type followed by a list of variables of that type")
+  # Duplicated variable
+  rblf.mk2rbc_error("product.mk:9", "Duplicate type hint for variable MY_VAR_2")
+  g["MY_VAR"] = ["foo"]
+  g["MY_VAR_UNHINTED"] = "foo"
+  # Vars set after other statements still get the hint
+  g["MY_VAR_2"] = ["foo"]
+  # You can't specify a type hint after the first statement
+  rblf.mk2rbc_error("product.mk:19", "type_hint annotations must come before the first Makefile statement")
+  g["MY_VAR_4"] = "foo"
+  _my_local_var_with_dashes = ["foo"]
 `,
 	},
 }

mk2rbc/variable.go

@@ -291,6 +291,12 @@ var presetVariables = map[string]bool{
 // addVariable returns a variable with a given name. A variable is
 // added if it does not exist yet.
 func (ctx *parseContext) addVariable(name string) variable {
+	// Get the hintType before potentially changing the variable name
+	var hintType starlarkType
+	var ok bool
+	if hintType, ok = ctx.typeHints[name]; !ok {
+		hintType = starlarkTypeUnknown
+	}
 	// Heuristics: if variable's name is all lowercase, consider it local
 	// string variable.
 	isLocalVariable := name == strings.ToLower(name)
@@ -301,8 +307,8 @@ func (ctx *parseContext) addVariable(name string) variable {
 	}
 	v, found := ctx.variables[name]
 	if !found {
-		_, preset := presetVariables[name]
 		if vi, found := KnownVariables[name]; found {
+			_, preset := presetVariables[name]
 			switch vi.class {
 			case VarClassConfig:
 				v = &productConfigVariable{baseVariable{nam: name, typ: vi.valueType, preset: preset}}
@@ -310,10 +316,10 @@ func (ctx *parseContext) addVariable(name string) variable {
 				v = &otherGlobalVariable{baseVariable{nam: name, typ: vi.valueType, preset: preset}}
 			}
 		} else if isLocalVariable {
-			v = &localVariable{baseVariable{nam: name, typ: starlarkTypeUnknown}}
+			v = &localVariable{baseVariable{nam: name, typ: hintType}}
 		} else {
-			vt := starlarkTypeUnknown
+			vt := hintType
-			if strings.HasPrefix(name, "LOCAL_") {
+			if strings.HasPrefix(name, "LOCAL_") && vt == starlarkTypeUnknown {
 				// Heuristics: local variables that contribute to corresponding config variables
 				if cfgVarName, found := localProductConfigVariables[name]; found {
 					vi, found2 := KnownVariables[cfgVarName]