Feature : parser config

[faascape]

In order to be able to use the parser in different contexts, it would be nice to set some parameters enabling / disabling some features.

For example :

var config map[string]string
config["stringOps"] = "false"
config["arrayOps"] = "false"
config["floatRound"] = "10"

eval := goval.NewEvaluator(config)

no operation on string allowed no operation on array allowed all float values are rounded to 10 decimals (useful for comparison, we can use function for that but the expression is less simple to input)

These features should only impact the ParserUtils.go file and not the parser at all.

If you are interested, i can work on some PR.

Thank you.

[maja42]

I really like the idea of having an optional configuration to disable certain functionality.

The actual configuration options are, however, very limited. Ideally, if a user uses a disabled feature, the error messages look the same as if the feature was not implemented at all - without changing the underlying parser.

It's certainly doable for things like concatenation. Which operations would you count to "stringOps" and "arrayOps"?

Regarding "floatRound": Usually, float comparisons are always implemented by explicitly calling a function and passing an epsilon. But I don't see a reason why we shouldn't do that in the == operator automatically. Instead of specifying the number of decimals, we should propably use an epsilon value (which must be >=0 and <1).

Also, instead of a config map, it should be a struct for type safety. And the options should be set via eval.Configure(conf) for backwards compatibility.

Yes, a pull request would be great.

[faascape]

Great :-)

When i say "disable stringOps or arrayOps", that means that we don't want to accept string or array representation in expression.

For example, if we disable string operations (because we want our user to input only numerical literals and variables) all string values will trigger an error during evaluation (in ParserUtils.go).

That also helps if we want a typed environment, i do not want the user the be able to generate value of some types.

I'll work on that and propose you something.

[maja42]

Do you want to forbid objects too? In that case your variables could be flat (numbers only), and it might actually be better for you to change the lexer to forbid string literals, brackets and dots. Otherwise, the user would still be able to use string or array literals and return them, as long as he uses no operator.

If you don't want to forbid objects, you can still return non-number literals but forbid things like var["field" + fieldNumber], which is inconsistent.

[faascape]

For me an object is like a variable, what is important is the type of the field.

So I don't think I need to forbid object, but if I don't want string, I'll check that Foo["bar"] is no string. "bar" is not a string by itself in the expression but a way to reach the final value used in the expression.

Le mar. 7 août 2018 à 07:01, maja42 notifications@github.com a écrit :

Do you also want to forbid objects too? In that case your variables could be flat (numbers only), and it might actually be better for you to change the lexer to forbid string literals, brackets and dots. Otherwise, the user would still be able to use string or array literals and return them, as long as he uses no operator.

If you don't want to forbid objects, you can still return non-number literals but forbid things like var["field" + fieldNumber], which is inconsistent.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/maja42/goval/issues/3#issuecomment-410933921, or mute the thread https://github.com/notifications/unsubscribe-auth/AKbwaT8QgodWz2GbCesEZ-jgHZzLOrAHks5uOR84gaJpZM4VwOIC .

[maja42]

Still, the library does not forbid the expression "text", or more complex things like ["field1", "field2"][0] or {"key": "value"}["key"], and therefore the expression result could be of type string. So you would need to type-check the resulting value anyways. The same happens if the expression does not access the field of an object-variable, in which case the result would be the object (variable) itself.

In one of my own applications, I have the same issue where the result of an expression must always be a string. I use this:

res, _ := eval.Evaluate(expr, vars, funcs)
resStr, ok := res.(string)
if !ok {
    return fmt.Errorf("evaluation error: expected type string, but got %s", typeOf(res))
}`

Since you want to use objects and access fields with string-literals, you can't avoid getting string-results and need to write such a type-check anyways - in which case I don't think that forbidding certain string-operators is a good idea.

Alternatively, it would be possible to configure the expected result type. So the Evaluate() function will perform the above check automatically before returning.

[maja42]

Hi, sorry for the long delay. I thought quite some time about the issue, and I don't like the idea about a dedicated configuration for each possible return type. Most people who want to limit the type - including you - will probably want to limit it to one specific type. And in this case, a dedicated type assertion outside the library is more readable than a configuration struct - which might get expanded unexpectedly if a new feature or type is added.

[maja42]

Regarding floating point calculations: Adding an epsilon to the equals method should be trivial, but operator overloading is also an option (the user can replace any operator-method with a custom implementation). This would, however, increase the complexity tremendously.

[maja42]

Closing: I like the simplicity and correctness of the current implementation and adding a configuration object is out-of-scope.

The main issue with configurations is that there are many features that we might want to configure - but we simply can't. The approach if this libary is to evaluate the result of an expression while parsing it. This reduces complexity and helps with performance tremendously because we don't need an intermediate AST representation. It would be easy to handle different configurations when evaluating an AST. But in goval's case, we would pretty quickly come into a situation where a specific configuration requires a dedicated yacc file, effectively duplicating the source code.