Solved comments to return out of bound errors

Author: alexcams

pkg/ottl/ottlfuncs/func_delete.go

@@ -29,14 +29,10 @@ func createDeleteFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (
 		return nil, errors.New("DeleteFactory args must be of type *DeleteArguments[K]")
 	}
 
-	return deleteFrom(args.Target, args.Index, args.Length)
+	return deleteFrom(args.Target, args.Index, args.Length), nil
 }
 
-func deleteFrom[K any](target ottl.GetSetter[K], indexGetter ottl.IntGetter[K], lengthGetter ottl.Optional[ottl.IntGetter[K]]) (ottl.ExprFunc[K], error) {
-	if indexGetter == nil {
-		return nil, errors.New("indexGetter cannot be nil for delete function")
-	}
-
+func deleteFrom[K any](target ottl.GetSetter[K], indexGetter ottl.IntGetter[K], lengthGetter ottl.Optional[ottl.IntGetter[K]]) ottl.ExprFunc[K] {
 	return func(ctx context.Context, tCtx K) (any, error) {
 		t, err := target.Get(ctx, tCtx)
 		if err != nil {
@@ -57,14 +53,6 @@ func deleteFrom[K any](target ottl.GetSetter[K], indexGetter ottl.IntGetter[K],
 			return nil, err
 		}
 
-		if index == -1 {
-			// If we get -1 as index, do nothing and return nil.
-			// Example: If we execute 'delete(attributes["tags"], Index(attributes["tags"], "error"))' statement
-			// and 'error' is not found in tags, Index func will return -1, To avoid errors on this scenario,
-			//  in this case delete function will be a no-op.
-			return nil, target.Set(ctx, tCtx, t)
-		}
-
 		length := int64(1)
 		if !lengthGetter.IsEmpty() {
 			length, err = lengthGetter.Get().Get(ctx, tCtx)
@@ -74,17 +62,14 @@ func deleteFrom[K any](target ottl.GetSetter[K], indexGetter ottl.IntGetter[K],
 		}
 
 		sliceLen := int64(targetSlice.Len())
-		if index < -1 || index >= sliceLen {
-			return nil, fmt.Errorf("index %d out of bounds for slice of length %d", index, sliceLen)
-		}
-
-		if length <= 0 {
-			return nil, fmt.Errorf("length must be positive, got %d", length)
+		endIndex, err := validateBounds(index, length, sliceLen)
+		if err != nil {
+			return nil, err
 		}
 
-		endIndex := index + length
-		if endIndex > sliceLen {
-			endIndex = sliceLen
+		if index == 0 && endIndex == sliceLen {
+			// If deleting all elements, return an empty slice without looping
+			return nil, target.Set(ctx, tCtx, pcommon.NewSlice())
 		}
 
 		resSlice := pcommon.NewSlice()
@@ -96,5 +81,21 @@ func deleteFrom[K any](target ottl.GetSetter[K], indexGetter ottl.IntGetter[K],
 		}
 
 		return nil, target.Set(ctx, tCtx, resSlice)
-	}, nil
+	}
+}
+
+func validateBounds(index, length, sliceLen int64) (endIndex int64, err error) {
+	if index < 0 || index >= sliceLen {
+		return 0, fmt.Errorf("index %d out of bounds for slice of length %d", index, sliceLen)
+	}
+
+	if length <= 0 {
+		return 0, fmt.Errorf("length must be positive, got %d", length)
+	}
+
+	endIndex = index + length
+	if endIndex > sliceLen {
+		return 0, fmt.Errorf("deletion range [%d:%d] out of bounds for slice of length %d", index, endIndex, sliceLen)
+	}
+	return endIndex, nil
 }

pkg/ottl/ottlfuncs/func_delete_test.go

@@ -247,18 +247,6 @@ func TestDelete(t *testing.T) {
 				[]float64{1.1, 2.2, 3.3},
 			}),
 		},
-		{
-			name: "delete with index -1 does nothing",
-			target: ottl.StandardGetSetter[any]{
-				Getter: func(_ context.Context, _ any) (any, error) {
-					return getSlice(t, []any{0, 1, 2, 3, 4, 5}), nil
-				},
-				Setter: setter,
-			},
-			index:    getIntGetter(-1),
-			length:   nilOptional,
-			expected: getSlice(t, []any{0, 1, 2, 3, 4, 5}),
-		},
 		{
 			name: "delete all elements",
 			target: ottl.StandardGetSetter[any]{
@@ -271,18 +259,6 @@ func TestDelete(t *testing.T) {
 			length:   ottl.NewTestingOptional(getIntGetter(5)),
 			expected: getSlice(t, []any{}),
 		},
-		{
-			name: "endIndex exceeds slice length",
-			target: ottl.StandardGetSetter[any]{
-				Getter: func(_ context.Context, _ any) (any, error) {
-					return getSlice(t, []any{0, 1, 2, 3}), nil
-				},
-				Setter: setter,
-			},
-			index:    getIntGetter(1),
-			length:   ottl.NewTestingOptional(getIntGetter(5)),
-			expected: getSlice(t, []any{0}),
-		},
 		{
 			name: "target is pcommon.Value slice",
 			target: ottl.StandardGetSetter[any]{
@@ -305,8 +281,7 @@ func TestDelete(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			exprFunc, err := deleteFrom(tc.target, tc.index, tc.length)
-			assert.NoError(t, err)
+			exprFunc := deleteFrom(tc.target, tc.index, tc.length)
 
 			res := pcommon.NewSlice()
 			result, err := exprFunc(t.Context(), res)
@@ -418,14 +393,37 @@ func TestDelete_Errors(t *testing.T) {
 			length:      ottl.NewTestingOptional(getIntGetter(0)),
 			expectedErr: "length must be positive, got 0",
 		},
+		{
+			name: "deletion range out of bounds",
+			target: ottl.StandardGetSetter[any]{
+				Getter: func(_ context.Context, _ any) (any, error) {
+					return getSlice(t, []any{0, 1, 2, 3}), nil
+				},
+				Setter: setter,
+			},
+			index:       getIntGetter(2),
+			length:      ottl.NewTestingOptional(getIntGetter(5)),
+			expectedErr: "deletion range [2:7] out of bounds",
+		},
+		{
+			name: "negative index",
+			target: ottl.StandardGetSetter[any]{
+				Getter: func(_ context.Context, _ any) (any, error) {
+					return getSlice(t, []any{0, 1, 2, 3}), nil
+				},
+				Setter: setter,
+			},
+			index:       getIntGetter(-1),
+			length:      nilOptional,
+			expectedErr: "index -1 out of bounds",
+		},
 	}
 	for _, etc := range errorTestCases {
 		t.Run(etc.name, func(t *testing.T) {
-			exprFunc, err := deleteFrom(etc.target, etc.index, etc.length)
-			assert.NoError(t, err)
+			exprFunc := deleteFrom(etc.target, etc.index, etc.length)
 
 			res := pcommon.NewSlice()
-			_, err = exprFunc(t.Context(), res)
+			_, err := exprFunc(t.Context(), res)
 			assert.ErrorContains(t, err, etc.expectedErr)
 		})
 	}