Merge remote-tracking branch 'aosp/upstream' into uppercase_properties am: f0e6d47072 am: 99ea787237 am: fd33f0d54d

Change-Id: I7c844439f525ee8f59114140cf4d35885a7a5d99

context.go

@@ -96,15 +96,15 @@ type Context struct {
 	// set during PrepareBuildActions
 	pkgNames        map[*packageContext]string
 	liveGlobals     *liveTracker
-	globalVariables map[Variable]*ninjaString
+	globalVariables map[Variable]ninjaString
 	globalPools     map[Pool]*poolDef
 	globalRules     map[Rule]*ruleDef
 
 	// set during PrepareBuildActions
-	ninjaBuildDir      *ninjaString // The builddir special Ninja variable
+	ninjaBuildDir      ninjaString // The builddir special Ninja variable
-	requiredNinjaMajor int          // For the ninja_required_version variable
+	requiredNinjaMajor int         // For the ninja_required_version variable
-	requiredNinjaMinor int          // For the ninja_required_version variable
+	requiredNinjaMinor int         // For the ninja_required_version variable
-	requiredNinjaMicro int          // For the ninja_required_version variable
+	requiredNinjaMicro int         // For the ninja_required_version variable
 
 	subninjas []string
 
@@ -2788,7 +2788,7 @@ func (c *Context) requireNinjaVersion(major, minor, micro int) {
 	}
 }
 
