Add `.toXOption` methods from `StringOps`

[jxnu-liguobin]

There are some useful method in scala2.13.x, such as toLongOption, But it throws an NPE when the string is null, so probably not many people use it?

[SethTisue]

The usual style in Scala is to assume that things aren't null. So I doubt that many people are avoiding toLongOption and friends for that reason. If a String is null, throwing seems expected to me. The idiomatic way to call toLongOption on a String which the programmer suspects might be null, probably because it came from some Java code, is Option(myStringOrNull).flatMap(_.toLongOption).

(I feel like there was a pretty thorough discussion in scala/bug about nulls and extension methods, but I can't seem to find it...)

In any case, I do think that toLongOption and friends would be welcome additions to this library.

[jxnu-liguobin]

The usual style in Scala is to assume that things aren't null. So I doubt that many people are avoiding toLongOption and friends for that reason. If a String is null, throwing seems expected to me. The idiomatic way to call toLongOption on a String which the programmer suspects might be null, probably because it came from some Java code, is Option(myStringOrNull).flatMap(_.toLongOption).

Because I find many people like using Try(catch NPE). 😸

[SethTisue]

@rjolly and I are looking at this at the Open Source Spree in Paris.

We're starting with just toLongOption as an example and we can worry later about whether anything needs to change when we generalize to other methods.

One question here is where the new code should live. compat/src/main/scala-2.12/scala/collection/compat/package.scala already exists and is a grab bag of different conversions. One could argue that the /collection/ part is inappropriate for String, but I looked in the Scala 2.13 standard library and all of StringOps, including non-collection methods such as toLongOption, is in scala.collection, so I think we could follow that precedent and put our code for this issue in the scala.collection.compat package.

[SethTisue]

I think the steps to a solution here are:

• work on Scala 2.12 only to start with
• add the implicit conversion with a bogus implementation such as Some(s.toLong)
• add a simple test case to test the success case the new code and make sure it passes
• replace the bogus implementation with a real one copied from Scala 2.13's StringParsers
• add extension methods for the other types (Int, Boolean, Double, Float, ...)
• backport the full test suite from Scala 2.13
• once all tests pass on Scala 2.12, backport to Scala 2.11
• add Scaladoc for the new method(s)
• update the readme

[SethTisue]

Note that I've narrowed the scope of the ticket to just the .toXOption methods. If there are are other extension methods on String in 2.13 that are desired, please open separate tickets on them, to make it easier for contributors to make bite-sized contributions.

[SethTisue]

@jxnu-liguobin about null handling, I see now that Martijn gave a design justification at scala/scala#6538 (comment)

[martijnhoekstra]

My own justification there is very thin: It chooses to throw consistently throw a NPE over a NumberFormatException, with an at-least-better-than-its-now justification.

The probably almost zero cost of the null check (the benchmarks in the original MR could be used to verify that assumption of near zero cost) means to me that added convenience can be pretty small to be worth it too.

In this specific case, because these things are explicitly for validation, I can get behind the idea of doing the null check, even if it doesn't occur in idiomatic scala. The whole point is to do quick validation, and Option(str).flatMap(_.toXoption) might slow down the happy path by more than reasonable.

I thought it was only called an Open Source Spree if it was held around the Spree region in Berlin, and that in Paris it's just a sparkling source Seine.