Null handling changed from version 2 to 3

[Coder3333]

I have this code in my application, and it worked properly with every version of NCalc prior to 3.0. Now, this same code throws an exception when evaluating the expression.

It seems to be ok if I remove the {} brackets. Was something changed to make curly braces treated differently?

            var expression = "{null} = 'WrongValue'";
            var eap = new NCalc.Expression(expression
    // need to indicate NoCache in order to avoid threading issues
    , NCalc.EvaluateOptions.NoCache);

            eap.EvaluateParameter += delegate (string name, NCalc.ParameterArgs args)
            {
                if (name == "null")
                {
                    // if 'null' appears in an expression, it is considered a parameter, so must replace it with
                    // a string.  This will be an issue if comparing "null = 'null'", as it would evaluate to true, instead
                    // of false.  will need to investigate if there is a better way to handle null
                    args.Result = "null";
                }
            };
            var evaluated = eap.Evaluate();

Exception:

  Message: 
NCalc.EvaluationException : token recognition error at: '{':1:0
token recognition error at: '}':1:5
---- NCalc.EvaluationException : token recognition error at: '{':1:0
token recognition error at: '}':1:5

  Stack Trace: 
Expression.Evaluate()
ExpressionTests.DemonstrateNullException() line 92
----- Inner Stack Trace -----
Expression.Compile(String expression, Boolean nocache)
Expression.HasErrors()

``

[Bykiev]

Hi, curly brackets are not officially supported by NCalc, you should replace it with square brackets

[Coder3333]

Hi, curly brackets are not officially supported by NCalc, you should replace it with square brackets

Was there a reason this changed from version 2.2 to 3? There was never an issue before.

[Bykiev]

I believe it's due to Antlr upgrade, don't see the exact reason though. @david-brink-talogy, do you have any ideas or can we restore the old behavior?

[david-brink-talogy]

Agree it does seem to be an issue after the antlr4, though I'm not sure why this worked before since braces aren't supported in the grammar from what I see. This isn't an issue with null handling, as {1+2}*3 works in ncalc-coreclr v2, but not v3. The really fun part is that the expression evaluates to 7 so they're not treated like parenthesis.

I'd suggest not adding support for braces unless there's a good reason; especially if support was never really intended. I also think there should be a good explanation of how they should behave mathematically.

Having said that, they could be stripped by adding them to the whitespace (WS) definition.

[Bykiev]

Agree it does seem to be an issue after the antlr4, though I'm not sure why this worked before since braces aren't supported in the grammar from what I see. This isn't an issue with null handling, as {1+2}*3 works in ncalc-coreclr v2, but not v3. The really fun part is that the expression evaluates to 7 so they're not treated like parenthesis.

I'd suggest not adding support for braces unless there's a good reason; especially if support was never really intended. I also think there should be a good explanation of how they should behave mathematically.

Having said that, they could be stripped by adding them to the whitespace (WS) definition.

Treating curly braces as whitespace seems to be a good idea. Still don't understand how curly braces handled in Antlr in math operations. I think we can ask the question in Antlr repository to better understand what is going under the hood

[david-brink-talogy]

Antlr is a lexer/parser library. It doesn't handle math operations or any other specific operation. Antlr defines a grammar syntax which can be used to parse a mathematical language (like ncalc); languages like C, SQL, etc; or any other domain specific language someone may design. The ncalc grammar defines tokens and how they are stitched together to form an expression. Antlr is then used to generate a C# parser from this grammar which the ncalc library sits on top of.

I'm not convinced treating curly braces as whitespace is a good idea. {{}{1}{{{}}}}}+{}{}2 doesn't seem like it should yield 3; it seems like a syntax violation.

[Bykiev]

Indeed, but why do

Antlr is a lexer/parser library. It doesn't handle math operations or any other specific operation. Antlr defines a grammar syntax which can be used to parse a mathematical language (like ncalc); languages like C, SQL, etc; or any other domain specific language someone may design. The ncalc grammar defines tokens and how they are stitched together to form an expression. Antlr is then used to generate a C# parser from this grammar which the ncalc library sits on top of.

I'm not convinced treating curly braces as whitespace is a good idea. {{}{1}{{{}}}}}+{}{}2 doesn't seem like it should yield 3; it seems like a syntax violation.

Thank you, agreed

[Bykiev]

@david-brink-talogy, curly braces support was proposed in https://github.com/ncalc/ncalc/pull/112 What do you think about it?

[david-brink-talogy]

That PR supports curly braces as a substitute for square brackets only. The expression {1+2}*3 evaluates to 7 in the older versions of ncalc and still wouldn't be supported under the PR which doesn't restore full compatibility with all expressions.

I think curly braces were unintentionally allowed in earlier versions. Perhaps due to overly greedy matching or some gap in the grammar/antlr3.

Since you asked my opinion, I don't see why the grammar should be made more complicated by supporting an alternative syntax when a simple string replacement would seem to suffice. This seems even more appropriate given that in /ncalc/ncalc the poster indicated that the braces are from a different (but similar) syntax supported by DataTable.Compute. There are likely other incompatibilities between the two languages that need to be resolved. Curly braces could also be used in the future to define arrays/matricies since parenthesis and brackets are already used.

public static string ConvertComputeToNcalc(this string compute) => 
  compute.Replace("{", string.Empty).Replace("}", string.Empty));

[sklose]

I agree with @david-brink-talogy , this was never meant to be supported and adding support for it might have some other side effects that we don't want or prevent us from using curlies for other things in the future.