Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 6, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b55d68
strings like ".Exxx" are not valid floats/doubles
Jun 5, 2018
4570f10
consistent space after if
Jun 5, 2018
7108f73
replace if/else with ||/&&
Jun 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 33 additions & 43 deletions src/library/scala/collection/StringParsers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ final private[scala] object StringParsers {
def rec(i: Int, agg: Int): Option[Int] =
if (agg < min) None
else if (i == len) {
if(!isPositive) Some(agg)
if (!isPositive) Some(agg)
else if (agg == min) None
else Some(-agg)
}
Expand All @@ -41,8 +41,8 @@ final private[scala] object StringParsers {
//bool
@inline
final def parseBool(from: String): Option[Boolean] =
if(from.equalsIgnoreCase("true")) Some(true)
else if(from.equalsIgnoreCase("false")) Some(false)
if (from.equalsIgnoreCase("true")) Some(true)
else if (from.equalsIgnoreCase("false")) Some(false)
else None

//integral types
Expand Down Expand Up @@ -90,11 +90,11 @@ final private[scala] object StringParsers {
@tailrec
def step(i: Int, agg: Int, isPositive: Boolean): Option[Int] = {
if (i == len) {
if(!isPositive) Some(agg)
if (!isPositive) Some(agg)
else if (agg == Int.MinValue) None
else Some(-agg)
}
else if(agg < intOverflowBoundary) None
else if (agg < intOverflowBoundary) None
else {
val digit = decValue(from.charAt(i))
if (digit == -1 || (agg == intOverflowBoundary && digit == intOverflowDigit)) None
Expand Down Expand Up @@ -125,11 +125,11 @@ final private[scala] object StringParsers {
@tailrec
def step(i: Int, agg: Long, isPositive: Boolean): Option[Long] = {
if (i == len) {
if(isPositive && agg == Long.MinValue) None
if (isPositive && agg == Long.MinValue) None
else if (isPositive) Some(-agg)
else Some(agg)
}
else if(agg < longOverflowBoundary) None
else if (agg < longOverflowBoundary) None
else {
val digit = decValue(from.charAt(i))
if (digit == -1 || (agg == longOverflowBoundary && digit == longOverflowDigit)) None
Expand Down Expand Up @@ -166,7 +166,7 @@ final private[scala] object StringParsers {
def forAllBetween(start: Int, end: Int, pred: Char => Boolean): Boolean = {
def rec(i: Int): Boolean = {
if (i >= end) true
else if(pred(format.charAt(i))) rec(i + 1)
else if (pred(format.charAt(i))) rec(i + 1)
else false
}
rec(start)
Expand All @@ -190,36 +190,32 @@ final private[scala] object StringParsers {

def prefixOK(startIndex: Int, endIndex: Int): Boolean = {
val len = endIndex - startIndex
if(len == 0) false
else {
(len > 0) && {
//the prefix part is
//hexDigits
//hexDigits.
//hexDigits.hexDigits
//.hexDigits
//but notnot .
if(format.charAt(startIndex) == '.') {
if (format.charAt(startIndex) == '.') {
(len > 1) && forAllBetween(startIndex + 1, endIndex, isHexDigit)
} else {
val noLeading = skipIndexWhile(isHexDigit, startIndex, endIndex)
if(noLeading >= endIndex) true
else if(format.charAt(noLeading) == '.') forAllBetween(noLeading + 1, endIndex, isHexDigit)
else false
(noLeading >= endIndex) ||
((format.charAt(noLeading) == '.') && forAllBetween(noLeading + 1, endIndex, isHexDigit))
}
}
}

def postfixOK(startIndex: Int, endIndex: Int): Boolean = {
if (startIndex >= endIndex) false
else {
if (forAllBetween(startIndex, endIndex, ch => ch >= '0' && ch <= '9')) true
else {
def postfixOK(startIndex: Int, endIndex: Int): Boolean =
(startIndex < endIndex) && {
(forAllBetween(startIndex, endIndex, ch => ch >= '0' && ch <= '9')) || {
val startchar = format.charAt(startIndex)
(startchar == '+' || startchar == '-') && (endIndex - startIndex > 1) && forAllBetween(startIndex + 1, endIndex, ch => ch >= '0' && ch <= '9')
(startchar == '+' || startchar == '-') &&
(endIndex - startIndex > 1) &&
forAllBetween(startIndex + 1, endIndex, ch => ch >= '0' && ch <= '9')
}
}

}
// prefix [pP] postfix
val pIndex = format.indexWhere(ch => ch == 'p' || ch == 'P', startIndex)
(pIndex <= endIndex) && prefixOK(startIndex, pIndex) && postfixOK(pIndex + 1, endIndex)
Expand All @@ -228,47 +224,41 @@ final private[scala] object StringParsers {
def isDecFloatLiteral(startIndex: Int, endIndex: Int): Boolean = {
//invariant: endIndex > startIndex

def expOK(startIndex: Int, endIndex: Int): Boolean = {
if(startIndex >= endIndex) false
else {
def expOK(startIndex: Int, endIndex: Int): Boolean =
(startIndex < endIndex) && {
val startChar = format.charAt(startIndex)
if(startChar == '+' || startChar == '-')
if (startChar == '+' || startChar == '-')
(endIndex > (startIndex + 1)) &&
skipIndexWhile(ch => ch >= '0' && ch <= '9', startIndex + 1, endIndex) == endIndex
else skipIndexWhile(ch => ch >= '0' && ch <= '9', startIndex, endIndex) == endIndex
}
}

//significant can be one of
//* digits.digits
//* .digits
//* digits.
//but not just .
val startChar = format.charAt(startIndex)
if(startChar == '.') {
val noSignificant = skipIndexWhile(ch => ch >= '0' && ch <= '9', startIndex + 1, endIndex)
if(noSignificant == endIndex) (noSignificant != startIndex + 1) //not just "."
else {
val e = format.charAt(noSignificant)
if (e == 'e' || e == 'E') expOK(noSignificant + 1, endIndex)
else false
}
if (startChar == '.') {
val noSignificant = skipIndexWhile(ch => ch >= '0' && ch <= '9', startIndex + 1, endIndex)
(noSignificant != startIndex + 1) && { //not just "." or ".Exxx"
val e = format.charAt(noSignificant)
(e == 'e' || e == 'E') && expOK(noSignificant + 1, endIndex)
}
}
else if (startChar >= '0' && startChar <= '9'){
//one set of digits, then optionally a period, then optionally another set of digits, then optionally an exponent
val noInt = skipIndexWhile(ch => ch >= '0' && ch <= '9', startIndex, endIndex)
if(noInt == endIndex) true //just the digits
else {
(noInt == endIndex) || { //just the digits
val afterIntChar = format.charAt(noInt)
if(afterIntChar == '.') {
if (afterIntChar == '.') {
val noSignificant = skipIndexWhile(ch => ch >= '0' && ch <= '9', noInt + 1, endIndex)
if(noSignificant >= endIndex) true //no exponent
else {
(noSignificant >= endIndex) || { //no exponent
val e = format.charAt(noSignificant)
(e == 'e' || e == 'E') && expOK(noSignificant + 1, endIndex)
}
}
else if(afterIntChar == 'e' || afterIntChar == 'E') expOK(noInt + 1, endIndex)
else if (afterIntChar == 'e' || afterIntChar == 'E') expOK(noInt + 1, endIndex)
else false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
Expand All @@ -280,14 +270,14 @@ final private[scala] object StringParsers {
val unspacedStart = format.indexWhere(ch => ch.toInt > 0x20)
val unspacedEnd = format.lastIndexWhere(ch => ch.toInt > 0x20) + 1

if(unspacedStart == -1 || unspacedStart >= unspacedEnd || unspacedEnd <= 0) false
if (unspacedStart == -1 || unspacedStart >= unspacedEnd || unspacedEnd <= 0) false
else {
//all formats can have a sign
val unsigned = {
val startchar = format.charAt(unspacedStart)
if (startchar == '-' || startchar == '+') unspacedStart + 1 else unspacedStart
}
if(unsigned >= unspacedEnd) false
if (unsigned >= unspacedEnd) false
//that's it for NaN and Infinity
else if (format.charAt(unsigned) == 'N') format.substring(unsigned, unspacedEnd) == "NaN"
else if (format.charAt(unsigned) == 'I') format.substring(unsigned, unspacedEnd) == "Infinity"
Expand Down
6 changes: 6 additions & 0 deletions test/junit/scala/collection/StringParsersTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ class StringParsersTest {
"-0x0.000000000000090000000000000000001p-1022",
"-0x0.00000000000009fffffffffffffffffffffffffffffffffp-1022",
"-0x0.0000000000000fffffffffffffffffffffffffffffffffffp-1022",
".E4",
"1.1E4",
"1.E4",
".1E4",
"1E4",
"E4",
"0.0",
"+0.0",
"-0.0",
Expand Down
10 changes: 5 additions & 5 deletions test/scalacheck/scala/collection/FloatFormatTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,21 +92,21 @@ object FloatFormatTest extends Properties("FloatFormat") {
case (ll, rr) => ll == rr
}

property("plausible string are same")= forAll(genPlausibleFloatString) {
property("for all strings (plausible examples) str.toDoubleOption == Try(str.toDouble).toOption") = forAll(genPlausibleFloatString) {
str => deq(str.toDoubleOption, Try(str.toDouble).toOption)
}

property("possible strings are same") = forAll(genPossibleFloatString) {
property("for all strings (possible examples) str.toDoubleOption == Try(str.toDouble).toOption") = forAll(genPossibleFloatString) {
str => deq(str.toDoubleOption, Try(str.toDouble).toOption)
}

property("arbitrary strings are same") = forAll{ (str: String) =>
property("for all strings (random strings) str.toDoubleOption == Try(str.toDouble).toOption") = forAll{ (str: String) =>
deq(str.toDoubleOption, Try(str.toDouble).toOption)
}

property("double.toString checks") = forAll{ (d: Double) => checkFloatFormat(d.toString)}
property("double.toString is a valid float format") = forAll{ (d: Double) => checkFloatFormat(d.toString)}

property("nan and infinity check like toDouble") = forAll(genNanInf){ (str: String) =>
property("nan and infinity toDouble == toDoubleOption.get") = forAll(genNanInf){ (str: String) =>
deq(str.toDoubleOption, Try(str.toDouble).toOption)
}

Expand Down