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/11 16:42:31 UTC
[GitHub] [incubator-daffodil] stevedlawrence opened a new pull request #417: Correctly expand varargs in String.format call
stevedlawrence opened a new pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417
Without using _ :* for the varargs, the format string is not properly
expanded and results in an IllegalFormatConversionException. This
currently only occurs when the BacktrackingException is created because
it's the only exception that uses the var args constructor with the
ThinException.
DAFFODIL-2394
----------------------------------------------------------------
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 #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417#discussion_r487180790
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
----------------------------------------------------------------
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 pull request #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417#issuecomment-692011234
With your idea of just grepping, I looked for a places in the codebase that for ``\.format(`` or ``Any\*`` and did my best to verify that all places are now doing the right thing. Based on visual inspection, I think this was the only case where we weren't handling varargs correctly, or where we were incorretly passing a Seq to format call where we intended to expand the sequence.
It's definitely not a perfect check, and it wouldn't surprise me if there are some potential placeses missed due to other things that we should grep for, but as far as I can tell this was the only incorrect usage.
----------------------------------------------------------------
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 #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
stevedlawrence merged pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417
----------------------------------------------------------------
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 #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417#discussion_r487180790
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
----------------------------------------------------------------
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 #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417#discussion_r487173958
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
----------------------------------------------------------------
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 #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
mbeckerle commented on a change in pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417#discussion_r487173958
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
I can see how this particular piece of list/argsList polymorphism issometimes useful, but generally, I wish this could be a scala type error so that it's caught at compile time, not run time. Oh well.
----------------------------------------------------------------
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 #417: Correctly expand varargs in String.format call
Posted by GitBox <gi...@apache.org>.
stevedlawrence commented on a change in pull request #417:
URL: https://github.com/apache/incubator-daffodil/pull/417#discussion_r487180790
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
##########
File path: daffodil-lib/src/main/scala/org/apache/daffodil/exceptions/Assert.scala
##########
@@ -33,7 +33,7 @@ abstract class ThinException protected (dummy: Int, cause: Throwable, fmt: Strin
extends Exception(null, cause, false, false) {
private lazy val msg_ =
- if (fmt ne null) fmt.format(args)
+ if (fmt ne null) fmt.format(args: _*)
Review comment:
Yeah, I think it's just a genearl issue with varargs. Those functions can accept anything, and it's hard to distinguish between a a single list as being the argument or the individual fields being all the arguments.
----------------------------------------------------------------
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