From ba48366969a55dc2a28ce278a01d6d8ecef4adc2 Mon Sep 17 00:00:00 2001 From: Cole Faust Date: Mon, 17 Jun 2024 15:03:33 -0700 Subject: [PATCH] Update selects_test for selects with deferred expressions Bug: 323382414 Test: m nothing --no-skip-soong-tests Change-Id: I18257ff586fb31d4e0b012a249726a925832cdac --- android/Android.bp | 1 + android/blueprint_e2e_test.go | 105 +++++++++++++++++++++++++++++++ android/selects_test.go | 59 ++++++++++++----- androidmk/androidmk/android.go | 3 - androidmk/androidmk/androidmk.go | 2 - androidmk/androidmk/values.go | 6 +- bpfix/bpfix/bpfix.go | 2 +- bpfix/bpfix/bpfix_test.go | 4 +- bpfix/cmd_lib/bpfix.go | 2 +- 9 files changed, 157 insertions(+), 27 deletions(-) create mode 100644 android/blueprint_e2e_test.go diff --git a/android/Android.bp b/android/Android.bp index 985ffa9f1..3c38148b6 100644 --- a/android/Android.bp +++ b/android/Android.bp @@ -113,6 +113,7 @@ bootstrap_go_package { "androidmk_test.go", "apex_test.go", "arch_test.go", + "blueprint_e2e_test.go", "config_test.go", "configured_jars_test.go", "csuite_config_test.go", diff --git a/android/blueprint_e2e_test.go b/android/blueprint_e2e_test.go new file mode 100644 index 000000000..b27451218 --- /dev/null +++ b/android/blueprint_e2e_test.go @@ -0,0 +1,105 @@ +// Copyright 2024 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 android + +import ( + "testing" +) + +var testCases []struct { + name string + fs MockFS + expectedError string +} = []struct { + name string + fs MockFS + expectedError string +}{ + { + name: "Can't reference variable before assignment", + fs: map[string][]byte{ + "Android.bp": []byte(` +x = foo +foo = "hello" +`), + }, + expectedError: "undefined variable foo", + }, + { + name: "Can't append to variable before assigned to", + fs: map[string][]byte{ + "Android.bp": []byte(` +foo += "world" +foo = "hello" +`), + }, + expectedError: "modified non-existent variable \"foo\" with \\+=", + }, + { + name: "Can't reassign variable", + fs: map[string][]byte{ + "Android.bp": []byte(` +foo = "hello" +foo = "world" +`), + }, + expectedError: "variable already set, previous assignment:", + }, + { + name: "Can't reassign variable in inherited scope", + fs: map[string][]byte{ + "Android.bp": []byte(` +foo = "hello" +`), + "foo/Android.bp": []byte(` +foo = "world" +`), + }, + expectedError: "variable already set in inherited scope, previous assignment:", + }, + { + name: "Can't modify variable in inherited scope", + fs: map[string][]byte{ + "Android.bp": []byte(` +foo = "hello" +`), + "foo/Android.bp": []byte(` +foo += "world" +`), + }, + expectedError: "modified non-local variable \"foo\" with \\+=", + }, + { + name: "Can't modify variable after referencing", + fs: map[string][]byte{ + "Android.bp": []byte(` +foo = "hello" +x = foo +foo += "world" +`), + }, + expectedError: "modified variable \"foo\" with \\+= after referencing", + }, +} + +func TestBlueprintErrors(t *testing.T) { + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fixtures := FixtureMergeMockFs(tc.fs) + fixtures = fixtures.ExtendWithErrorHandler(FixtureExpectsOneErrorPattern(tc.expectedError)) + fixtures.RunTest(t) + }) + } +} diff --git a/android/selects_test.go b/android/selects_test.go index e2dc4035d..bfaabd1eb 100644 --- a/android/selects_test.go +++ b/android/selects_test.go @@ -97,6 +97,26 @@ func TestSelects(t *testing.T) { my_paths: &[]string{"baz.txt"}, }, }, + { + name: "Expression in select", + bp: ` + my_module_type { + name: "foo", + my_string: select(soong_config_variable("my_namespace", "my_variable"), { + "a": "foo" + "bar", + default: "baz", + }), + } + `, + provider: selectsTestProvider{ + my_string: proptools.StringPtr("foobar"), + }, + vendorVars: map[string]map[string]string{ + "my_namespace": { + "my_variable": "a", + }, + }, + }, { name: "paths with module references", bp: ` @@ -111,20 +131,6 @@ func TestSelects(t *testing.T) { `, expectedError: `"foo" depends on undefined module "c"`, }, - { - name: "Differing types", - bp: ` - my_module_type { - name: "foo", - my_string: select(soong_config_variable("my_namespace", "my_variable"), { - "a": "a.cpp", - "b": true, - default: "c.cpp", - }), - } - `, - expectedError: `Android.bp:8:5: Found select statement with differing types "string" and "bool" in its cases`, - }, { name: "Select type doesn't match property type", bp: ` @@ -137,7 +143,7 @@ func TestSelects(t *testing.T) { }), } `, - expectedError: `can't assign bool value to string property "my_string\[0\]"`, + expectedError: `can't assign bool value to string property`, }, { name: "String list non-default", @@ -825,6 +831,24 @@ func TestSelects(t *testing.T) { my_string_list: &[]string{"a.cpp", "c.cpp", "foo.cpp"}, }, }, + { + name: "Arch variant bool", + bp: ` + my_variable = ["b.cpp"] + my_module_type { + name: "foo", + arch_variant_configurable_bool: false, + target: { + bionic_arm64: { + enabled: true, + }, + }, + } + `, + provider: selectsTestProvider{ + arch_variant_configurable_bool: proptools.BoolPtr(false), + }, + }, } for _, tc := range testCases { @@ -873,6 +897,7 @@ type selectsTestProvider struct { my_string_list *[]string my_paths *[]string replacing_string_list *[]string + arch_variant_configurable_bool *bool my_nonconfigurable_bool *bool my_nonconfigurable_string *string my_nonconfigurable_string_list []string @@ -897,6 +922,7 @@ func (p *selectsTestProvider) String() string { my_string_list: %s, my_paths: %s, replacing_string_list %s, + arch_variant_configurable_bool %v my_nonconfigurable_bool: %v, my_nonconfigurable_string: %s, my_nonconfigurable_string_list: %s, @@ -906,6 +932,7 @@ func (p *selectsTestProvider) String() string { p.my_string_list, p.my_paths, p.replacing_string_list, + p.arch_variant_configurable_bool, p.my_nonconfigurable_bool, myNonconfigurableStringStr, p.my_nonconfigurable_string_list, @@ -920,6 +947,7 @@ type selectsMockModuleProperties struct { My_string_list proptools.Configurable[[]string] My_paths proptools.Configurable[[]string] `android:"path"` Replacing_string_list proptools.Configurable[[]string] `android:"replace_instead_of_append,arch_variant"` + Arch_variant_configurable_bool proptools.Configurable[bool] `android:"replace_instead_of_append,arch_variant"` My_nonconfigurable_bool *bool My_nonconfigurable_string *string My_nonconfigurable_string_list []string @@ -950,6 +978,7 @@ func (p *selectsMockModule) GenerateAndroidBuildActions(ctx ModuleContext) { my_string_list: optionalToPtr(p.properties.My_string_list.Get(ctx)), my_paths: optionalToPtr(p.properties.My_paths.Get(ctx)), replacing_string_list: optionalToPtr(p.properties.Replacing_string_list.Get(ctx)), + arch_variant_configurable_bool: optionalToPtr(p.properties.Arch_variant_configurable_bool.Get(ctx)), my_nonconfigurable_bool: p.properties.My_nonconfigurable_bool, my_nonconfigurable_string: p.properties.My_nonconfigurable_string, my_nonconfigurable_string_list: p.properties.My_nonconfigurable_string_list, diff --git a/androidmk/androidmk/android.go b/androidmk/androidmk/android.go index 8a8bb2eaa..570f36c16 100644 --- a/androidmk/androidmk/android.go +++ b/androidmk/androidmk/android.go @@ -341,9 +341,6 @@ func classifyLocalOrGlobalPath(value bpparser.Expression) (string, bpparser.Expr firstOperand := v.Args[0] secondOperand := v.Args[1] - if firstOperand.Type() != bpparser.StringType { - return "global", value, nil - } if _, ok := firstOperand.(*bpparser.Operator); ok { return "global", value, nil diff --git a/androidmk/androidmk/androidmk.go b/androidmk/androidmk/androidmk.go index 2e8810fe8..6fb20dcc1 100644 --- a/androidmk/androidmk/androidmk.go +++ b/androidmk/androidmk/androidmk.go @@ -493,7 +493,6 @@ func setVariable(file *bpFile, plusequals bool, prefix, name string, value bppar Name: name, NamePos: pos, Value: value, - OrigValue: value, EqualsPos: pos, Assigner: "+=", } @@ -506,7 +505,6 @@ func setVariable(file *bpFile, plusequals bool, prefix, name string, value bppar Name: name, NamePos: pos, Value: value, - OrigValue: value, EqualsPos: pos, Assigner: "=", } diff --git a/androidmk/androidmk/values.go b/androidmk/androidmk/values.go index 9618142fa..701c708e2 100644 --- a/androidmk/androidmk/values.go +++ b/androidmk/androidmk/values.go @@ -81,7 +81,7 @@ func makeToStringExpression(ms *mkparser.MakeString, file *bpFile) (bpparser.Exp } tmp := &bpparser.Variable{ Name: name, - Value: &bpparser.String{}, + Type_: bpparser.StringType, } if tmp.Name == "TOP" { @@ -150,7 +150,7 @@ func makeToListExpression(ms *mkparser.MakeString, file *bpFile) (bpparser.Expre } listOfListValues = append(listOfListValues, &bpparser.Variable{ Name: name, - Value: &bpparser.List{}, + Type_: bpparser.ListType, }) listValue = &bpparser.List{} } @@ -215,7 +215,7 @@ func makeToBoolExpression(ms *mkparser.MakeString, file *bpFile) (bpparser.Expre } return &bpparser.Variable{ Name: name, - Value: &bpparser.Bool{}, + Type_: bpparser.BoolType, }, nil } else { return nil, fmt.Errorf("non-const bool expression %s", ms.Dump()) diff --git a/bpfix/bpfix/bpfix.go b/bpfix/bpfix/bpfix.go index ddaa98aa3..9163ab759 100644 --- a/bpfix/bpfix/bpfix.go +++ b/bpfix/bpfix/bpfix.go @@ -286,7 +286,7 @@ func (f *Fixer) reparse() ([]byte, error) { } func parse(name string, r io.Reader) (*parser.File, error) { - tree, errs := parser.Parse(name, r, parser.NewScope(nil)) + tree, errs := parser.Parse(name, r) if errs != nil { s := "parse error: " for _, err := range errs { diff --git a/bpfix/bpfix/bpfix_test.go b/bpfix/bpfix/bpfix_test.go index b5b49b1ab..f487d3c7c 100644 --- a/bpfix/bpfix/bpfix_test.go +++ b/bpfix/bpfix/bpfix_test.go @@ -46,7 +46,7 @@ func buildTree(local_include_dirs []string, export_include_dirs []string) (file } `, printListOfStrings(local_include_dirs), printListOfStrings(export_include_dirs)) - tree, errs := parser.Parse("", strings.NewReader(input), parser.NewScope(nil)) + tree, errs := parser.Parse("", strings.NewReader(input)) if len(errs) > 0 { errs = append([]error{fmt.Errorf("failed to parse:\n%s", input)}, errs...) } @@ -167,7 +167,7 @@ func preProcessIn(in string) (fixer *Fixer, err error) { return fixer, err } - tree, errs := parser.Parse("", bytes.NewBufferString(in), parser.NewScope(nil)) + tree, errs := parser.Parse("", bytes.NewBufferString(in)) if errs != nil { return fixer, err } diff --git a/bpfix/cmd_lib/bpfix.go b/bpfix/cmd_lib/bpfix.go index 1106d4af7..41430f8e4 100644 --- a/bpfix/cmd_lib/bpfix.go +++ b/bpfix/cmd_lib/bpfix.go @@ -66,7 +66,7 @@ func processFile(filename string, in io.Reader, out io.Writer, fixRequest bpfix. return err } r := bytes.NewBuffer(append([]byte(nil), src...)) - file, errs := parser.Parse(filename, r, parser.NewScope(nil)) + file, errs := parser.Parse(filename, r) if len(errs) > 0 { for _, err := range errs { fmt.Fprintln(os.Stderr, err)