Provide more detail for failed assertions

[jadams-tresys]

We used to have more detailed assertion failed messages that included
the value that was parsed, but it seems to have been lost over the
course of some redesigns to the error system. This commit adds the extra
detail back in.

DAFFODIL-752

[mbeckerle]

+1 Looks good.

[stevedlawrence]

I'm not sure if this change makes sense in all cases. One case might be if a dfdl:assert has a message attribute. I would argue that in that case we do not want to alter the users message at all, which is what this does.

Instead, maybe the bug can be inerpreted as really asking for a better default assertion failed message when a schema doesn't provide a message attribute. Right now, that default is defined in PrimitievsExpression.scala as just exprWithBraces + " failed". Considering that the assertion could check for anything, this actually seems somehwat reasonsable to me, especially since just knowing the value of the current element isn't going to that useful in many cases. For example, what if the assertion expression was { xs:int(.) gt ../SomeElement }. In this case, knowing the value of the current element isn't enough information. You also want to know the value of ../SomeElement.

I've seen some assertion libraries that will generate an assertion message that is the string version of the assertion, but with variables replaced with their runtime values. So for the above assertion, we might generate a default message expression that look like this:

{ fn:concat("{ xs:int(", ., ") gt ", ../SomeElement, " }", "failed") } 

And when evaluated would result in somethign like this:

{ xs:int(2) gt 5 } failed

That of course gets really complicated since you need to be able to convert your expression to a fn:concat thing with all the pieces being a string except for the variables. That might not be too difficult with our expression tree stuff, but I'm not sure it's worth the effort, especially since if a user really cared they could make that message theirself.

[jadams-tresys]

I looked into this a bit and I think the problem is there doesn't seem to be a way to determine if a message was predefined from the CompiledExpression that gets passed into the Assert parsers. We seem to be automatically generating messages when one isn't supplied. For example:
<dfdl:assert test="{ xs:int(.) eq 42}"> doesn't specify a message, but when getAssertFailureMessage it returns { xs:int(.) eq 42 } failed

Should we be automatically creating these messages? I didn't see a straightforward way to determine if the message returned by getAssertFailureMessage is generated or not.

[stevedlawrence]

Yeah, I would say that if the current default assert message isn't sufficient, then we should change the lazy val msgExpr value in PrimitivesExpressions.scala. The default is set to just a constant string, e.g.:

new ConstantExpression[String](qn, NodeInfo.String, exprWithBraces + " failed")

But there's nothing that's stopping us from generating and compiling an expression that outputs more useful information (e.g. like the fn:concat thing from above). I think if we want more helpful default asserts, that might be a good thing to strive for. I imagine generating something like the fn:concat thing would require adding a new method to CompiledDPath that would be similar to "toXML" (e.g. toAssertExpression) but would generate a different DFDL exprsession that could then be compiled into an assert message that would be evaluated on error.

That said, that might be decent amount of effrot and might not be worth the time. I think our asserts are actually prettty decent now. This bug was opened in 2013, things have changed a lot since then. We also have the debugger, so we could just recommend people switch to that if they need details on why an assert fails.

[jadams-tresys]

So, are you of the opinion to just leave things the way they are then and drop this pull request?

[stevedlawrence]

Yeah, I think that's how I'm leaning.

I think the default message has improved alot since this bug was originally created. And we also fixed the issue that prevented the message attribute from being an expression. So if someone really wants extra information they can create a custom message expression to get exactly what information is relevant.

[jadams-tresys]

OK, sounds good.