Skip to content

Commit c49f81d

Browse files
migoff875migoff875
authored
Merge pull request #43 from CiscoM31/case-statement
Ensure functions return a boolean value in required situations
2 parents e424590 + b0e3c08 commit c49f81d

File tree

4 files changed

+96
-76
lines changed

4 files changed

+96
-76
lines changed

‎expression_parser.go‎

Lines changed: 41 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -121,23 +121,8 @@ func (p *ExpressionParser) ParseExpressionString(ctx context.Context, expression
121121
if tree == nil || tree.Token == nil {
122122
return nil, BadRequestError("Expression cannot be nil")
123123
}
124-
if p.ExpectBoolExpr {
125-
switch tree.Token.Type {
126-
case ExpressionTokenBoolean:
127-
// Valid boolean expression
128-
case ExpressionTokenLogical:
129-
// eq|ne|gt|ge|lt|le|and|or|not|has|in
130-
// Valid boolean expression
131-
case ExpressionTokenFunc:
132-
// We need to know the return type of the function.
133-
// TODO
134-
case ExpressionTokenLambdaNav:
135-
// Lambda Navigation.
136-
// Valid boolean expression
137-
default:
138-
// Not a boolean expression
139-
return nil, BadRequestError("Expression does not return a boolean value")
140-
}
124+
if p.ExpectBoolExpr && !p.isBooleanExpression(tree.Token) {
125+
return nil, BadRequestError("Expression does not return a boolean value")
141126
}
142127
return &GoDataExpression{tree, expression}, nil
143128
}
@@ -288,52 +273,55 @@ func NewExpressionParser() *ExpressionParser {
288273
parser.DefineOperator("and", 2, OpAssociationLeft, 2)
289274
parser.DefineOperator("or", 2, OpAssociationLeft, 1)
290275
parser.DefineOperator("=", 2, OpAssociationRight, 0) // Function argument assignment. E.g. MyFunc(Arg1='abc')
291-
parser.DefineFunction("contains", []int{2})
292-
parser.DefineFunction("endswith", []int{2})
293-
parser.DefineFunction("startswith", []int{2})
294-
parser.DefineFunction("exists", []int{2})
295-
parser.DefineFunction("length", []int{1})
296-
parser.DefineFunction("indexof", []int{2})
297-
parser.DefineFunction("substring", []int{2, 3})
298-
parser.DefineFunction("substringof", []int{2})
299-
parser.DefineFunction("tolower", []int{1})
300-
parser.DefineFunction("toupper", []int{1})
301-
parser.DefineFunction("trim", []int{1})
302-
parser.DefineFunction("concat", []int{2})
303-
parser.DefineFunction("year", []int{1})
304-
parser.DefineFunction("month", []int{1})
305-
parser.DefineFunction("day", []int{1})
306-
parser.DefineFunction("hour", []int{1})
307-
parser.DefineFunction("minute", []int{1})
308-
parser.DefineFunction("second", []int{1})
309-
parser.DefineFunction("fractionalseconds", []int{1})
310-
parser.DefineFunction("date", []int{1})
311-
parser.DefineFunction("time", []int{1})
312-
parser.DefineFunction("totaloffsetminutes", []int{1})
313-
parser.DefineFunction("now", []int{0})
314-
parser.DefineFunction("maxdatetime", []int{0})
315-
parser.DefineFunction("mindatetime", []int{0})
316-
parser.DefineFunction("totalseconds", []int{1})
317-
parser.DefineFunction("round", []int{1})
318-
parser.DefineFunction("floor", []int{1})
319-
parser.DefineFunction("ceiling", []int{1})
320-
parser.DefineFunction("isof", []int{1, 2}) // isof function can take one or two arguments.
321-
parser.DefineFunction("cast", []int{2})
322-
parser.DefineFunction("geo.distance", []int{2})
276+
parser.DefineFunction("contains", []int{2}, true)
277+
parser.DefineFunction("endswith", []int{2}, true)
278+
parser.DefineFunction("startswith", []int{2}, true)
279+
parser.DefineFunction("exists", []int{2}, true)
280+
parser.DefineFunction("length", []int{1}, false)
281+
parser.DefineFunction("indexof", []int{2}, false)
282+
parser.DefineFunction("substring", []int{2, 3}, false)
283+
parser.DefineFunction("substringof", []int{2}, false)
284+
parser.DefineFunction("tolower", []int{1}, false)
285+
parser.DefineFunction("toupper", []int{1}, false)
286+
parser.DefineFunction("trim", []int{1}, false)
287+
parser.DefineFunction("concat", []int{2}, false)
288+
parser.DefineFunction("year", []int{1}, false)
289+
parser.DefineFunction("month", []int{1}, false)
290+
parser.DefineFunction("day", []int{1}, false)
291+
parser.DefineFunction("hour", []int{1}, false)
292+
parser.DefineFunction("minute", []int{1}, false)
293+
parser.DefineFunction("second", []int{1}, false)
294+
parser.DefineFunction("fractionalseconds", []int{1}, false)
295+
parser.DefineFunction("date", []int{1}, false)
296+
parser.DefineFunction("time", []int{1}, false)
297+
parser.DefineFunction("totaloffsetminutes", []int{1}, false)
298+
parser.DefineFunction("now", []int{0}, false)
299+
parser.DefineFunction("maxdatetime", []int{0}, false)
300+
parser.DefineFunction("mindatetime", []int{0}, false)
301+
parser.DefineFunction("totalseconds", []int{1}, false)
302+
parser.DefineFunction("round", []int{1}, false)
303+
parser.DefineFunction("floor", []int{1}, false)
304+
parser.DefineFunction("ceiling", []int{1}, false)
305+
parser.DefineFunction("isof", []int{1, 2}, true) // isof function can take one or two arguments.
306+
parser.DefineFunction("cast", []int{2}, false)
307+
parser.DefineFunction("geo.distance", []int{2}, false)
323308
// The geo.intersects function has the following signatures:
324309
// Edm.Boolean geo.intersects(Edm.GeographyPoint,Edm.GeographyPolygon)
325310
// Edm.Boolean geo.intersects(Edm.GeometryPoint,Edm.GeometryPolygon)
326311
// The geo.intersects function returns true if the specified point lies within the interior
327312
// or on the boundary of the specified polygon, otherwise it returns false.
328-
parser.DefineFunction("geo.intersects", []int{2})
313+
parser.DefineFunction("geo.intersects", []int{2}, false)
329314
// The geo.length function has the following signatures:
330315
// Edm.Double geo.length(Edm.GeographyLineString)
331316
// Edm.Double geo.length(Edm.GeometryLineString)
332317
// The geo.length function returns the total length of its line string parameter
333318
// in the coordinate reference system signified by its SRID.
334-
parser.DefineFunction("geo.length", []int{1})
335-
parser.DefineFunction("any", []int{0, 2}) // 'any' can take either zero or one argument.
336-
parser.DefineFunction("all", []int{2})
319+
parser.DefineFunction("geo.length", []int{1}, false)
320+
// 'any' can take either zero or two arguments with the later having the form any(d:d/Prop eq 1).
321+
// Godata interprets the colon as an argument delimiter and considers the function to have two arguments.
322+
parser.DefineFunction("any", []int{0, 2}, true)
323+
// 'all' requires two arguments of a form similar to 'any'.
324+
parser.DefineFunction("all", []int{2}, true)
337325

338326
return parser
339327
}

‎expression_parser_test.go‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,18 +123,15 @@ func TestValidBooleanExpressionSyntax(t *testing.T) {
123123
"ceiling(Freight) eq 33",
124124
"Rating mod 5 eq 0",
125125
"Price div 2 eq 3",
126-
// Type functions
126+
// Functions
127+
"contains(Name,'Ted')",
128+
"startswith(Name,'Ted')",
129+
"endswith(Name,'Lasso')",
127130
"isof(ShipCountry,Edm.String)",
128131
"isof(NorthwindModel.BigOrder)",
129-
"cast(ShipCountry,Edm.String)",
130132
// Parameter aliases
131133
// See http://docs.oasis-open.org/odata/odata/v4.0/errata03/os/complete/part1-protocol/odata-v4.0-errata03-os-part1-protocol-complete.html#_Toc453752288
132134
"Region eq @p1", // Aliases start with @
133-
// Geo functions
134-
"geo.distance(CurrentPosition,TargetPosition)",
135-
"geo.length(DirectRoute)",
136-
"geo.intersects(Position,TargetArea)",
137-
"GEO.INTERSECTS(Position,TargetArea)", // functions are case insensitive in ODATA 4.0.1
138135
// Logical operators
139136
"'Milk' eq 'Milk'", // Compare two literals
140137
"'Water' ne 'Milk'", // Compare two literals
@@ -240,6 +237,15 @@ func TestInvalidBooleanExpressionSyntax(t *testing.T) {
240237
"City", // Just a single literal
241238
"Tags/any(var:var/Key eq 'Site') orTags/any(var:var/Key eq 'Site')",
242239
"contains(Name, 'a', 'b', 'c', 'd')", // Too many function arguments
240+
"cast(ShipCountry,Edm.String)",
241+
// Geo functions
242+
"geo.distance(CurrentPosition,TargetPosition)",
243+
"geo.length(DirectRoute)",
244+
"geo.intersects(Position,TargetArea)",
245+
"GEO.INTERSECTS(Position,TargetArea)", // functions are case insensitive in ODATA 4.0.1
246+
"now()",
247+
"tolower(Name)",
248+
"concat(First,Last)",
243249
}
244250
p := NewExpressionParser()
245251
p.ExpectBoolExpr = true

‎parser.go‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ func (o *Operator) WithListExprPreference(v bool) *Operator {
233233
type Function struct {
234234
Token string // The function token
235235
Params []int // The number of parameters this function accepts
236+
ReturnsBool bool // Indicates if the function has a boolean return value
236237
}
237238

238239
type ParseNode struct {
@@ -291,11 +292,12 @@ func (p *Parser) DefineOperator(token string, operands, assoc, precedence int) *
291292
return op
292293
}
293294

294-
// DefineFunction adds a function to the language
295-
// params is the number of parameters this function accepts
296-
func (p *Parser) DefineFunction(token string, params []int) *Function {
295+
// DefineFunction adds a function to the language.
296+
// - params is the number of parameters this function accepts
297+
// - returnsBool indicates if the function has a boolean return value
298+
func (p *Parser) DefineFunction(token string, params []int, returnsBool bool) *Function {
297299
sort.Sort(sort.Reverse(sort.IntSlice(params)))
298-
f := &Function{token, params}
300+
f := &Function{token, params, returnsBool}
299301
p.Functions[token] = f
300302
return f
301303
}
@@ -304,6 +306,7 @@ func (p *Parser) DefineFunction(token string, params []int) *Function {
304306
type CustomFunctionInput struct {
305307
Name string // case-insensitive function name
306308
NumParams []int // number of allowed parameters
309+
ReturnsBool bool // indicates if the function has a boolean return value
307310
}
308311

309312
// DefineCustomFunctions introduces additional function names to be considered as legal function
@@ -322,7 +325,7 @@ func DefineCustomFunctions(functions []CustomFunctionInput) error {
322325
return fmt.Errorf("custom function '%s' may not override odata operator", name)
323326
}
324327

325-
GlobalExpressionParser.DefineFunction(name, v.NumParams)
328+
GlobalExpressionParser.DefineFunction(name, v.NumParams, v.ReturnsBool)
326329
funcNames = append(funcNames, name)
327330
}
328331

@@ -362,6 +365,29 @@ func (p *Parser) isOperator(token *Token) bool {
362365
return ok
363366
}
364367

368+
// isBooleanExpression returns True when the expression token 't' has a resulting boolean value
369+
func (p *Parser) isBooleanExpression(t *Token) bool {
370+
switch t.Type {
371+
case ExpressionTokenBoolean:
372+
// Valid boolean expression
373+
case ExpressionTokenLogical:
374+
// eq|ne|gt|ge|lt|le|and|or|not|has|in
375+
// Valid boolean expression
376+
case ExpressionTokenFunc:
377+
// Depends on function return type
378+
f := p.Functions[t.Value]
379+
if !f.ReturnsBool {
380+
return false
381+
}
382+
case ExpressionTokenLambdaNav:
383+
// Lambda Navigation.
384+
// Valid boolean expression
385+
default:
386+
return false
387+
}
388+
return true
389+
}
390+
365391
// InfixToPostfix parses the input string of tokens using the given definitions of operators
366392
// and functions.
367393
// Everything else is assumed to be a literal.

‎parser_test.go‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ import (
99
func TestPEMDAS(t *testing.T) {
1010
ctx := context.Background()
1111
parser := EmptyParser()
12-
parser.DefineFunction("sin", []int{1})
13-
parser.DefineFunction("max", []int{2})
12+
parser.DefineFunction("sin", []int{1}, false)
13+
parser.DefineFunction("max", []int{2}, false)
1414
parser.DefineOperator("^", 2, OpAssociationRight, 5)
1515
parser.DefineOperator("*", 2, OpAssociationLeft, 5)
1616
parser.DefineOperator("/", 2, OpAssociationLeft, 5)
@@ -64,8 +64,8 @@ func TestPEMDAS(t *testing.T) {
6464
func BenchmarkPEMDAS(b *testing.B) {
6565
ctx := context.Background()
6666
parser := EmptyParser()
67-
parser.DefineFunction("sin", []int{1})
68-
parser.DefineFunction("max", []int{2})
67+
parser.DefineFunction("sin", []int{1}, false)
68+
parser.DefineFunction("max", []int{2}, false)
6969
parser.DefineOperator("^", 2, OpAssociationRight, 5)
7070
parser.DefineOperator("*", 2, OpAssociationLeft, 5)
7171
parser.DefineOperator("/", 2, OpAssociationLeft, 5)
@@ -145,9 +145,9 @@ func TestBoolean(t *testing.T) {
145145
func TestFunc(t *testing.T) {
146146
ctx := context.Background()
147147
parser := EmptyParser()
148-
parser.DefineFunction("sin", []int{1})
149-
parser.DefineFunction("max", []int{2})
150-
parser.DefineFunction("volume", []int{3})
148+
parser.DefineFunction("sin", []int{1}, false)
149+
parser.DefineFunction("max", []int{2}, false)
150+
parser.DefineFunction("volume", []int{3}, false)
151151
parser.DefineOperator("^", 2, OpAssociationRight, 5)
152152
parser.DefineOperator("*", 2, OpAssociationLeft, 5)
153153
parser.DefineOperator("/", 2, OpAssociationLeft, 5)
@@ -216,8 +216,8 @@ func TestFunc(t *testing.T) {
216216
func TestTree(t *testing.T) {
217217
ctx := context.Background()
218218
parser := EmptyParser()
219-
parser.DefineFunction("sin", []int{1})
220-
parser.DefineFunction("max", []int{2})
219+
parser.DefineFunction("sin", []int{1}, false)
220+
parser.DefineFunction("max", []int{2}, false)
221221
parser.DefineOperator("^", 2, OpAssociationRight, 5)
222222
parser.DefineOperator("*", 2, OpAssociationLeft, 5)
223223
parser.DefineOperator("/", 2, OpAssociationLeft, 5)
@@ -280,8 +280,8 @@ func TestTree(t *testing.T) {
280280
func BenchmarkBuildTree(b *testing.B) {
281281
ctx := context.Background()
282282
parser := EmptyParser()
283-
parser.DefineFunction("sin", []int{1})
284-
parser.DefineFunction("max", []int{2})
283+
parser.DefineFunction("sin", []int{1}, false)
284+
parser.DefineFunction("max", []int{2}, false)
285285
parser.DefineOperator("^", 2, OpAssociationRight, 5)
286286
parser.DefineOperator("*", 2, OpAssociationLeft, 5)
287287
parser.DefineOperator("/", 2, OpAssociationLeft, 5)

0 commit comments

Comments
 (0)