b3b00/csly

`ArgumentNullException` thrown on parse failure instead of helpful message

RoystonS opened this issue · 7 comments

RoystonS commented

Hi there,

I've just added some extra unit tests to my parsing code (following on from #254), and I'm seeing an ArgumentNullException thrown from inside the parser in one particular test case.

I'm using csly 2.8.0.

I've produced a simplified version of my app (with no extra dependencies this time!) which shows a failing test:
https://github.com/RoystonS/CslyNullIssue

It's the same parser as in my real app, but instead of generating an AST I've made this one simply generate a string during parsing to make reproduction easier.

The test output for the failing test is:

Message: 
System.ArgumentNullException : Value cannot be null. (Parameter 'collection')

Stack Trace: 
List`1.InsertRange(Int32 index, IEnumerable`1 collection)
List`1.AddRange(IEnumerable`1 collection)
SyntaxParseResult`1.AddExpectings(IEnumerable`1 expected)
EBNFRecursiveDescentSyntaxParser`2.ParseExpressionRule(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
EBNFRecursiveDescentSyntaxParser`2.Parse(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
RecursiveDescentSyntaxParser`2.ParseNonTerminal(IList`1 tokens, String nonTerminalName, Int32 currentPosition)
EBNFRecursiveDescentSyntaxParser`2.Parse(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
RecursiveDescentSyntaxParser`2.ParseNonTerminal(IList`1 tokens, String nonTerminalName, Int32 currentPosition)
RecursiveDescentSyntaxParser`2.ParseNonTerminal(IList`1 tokens, NonTerminalClause`1 nonTermClause, Int32 currentPosition)
EBNFRecursiveDescentSyntaxParser`2.ParseExpressionRule(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
<34 more frames...>
RecursiveDescentSyntaxParser`2.ParseNonTerminal(IList`1 tokens, NonTerminalClause`1 nonTermClause, Int32 currentPosition)
EBNFRecursiveDescentSyntaxParser`2.ParseExpressionRule(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
EBNFRecursiveDescentSyntaxParser`2.Parse(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
RecursiveDescentSyntaxParser`2.ParseNonTerminal(IList`1 tokens, String nonTerminalName, Int32 currentPosition)
EBNFRecursiveDescentSyntaxParser`2.Parse(IList`1 tokens, Rule`1 rule, Int32 position, String nonTerminalName)
RecursiveDescentSyntaxParser`2.Parse(IList`1 tokens, String startingNonTerminal)
Parser`2.ParseWithContext(IList`1 tokens, Object parsingContext, String startingNonTerminal)
Parser`2.Parse(String source, String startingNonTerminal)
ExpressionParser.Parse[T](String expression) line 94
UnitTest1.BooleanParser(String expression, String expected) line 52

This particular test is a fairly heavily nested expression

1 < 2 && 1 <= 2 && 1 == 2 && 1 >= 2 && 1 > 2 && 1 != 2 && 1 <> 2
This test case is checking grouping behaviour and also doing exhaustive testing of all my operators. The expected parse result should be
(((((((1 < 2) AND (1 <= 2)) AND (1 == 2)) AND (1 >= 2)) AND (1 > 2)) AND (1 != 2)) AND (1 != 2))
but the parse is throwing ArgumentNullException.

b3b00 commented

looking at the callstack I suspect that issue #254 fix is the root of this bug. I will look a it tomorrow. thanks for reporting

RoystonS commented

I've just done a little more debugging myself, and it looks like this particular expression is actually invalid. (This parser grammar is missing the actual parsing logic for the <> operator, so should be failing.)

I've just wound back to csly 2.7.0.5 and I get different behaviour. I get a nice clear error message instead:

System.Exception : unexpected "COMPARISON [>] @line 1, column 80" (COMPARISON). Expecting MINUS, LVAR, SIMVAR, LPAREN, OFF, ON, HEX_NUMBER, DECIMAL_NUMBER, .

So the problem is a pretty minor one: the error message has simply gotten lost in the 2.8.0 update, not that something has fundamentally gone wrong with the parser.

b3b00 commented

hello @RoystonS ,
I 've found a fix. You can test it on branch bugfix/#259
As soon as you're ok , i will build a new release

RoystonS commented

Yep, looks good!

b3b00 commented

I am not totally satisified with the error message that states :

unexpected "COMPARISON [<] @line 1, column 67" (COMPARISON). Expecting LOGICAL_OR, .

the "<" token is legit at position 67. I was hoping something like

unexpected "COMPARISON [>] @line 1, column 68" (COMPARISON). Expecting ...... .

I guess the expected tokens should be operands :

  • LVAR
  • SIMVAR
  • LPAREN
  • OFF
  • ON
  • HEX_NUMBER
  • DECIMAL_NUMBER
  • maybe i'm missing some
b3b00 commented

I've pushed one more fix that help deliver better error message :

unexpected "COMPARISON [>] @line 1, column 72" (COMPARISON). Expecting MINUS, OFF, ON, HEX_NUMBER, DECIMAL_NUMBER, LVAR, SIMVAR, LPAREN, .

You can test it on the same branch as previously

b3b00 commented

I 've release nuget 2.8.0.1

  • 👍1