Add java.time._ Instances For The JVM

[isomarcte]

This commit adds Scalacheck instances for almost all of the types in the java.time package and sub-packages, for which meaningful definitions seem to exist.

Almost all implemented types have Gen.Choose instances. Shrink instances were defined for the types that seemed to have a reasonably straight forward definition. And all implemented types have an Arbitrary instance and a Cogen instance.

The instances were built against JRE 8, though I'm not aware of any changes between the JRE definitions and the current (JRE 15) definitions.

The types are scoped to org.scalacheck.time, with the intent that import org.scalacheck.time._ would bring them all into the implicit scope.

It is likely worth discussing why these types are being added. Scalacheck instances for java.time types are hardly new. There are several libraries which provide some instances for some java.time types. There is however, no library of which I am aware, which provides a relatively complete implementation of these types, e.g. instances for most types, with full range support, and Choose instances. If such a library already exists, then I am more than happy to withdraw this PR.

Further, java.time types have a relatively large user base in the Scala community. By hosting instances in Scalacheck proper, we increase exposure to these instances and lessen the likelihood that developers will attempt to re-invent the wheel, or just use stub instances, e.g. Gen.const(LocalDate.now).

While these types are currently scoped to the JVM only, it is arguable that they could be provided for ScalaJs and ScalaNative too. Both ScalaJs and ScalaNative have, or are working on, replacement API compatible implementations. However adding support in Scalacheck proper for this would require adding dependencies and is thus punted to a later discussion for now.

[isomarcte]

I've encoded trait which brings the implicits into scope as such,

package object time extends time.JavaTimeInstances

Though I acknowledge that several other popular encodings exist. For example,

package object time {
    object implicits extends time.JavaTimeInstances
}

or

package object time {
    object syntax {
      object all extends time.JavaTimeInstances
    }
}

If the maintainers have any preference's here, I'm more than happy to make changes to that structure.

[isomarcte]

Also totally happy to move these if they would be better in another module or to close the PR if they aren't wanted. :)

[isomarcte]

Looks like Scala is missing an Ordering for some types on 2.11...I'll look into that.

[isomarcte]

Fixed the compilation issues on Scala < 2.13. It was having issues resolving Ordering instances for three of the java.time types.

[ashawley]

This is a nice contribution. Would a better home for this be https://github.com/47degrees/scalacheck-toolbox though?

[isomarcte]

@ashawley So that's a bit of a tricky question. I considered proposing this PR there, but I decided not to do so for these reasons,

• https://github.com/typelevel/scalacheck already implements instances for other core JVM types. For the purposes of exposure to the user base, it seemed to me that it would make most sense in this repo. Especially since it doesn't require any new dependencies.
• https://github.com/47degrees/scalacheck-toolbox already contains a subset of instances for the types in this PR, but they are implemented in a radically different way. In order to add these to that repo, it would effectively replace the entirety of the implementation there.
• That codebase also deals with JodaTime. Any contributions like this to that code base might reasonably expect to have similar Joda implementations.
• Adds new typeclasses on top of their implementation, which are required in the interface, e.g. Granularity. There's nothing objectively wrong with that, but it is nice to just have a core implementation in terms of the JRE APIs and Scalacheck. Other items could be built on top of that in other libraries.

[isomarcte]

Anything I can do to move this along? Or should I close it? Or do people just need more time to think it over?

[larsrh]

I'm leaning towards inclusion. @ashawley, do you veto it?

[SethTisue]

is there somebody at 47 Degrees we should solicit an opinion from? (@alejandrohdezma, perhaps?)

[ashawley]

FWIW, there's also this library: https://github.com/tmccarthy/intime

[ashawley]

@larsrh No, I wasn't trying to veto.  I was attempting, but wasn't convincing enough, to facilitate getting this contributed to a library elsewhere in the ScalaCheck ecosystem.  In the spirit of what Richard wrote at #351 (comment) about keeping the library minimal and because we don't have a lot of capacity for maintaining the project, we should probably lean towards leaning the capabilities of the library to some definition, like Scala its standard library and some Java primitives.

[SethTisue]

cc @tmccarthy

[tmccarthy]

Just providing my humble opinion since my library was tagged above.

This seems like a nice addition that provides basically the same functionality as the intime-scalacheck library I wrote. Obviously I'm not sufficiently involved in this project to weigh in on the balance between maintaining additional instances vs contributing this to another library. But I'd note that scalacheck already provides instances for the java.util.Calendar class, which is de facto deprecated. Given that the java.time classes are today the standard representation of datetime concepts in Scala, it seems appropriate for instances to be provided as part of the main scalacheck library.

Probably the only value intime-scalacheck would provide over the instances in this PR is what I've termed "sensible generators". I've found these more useful than using Arbitrary instances in many cases. But I think it would be reasonable for something like that to be provided by an external library.

[larsrh]

Thanks @tmccarthy for chiming in and @isomarcte for the contribution. I'm a friend of optimistic merging, so I'm suggesting to do that here. If noone objects, I'll push the merge button in a few days.

[ashawley]

If contributing this to other ScalaCheck libraries isn't ideal, then adding to ScalaCheck core seems fine. Time values aren't Java primitives, but they are used enough that it seems like a valid exception.

[ashawley]

The types are scoped to org.scalacheck.time, with the intent that import org.scalacheck.time._ would bring them all into the implicit scope.

This is very smart, but I would rather they just be added to the library the same way as other types. I don't think we need to add the namespace. It might be something to consider in the future.

[isomarcte]

@ashawley just to clarify, are you saying you'd like the instances to be mixed into the companion objects for the various typeclasses, e.g. object Arbitrary extends ArbitraryLowPriority with ArbitraryArities with JavaTimeArbitraries or similar?

If so, do you want me to remove the org.scalacheck.time package entirely and just scope everything to org.scalacheck or should I keep the actual private[scalacheck] trait JavaTimeArbitraries scoped to org.scalacheck.time?

[ashawley]

Since it's private and if it doesn't get in the way, then I don't see any reason not to use the org.scalacheck.time namespace if you wish to.

[isomarcte]

@ashawley I'll update it to reflect those changes.

[isomarcte]

@ashawley @larsrh the latest commit splits up the JavaTimeInstances trait into 4 traits and has the companion object for each typeclass extends the corresponding trait. Let me know if you'd like me to make any further changes.

[larsrh]

Thanks a ton @isomarcte!