strings like ".Exxx" are not valid floats/doubles

[martijnhoekstra]

Fixes scala/bug#10925

Should fix the test failure on #6559

Added the failing testcase that was generated by scalacheck, and a few close variations.

Make test failure message on the scalacheck tests clearer

[som-snytt]

Speaking of formatting, no one mentioned the variant space after if. This code base is mostly but not absolutely for the space, which is my preference; but a couple of people omit it. Probably it's best to pick one and gird for battle.

[martijnhoekstra]

I tend to be fully consistent with the rule that I use, like, whatever feels right at the time, man.

But considering I've annoyed you several times in the last week already, and I don't want to stay consistent in that regard, I'll put in some spaces.

[som-snytt]

i.e. (e == 'e' || e == 'E') && expOK(...)

[som-snytt]

I read that we're going for parseDouble compatibility. Just to mention that Scala doesn't take 5.E2 as a constant anymore, because of the ambiguity with selection.

[som-snytt]

And then:

noSignificant != startIndex + 1 && {
  val e = ???
 (e == 'e' || ...) && expOK(...)
}

[martijnhoekstra]

@som-snytt This entire validation monster returns a boolean and is mostly made out of expressions that return booleans, so I could make it all in to a vary long series of nested and chained && and || 's. This fragment could be

(startChar == '.' && {
  val noSignificant = skipIndexWhile(ch => ch >= '0' && ch <= '9', startIndex + 1, endIndex)
  (noSignificant != startIndex +1)
} && {
  val e = format.charAt(noSignificant)
  (e == 'e' || e == 'E') && expOP(noSignificant + 1, endIndex)
} || (...)

But the explicit if/elses with boolean literal returns in some places make it much easier to follow for me here. There probably is some arbitrary line where opinions can differ. I have no strong feelings where that line should be, so if there are particular places where you'd like to see it moved, I'm fine with that.

[som-snytt]

Yes, there is a line of taste. Returning true might be a code smell, but I trust your judgment.

[som-snytt]

Here you didn't if-else it, so I would boost the other expressions, too,

[som-snytt]

Like this if-else is exp1 || { val e = ???; exp2 } || exp3.

[som-snytt]

I think some of the if-else make the code read more imperative than necessary, when a boolean expression suffices, assuming it doesn't become too unwieldy.

It was neat to see scalacheck at work. It's also interesting to read the history of the API upgrade.

[som-snytt]

Thanks! You may have used extra parens, but who's counting?

[adriaanm]

Could this explain the new test failure we’re seeing in #6635?

[lrytz]

Yes, there's already a PR #6746