-func (c *Context) setNinjaBuildDir(value *ninjaString) {
+func (c *Context) setNinjaBuildDir(value ninjaString) {
 	if c.ninjaBuildDir == nil {
 		c.ninjaBuildDir = value
 	}
@@ -2854,7 +2854,7 @@ func (c *Context) makeUniquePackageNames(
 }
 
 func (c *Context) checkForVariableReferenceCycles(
-	variables map[Variable]*ninjaString, pkgNames map[*packageContext]string) {
+	variables map[Variable]ninjaString, pkgNames map[*packageContext]string) {
 
 	visited := make(map[Variable]bool)  // variables that were already checked
 	checking := make(map[Variable]bool) // variables actively being checked
@@ -2867,7 +2867,7 @@ func (c *Context) checkForVariableReferenceCycles(
 		defer delete(checking, v)
 
 		value := variables[v]
-		for _, dep := range value.variables {
+		for _, dep := range value.Variables() {
 			if checking[dep] {
 				// This is a cycle.
 				return []Variable{dep, v}
@@ -3352,7 +3352,7 @@ func (c *Context) writeGlobalVariables(nw *ninjaWriter) error {
 
 		// First visit variables on which this variable depends.
 		value := c.globalVariables[v]
-		for _, dep := range value.variables {
+		for _, dep := range value.Variables() {
 			if !visited[dep] {
 				err := walk(dep)
 				if err != nil {

live_tracker.go

@@ -24,7 +24,7 @@ type liveTracker struct {
 	sync.Mutex
 	config interface{} // Used to evaluate variable, rule, and pool values.
 
-	variables map[Variable]*ninjaString
+	variables map[Variable]ninjaString
 	pools     map[Pool]*poolDef
 	rules     map[Rule]*ruleDef
 }
@@ -32,7 +32,7 @@ type liveTracker struct {
 func newLiveTracker(config interface{}) *liveTracker {
 	return &liveTracker{
 		config:    config,
-		variables: make(map[Variable]*ninjaString),
+		variables: make(map[Variable]ninjaString),
 		pools:     make(map[Pool]*poolDef),
 		rules:     make(map[Rule]*ruleDef),
 	}
@@ -170,7 +170,7 @@ func (l *liveTracker) addVariable(v Variable) error {
 	return nil
 }
 
-func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error {
+func (l *liveTracker) addNinjaStringListDeps(list []ninjaString) error {
 	for _, str := range list {
 		err := l.addNinjaStringDeps(str)
 		if err != nil {
@@ -180,8 +180,8 @@ func (l *liveTracker) addNinjaStringListDeps(list []*ninjaString) error {
 	return nil
 }
 
-func (l *liveTracker) addNinjaStringDeps(str *ninjaString) error {
+func (l *liveTracker) addNinjaStringDeps(str ninjaString) error {
-	for _, v := range str.variables {
+	for _, v := range str.Variables() {
 		err := l.addVariable(v)
 		if err != nil {
 			return err

ninja_defs.go

@@ -128,11 +128,11 @@ func (p *poolDef) WriteTo(nw *ninjaWriter, name string) error {
 // A ruleDef describes a rule definition.  It does not include the name of the
 // rule.
 type ruleDef struct {
-	CommandDeps      []*ninjaString
+	CommandDeps      []ninjaString
-	CommandOrderOnly []*ninjaString
+	CommandOrderOnly []ninjaString
 	Comment          string
 	Pool             Pool
-	Variables        map[string]*ninjaString
+	Variables        map[string]ninjaString
 }
 
 func parseRuleParams(scope scope, params *RuleParams) (*ruleDef,
@@ -141,7 +141,7 @@ func parseRuleParams(scope scope, params *RuleParams) (*ruleDef,
 	r := &ruleDef{
 		Comment:   params.Comment,
 		Pool:      params.Pool,
-		Variables: make(map[string]*ninjaString),
+		Variables: make(map[string]ninjaString),
 	}
 
 	if params.Command == "" {
@@ -252,13 +252,13 @@ type buildDef struct {
 	Comment         string
 	Rule            Rule
 	RuleDef         *ruleDef
-	Outputs         []*ninjaString
+	Outputs         []ninjaString
-	ImplicitOutputs []*ninjaString
+	ImplicitOutputs []ninjaString
-	Inputs          []*ninjaString
+	Inputs          []ninjaString
-	Implicits       []*ninjaString
+	Implicits       []ninjaString
-	OrderOnly       []*ninjaString
+	OrderOnly       []ninjaString
-	Args            map[Variable]*ninjaString
+	Args            map[Variable]ninjaString
-	Variables       map[string]*ninjaString
+	Variables       map[string]ninjaString
 	Optional        bool
 }
 
@@ -273,9 +273,9 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef,
 		Rule:    rule,
 	}
 
-	setVariable := func(name string, value *ninjaString) {
+	setVariable := func(name string, value ninjaString) {
 		if b.Variables == nil {
-			b.Variables = make(map[string]*ninjaString)
+			b.Variables = make(map[string]ninjaString)
 		}
 		b.Variables[name] = value
 	}
@@ -339,7 +339,7 @@ func parseBuildParams(scope scope, params *BuildParams) (*buildDef,
 	argNameScope := rule.scope()
 
 	if len(params.Args) > 0 {
-		b.Args = make(map[Variable]*ninjaString)
+		b.Args = make(map[Variable]ninjaString)
 		for name, value := range params.Args {
 			if !rule.isArg(name) {
 				return nil, fmt.Errorf("unknown argument %q", name)
@@ -419,7 +419,7 @@ func (b *buildDef) WriteTo(nw *ninjaWriter, pkgNames map[*packageContext]string)
 	return nw.BlankLine()
 }
 
-func valueList(list []*ninjaString, pkgNames map[*packageContext]string,
+func valueList(list []ninjaString, pkgNames map[*packageContext]string,
 	escaper *strings.Replacer) []string {
 
 	result := make([]string, len(list))
@@ -429,7 +429,7 @@ func valueList(list []*ninjaString, pkgNames map[*packageContext]string,
 	return result
 }
 
-func writeVariables(nw *ninjaWriter, variables map[string]*ninjaString,
+func writeVariables(nw *ninjaWriter, variables map[string]ninjaString,
 	pkgNames map[*packageContext]string) error {
 	var keys []string
 	for k := range variables {

ninja_strings.go

@@ -34,21 +34,28 @@ var (
 		":", "$:")
 )
 
-type ninjaString struct {
+type ninjaString interface {
+	Value(pkgNames map[*packageContext]string) string
+	ValueWithEscaper(pkgNames map[*packageContext]string, escaper *strings.Replacer) string
+	Eval(variables map[Variable]ninjaString) (string, error)
+	Variables() []Variable
+}
+
+type varNinjaString struct {
 	strings   []string
 	variables []Variable
 }
 
+type literalNinjaString string
+
 type scope interface {
 	LookupVariable(name string) (Variable, error)
 	IsRuleVisible(rule Rule) bool
 	IsPoolVisible(pool Pool) bool
 }
 
-func simpleNinjaString(str string) *ninjaString {
+func simpleNinjaString(str string) ninjaString {
-	return &ninjaString{
+	return literalNinjaString(str)
-		strings: []string{str},
-	}
 }
 
 type parseState struct {
@@ -57,7 +64,7 @@ type parseState struct {
 	pendingStr  string
 	stringStart int
 	varStart    int
-	result      *ninjaString
+	result      *varNinjaString
 }
 
 func (ps *parseState) pushVariable(v Variable) {
@@ -84,10 +91,16 @@ type stateFunc func(*parseState, int, rune) (stateFunc, error)
 // parseNinjaString parses an unescaped ninja string (i.e. all $<something>
 // occurrences are expected to be variables or $$) and returns a list of the
 // variable names that the string references.
-func parseNinjaString(scope scope, str string) (*ninjaString, error) {
+func parseNinjaString(scope scope, str string) (ninjaString, error) {
 	// naively pre-allocate slices by counting $ signs
 	n := strings.Count(str, "$")
-	result := &ninjaString{
+	if n == 0 {
+		if strings.HasPrefix(str, " ") {
+			str = "$" + str
+		}
+		return literalNinjaString(str), nil
+	}
+	result := &varNinjaString{
 		strings:   make([]string, 0, n+1),
 		variables: make([]Variable, 0, n),
 	}
@@ -253,13 +266,13 @@ func parseBracketsState(state *parseState, i int, r rune) (stateFunc, error) {
 	}
 }
 
-func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
+func parseNinjaStrings(scope scope, strs []string) ([]ninjaString,
 	error) {
 
 	if len(strs) == 0 {
 		return nil, nil
 	}
-	result := make([]*ninjaString, len(strs))
+	result := make([]ninjaString, len(strs))
 	for i, str := range strs {
 		ninjaStr, err := parseNinjaString(scope, str)
 		if err != nil {
@@ -270,11 +283,11 @@ func parseNinjaStrings(scope scope, strs []string) ([]*ninjaString,
 	return result, nil
 }
 
-func (n *ninjaString) Value(pkgNames map[*packageContext]string) string {
+func (n varNinjaString) Value(pkgNames map[*packageContext]string) string {
 	return n.ValueWithEscaper(pkgNames, defaultEscaper)
 }
 
-func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
+func (n varNinjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
 	escaper *strings.Replacer) string {
 
 	if len(n.strings) == 1 {
@@ -293,7 +306,7 @@ func (n *ninjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
 	return str.String()
 }
 
-func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error) {
+func (n varNinjaString) Eval(variables map[Variable]ninjaString) (string, error) {
 	str := n.strings[0]
 	for i, v := range n.variables {
 		variable, ok := variables[v]
@@ -309,6 +322,27 @@ func (n *ninjaString) Eval(variables map[Variable]*ninjaString) (string, error)
 	return str, nil
 }
 
+func (n varNinjaString) Variables() []Variable {
+	return n.variables
+}
+
+func (l literalNinjaString) Value(pkgNames map[*packageContext]string) string {
+	return l.ValueWithEscaper(pkgNames, defaultEscaper)
+}
+
+func (l literalNinjaString) ValueWithEscaper(pkgNames map[*packageContext]string,
+	escaper *strings.Replacer) string {
+	return escaper.Replace(string(l))
+}
+
+func (l literalNinjaString) Eval(variables map[Variable]ninjaString) (string, error) {
+	return string(l), nil
+}
+
+func (l literalNinjaString) Variables() []Variable {
+	return nil
+}
+
 func validateNinjaName(name string) error {
 	for i, r := range name {
 		valid := (r >= 'a' && r <= 'z') ||

ninja_strings_test.go

@@ -22,10 +22,11 @@ import (
 )
 
 var ninjaParseTestCases = []struct {
-	input string
+	input   string
-	vars  []string
+	vars    []string
-	strs  []string
+	strs    []string
-	err   string
+	literal bool
+	err     string
 }{
 	{
 		input: "abc def $ghi jkl",
@@ -56,6 +57,7 @@ var ninjaParseTestCases = []struct {
 		input: "foo $$ bar",
 		vars:  nil,
 		strs:  []string{"foo $$ bar"},
+		// this is technically a literal, but not recognized as such due to the $$
 	},
 	{
 		input: "$foo${bar}",
@@ -68,16 +70,22 @@ var ninjaParseTestCases = []struct {
 		strs:  []string{"", "$$"},
 	},
 	{
-		input: "foo bar",
+		input:   "foo bar",
-		vars:  nil,
+		vars:    nil,
-		strs:  []string{"foo bar"},
+		strs:    []string{"foo bar"},
+		literal: true,
 	},
 	{
-		input: " foo ",
+		input:   " foo ",
-		vars:  nil,
+		vars:    nil,
-		strs:  []string{"$ foo "},
+		strs:    []string{"$ foo "},
+		literal: true,
 	},
 	{
+		input: " $foo ",
+		vars:  []string{"foo"},
+		strs:  []string{"$ ", " "},
+	}, {
 		input: "foo $ bar",
 		err:   "invalid character after '$' at byte offset 5",
 	},
@@ -114,19 +122,25 @@ func TestParseNinjaString(t *testing.T) {
 			expectedVars = append(expectedVars, v)
 		}
 
+		var expected ninjaString
+		if len(testCase.strs) > 0 {
+			if testCase.literal {
+				expected = literalNinjaString(testCase.strs[0])
+			} else {
+				expected = &varNinjaString{
+					strings:   testCase.strs,
+					variables: expectedVars,
+				}
+			}
+		}
+
 		output, err := parseNinjaString(scope, testCase.input)
 		if err == nil {
-			if !reflect.DeepEqual(output.variables, expectedVars) {
+			if !reflect.DeepEqual(output, expected) {
-				t.Errorf("incorrect variable list:")
+				t.Errorf("incorrect ninja string:")
 				t.Errorf("     input: %q", testCase.input)
-				t.Errorf("  expected: %#v", expectedVars)
+				t.Errorf("  expected: %#v", expected)
-				t.Errorf("       got: %#v", output.variables)
+				t.Errorf("       got: %#v", output)
-			}
-			if !reflect.DeepEqual(output.strings, testCase.strs) {
-				t.Errorf("incorrect string list:")
-				t.Errorf("     input: %q", testCase.input)
-				t.Errorf("  expected: %#v", testCase.strs)
-				t.Errorf("       got: %#v", output.strings)
 			}
 		}
 		var errStr string
@@ -156,7 +170,7 @@ func TestParseNinjaStringWithImportedVar(t *testing.T) {
 	}
 
 	expect := []Variable{ImpVar}
-	if !reflect.DeepEqual(output.variables, expect) {
+	if !reflect.DeepEqual(output.(*varNinjaString).variables, expect) {
 		t.Errorf("incorrect output:")
 		t.Errorf("     input: %q", input)
 		t.Errorf("  expected: %#v", expect)

package_ctx.go

@@ -292,7 +292,7 @@ func (v *staticVariable) fullName(pkgNames map[*packageContext]string) string {
 	return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_
 }
 
-func (v *staticVariable) value(interface{}) (*ninjaString, error) {
+func (v *staticVariable) value(interface{}) (ninjaString, error) {
 	ninjaStr, err := parseNinjaString(v.pctx.scope, v.value_)
 	if err != nil {
 		err = fmt.Errorf("error parsing variable %s value: %s", v, err)
@@ -392,7 +392,7 @@ func (v *variableFunc) fullName(pkgNames map[*packageContext]string) string {
 	return packageNamespacePrefix(pkgNames[v.pctx]) + v.name_
 }
 
-func (v *variableFunc) value(config interface{}) (*ninjaString, error) {
+func (v *variableFunc) value(config interface{}) (ninjaString, error) {
 	value, err := v.value_(config)
 	if err != nil {
 		return nil, err
@@ -452,7 +452,7 @@ func (v *argVariable) fullName(pkgNames map[*packageContext]string) string {
 	return v.name_
 }
 
-func (v *argVariable) value(config interface{}) (*ninjaString, error) {
+func (v *argVariable) value(config interface{}) (ninjaString, error) {
 	return nil, errVariableIsArg
 }
 

proptools/proptools.go

@@ -16,19 +16,34 @@ package proptools
 
 import (
 	"reflect"
+	"strings"
 	"unicode"
 	"unicode/utf8"
 )
 
+// PropertyNameForField converts the name of a field in property struct to the property name that
+// might appear in a Blueprints file.  Since the property struct fields must always be exported
+// to be accessed with reflection and the canonical Blueprints style is lowercased names, it
+// lower cases the first rune in the field name unless the field name contains an uppercase rune
+// after the first rune (which is always uppercase), and no lowercase runes.
 func PropertyNameForField(fieldName string) string {
 	r, size := utf8.DecodeRuneInString(fieldName)
 	propertyName := string(unicode.ToLower(r))
+	if size == len(fieldName) {
+		return propertyName
+	}
+	if strings.IndexFunc(fieldName[size:], unicode.IsLower) == -1 &&
+		strings.IndexFunc(fieldName[size:], unicode.IsUpper) != -1 {
+		return fieldName
+	}
 	if len(fieldName) > size {
 		propertyName += fieldName[size:]
 	}
 	return propertyName
 }
 
+// FieldNameForProperty converts the name of a property that might appear in a Blueprints file to
+// the name of a field in property struct by uppercasing the first rune.
 func FieldNameForProperty(propertyName string) string {
 	r, size := utf8.DecodeRuneInString(propertyName)
 	fieldName := string(unicode.ToUpper(r))

proptools/proptools_test.go

@@ -0,0 +1,114 @@
+// Copyright 2020 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package proptools
+
+import "testing"
+
+func TestPropertyNameForField(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "short",
+			input: "S",
+			want:  "s",
+		},
+		{
+			name:  "long",
+			input: "String",
+			want:  "string",
+		},
+		{
+			name:  "uppercase",
+			input: "STRING",
+			want:  "STRING",
+		},
+		{
+			name:  "mixed",
+			input: "StRiNg",
+			want:  "stRiNg",
+		},
+		{
+			name:  "underscore",
+			input: "Under_score",
+			want:  "under_score",
+		},
+		{
+			name:  "uppercase underscore",
+			input: "UNDER_SCORE",
+			want:  "UNDER_SCORE",
+		},
+		{
+			name:  "x86",
+			input: "X86",
+			want:  "x86",
+		},
+		{
+			name:  "x86_64",
+			input: "X86_64",
+			want:  "x86_64",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := PropertyNameForField(tt.input); got != tt.want {
+				t.Errorf("PropertyNameForField(%v) = %v, want %v", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestFieldNameForProperty(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "short lowercase",
+			input: "s",
+			want:  "S",
+		},
+		{
+			name:  "short uppercase",
+			input: "S",
+			want:  "S",
+		},
+		{
+			name:  "long lowercase",
+			input: "string",
+			want:  "String",
+		},
+		{
+			name:  "long uppercase",
+			input: "STRING",
+			want:  "STRING",
+		},
+		{
+			name:  "mixed",
+			input: "StRiNg",
+			want:  "StRiNg",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := FieldNameForProperty(tt.input); got != tt.want {
+				t.Errorf("FieldNameForProperty(%v) = %v, want %v", tt.input, got, tt.want)
+			}
+		})
+	}
+}

proptools/unpack_test.go

@@ -508,6 +508,21 @@ var validUnpackTestCases = []struct {
 			},
 		},
 	},
+	// Captitalized property
+	{
+		input: `
+			m {
+				CAPITALIZED: "foo",
+			}
+		`,
+		output: []interface{}{
+			&struct {
+				CAPITALIZED string
+			}{
+				CAPITALIZED: "foo",
+			},
+		},
+	},
 }
 
 func TestUnpackProperties(t *testing.T) {

scope.go

@@ -28,7 +28,7 @@ type Variable interface {
 	packageContext() *packageContext
 	name() string                                        // "foo"
 	fullName(pkgNames map[*packageContext]string) string // "pkg.foo" or "path.to.pkg.foo"
-	value(config interface{}) (*ninjaString, error)
+	value(config interface{}) (ninjaString, error)
 	String() string
 }
 
@@ -351,7 +351,7 @@ func (s *localScope) AddLocalRule(name string, params *RuleParams,
 type localVariable struct {
 	namePrefix string
 	name_      string
-	value_     *ninjaString
+	value_     ninjaString
 }
 
 func (l *localVariable) packageContext() *packageContext {
@@ -366,7 +366,7 @@ func (l *localVariable) fullName(pkgNames map[*packageContext]string) string {
 	return l.namePrefix + l.name_
 }
 
-func (l *localVariable) value(interface{}) (*ninjaString, error) {
+func (l *localVariable) value(interface{}) (ninjaString, error) {
 	return l.value_, nil
 }