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

Math and comparisons #23

Merged
merged 51 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
c9a750f
Added functions to grammar only, and removed () around sub-expr
jaredoconnell Nov 22, 2023
5d1b5ff
Finished evaluation of expressions
jaredoconnell Dec 1, 2023
9374a44
Added support for function type and dependency resolution
jaredoconnell Dec 7, 2023
96b65c1
Fix missing NoError assertions
jaredoconnell Dec 7, 2023
b2638b2
Update to latest SDK
jaredoconnell Dec 14, 2023
49fa66c
Finished grammar for comparisons, logical operators, math, and booleans
jaredoconnell Dec 16, 2023
a6c53e7
Fix test and tree comment
jaredoconnell Dec 18, 2023
2e958c5
Added test for eat failure case
jaredoconnell Dec 18, 2023
01bb0f7
Fix test case broken by recent changes
jaredoconnell Dec 18, 2023
5c7fc3a
Added initial eval for integer math, and added floats
jaredoconnell Dec 19, 2023
4154408
Merge branch 'main' into math-and-comparisons
jaredoconnell Dec 20, 2023
6bfaaf3
Added evaluation of operators with bools or strings
jaredoconnell Dec 20, 2023
94782ce
Added tests for binary operator evaluation
jaredoconnell Dec 21, 2023
d65ba81
Added unary operator evaluation
jaredoconnell Dec 21, 2023
53319d3
Revert change to support older go versions
jaredoconnell Dec 21, 2023
eefe8f9
Properly track dependencies
jaredoconnell Jan 5, 2024
048a910
Properly handle paths with extraneous data
jaredoconnell Jan 6, 2024
aeff7ac
Fix missed error check
jaredoconnell Jan 6, 2024
db5aabe
Improved test converage, fixed bugs
jaredoconnell Jan 9, 2024
781c026
Reduce redundancy, improve clarity, and better extraneous path solution
jaredoconnell Jan 10, 2024
0048692
Remove old code
jaredoconnell Jan 10, 2024
4aed823
Merge branch 'main' into math-and-comparisons
jaredoconnell Jan 10, 2024
858f1d9
Merge branch 'improved-dependency-resolution' into math-and-comparisons
jaredoconnell Jan 10, 2024
78b453d
Untested dependency eval for binary and unary ops
jaredoconnell Jan 11, 2024
5e115e3
Merge branch 'main' into math-and-comparisons
jaredoconnell Jan 19, 2024
65e2c81
Finish tests for type and dependency resolution
jaredoconnell Jan 19, 2024
0e615e8
Undo line deletion
jaredoconnell Jan 19, 2024
5869188
Undo line deletion
jaredoconnell Jan 19, 2024
4b40f22
Fix copy-paste comment error
jaredoconnell Jan 20, 2024
1012a08
Added missing not logic
jaredoconnell Jan 20, 2024
d819297
Addressed review comments
jaredoconnell Jan 23, 2024
c534f4f
Addressed review comments
jaredoconnell Jan 24, 2024
d33870d
Addressed review feedback
jaredoconnell Jan 25, 2024
5b6023b
Addressed review comments
jaredoconnell Jan 27, 2024
34092b9
Addressed review comments
jaredoconnell Jan 30, 2024
2b0caa1
Fix linting error
jaredoconnell Jan 30, 2024
beaa826
Addressed review comments for binary operations
jaredoconnell Jan 30, 2024
5328af0
Addressed review comments
jaredoconnell Jan 30, 2024
666d8db
Address review comments
jaredoconnell Jan 31, 2024
6d3984e
Update CI
jaredoconnell Jan 31, 2024
efa9f37
Update dependencies
jaredoconnell Jan 31, 2024
18e938d
Fix CI-specific linter error
jaredoconnell Jan 31, 2024
b098ddc
Revert "Fix CI-specific linter error"
jaredoconnell Jan 31, 2024
01837cf
Merge branch 'main' into math-and-comparisons
jaredoconnell Feb 1, 2024
3f8150f
Use main's CI config
jaredoconnell Feb 1, 2024
2824b2c
Switched to my local Golang version
jaredoconnell Feb 1, 2024
954e2b5
Setup go version in lint job
jaredoconnell Feb 1, 2024
77f455f
Address review comments
jaredoconnell Feb 1, 2024
c8a734d
Address review comment
jaredoconnell Feb 1, 2024
1cd13a9
Addressed final review comments
jaredoconnell Feb 1, 2024
0a7da10
Use variable for go version in CI
jaredoconnell Feb 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions .github/workflows/test_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@ on:
branches:
- main
pull_request:

