diff --git a/mk2rbc/mk2rbc.go b/mk2rbc/mk2rbc.go index d8b88b2cd..35d7d4de4 100644 --- a/mk2rbc/mk2rbc.go +++ b/mk2rbc/mk2rbc.go @@ -407,6 +407,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 { @@ -450,6 +452,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 { @@ -1680,7 +1684,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{} } @@ -1690,6 +1695,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. @@ -1714,6 +1725,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 } diff --git a/mk2rbc/mk2rbc_test.go b/mk2rbc/mk2rbc_test.go index 35c54d268..408466041 100644 --- a/mk2rbc/mk2rbc_test.go +++ b/mk2rbc/mk2rbc_test.go @@ -1404,6 +1404,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"] `, }, } diff --git a/mk2rbc/variable.go b/mk2rbc/variable.go index 3241a3854..506266a29 100644 --- a/mk2rbc/variable.go +++ b/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 - if strings.HasPrefix(name, "LOCAL_") { + vt := hintType + 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]