Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/ottl-lambda-arity-validation.yaml
Original file line number Diff line number Diff line change
@@ -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: []
Comment thread
evan-bradley marked this conversation as resolved.
Outdated

# (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]
5 changes: 4 additions & 1 deletion pkg/ottl/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2854,7 +2854,10 @@ func createLambdaEvalFunction[K any](_ ottl.FunctionContext, oArgs ottl.Argument
}

return func(ctx context.Context, tCtx K) (any, error) {
lambda, err := args.Expr.Activate(ctx, len(args.Params))
if err := args.Expr.ValidateArity(len(args.Params)); err != nil {
return nil, err
}
lambda, err := args.Expr.Activate(ctx)
if err != nil {
return nil, err
}
Expand Down
5 changes: 4 additions & 1 deletion pkg/ottl/functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
18 changes: 14 additions & 4 deletions pkg/ottl/lambda.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,17 +51,27 @@ 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. before the closure the factory returns. It's
// meant to verify the passed in lambda 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 expects exactly %d argument(s), got %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
Comment thread
evan-bradley marked this conversation as resolved.
Outdated
// [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) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats the error if a function calls activate with the wrong parity? Will it panic?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't panic, you have one of two things happen:

  1. If there are more arguments than parameters, you'll get an error returned when you try to bind an argument that doesn't have a defined parameter.
  2. If there are fewer arguments than parameters, some parameters are not bound and are essentially nil from my rudimentary testing.

If we're really concerned about ValidateArity not being called before Activate, we could include a flag on the lambda expression to check that ValidateArity has been called.

@edmocosta edmocosta Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're really concerned about ValidateArity not being called before Activate, we could include a flag on the lambda expression to check that ValidateArity has been called.

I'd add this validation to ensure usages are consistent and all functions implementations are correct.
If for some reason the function do not to validate it, Eval((a, b, c) => a, [1, 2]) for example, would be accepted.

v := l.activationPool.Get().(*LambdaActivation[K])
v.ctx = pushLocalActivation(ctx, v.activation)
return v, nil
Expand Down
92 changes: 59 additions & 33 deletions pkg/ottl/lambda_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,30 +55,71 @@ func TestLambdaExpression_Formals(t *testing.T) {
}
}

func TestLambdaExpression_Eval(t *testing.T) {
func TestLambdaExpression_ValidateArity(t *testing.T) {
tests := []struct {
name string
expr *LambdaExpression[any]
ctx context.Context
formals []LocalIdentifierDecl
arity int
params []any
want any
wantErr string
}{
{
name: "arity mismatch",
expr: newLambdaExpression[any](makeLocalIdentifiers("a", "b"), nil, nil),
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 expects exactly 1 argument(s), got 2",
},
{
name: "too many arguments",
formals: makeLocalIdentifiers("a"),
arity: 3,
wantErr: "lambda expects exactly 3 argument(s), got 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
params []any
want any
wantErr string
}{
{
name: "literal body evaluate as-is",
expr: newLambdaExpression[any](
makeLocalIdentifiers("a"),
newLiteral[any, any]("literal"),
nil,
),
arity: 1,
params: []any{"a value"},
want: "literal",
},
Expand All @@ -98,7 +139,6 @@ func TestLambdaExpression_Eval(t *testing.T) {
},
},
),
arity: 1,
params: []any{"bound"},
want: true,
},
Expand All @@ -113,7 +153,6 @@ func TestLambdaExpression_Eval(t *testing.T) {
},
},
),
arity: 1,
params: []any{"bound"},
wantErr: "failed to evaluate",
},
Expand All @@ -126,7 +165,6 @@ func TestLambdaExpression_Eval(t *testing.T) {
},
nil,
),
arity: 1,
params: []any{42},
want: 42,
},
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -173,7 +209,6 @@ func TestLambdaExpression_Eval(t *testing.T) {
},
nil,
),
arity: 1,
params: []any{
map[string]any{"name": []any{"zero", "one"}},
},
Expand All @@ -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",
},
Expand All @@ -195,7 +229,6 @@ func TestLambdaExpression_Eval(t *testing.T) {
},
nil,
),
arity: 2,
params: []any{"skip", "bound"},
want: "bound",
},
Expand All @@ -215,7 +248,6 @@ func TestLambdaExpression_Eval(t *testing.T) {
},
},
),
arity: 1,
params: []any{"skip"},
want: true,
},
Expand All @@ -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 {
Expand Down Expand Up @@ -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))
Expand All @@ -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"))
Expand All @@ -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")
Expand All @@ -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"))
Expand Down Expand Up @@ -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"))

Expand All @@ -382,15 +408,15 @@ 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)
require.NoError(t, err)
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)
Expand Down
3 changes: 2 additions & 1 deletion pkg/ottl/ottlfuncs/internal/funcutil/lambda_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading