From 0173a2268bce78e353b9c12fe19c144c8faf350d Mon Sep 17 00:00:00 2001
From: Cole Faust <colefaust@google.com>
Date: Tue, 2 Apr 2024 16:42:58 -0700
Subject: [PATCH] Rename default select branch to 'default' keyword

Previously I was using an underscore to denote the default branch
because I was thinking that I would allow variable bindings in the
select branches, and 'default' could be mistaken for the name of a
variable. But I think it's better to just introduce alternate syntax,
like `default @ my_var: "foo" + my_var,` to do the variable bindings,
so that we can have a clearer name for the default case.

Bug: 323382414
Test: m nothing --no-skip-soong-tests
Change-Id: Ied762694e453855c03dd471898ebb52e97a5a671
---
 parser/parser.go         | 12 ++++++++++--
 parser/printer.go        |  2 +-
 parser/printer_test.go   | 30 +++++++++++++++---------------
 proptools/unpack_test.go |  6 +++---
 4 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/parser/parser.go b/parser/parser.go
index 76cc654..07f4681 100644
--- a/parser/parser.go
+++ b/parser/parser.go
@@ -214,6 +214,14 @@ func (p *parser) parseDefinitions() (defs []Definition) {
 func (p *parser) parseAssignment(name string, namePos scanner.Position,
 	assigner string) (assignment *Assignment) {
 
+	// These are used as keywords in select statements, prevent making variables
+	// with the same name to avoid any confusion.
+	switch name {
+	case "default", "unset":
+		p.errorf("'default' and 'unset' are reserved keywords, and cannot be used as variable names")
+		return nil
+	}
+
 	assignment = new(Assignment)
 
 	pos := p.scanner.Position
@@ -639,8 +647,8 @@ func (p *parser) parseSelect() Expression {
 
 	// Default must be last
 	if p.tok == scanner.Ident {
-		if p.scanner.TokenText() != "_" {
-			p.errorf("select cases can either be quoted strings or '_' to match any value")
+		if p.scanner.TokenText() != "default" {
+			p.errorf("select cases can either be quoted strings or 'default' to match any value")
 			return nil
 		}
 		c := &SelectCase{Pattern: String{
diff --git a/parser/printer.go b/parser/printer.go
index 47f8121..9d83e7a 100644
--- a/parser/printer.go
+++ b/parser/printer.go
@@ -183,7 +183,7 @@ func (p *printer) printSelect(s *Select) {
 		if c.Pattern.Value != "__soong_conditions_default__" {
 			p.printToken(strconv.Quote(c.Pattern.Value), c.Pattern.LiteralPos)
 		} else {
-			p.printToken("_", c.Pattern.LiteralPos)
+			p.printToken("default", c.Pattern.LiteralPos)
 		}
 		p.printToken(":", c.ColonPos)
 		p.requestSpace()
diff --git a/parser/printer_test.go b/parser/printer_test.go
index 8aefa5d..e273115 100644
--- a/parser/printer_test.go
+++ b/parser/printer_test.go
@@ -567,7 +567,7 @@ foo {
         // test2
         "b": "b2",
         // test3
-        _: "c2",
+        default: "c2",
     }),
 }
 `,
@@ -579,7 +579,7 @@ foo {
         // test2
         "b": "b2",
         // test3
-        _: "c2",
+        default: "c2",
     }),
 }
 `,
@@ -591,7 +591,7 @@ foo {
 foo {
     stuff: select(soong_config_variable("my_namespace", "my_variable"), {
         // test2
-        _: "c2",
+        default: "c2",
     }),
 }
 `,
@@ -612,11 +612,11 @@ foo {
         // test2
         "b": "b2",
         // test3
-        _: "c2",
+        default: "c2",
     }) + select(release_variable("RELEASE_TEST"), {
         "d": "d2",
         "e": "e2",
-        _: "f2",
+        default: "f2",
     }),
 }
 `,
@@ -628,11 +628,11 @@ foo {
         // test2
         "b": "b2",
         // test3
-        _: "c2",
+        default: "c2",
     }) + select(release_variable("RELEASE_TEST"), {
         "d": "d2",
         "e": "e2",
-        _: "f2",
+        default: "f2",
     }),
 }
 `,
@@ -643,7 +643,7 @@ foo {
 foo {
     stuff: select(soong_config_variable("my_namespace", "my_variable"), {
         "foo": unset,
-        _: "c2",
+        default: "c2",
     }),
 }
 `,
@@ -651,7 +651,7 @@ foo {
 foo {
     stuff: select(soong_config_variable("my_namespace", "my_variable"), {
         "foo": unset,
-        _: "c2",
+        default: "c2",
     }),
 }
 `,
@@ -662,7 +662,7 @@ foo {
 foo {
     stuff: select(soong_config_variable("my_namespace", "my_variable"), {
         "foo": unset,
-        _: unset,
+        default: unset,
     }),
 }
 `,
@@ -678,13 +678,13 @@ foo {
 foo {
     stuff: select(soong_config_variable("my_namespace", "my_variable"), {
         "foo": "a",
-        _: "b",
+        default: "b",
     }) + select(soong_config_variable("my_namespace", "my_variable2"), {
         "foo": unset,
-        _: unset,
+        default: unset,
     }) + select(soong_config_variable("my_namespace", "my_variable3"), {
         "foo": "c",
-        _: "d",
+        default: "d",
     }),
 }
 `,
@@ -694,12 +694,12 @@ foo {
 foo {
     stuff: select(soong_config_variable("my_namespace", "my_variable"), {
         "foo": "a",
-        _: "b",
+        default: "b",
     }) +
 
         select(soong_config_variable("my_namespace", "my_variable3"), {
             "foo": "c",
-            _: "d",
+            default: "d",
         }),
 }
 `,
diff --git a/proptools/unpack_test.go b/proptools/unpack_test.go
index 96cbfb9..7b7b46b 100644
--- a/proptools/unpack_test.go
+++ b/proptools/unpack_test.go
@@ -794,7 +794,7 @@ var validUnpackTestCases = []struct {
 				foo: select(soong_config_variable("my_namespace", "my_variable"), {
 					"a": "a2",
 					"b": "b2",
-					_: "c2",
+					default: "c2",
 				})
 			}
 		`,
@@ -823,11 +823,11 @@ var validUnpackTestCases = []struct {
 				foo: select(soong_config_variable("my_namespace", "my_variable"), {
 					"a": "a2",
 					"b": "b2",
-					_: "c2",
+					default: "c2",
 				}) + select(soong_config_variable("my_namespace", "my_2nd_variable"), {
 					"d": "d2",
 					"e": "e2",
-					_: "f2",
+					default: "f2",
 				})
 			}
 		`,