Repository: ariga/atlas
Pull request #219 (2021-11-13 21:22:22)

schema/schemaspec/schemahcl: fixing a bug where list literals weren't…
… handled correctly

discovered while working on support for lists of literals in HCL


Patch:
diff --git a/cmd/atlas/schema.go b/cmd/atlas/schema.go
index d29f76ce..c3b8184a 100644
--- a/cmd/atlas/schema.go
+++ b/cmd/atlas/schema.go
@@ -1,4 +1,3 @@
-
 package main
 
 import (
@@ -9,7 +8,7 @@ import (
 var schemaCmd = &cobra.Command{
 	Use:   "schema",
 	Short: "Work with atlas schemas.",
-	Long: `Provides ability to interact with schema and data source.`,
+	Long:  `Provides ability to interact with schema and data source.`,
 }
 
 func init() {
diff --git a/schema/schemaspec/schemahcl/hcl.go b/schema/schemaspec/schemahcl/hcl.go
index 27968c01..d06495a9 100644
--- a/schema/schemaspec/schemahcl/hcl.go
+++ b/schema/schemaspec/schemahcl/hcl.go
@@ -238,11 +238,15 @@ func writeAttr(attr *schemaspec.Attr, body *hclwrite.Body) error {
 	case *schemaspec.ListValue:
 		lst := make([]string, 0, len(v.V))
 		for _, item := range v.V {
-			val, err := schemaspec.StrVal(item)
-			if err != nil {
-				return err
+			switch v := item.(type) {
+			case *schemaspec.Ref:
+				expr := strings.ReplaceAll(v.V, "$", "")
+				lst = append(lst, expr)
+			case *schemaspec.LiteralValue:
+				lst = append(lst, v.V)
+			default:
+				return fmt.Errorf("cannot write elem type %T of attr %q to HCL list", v, attr)
 			}
-			lst = append(lst, val)
 		}
 		body.SetAttributeRaw(attr.K, hclRawList(lst))
 	default:
diff --git a/sql/internal/specutil/spec.go b/sql/internal/specutil/spec.go
index 28456378..717c0ec9 100644
--- a/sql/internal/specutil/spec.go
+++ b/sql/internal/specutil/spec.go
@@ -1,8 +1,6 @@
 package specutil
 
 import (
-	"strconv"
-
 	"ariga.io/atlas/schema/schemaspec"
 	"ariga.io/atlas/sql/sqlspec"
 )
@@ -26,17 +24,11 @@ func LitAttr(k, v string) *schemaspec.Attr {
 	}
 }
 
-// StrLitAttr is a helper method for constructing *schemaspec.Attr instances that contain literal values
-// representing string literals.
-func StrLitAttr(k, v string) *schemaspec.Attr {
-	return LitAttr(k, strconv.Quote(v))
-}
-
 // ListAttr is a helper method for constructing *schemaspec.Attr instances that contain list values.
-func ListAttr(k string, values ...string) *schemaspec.Attr {
+func ListAttr(k string, litValues ...string) *schemaspec.Attr {
 	lv := &schemaspec.ListValue{}
-	for _, v := range values {
-		lv.V = append(lv.V, &schemaspec.LiteralValue{V: strconv.Quote(v)})
+	for _, v := range litValues {
+		lv.V = append(lv.V, &schemaspec.LiteralValue{V: v})
 	}
 	return &schemaspec.Attr{
 		K: k,
diff --git a/sql/mysql/sqlspec.go b/sql/mysql/sqlspec.go
index 08d74548..9e486945 100644
--- a/sql/mysql/sqlspec.go
+++ b/sql/mysql/sqlspec.go
@@ -340,4 +340,4 @@ func enumSpec(t *schema.EnumType) (*sqlspec.Column, error) {
 		quoted = append(quoted, strconv.Quote(v))
 	}
 	return specutil.NewCol("", "enum", specutil.ListAttr("values", quoted...)), nil
-}
\ No newline at end of file
+}
diff --git a/sql/postgres/sqlspec.go b/sql/postgres/sqlspec.go
index e36c820e..a7c74d7f 100644
--- a/sql/postgres/sqlspec.go
+++ b/sql/postgres/sqlspec.go
@@ -327,4 +327,3 @@ func bitSpec(t *BitType) (*sqlspec.Column, error) {
 		return nil, errors.New("schema bit failed to convert")
 	}
 }
-


Test patch:
diff --git a/schema/schemaspec/schemahcl/context_test.go b/schema/schemaspec/schemahcl/context_test.go
index b3f088fe..51a93fff 100644
--- a/schema/schemaspec/schemahcl/context_test.go
+++ b/schema/schemaspec/schemahcl/context_test.go
@@ -238,6 +238,8 @@ group "lion_kings" {
 			{V: "$user.mufasa"},
 		},
 	}, test.Groups[0])
+	_, err = Marshal(&test)
+	require.NoError(t, err)
 }
 
 func TestNestedDifference(t *testing.T) {
diff --git a/sql/mysql/sqlspec_test.go b/sql/mysql/sqlspec_test.go
index 0c7b1e0c..d23141fc 100644
--- a/sql/mysql/sqlspec_test.go
+++ b/sql/mysql/sqlspec_test.go
@@ -399,7 +399,7 @@ func TestMarshalSpecColumnType(t *testing.T) {
 		},
 		{
 			schem:    &schema.EnumType{Values: []string{"a", "b", "c"}},
-			expected: specutil.NewCol("column", "enum", specutil.ListAttr("values", `a`, `b`, `c`)),
+			expected: specutil.NewCol("column", "enum", specutil.ListAttr("values", `"a"`, `"b"`, `"c"`)),
 		},
 		{
 			schem:    &schema.BoolType{T: "boolean"},
diff --git a/sql/postgres/sqlspec_test.go b/sql/postgres/sqlspec_test.go
index 1948bd11..f6db34a0 100644
--- a/sql/postgres/sqlspec_test.go
+++ b/sql/postgres/sqlspec_test.go
@@ -340,7 +340,7 @@ func TestMarshalSpecColumnType(t *testing.T) {
 				Values: []string{"a", "b", "c"},
 			},
 			expected: specutil.NewCol("enum", "enum",
-				specutil.ListAttr("values", "a", "b", "c")),
+				specutil.ListAttr("values", `"a"`, `"b"`, `"c"`)),
 		},
 		{
 			schem: &schema.BoolType{
@@ -376,7 +376,7 @@ func TestMarshalSpecColumnType(t *testing.T) {
 		},
 		{
 			schem: &BitType{
-				T: tBit,
+				T:   tBit,
 				Len: 1,
 			},
 			expected: specutil.NewCol("bit", "bit"),
@@ -389,14 +389,14 @@ func TestMarshalSpecColumnType(t *testing.T) {
 		},
 		{
 			schem: &BitType{
-				T: tBit,
+				T:   tBit,
 				Len: 8,
 			},
 			expected: specutil.NewCol("bit8", "bit(8)"),
 		},
 		{
 			schem: &BitType{
-				T: tBitVar,
+				T:   tBitVar,
 				Len: 8,
 			},
 			expected: specutil.NewCol("bitvar8", "bit varying(8)"),
diff --git a/sql/sqlite/sqlspec_test.go b/sql/sqlite/sqlspec_test.go
index 95ce5e52..a8a07ac2 100644
--- a/sql/sqlite/sqlspec_test.go
+++ b/sql/sqlite/sqlspec_test.go
@@ -316,7 +316,7 @@ func TestMarshalSpecColumnType(t *testing.T) {
 		},
 		{
 			schem:    &schema.EnumType{T: "enum", Values: []string{"a", "b", "c"}},
-			expected: specutil.NewCol("column", "enum", specutil.ListAttr("values", "a", "b", "c")),
+			expected: specutil.NewCol("column", "enum", specutil.ListAttr("values", `"a"`, `"b"`, `"c"`)),
 		},
 		{
 			schem:    &schema.BoolType{T: "boolean"},