From 05732b52c412dee1f23e6895392504d9d1cb3e16 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 1 Mar 2023 18:19:33 -0500 Subject: [PATCH 1/3] Add version option to Strings extension library --- ext/strings.go | 74 +++++++++++++++++++++++++++++---------------- ext/strings_test.go | 66 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 26 deletions(-) diff --git a/ext/strings.go b/ext/strings.go index fbe140e3..e2ba46c6 100644 --- a/ext/strings.go +++ b/ext/strings.go @@ -59,12 +59,16 @@ const ( // // # Format // +// Introduced at version: 1 +// // Returns a new string with substitutions being performed, printf-style. // The valid formatting clauses are: // -// `%s` - substitutes a string. This can also be used on bools, lists, maps, bytes, Duration and Timestamp, in addition to all numerical types (int, uint, and double). -// Note that the dot/period decimal separator will always be used when printing a list or map that contains a double, -// and that null can be passed (which results in the string "null") in addition to types. +// `%s` - substitutes a string. This can also be used on bools, lists, maps, bytes, +// Duration and Timestamp, in addition to all numerical types (int, uint, and double). +// Note that the dot/period decimal separator will always be used when printing a list +// or map that contains a double, and that null can be passed (which results in the +// string "null") in addition to types. // `%d` - substitutes an integer. // `%f` - substitutes a double with fixed-point precision. The default precision is 6, but // this can be adjusted. The strings `Infinity`, `-Infinity`, and `NaN` are also valid input @@ -81,19 +85,19 @@ const ( // // Examples: // -// "this is a string: %s\nand an integer: %d".format(["str", 42]) // returns "this is a string: str\nand an integer: 42" -// "a double substituted with %%s: %s".format([64.2]) // returns "a double substituted with %s: 64.2" -// "string type: %s".format([type(string)]) // returns "string type: string" -// "timestamp: %s".format([timestamp("2023-02-03T23:31:20+00:00")]) // returns "timestamp: 2023-02-03T23:31:20Z" -// "duration: %s".format([duration("1h45m47s")]) // returns "duration: 6347s" -// "%f".format([3.14]) // returns "3.140000" -// "scientific notation: %e".format([2.71828]) // returns "scientific notation: 2.718280\u202f\u00d7\u202f10\u2070\u2070" -// "5 in binary: %b".format([5]), // returns "5 in binary; 101" -// "26 in hex: %x".format([26]), // returns "26 in hex: 1a" -// "26 in hex (uppercase): %X".format([26]) // returns "26 in hex (uppercase): 1A" -// "30 in octal: %o".format([30]) // returns "30 in octal: 36" -// "a map inside a list: %s".format([[1, 2, 3, {"a": "x", "b": "y", "c": "z"}]]) // returns "a map inside a list: [1, 2, 3, {"a":"x", "b":"y", "c":"d"}]" -// "true bool: %s - false bool: %s\nbinary bool: %b".format([true, false, true]) // returns "true bool: true - false bool: false\nbinary bool: 1" +// "this is a string: %s\nand an integer: %d".format(["str", 42]) // returns "this is a string: str\nand an integer: 42" +// "a double substituted with %%s: %s".format([64.2]) // returns "a double substituted with %s: 64.2" +// "string type: %s".format([type(string)]) // returns "string type: string" +// "timestamp: %s".format([timestamp("2023-02-03T23:31:20+00:00")]) // returns "timestamp: 2023-02-03T23:31:20Z" +// "duration: %s".format([duration("1h45m47s")]) // returns "duration: 6347s" +// "%f".format([3.14]) // returns "3.140000" +// "scientific notation: %e".format([2.71828]) // returns "scientific notation: 2.718280\u202f\u00d7\u202f10\u2070\u2070" +// "5 in binary: %b".format([5]), // returns "5 in binary; 101" +// "26 in hex: %x".format([26]), // returns "26 in hex: 1a" +// "26 in hex (uppercase): %X".format([26]) // returns "26 in hex (uppercase): 1A" +// "30 in octal: %o".format([30]) // returns "30 in octal: 36" +// "a map inside a list: %s".format([[1, 2, 3, {"a": "x", "b": "y", "c": "z"}]]) // returns "a map inside a list: [1, 2, 3, {"a":"x", "b":"y", "c":"d"}]" +// "true bool: %s - false bool: %s\nbinary bool: %b".format([true, false, true]) // returns "true bool: true - false bool: false\nbinary bool: 1" // // Passing an incorrect type (an integer to `%s`) or mismatching the number of arguments (putting 3 // formatting clauses but passing two variables, or vice-versa) is considered an error. @@ -251,7 +255,7 @@ const ( // 'TacoCat'.upperAscii() // returns 'TACOCAT' // 'TacoCÆt Xii'.upperAscii() // returns 'TACOCÆT XII' func Strings(options ...StringsOption) cel.EnvOption { - s := &stringLib{} + s := &stringLib{version: math.MaxUint32} for _, o := range options { s = o(s) } @@ -259,7 +263,8 @@ func Strings(options ...StringsOption) cel.EnvOption { } type stringLib struct { - locale string + locale string + version uint32 } // LibraryName implements the SingletonLibrary interface method. @@ -280,6 +285,20 @@ func StringsLocale(locale string) StringsOption { } } +// Version configures the version of the string library. The version limits which +// functions are available. Only functions introduced below or equal to the given +// version included in the library. See the library documentation to determine +// which version a function was introduced at. If the documentation does not +// state which version a function was introduced at, it can be assumed to be +// introduced at version 0, when the library was first created. +// If this option is not set, all functions are available. +func Version(version uint32) func(lib *stringLib) *stringLib { + return func(sl *stringLib) *stringLib { + sl.version = version + return sl + } +} + // CompileOptions implements the Library interface method. func (sl stringLib) CompileOptions() []cel.EnvOption { formatLocale := "en_US" @@ -295,14 +314,8 @@ func (sl stringLib) CompileOptions() []cel.EnvOption { } formatLocale = sl.locale } - formatOverload := cel.MemberOverload("string_format", []*cel.Type{cel.StringType, cel.ListType(cel.DynType)}, cel.StringType, - cel.FunctionBinding(func(args ...ref.Val) ref.Val { - s := args[0].(types.String).Value().(string) - formatArgs := args[1].(traits.Lister) - return stringOrError(stringFormatWithLocale(s, formatArgs, formatLocale)) - })) - return []cel.EnvOption{ + opts := []cel.EnvOption{ cel.Function("charAt", cel.MemberOverload("string_char_at_int", []*cel.Type{cel.StringType, cel.IntType}, cel.StringType, cel.BinaryBinding(func(str, ind ref.Val) ref.Val { @@ -420,8 +433,17 @@ func (sl stringLib) CompileOptions() []cel.EnvOption { d := delim.(types.String) return stringOrError(joinSeparator(l.([]string), string(d))) }))), - cel.Function("format", formatOverload), } + if sl.version >= 1 { + opts = append(opts, cel.Function("format", + cel.MemberOverload("string_format", []*cel.Type{cel.StringType, cel.ListType(cel.DynType)}, cel.StringType, + cel.FunctionBinding(func(args ...ref.Val) ref.Val { + s := args[0].(types.String).Value().(string) + formatArgs := args[1].(traits.Lister) + return stringOrError(stringFormatWithLocale(s, formatArgs, formatLocale)) + })))) + } + return opts } // ProgramOptions implements the Library interface method. diff --git a/ext/strings_test.go b/ext/strings_test.go index 777a77ef..08d05809 100644 --- a/ext/strings_test.go +++ b/ext/strings_test.go @@ -31,6 +31,7 @@ import ( // TODO: move these tests to a conformance test. var stringTests = []struct { expr string + version *uint32 err string parseOnly bool }{ @@ -325,6 +326,71 @@ func TestStrings(t *testing.T) { } } +func TestVersions(t *testing.T) { + versionCases := []struct { + version uint32 + supportedFunctions map[string]string + }{ + { + version: 0, + supportedFunctions: map[string]string{ + "chatAt": "''.charAt(0)", + "indexOf": "'a'.indexOf('a')", + "lastIndexOf": "'a'.lastIndexOf('a')", + "join": "['a', 'b'].join()", + "lowerAscii": "'a'.lowerAscii()", + "replace": "'hello hello'.replace('he', 'we')", + "split": "'hello hello hello'.split(' ')", + "substring": "'tacocat'.substring(4)", + "trim": "' \\ttrim\\n '.trim()", + "upperAscii": "'TacoCat'.upperAscii()", + }, + }, + { + version: 1, + supportedFunctions: map[string]string{ + "format": "'a %d'.format([1])", + }, + }, + } + for _, lib := range versionCases { + env, err := cel.NewEnv(Strings(Version(lib.version))) + if err != nil { + t.Fatalf("cel.NewEnv(Strings(Version(%d))) failed: %v", lib.version, err) + } + t.Run(fmt.Sprintf("version=%d", lib.version), func(t *testing.T) { + for _, tc := range versionCases { + for name, expr := range tc.supportedFunctions { + supported := lib.version >= tc.version + t.Run(fmt.Sprintf("%s-supported=%t", name, supported), func(t *testing.T) { + var asts []*cel.Ast + pAst, iss := env.Parse(expr) + if iss.Err() != nil { + t.Fatalf("env.Parse(%v) failed: %v", expr, iss.Err()) + } + asts = append(asts, pAst) + _, iss = env.Check(pAst) + + if supported { + if iss.Err() != nil { + t.Errorf("unexpected error: %v", iss.Err()) + } + } else { + if iss.Err() == nil || !strings.Contains(iss.Err().Error(), "undeclared reference") { + t.Errorf("got error %v, wanted error %s for expr: %s, version: %d", iss.Err(), "undeclared reference", expr, tc.version) + } + } + }) + } + } + }) + } +} + +func version(v uint32) *uint32 { + return &v +} + func TestStringsWithExtension(t *testing.T) { env, err := cel.NewEnv(Strings()) if err != nil { From 0f28cf42ec48b3624204308b93a680024e4fd003 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 1 Mar 2023 18:38:53 -0500 Subject: [PATCH 2/3] Use ptr receiver for stringlib --- ext/strings.go | 6 +++--- ext/strings_test.go | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ext/strings.go b/ext/strings.go index e2ba46c6..57dd680c 100644 --- a/ext/strings.go +++ b/ext/strings.go @@ -268,7 +268,7 @@ type stringLib struct { } // LibraryName implements the SingletonLibrary interface method. -func (stringLib) LibraryName() string { +func (*stringLib) LibraryName() string { return "cel.lib.ext.strings" } @@ -300,7 +300,7 @@ func Version(version uint32) func(lib *stringLib) *stringLib { } // CompileOptions implements the Library interface method. -func (sl stringLib) CompileOptions() []cel.EnvOption { +func (sl *stringLib) CompileOptions() []cel.EnvOption { formatLocale := "en_US" if sl.locale != "" { // ensure locale is properly-formed if set @@ -447,7 +447,7 @@ func (sl stringLib) CompileOptions() []cel.EnvOption { } // ProgramOptions implements the Library interface method. -func (stringLib) ProgramOptions() []cel.ProgramOption { +func (*stringLib) ProgramOptions() []cel.ProgramOption { return []cel.ProgramOption{} } diff --git a/ext/strings_test.go b/ext/strings_test.go index 08d05809..bb81eece 100644 --- a/ext/strings_test.go +++ b/ext/strings_test.go @@ -31,7 +31,6 @@ import ( // TODO: move these tests to a conformance test. var stringTests = []struct { expr string - version *uint32 err string parseOnly bool }{ From 2beaf44c14076b8c62b854f12d33d34176a8a532 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 1 Mar 2023 18:42:25 -0500 Subject: [PATCH 3/3] Rename option to StringsVersion --- ext/strings.go | 4 ++-- ext/strings_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ext/strings.go b/ext/strings.go index 57dd680c..0a7967e8 100644 --- a/ext/strings.go +++ b/ext/strings.go @@ -285,14 +285,14 @@ func StringsLocale(locale string) StringsOption { } } -// Version configures the version of the string library. The version limits which +// StringsVersion configures the version of the string library. The version limits which // functions are available. Only functions introduced below or equal to the given // version included in the library. See the library documentation to determine // which version a function was introduced at. If the documentation does not // state which version a function was introduced at, it can be assumed to be // introduced at version 0, when the library was first created. // If this option is not set, all functions are available. -func Version(version uint32) func(lib *stringLib) *stringLib { +func StringsVersion(version uint32) func(lib *stringLib) *stringLib { return func(sl *stringLib) *stringLib { sl.version = version return sl diff --git a/ext/strings_test.go b/ext/strings_test.go index bb81eece..82ba87e6 100644 --- a/ext/strings_test.go +++ b/ext/strings_test.go @@ -353,9 +353,9 @@ func TestVersions(t *testing.T) { }, } for _, lib := range versionCases { - env, err := cel.NewEnv(Strings(Version(lib.version))) + env, err := cel.NewEnv(Strings(StringsVersion(lib.version))) if err != nil { - t.Fatalf("cel.NewEnv(Strings(Version(%d))) failed: %v", lib.version, err) + t.Fatalf("cel.NewEnv(Strings(StringsVersion(%d))) failed: %v", lib.version, err) } t.Run(fmt.Sprintf("version=%d", lib.version), func(t *testing.T) { for _, tc := range versionCases {