diff --git a/.chloggen/ottl-lambda-arity-validation.yaml b/.chloggen/ottl-lambda-arity-validation.yaml new file mode 100644 index 0000000000000..283754ba76106 --- /dev/null +++ b/.chloggen/ottl-lambda-arity-validation.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog) +component: pkg/ottl + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Break lambda arity validation into a separate function. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [49421] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: This allows statically verifying arity. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/pkg/ottl/e2e/e2e_test.go b/pkg/ottl/e2e/e2e_test.go index d5b669e7d90ce..093e6fa403962 100644 --- a/pkg/ottl/e2e/e2e_test.go +++ b/pkg/ottl/e2e/e2e_test.go @@ -2293,19 +2293,24 @@ func Test_e2e_lambda_expression(t *testing.T) { want: wantValue("result"), }, { - name: "not enough arguments (0 args)", - expression: `Eval((a, b) => a, [])`, - wantErr: "lambda expects exactly 0 argument(s), got 2", + name: "too many formals (0 formals)", + expression: `Eval((a, b) => a, [])`, + wantParseErr: "lambda should be defined with exactly 0 formal(s), but has 2", }, { - name: "not enough arguments (1 arg)", - expression: `Eval((a, b) => a, [1])`, - wantErr: "lambda expects exactly 1 argument(s), got 2", + name: "too many formals (1 formal)", + expression: `Eval((a, b) => a, [1])`, + wantParseErr: "lambda should be defined with exactly 1 formal(s), but has 2", }, { - name: "not enough arguments in nested lambda", - expression: `Eval((a) => Eval((b, c, d) => a + b + c + d, [2, 3]), [1])`, - wantErr: "lambda expects exactly 2 argument(s), got 3", + name: "not enough formals (2 formals)", + expression: `Eval((a) => a, [1, 2])`, + wantParseErr: "lambda should be defined with exactly 2 formal(s), but has 1", + }, + { + name: "too many formals in nested lambda", + expression: `Eval((a) => Eval((b, c, d) => a + b + c + d, [2, 3]), [1])`, + wantParseErr: "lambda should be defined with exactly 2 formal(s), but has 3", }, { name: "lambdas can't return another lambda", @@ -2853,8 +2858,12 @@ func createLambdaEvalFunction[K any](_ ottl.FunctionContext, oArgs ottl.Argument return nil, errors.New("lambdaEvalArguments args must be of type *lambdaEvalArguments[K]") } + if err := args.Expr.ValidateArity(len(args.Params)); err != nil { + return nil, err + } + return func(ctx context.Context, tCtx K) (any, error) { - lambda, err := args.Expr.Activate(ctx, len(args.Params)) + lambda, err := args.Expr.Activate(ctx) if err != nil { return nil, err } diff --git a/pkg/ottl/functions_test.go b/pkg/ottl/functions_test.go index 080574476365b..6939c40267ccf 100644 --- a/pkg/ottl/functions_test.go +++ b/pkg/ottl/functions_test.go @@ -2539,7 +2539,10 @@ type evalLambdaArguments[K any] struct { //nolint:unparam // returning (ExprFunc[K], error) is required by this local test framework func evalLambdaFunction[K any](expr LambdaExpression[any], args []Getter[K]) (ExprFunc[K], error) { return func(ctx context.Context, tCtx K) (any, error) { - lambda, err := expr.Activate(ctx, len(args)) + if err := expr.ValidateArity(len(args)); err != nil { + return nil, err + } + lambda, err := expr.Activate(ctx) if err != nil { return nil, err } diff --git a/pkg/ottl/lambda.go b/pkg/ottl/lambda.go index c4b8db9793ad2..c0f530983ff38 100644 --- a/pkg/ottl/lambda.go +++ b/pkg/ottl/lambda.go @@ -51,17 +51,24 @@ func (l *LambdaExpression[K]) Formals() []LocalIdentifierDecl { return slices.Clone(l.formals) } +// ValidateArity checks that the number of arguments that will be passed to the +// lambda matches the number of declared formals. If the counts differ, an error +// is returned. This allows statically verifying arity: it should be run inside +// the OTTL function factory, i.e. outside the closure the factory returns. It's +// meant to verify the lambda passed by the user has the correct number of +// formals before calling [LambdaExpression.Activate]. +func (l *LambdaExpression[K]) ValidateArity(arity int) error { + if len(l.formals) != arity { + return fmt.Errorf("lambda should be defined with exactly %d formal(s), but has %d", arity, len(l.formals)) + } + return nil +} + // Activate creates a [LambdaActivation] for a single outer function invocation, allocating // its own activation and argument storage, linking the resulting activation to the given ctx. -// The arity is the number of formals the caller must pass, and their values are set via -// [LambdaActivation.SetArg]. If the lambda's declared formal count differs from arity, an -// error is returned. // Call [LambdaActivation.SetArg] for every index in (0...arity) before each [LambdaActivation.Eval], // and then [LambdaActivation.Close] on the returned activation when it is no longer needed. -func (l *LambdaExpression[K]) Activate(ctx context.Context, arity int) (*LambdaActivation[K], error) { - if len(l.formals) != arity { - return nil, fmt.Errorf("lambda expects exactly %d argument(s), got %d", arity, len(l.formals)) - } +func (l *LambdaExpression[K]) Activate(ctx context.Context) (*LambdaActivation[K], error) { v := l.activationPool.Get().(*LambdaActivation[K]) v.ctx = pushLocalActivation(ctx, v.activation) return v, nil diff --git a/pkg/ottl/lambda_test.go b/pkg/ottl/lambda_test.go index 4938eeace3ea1..cb75f0b1bb53b 100644 --- a/pkg/ottl/lambda_test.go +++ b/pkg/ottl/lambda_test.go @@ -55,22 +55,64 @@ func TestLambdaExpression_Formals(t *testing.T) { } } +func TestLambdaExpression_ValidateArity(t *testing.T) { + tests := []struct { + name string + formals []LocalIdentifierDecl + arity int + wantErr string + }{ + { + name: "matching arity", + formals: makeLocalIdentifiers("a", "b"), + arity: 2, + }, + { + name: "no formals matches zero arity", + formals: nil, + arity: 0, + }, + { + name: "blank formals count toward arity", + formals: makeLocalIdentifiers("_", "a"), + arity: 2, + }, + { + name: "too few arguments", + formals: makeLocalIdentifiers("a", "b"), + arity: 1, + wantErr: "lambda should be defined with exactly 1 formal(s), but has 2", + }, + { + name: "too many arguments", + formals: makeLocalIdentifiers("a"), + arity: 3, + wantErr: "lambda should be defined with exactly 3 formal(s), but has 1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expr := newLambdaExpression[any](tt.formals, nil, nil) + err := expr.ValidateArity(tt.arity) + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + return + } + require.NoError(t, err) + }) + } +} + func TestLambdaExpression_Eval(t *testing.T) { tests := []struct { name string expr *LambdaExpression[any] ctx context.Context - arity int params []any want any wantErr string }{ - { - name: "arity mismatch", - expr: newLambdaExpression[any](makeLocalIdentifiers("a", "b"), nil, nil), - arity: 1, - wantErr: "lambda expects exactly 1 argument(s), got 2", - }, { name: "literal body evaluate as-is", expr: newLambdaExpression[any]( @@ -78,7 +120,6 @@ func TestLambdaExpression_Eval(t *testing.T) { newLiteral[any, any]("literal"), nil, ), - arity: 1, params: []any{"a value"}, want: "literal", }, @@ -98,7 +139,6 @@ func TestLambdaExpression_Eval(t *testing.T) { }, }, ), - arity: 1, params: []any{"bound"}, want: true, }, @@ -113,7 +153,6 @@ func TestLambdaExpression_Eval(t *testing.T) { }, }, ), - arity: 1, params: []any{"bound"}, wantErr: "failed to evaluate", }, @@ -126,7 +165,6 @@ func TestLambdaExpression_Eval(t *testing.T) { }, nil, ), - arity: 1, params: []any{42}, want: 42, }, @@ -140,7 +178,6 @@ func TestLambdaExpression_Eval(t *testing.T) { nil, ), ctx: context.WithValue(t.Context(), localActivationKey{}, &localActivation{bindings: map[string]any{"parent": "value"}}), - arity: 0, params: []any{}, want: "value", }, @@ -154,7 +191,6 @@ func TestLambdaExpression_Eval(t *testing.T) { nil, ), ctx: context.WithValue(t.Context(), localActivationKey{}, &localActivation{bindings: map[string]any{"a": "old"}}), - arity: 1, params: []any{"new"}, want: "new", }, @@ -173,7 +209,6 @@ func TestLambdaExpression_Eval(t *testing.T) { }, nil, ), - arity: 1, params: []any{ map[string]any{"name": []any{"zero", "one"}}, }, @@ -182,7 +217,6 @@ func TestLambdaExpression_Eval(t *testing.T) { { name: "invalid lambda without body", expr: newLambdaExpression[any](nil, nil, nil), - arity: 0, params: []any{}, wantErr: "invalid lambda: no body", }, @@ -195,7 +229,6 @@ func TestLambdaExpression_Eval(t *testing.T) { }, nil, ), - arity: 2, params: []any{"skip", "bound"}, want: "bound", }, @@ -215,7 +248,6 @@ func TestLambdaExpression_Eval(t *testing.T) { }, }, ), - arity: 1, params: []any{"skip"}, want: true, }, @@ -228,13 +260,7 @@ func TestLambdaExpression_Eval(t *testing.T) { ctx = t.Context() } - lb, err := tt.expr.Activate(ctx, tt.arity) - if tt.wantErr != "" && lb == nil { - require.Error(t, err) - assert.EqualError(t, err, tt.wantErr) - return - } - + lb, err := tt.expr.Activate(ctx) require.NoError(t, err) defer lb.Close() for i, param := range tt.params { @@ -262,7 +288,7 @@ func TestLambdaExpression_Activate(t *testing.T) { nil, ) - lb, err := expr.Activate(t.Context(), 1) + lb, err := expr.Activate(t.Context()) require.NoError(t, err) require.NoError(t, lb.SetArg(0, 1)) @@ -277,9 +303,9 @@ func TestLambdaExpression_Activate(t *testing.T) { // Each Activate yields independent state, so overlapping activations of the same expression do not // interfere with one another. - lb1, err := expr.Activate(t.Context(), 1) + lb1, err := expr.Activate(t.Context()) require.NoError(t, err) - lb2, err := expr.Activate(t.Context(), 1) + lb2, err := expr.Activate(t.Context()) require.NoError(t, err) require.NoError(t, lb1.SetArg(0, "one")) @@ -301,7 +327,7 @@ func TestLambdaActivation_SetArg(t *testing.T) { nil, ) - lb, err := expr.Activate(t.Context(), 1) + lb, err := expr.Activate(t.Context()) require.NoError(t, err) err = lb.SetArg(-1, "x") @@ -320,7 +346,7 @@ func TestLambdaActivation_StaleArg(t *testing.T) { nil, ) - lb, err := expr.Activate(t.Context(), 2) + lb, err := expr.Activate(t.Context()) require.NoError(t, err) require.NoError(t, lb.SetArg(0, "first-a")) @@ -358,13 +384,13 @@ func TestLambdaActivation_ParentChain(t *testing.T) { nil, ) - outerLb, err := outerExpr.Activate(t.Context(), 1) + outerLb, err := outerExpr.Activate(t.Context()) require.NoError(t, err) require.NoError(t, outerLb.SetArg(0, "from-outer")) _, err = outerLb.Eval(nil) require.NoError(t, err) - innerLb, err := innerExpr.Activate(outerLb.ctx, 1) + innerLb, err := innerExpr.Activate(outerLb.ctx) require.NoError(t, err) require.NoError(t, innerLb.SetArg(0, "inner-val")) @@ -382,7 +408,7 @@ func TestLambdaActivation_Close(t *testing.T) { nil, ) - lb, err := expr.Activate(t.Context(), 2) + lb, err := expr.Activate(t.Context()) require.NoError(t, err) require.NoError(t, lb.SetArg(0, 1)) eval, err := lb.Eval(nil) @@ -390,7 +416,7 @@ func TestLambdaActivation_Close(t *testing.T) { assert.Equal(t, 1, eval) lb.Close() - lb2, err := expr.Activate(t.Context(), 2) + lb2, err := expr.Activate(t.Context()) require.NoError(t, err) require.NotNil(t, lb2.activation) assert.Nil(t, lb2.activation.parent) diff --git a/pkg/ottl/ottlfuncs/internal/funcutil/lambda_test.go b/pkg/ottl/ottlfuncs/internal/funcutil/lambda_test.go index ec847c6fd0d19..d1e21905250d4 100644 --- a/pkg/ottl/ottlfuncs/internal/funcutil/lambda_test.go +++ b/pkg/ottl/ottlfuncs/internal/funcutil/lambda_test.go @@ -17,7 +17,8 @@ import ( func activateTestLambda(t *testing.T, expr *ottl.LambdaExpression[any], arity int) *ottl.LambdaActivation[any] { t.Helper() - lb, err := expr.Activate(t.Context(), arity) + require.NoError(t, expr.ValidateArity(arity)) + lb, err := expr.Activate(t.Context()) require.NoError(t, err) t.Cleanup(lb.Close) return lb