env:
go_version: 1.21.6
jobs:
golangci-lint:
name: golangci-lint
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4
- uses: actions/setup-go@v4
with:
go-version: ${{ env.go_version }}
cache: false
- name: Run golangci-lint
uses: golangci/golangci-lint-action@3a919529898de77ec3da873e3063ca4b10e7f5cc # v3
with:
Expand All @@ -25,7 +30,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@0c52d547c9bc32b1aa3301fd7a9cb496313a4491 # v5
with:
go-version: 1.18
go-version: ${{ env.go_version }}
- name: Set up gotestfmt
uses: GoTestTools/gotestfmt-action@v2
- uses: actions/cache@704facf57e6136b1bc63b828d79edcd491f0ee84 # v3
Expand Down
20 changes: 20 additions & 0 deletions expression_data_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ var testScope = schema.NewScopeSchema(
nil,
nil,
),
"simple_int_2": schema.NewPropertySchema(
schema.NewIntSchema(nil, nil, nil),
nil,
true,
nil,
nil,
nil,
nil,
nil,
),
"simple_any": schema.NewPropertySchema(
schema.NewAnySchema(),
nil,
Expand All @@ -77,6 +87,16 @@ var testScope = schema.NewScopeSchema(
nil,
nil,
),
"simple_bool": schema.NewPropertySchema(
schema.NewBoolSchema(),
nil,
true,
nil,
nil,
nil,
nil,
nil,
),
"int_list": schema.NewPropertySchema(
schema.NewListSchema(
schema.NewIntSchema(nil, nil, nil),
Expand Down
141 changes: 141 additions & 0 deletions expression_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"go.flow.arcalot.io/expressions/internal/ast"
"go.flow.arcalot.io/pluginsdk/schema"
"slices"
)

// dependencyContext holds the root data for a dependency evaluation in an expression. This is useful so that we
Expand Down Expand Up @@ -74,13 +75,153 @@ func (c *dependencyContext) dependencies(
return &dependencyResult{resolvedType: schema.NewStringSchema(nil, nil, nil)}, nil
case *ast.IntLiteral:
return &dependencyResult{resolvedType: schema.NewIntSchema(nil, nil, nil)}, nil
case *ast.FloatLiteral:
return &dependencyResult{resolvedType: schema.NewFloatSchema(nil, nil, nil)}, nil
case *ast.BooleanLiteral:
return &dependencyResult{resolvedType: schema.NewBoolSchema()}, nil
case *ast.BinaryOperation:
return c.binaryOperationDependencies(n)
case *ast.UnaryOperation:
return c.unaryOperationDependencies(n)
case *ast.FunctionCall:
return c.functionDependencies(n)
default:
return nil, fmt.Errorf("unsupported AST node type: %T", n)
}
}

func (c *dependencyContext) unaryOperationDependencies(
node *ast.UnaryOperation,
) (*dependencyResult, error) {
// Unary operations don't change dependencies, but the type must be validated.
innerResult, err := c.rootDependencies(node.RightNode)
if err != nil {
return nil, err
}
switch node.LeftOperation {
case ast.Subtract:
// Negation expects a numerical type
if innerResult.resolvedType.TypeID() != schema.TypeIDInt &&
innerResult.resolvedType.TypeID() != schema.TypeIDFloat {
return nil,
fmt.Errorf("attempted negation operation on non-numeric type %q",
innerResult.resolvedType.TypeID())
}
case ast.Not:
// 'not' expects a boolean input
if innerResult.resolvedType.TypeID() != schema.TypeIDBool {
return nil,
fmt.Errorf("attempted 'not' operation on non-boolean type %q",
innerResult.resolvedType.TypeID())
}
default:
return nil, fmt.Errorf("unsupported unary operation: %q", node.LeftOperation)
}
// Negation and 'not' do not change the type or dependencies.
return innerResult, nil
}

func (c *dependencyContext) binaryOperationDependencies(
node *ast.BinaryOperation,
) (*dependencyResult, error) {
leftResult, err := c.rootDependencies(node.LeftNode)
if err != nil {
return nil, err
}
// Right dependencies, using left type.
rightResult, err := c.rootDependencies(node.RightNode)
if err != nil {
return nil, err
}
var resultType schema.Type
// Validate operations with the resolved type, and compute the return type for the combination.
switch node.Operation {
case ast.Add:
// Add or concatenate
err = validateValidBinaryOpTypes(
node,
leftResult.resolvedType.TypeID(),
rightResult.resolvedType.TypeID(),
[]schema.TypeID{schema.TypeIDInt, schema.TypeIDFloat, schema.TypeIDString},
)
resultType = leftResult.resolvedType
case ast.Subtract, ast.Multiply, ast.Divide, ast.Modulus, ast.Power:
// Math. Same as type going in. Plus validate that it's numeric.
err = validateValidBinaryOpTypes(
node,
leftResult.resolvedType.TypeID(),
rightResult.resolvedType.TypeID(),
[]schema.TypeID{schema.TypeIDInt, schema.TypeIDFloat},
)
resultType = leftResult.resolvedType
case ast.And, ast.Or:
// Boolean operations. Bool in and out.
err = validateValidBinaryOpTypes(
node,
leftResult.resolvedType.TypeID(),
rightResult.resolvedType.TypeID(),
[]schema.TypeID{schema.TypeIDBool},
)
resultType = schema.NewBoolSchema()
case ast.GreaterThan, ast.LessThan, ast.GreaterThanEqualTo, ast.LessThanEqualTo:
// Inequality. Int, float, or string in; bool out.
err = validateValidBinaryOpTypes(
node,
leftResult.resolvedType.TypeID(),
rightResult.resolvedType.TypeID(),
[]schema.TypeID{schema.TypeIDInt, schema.TypeIDString, schema.TypeIDFloat},
)
resultType = schema.NewBoolSchema()
case ast.EqualTo, ast.NotEqualTo:
// Equality comparison. Any supported type in. Bool out.
err = validateValidBinaryOpTypes(
node,
leftResult.resolvedType.TypeID(),
rightResult.resolvedType.TypeID(),
[]schema.TypeID{schema.TypeIDInt, schema.TypeIDString, schema.TypeIDFloat, schema.TypeIDBool},
)
resultType = schema.NewBoolSchema()
case ast.Invalid:
panic(fmt.Errorf("attempted to perform invalid operation (binary operation type invalid)"))
default:
panic(fmt.Errorf("bug: binary operation %s missing from dependency evaluation code", node.Operation))
}
if err != nil {
return nil, err
}
// Combine the left and right dependencies.
finalDependencies := append(leftResult.completedPaths, rightResult.completedPaths...)
return &dependencyResult{
resolvedType: resultType,
rootPathResult: nil, // Cannot be chained. It's a primitive.
completedPaths: finalDependencies,
}, nil
}

func validateValidBinaryOpTypes(
node *ast.BinaryOperation,
leftType schema.TypeID,
rightType schema.TypeID,
expectedTypes []schema.TypeID,
) error {
// First validate left and right types are within the expected types.
leftIsValid := slices.Contains(expectedTypes, leftType)
if !leftIsValid {
return fmt.Errorf("invalid type %q from left expression %q for binary operation %q; expected one of %q",
leftType, node.LeftNode.String(), node.Operation, expectedTypes)
}
rightIsValid := slices.Contains(expectedTypes, rightType)
if !rightIsValid {
return fmt.Errorf("invalid type %q from right expression %q for binary operation %q; expected one of %q",
rightType, node.RightNode.String(), node.Operation, expectedTypes)
}
// Next, validate that left and right types match
if leftType != rightType {
return fmt.Errorf("left (%s) and right (%s) types do not match for binary expression %q", leftType, rightType, node.String())
}
return nil
}

func (c *dependencyContext) functionDependencies(node *ast.FunctionCall) (*dependencyResult, error) {
// Get the types and dependencies of all parameters.
functionSchema, found := c.functions[node.FuncIdentifier.IdentifierName]
Expand Down
150 changes: 150 additions & 0 deletions expression_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,153 @@ func TestFunctionDependencyResolution_dynamicTyping(t *testing.T) {
assert.Error(t, err)
assert.Contains(t, err.Error(), "unsupported data type")
}

func TestDependencyResolution_MathHomogeneousLiterals(t *testing.T) {
// Test simple literal integer math, same type
expr, err := expressions.New("5 + 5")
webbnh marked this conversation as resolved.
Show resolved Hide resolved
assert.NoError(t, err)
pathWithExtra, err := expr.Dependencies(nil, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(pathWithExtra), 0)
dbutenhof marked this conversation as resolved.
Show resolved Hide resolved
}

func TestDependencyResolution_MathHomogeneousReference(t *testing.T) {
// Test simple reference integer math, same type
expr, err := expressions.New("5 + $.simple_int")
webbnh marked this conversation as resolved.
Show resolved Hide resolved
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_int")
// Swapped
expr, err = expressions.New("$.simple_int + 5")
assert.NoError(t, err)
paths, err = expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_int")
}

func TestDependencyResolution_MathSameDependency(t *testing.T) {
// Test simple reference integer math, with both items having the same dependency
expr, err := expressions.New("$.simple_int + $.simple_int")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_int")
}

func TestDependencyResolution_MathDifferentDependencies(t *testing.T) {
// Test math with two references, with different references.
// Tests the duplicate removal logic, and the left and right paths.
expr, err := expressions.New("$.simple_int + $.simple_int_2")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 2)
assert.SliceContainsExtractor(t, pathStrExtractor, "$.simple_int", paths)
assert.SliceContainsExtractor(t, pathStrExtractor, "$.simple_int_2", paths)
}

func TestDependencyResolution_UnaryLiteral(t *testing.T) {
// Test unary operation with literals.
expr, err := expressions.New("-5")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 0)
}

