Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Add array.slice function #1243

Closed
teq0 opened this issue Mar 2, 2019 · 2 comments
Closed

Enhancement: Add array.slice function #1243

teq0 opened this issue Mar 2, 2019 · 2 comments

Comments

@teq0
Copy link
Contributor

teq0 commented Mar 2, 2019

#851 mentions both slice and concat operations on arrays. array.concat was added and #851 was closed, but I believe there is still a valid need for slice.

@Infi-Knight
Copy link
Contributor

I know this was supposed to be a "low-hanging-fruit" but I am stuck and would really appreciate some help:
Going through the code I think that I need to follow these steps:

  1. Register ArraySlice in ast/builtins.go
  2. Implement and register builtinArraySlice in topdown/array.go
  3. Write requisite tests and make sure it works

For the first step I wrote following code:

//Arrays
ArrayConcat,
ArraySlice


// ArraySlice returns a slice of a given array
var ArraySlice = &Builtin{
	Name: "array.slice",
	Decl: types.NewFunction(
		types.Args(
			types.NewArray(nil, types.A),
			types.NewNumber(),
			types.NewNumber(),
		),
		types.NewArray(nil, types.A),
	),
}

For the second step, I have setup the following skeleton:

func builtinArraySlice(a, i, j ast.Value) (ast.Value, error) {

	// initialize required variables

	// arrA, err := builtins.ArrayOperand(a, 1)
	// if err != nil {
	// 	return nil, err
	// }

	startIndex, err := builtins.IntOperand(i, 2)
	if err != nil {
		return nil, err
	}

	stopIndex, err := builtins.IntOperand(j, 3)
	if err != nil {
		return nil, err
	}

	arrB := make(ast.Array, 0, stopIndex-startIndex)

	// implement the logic for slicing

	return arrB, nil

}

Now when I try to register this new function:

func init() {
	RegisterFunctionalBuiltin2(ast.ArrayConcat.Name, builtinArrayConcat)
	RegisterFunctionalBuiltin2(ast.ArraySlice.Name, builtinArraySlice)
}

editor(vscode) hints
following error:
opa

cannot use builtinArraySlice (type func(ast.Value, ast.Value, ast.Value) (ast.Value, error)) as type FunctionalBuiltin2 in argument to RegisterFunctionalBuiltin2

What am I doing wrong here?

@tsandall
Copy link
Member

@Infi-Knight You'll need to use RegisterFunctionalBuiltin3 instead of RegisterFunctionalBuiltin2 here. The former takes accepts 3 parameters while the latter only accepts 2.

Infi-Knight pushed a commit to Infi-Knight/opa that referenced this issue Mar 26, 2019
Fixes open-policy-agent#1243

Signed-off-by: Ravi Soni <soniravi829@gmail.com>
Infi-Knight pushed a commit to Infi-Knight/opa that referenced this issue Mar 28, 2019
Fixes open-policy-agent#1243

Signed-off-by: Ravi Soni <soniravi829@gmail.com>
Infi-Knight pushed a commit to Infi-Knight/opa that referenced this issue Mar 31, 2019
Fixes open-policy-agent#1243

Signed-off-by: Ravi Soni <codespartan09@gmail.com>
tsandall pushed a commit that referenced this issue Apr 3, 2019
Fixes #1243

Signed-off-by: Ravi Soni <codespartan09@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants