You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2020/09/04 15:14:12 UTC

[GitHub] [incubator-daffodil] rsteinberger opened a new pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

rsteinberger opened a new pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415


   The diagnostic message only displays "." as the error.
   This commit adds the full expression text to the diagnostic message
   for the case of test_checkConstraintsComplexTypeFails.
   This case does not invlolve inputValueCalc.
   
   The case of test_assertFailShowsValue2 indicated in the ticket
   invloves inputValueCalc and is addressed in Daffodil-1035 Lone "."
   in a dfdl:inputValueCalc expression should be an error
   
   Daffodil-1043


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r498945046



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -623,24 +623,18 @@ case class WholeExpression(
     }
 
     if (!allowCoercion) {
-      val (relevantExpr: Expression, detailMsg: String) =
-        Conversion.polymorphicExpressionDiagnostics(inherentType, targetType, subExpr)
       if (tunable.allowExpressionResultCoercion) {
         SDW(
           WarnID.DeprecatedExpressionResultCoercion,
-          "In expression %s, result type (%s) should be manually cast to the expected type (%s) with the appropriate constructor." +
-            "Performing deprecated automatic conversion.%s",
-          relevantExpr.text,
+          "Result type (%s) should be manually cast to the expected type (%s) with the appropriate constructor." +
+            "Performing deprecated automatic conversion.",
           inherentType,
-          targetType,
-          detailMsg)
+          targetType)
       } else {
         SDE(
-          "In expression %s, result type (%s) must be manually cast to the expected type (%s) with the approrpriate constructor.%s",
-          relevantExpr.text,
+          "Result type (%s) must be manually cast to the expected type (%s) with the approrpriate constructor.",
           inherentType,
-          targetType,

Review comment:
       Should we add back "in expression %s" for these error messages?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492917360



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Cool




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r497542121



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -270,10 +268,10 @@ object Conversion {
       case (_, Exists) => Nil
       case (_, other) => {
         val (relevantExpr: Expression, detailMsg: String) =
-          polymorphicExpressionDiagnostics(st, tt, context)
+          context.polymorphicExpressionErrorDetails()

Review comment:
       Done.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r489770265



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Right, good call. I'll take a look and add them to the ticket.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493847138



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Thanks guys, very helpful.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493745482



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       I think I may be confused. Line 297 doesn't have an error message. Are you talking about the error message on line 317?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493732931



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       The error message in question is inside of case stepE: StepExpression => {} (Conversions LN 297). I placed a break point inside of this case and ran all tests. It is not hit. So, like you suggested it may make sense to create a new test to cover this code. Thus my effort to satisfy the StepExpression case in a new test. But it's possible I'm lost in a rabbit hole.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r497451156



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -270,10 +268,10 @@ object Conversion {
       case (_, Exists) => Nil
       case (_, other) => {
         val (relevantExpr: Expression, detailMsg: String) =
-          polymorphicExpressionDiagnostics(st, tt, context)
+          context.polymorphicExpressionErrorDetails()

Review comment:
       I thought you said that the contents of this function that do polymorphic work never get hit when called in this context? Which makes sense to me. The DownStep is really the only place where we need to detect polymorphic expressions. If there is one, ``DownStep`` will create an SDE and this function will never be hit. So in this usage, this function will just always return context and an error string of "", which I'm not sure gains us anything, aside from potential confusion and extra complications.
   
   I'd suggest just deleting this usage. If we're concerned that there does need to be some polymorphic check here, we could maybe add a function and do something like ``Assert.invariant(!isPolymorhpic)``, but I'm not sure if that's even necessary, as polymorphism isn't even relevant in this function. All this function does it get a source type and a target type and figures out how to convert between one and another. That fact that something is polymorphic is irrelevant to that decision.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493747613



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Right, the case statement is on 297.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492909669



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       The addition of 'eq 5' is what the wholeText returns.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#issuecomment-706144945


   Squashed and rebased.
   Thanks John
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493821079



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       A Step expression is one "step" in a Path expression. So foo/bar/baz/.././quux is a Path with 6 Step expressions in it. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] mbeckerle commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493821858



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       A step is either a named step (downward), or a ".." up step, or a "." self step. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492921552



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Perfect, I'll give that a try.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492920715



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Gotcha. This bit of code should be hit if an expression ever needs to convert one type to another where that conversion isn't allowed. For example, it is illegal to convert an ``xs:date`` to an ``xs:boolean``. So you could have a schema that does something like this:
   
   ```xml
   <xs:element name="foo" type="xs:boolean" dfdl:inputValueCalc="{ xs:boolean(xs:date("dateString")) }" />
   ```
   
   That inputValueCalc expression includes an illegal conversion that *should* trigger this code.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492906593



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       What is the full error here? Without context of the rest of the error message it's hard to tell if this change is correct.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       It would probably be worth adding additional tests, especially if we don't have any coverage. What questions do you have?

##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Right, but the full message is probably someting like "Expression failed because ..... some more text ... ../../poly eq 5 with xs:date ... maybe more text". The on snipp of ``../../poly eq 5 with xs:date`` feels like it doesn't make sense, but maybe it's because I don't have the full context of the full message. I was asking what the full message is to make sure this error message still makes sense with this change, since it isn't clear.

##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Gotcha, that seems like a reasonble (and more helpful with this change) diagnostic to me. Looks good.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Gotcha. This bit of code should be hit if an expression ever needs to convert one type to another where that conversion isn't allowed. For example, it is illegal to convert an ``xs:date`` to an ``xs:boolean``. So you could have a schema that does something like this:
   
   ```xml
   <xs:element name="foo" type="xs:boolean" dfdl:inputValueCalc="{ xs:boolean(xs:date("dateString")) }" />
   ```
   
   That inputValueCalc expression includes an illegal conversion that *should* trigger this code.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492915314



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Oh, here we go:
   Schema Definition Error: Subset Expression ../../poly has different types at different points of usage.
   The inconsistent type usages are listed here:
   element poly in expression ../../poly eq 5 with xs:int type at Location line 29 column 10 in file:/C:/Users/RSTEIN~1/AppData/Local/Temp/anon689374424817542066.dfdl.xsd
   element poly in expression ../../poly eq 5 with xs:date type at Location line 48 column 10 in 
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493716705



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       So this issue of a new test for Conversions requires the generation of a StepExpression rather than a FunctionCallExpression. Can you give me an example of how to form a StepExpression in the schema.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492911748



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Right, but the full message is probably someting like "Expression failed because ..... some more text ... ../../poly eq 5 with xs:date ... maybe more text". The on snipp of ``../../poly eq 5 with xs:date`` feels like it doesn't make sense, but maybe it's because I don't have the full context of the full message. I was asking what the full message is to make sure this error message still makes sense with this change, since it isn't clear.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493763867



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Gotcha. This error is a bit trickier to hit. In order to hit this, there must be a global element that has an expression that references elements outside of it's global declaration. Depending on the context of where this global element is referenced, it could reference element with the same name but with different types. This is called a polymorphic expression, and isn't allowed. Here's a schema that triggers this error:
   
   ```xml
   <?xml version="1.0" encoding="UTF-8"?> 
   
   <xs:schema
     xmlns:xs="http://www.w3.org/2001/XMLSchema"
     xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/">
   
     <xs:include schemaLocation="org/apache/daffodil/xsd/DFDLGeneralFormat.dfdl.xsd" />
   
     <xs:annotation>
       <xs:appinfo source="http://www.ogf.org/dfdl/">
         <dfdl:format ref="GeneralFormat"
           textBooleanTrueRep="t"
           textBooleanFalseRep="f" />
       </xs:appinfo>
     </xs:annotation>
   
     <xs:element name="root">
       <xs:complexType>
         <xs:sequence>
           <xs:element name="boolean">
             <xs:complexType>
               <xs:sequence>
                 <xs:element name="bar" type="xs:boolean" dfdl:lengthKind="explicit" dfdl:length="1" />
                 <xs:element ref="foo" />
               </xs:sequence>
             </xs:complexType>
           </xs:element>
           <xs:element name="date">
             <xs:complexType>
               <xs:sequence>
                 <xs:element name="bar" type="xs:date" dfdl:lengthKind="explicit" dfdl:length="1" />
                 <xs:element ref="foo" />
               </xs:sequence>
             </xs:complexType>
           </xs:element>
         </xs:sequence>
       </xs:complexType>
     </xs:element>
   
     <xs:element name="foo" type="xs:boolean" dfdl:inputValueCalc="{ xs:boolean(../bar) }" />
   
   </xs:schema>
   ```
   At the bottom we have a ``foo`` element that referenes a "../bar" element in the inputValueCalc expression, and converts ``../bar`` to a boolean. So what bar element is referenced must be able to be converted to a boolean.
   
   In the root element, we then reference ``foo`` twice. In the first usage, the ``../bar`` that is referenced has an ``xs:boolean`` type and the conversion is allowed. But in the second usage, the ``../bar`` that is referenced has an ``xs:date`` type, which doesn't allow the conversion
   
   Because ``../bar`` is referenced polymorphically (once to an xs:boolean, once to an xs:date) we should get the error you're looking for.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r497546860



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -269,74 +267,13 @@ object Conversion {
 
       case (_, Exists) => Nil
       case (_, other) => {
-        val (relevantExpr: Expression, detailMsg: String) =
-          polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
-          "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          "The type %s cannot be converted to %s.",

Review comment:
       It's probably still worth having "In expression %s", you just need to use ``context`` to get the whole text rather than ``relevantExpr``.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -624,7 +673,7 @@ case class WholeExpression(
 
     if (!allowCoercion) {
       val (relevantExpr: Expression, detailMsg: String) =
-        Conversion.polymorphicExpressionDiagnostics(inherentType, targetType, subExpr)
+        polymorphicExpressionErrorDetails()

Review comment:
       Is this one also essentially a no-op? Seems the only one that's needed is the DownStepExpression. If so, it probably makes sense to only move the polymorphic expression details function in DownStepExpression rather than Expression, since no other Expression classes should need this logic. It really is specific to DownStep.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#issuecomment-706144945


   Squashed and rebased.
   Thanks John
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r498923980



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -624,7 +673,7 @@ case class WholeExpression(
 
     if (!allowCoercion) {
       val (relevantExpr: Expression, detailMsg: String) =
-        Conversion.polymorphicExpressionDiagnostics(inherentType, targetType, subExpr)
+        polymorphicExpressionErrorDetails()

Review comment:
       Done

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -269,74 +267,13 @@ object Conversion {
 
       case (_, Exists) => Nil
       case (_, other) => {
-        val (relevantExpr: Expression, detailMsg: String) =
-          polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
-          "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          "The type %s cannot be converted to %s.",

Review comment:
       Done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492916429



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Gotcha, that seems like a reasonble (and more helpful with this change) diagnostic to me. Looks good.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492912048



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       I've been looking at it, and I don't see how to structure the test schema to trigger this section of the code. Didn't want to keep spinning my wheels.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492894645



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       I addressed the additional case in Expressions.
   The additional case in Conversions doesn't appear to be addressed by any unit test. The code appears to be largely duplicated in Expressions where the expression tests are applied.
   If we need to develop a test to address the code in Conversion, I'll have some questions on how to construct the xsd.

##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       The addition of 'eq 5' is what the wholeText returns.

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       I've been looking at it, and I don't see how to structure the test schema to trigger this section of the code. Didn't want to keep spinning my wheels.

##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Oh, here we go:
   Schema Definition Error: Subset Expression ../../poly has different types at different points of usage.
   The inconsistent type usages are listed here:
   element poly in expression ../../poly eq 5 with xs:int type at Location line 29 column 10 in file:/C:/Users/RSTEIN~1/AppData/Local/Temp/anon689374424817542066.dfdl.xsd
   element poly in expression ../../poly eq 5 with xs:date type at Location line 48 column 10 in 
   

##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       Cool

##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       Perfect, I'll give that a try.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r498964266



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Expression.scala
##########
@@ -623,24 +623,18 @@ case class WholeExpression(
     }
 
     if (!allowCoercion) {
-      val (relevantExpr: Expression, detailMsg: String) =
-        Conversion.polymorphicExpressionDiagnostics(inherentType, targetType, subExpr)
       if (tunable.allowExpressionResultCoercion) {
         SDW(
           WarnID.DeprecatedExpressionResultCoercion,
-          "In expression %s, result type (%s) should be manually cast to the expected type (%s) with the appropriate constructor." +
-            "Performing deprecated automatic conversion.%s",
-          relevantExpr.text,
+          "Result type (%s) should be manually cast to the expected type (%s) with the appropriate constructor." +
+            "Performing deprecated automatic conversion.",
           inherentType,
-          targetType,
-          detailMsg)
+          targetType)
       } else {
         SDE(
-          "In expression %s, result type (%s) must be manually cast to the expected type (%s) with the approrpriate constructor.%s",
-          relevantExpr.text,
+          "Result type (%s) must be manually cast to the expected type (%s) with the approrpriate constructor.",
           inherentType,
-          targetType,

Review comment:
       Done




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence merged pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#issuecomment-693599263


   Yes, I believe this is complete. Just waiting on comments and approvals. The case of test_assertFailShowsValue2 indicated in the ticket invloves inputValueCalc and is addressed in Daffodil-1035 Lone "." in a dfdl:inputValueCalc expression should be an error.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492906593



##########
File path: daffodil-core/src/test/scala/org/apache/daffodil/dsom/TestPolymorphicUpwardRelativeExpressions.scala
##########
@@ -144,8 +144,8 @@ class TestPolymorphicUpwardRelativeExpressions extends Logging {
     }
     val msg = e.getMessage()
     val hasSDE = msg.contains("Schema Definition Error")
-    val hasPolyInt = msg.contains("../../poly with xs:int")
-    val hasPolyDate = msg.contains("../../poly with xs:date")
+    val hasPolyInt = msg.contains("../../poly eq 5 with xs:int")
+    val hasPolyDate = msg.contains("../../poly eq 5 with xs:date")

Review comment:
       What is the full error here? Without context of the rest of the error message it's hard to tell if this change is correct.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r493722394



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       It's not clear to my why you need a step expression test. Is this because the original bug was about ``checkConstraints(.)``, which was about step expressions?
   
   When you fixed the original bug by changing ``.text`` to ``.wholeExpression``, I suggested there were other places in code that could use this change that were not related to the original bug. So we sort of expanded the issue of this bug to fix the underlying issue, and not just this one specific issue of checkConstriants(.).
   
   Conversions.scala doesn't really have anything specific to do with step expressions, but did have the incorrect .text. So I _think_ we just need a test that ensures this new conversion string outputs the full expression when this error occurs. There are probably existing tests that check illegal conversions, so if you could find those and just update the expected error message to ensure it includes the full expression. It might take some digging to find a test, so it might just be easier to create a new test.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] rsteinberger commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
rsteinberger commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492894645



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       I addressed the additional case in Expressions.
   The additional case in Conversions doesn't appear to be addressed by any unit test. The code appears to be largely duplicated in Expressions where the expression tests are applied.
   If we need to develop a test to address the code in Conversion, I'll have some questions on how to construct the xsd.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r492907034



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       It would probably be worth adding additional tests, especially if we don't have any coverage. What questions do you have?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence commented on a change in pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415#discussion_r489685258



##########
File path: daffodil-core/src/main/scala/org/apache/daffodil/dpath/Conversions.scala
##########
@@ -273,7 +273,7 @@ object Conversion {
           polymorphicExpressionDiagnostics(st, tt, context)
         context.SDE(
           "In expression %s, the type %s cannot be converted to %s.%s",
-          relevantExpr.text,
+          relevantExpr.wholeExpressionText,

Review comment:
       It looks like there are some other places where we reference ``.text`` an an Expression in a diagnostic in Expression.scala and Conversions.scala. Did you look at these and determine if a similar change to wholeExpressionText is needed to improve the diagnostics?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-daffodil] stevedlawrence merged pull request #415: Expand the diagnostic message for dfdl:checkConstraints(.)

Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #415:
URL: https://github.com/apache/incubator-daffodil/pull/415


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org