func TestDependencyResolution_UnaryReference(t *testing.T) {
// Test unary operation with references.
expr, err := expressions.New("-$.simple_int")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_int")
}

func TestDependencyResolution_TestBinaryComparisonLiterals(t *testing.T) {
// Test comparison with literals
expr, err := expressions.New("4 == 5")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 0)
}

func TestDependencyResolution_TestBinaryComparisonReferences(t *testing.T) {
// Test comparison with references
expr, err := expressions.New("$.simple_int == 5")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_int")
}

func TestDependencyResolution_TestBooleanOperationLiterals(t *testing.T) {
// Test boolean operations with literals
expr, err := expressions.New("true && false")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 0)
}

func TestDependencyResolution_TestBooleanOperationReferences(t *testing.T) {
// Test boolean operations with references
expr, err := expressions.New("true && $.simple_bool")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, nil, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_bool")
}

func TestDependencyResolution_TestMixedMathAndFunc(t *testing.T) {
// Test dependencies properly propagated from a function through an operation.
intInFunc, err := schema.NewCallableFunction(
"intToFloat",
[]schema.Type{schema.NewIntSchema(nil, nil, nil)},
schema.NewFloatSchema(nil, nil, nil),
false,
nil,
func(a int64) float64 {
return float64(a)
},
)
assert.NoError(t, err)
funcMap := map[string]schema.Function{"intToFloat": intInFunc}

expr, err := expressions.New("5.0 + intToFloat($.simple_int)")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, funcMap, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 1)
assert.Equals(t, paths[0].String(), "$.simple_int")
}

func TestDependencyResolution_TestMixedOperations(t *testing.T) {
// Test different types of operations that take different types.
intInFunc, err := schema.NewCallableFunction(
"giveFloat",
[]schema.Type{},
schema.NewFloatSchema(nil, nil, nil),
false,
nil,
func() float64 {
return 5.5
},
)
assert.NoError(t, err)
funcMap := map[string]schema.Function{"giveFloat": intInFunc}

expr, err := expressions.New("1.0 == (5.0 / giveFloat()) && !true")
assert.NoError(t, err)
paths, err := expr.Dependencies(testScope, funcMap, nil, fullDataRequirements)
assert.NoError(t, err)
assert.Equals(t, len(paths), 0)
}