Skip to content
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.

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

Create Java Collections module #391

Merged
merged 26 commits into from
Oct 5, 2023
Merged

Create Java Collections module #391

merged 26 commits into from
Oct 5, 2023

Conversation

MateuszKubuszok
Copy link
Member

@MateuszKubuszok MateuszKubuszok commented Sep 26, 2023

  • turn chimney-java-collections into publishable module
  • create instances for Java collections
    • java.util.Optional
      • test for total
      • test for partial
    • java.util.Iterable
      • test for total
      • test for partial
    • java.util.Enumeration
      • test for total
      • test for partial
    • java.util.Collection
      • test for total
      • test for partial
    • java.util.BitSet
      • test for total
      • test for partial
    • java.util.Dictionary
      • test for total
      • test for partial
    • java.util.Map
      • test for total
      • test for partial
    • java.util.Properties
      • test for total
      • test for partial
    • java.util.stream.Stream
      • test for total
      • test for partial
  • test that conversions produce correct error messages for partials
  • create Java collections documentation

@MateuszKubuszok MateuszKubuszok linked an issue Sep 26, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (8ae8524) 84.27% compared to head (49b91b2) 85.86%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
+ Coverage   84.27%   85.86%   +1.58%     
==========================================
  Files         108      117       +9     
  Lines        4071     4463     +392     
  Branches      182      171      -11     
==========================================
+ Hits         3431     3832     +401     
+ Misses        640      631       -9     
Files Coverage Δ
...nd/chimney/javacollections/JavaFactoryCompat.scala 100.00% <100.00%> (ø)
...s/JavaCollectionsPartialTransformerImplicits.scala 100.00% <100.00%> (ø)
...ons/JavaCollectionsTotalTransformerImplicits.scala 100.00% <100.00%> (ø)
...ollections/internal/PartialTransformOrUpcast.scala 100.00% <100.00%> (ø)
...y/javacollections/internal/TransformOrUpcast.scala 100.00% <100.00%> (ø)
...nd/chimney/javacollections/JavaFactoryCompat.scala 87.50% <87.50%> (ø)
...nd/chimney/javacollections/JavaFactoryCompat.scala 75.00% <75.00%> (ø)
...alaland/chimney/javacollections/JavaIterator.scala 88.23% <88.23%> (ø)
...calaland/chimney/javacollections/JavaFactory.scala 96.49% <96.49%> (ø)

... and 10 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MateuszKubuszok
Copy link
Member Author

MateuszKubuszok commented Sep 29, 2023

I am not entirely happy with the amount of implicits I have to write and that I cannot create JavaIterable[A, CC] similarly to JavaFactory to abstract over all possible iterations, since I get a lot of ambiguity errors that didn't happen with JavaFactory. For instance:

trait JavaIterator[CC] {
  type A

  def foreach(collection: CC)(f: A => Any): Unit
}
object JavaIterator {

  type Aux[A0, CC] = JavaIterator[CC] { type A = A0 }

  implicit def iterator[A]: Aux[A, java.util.Iterator[A]] = ???

  implicit def bitSet: Aux[Int, java.util.BitSet] = ???

  implicit def iterable[A, CC <: java.lang.Iterable[A]]: Aux[A, CC] = ???

  locally {
    import io.scalaland.chimney.PartialTransformer

    implicit def partialForJavaToJava[JColl1, JColl2, A, B](implicit
        iterator: JavaIterator.Aux[A, JColl1],
        factory: JavaFactory[B, JColl2],
        aToB: PartialTransformer[A, B]
    ): PartialTransformer[JColl1, JColl2] =
      ???

    implicit def strToInt: PartialTransformer[String, Int] = ???

    //implicitly[PartialTransformer[java.util.ArrayList[String], java.util.BitSet]].toString

    val foo: PartialTransformer[java.util.ArrayList[String], java.util.BitSet] = partialForJavaToJava
  }
}

produces (Scala 2.12)

[error] /Users/dev/Workspaces/GitHub/chimney/chimney-java-collections/src/main/scala/io/scalaland/chimney/javacollections/JavaIterator.scala:32:82: ambiguous implicit values:
[error]  both method iterator in object JavaIterator of type [A]=> io.scalaland.chimney.javacollections.JavaIterator.Aux[A,java.util.Iterator[A]]
[error]  and method bitSet in object JavaIterator of type => io.scalaland.chimney.javacollections.JavaIterator.Aux[Int,java.util.BitSet]
[error]  match expected type io.scalaland.chimney.javacollections.JavaIterator.Aux[A,JColl1]
[error]     val foo: PartialTransformer[java.util.ArrayList[String], java.util.BitSet] = partialForJavaToJava

which is absolutely absurd, as java.util.Iterator and java.util.BitSet have only java.lang.Object as the only common supertype, and none of them is s supertype to java.util.ArrayList, the actually resolved type (which is a subtype of java.util.Iterable).

If we managed to pull this off we could have only 2 implicit for all cases: Iterator, Enumeration, Iterable, Dictionary, Map, Properties, BitSet, etc. handling collections with and without type bounds, iterating over one or two values (maps and map-like).

For now however I am skipping on this effort as a lost a few hours and get nowhere with it.

@MateuszKubuszok
Copy link
Member Author

For now however I am skipping on this effort as a lost a few hours and get nowhere with it.

Actually I managed to fix that! Refactored

/** Since [[io.scalaland.chimney.Transformer]] is NOT automatically provided for identity/subtype transformation,
* and we want to allow such things without enabling whole recursive auto-derivation we use this trick.
*/
trait TransformOrUpcast[From, To] {
Copy link
Member Author

Choose a reason for hiding this comment

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

Question: shouldn't such structure live in core? Would we reuse it somewhere else? If we want to enure binary compatibility we should decide where to put this utility before releasing it since shoving it around would break binary compatibility.

@MateuszKubuszok
Copy link
Member Author

MateuszKubuszok commented Sep 30, 2023

I think that support for java.util.Enum can be done in a separate PR, as this one is already quite large and macro support is not needed for this PR to work. So java.util.Enum is removed from TODOs and java.lang.Enum support will be developed in #393.

@MateuszKubuszok MateuszKubuszok marked this pull request as ready for review October 2, 2023 15:51
@MateuszKubuszok MateuszKubuszok merged commit 051a3eb into master Oct 5, 2023
@MateuszKubuszok MateuszKubuszok deleted the better-java-support branch October 5, 2023 11:19
@MateuszKubuszok
Copy link
Member Author

While it wasn't intended for 0.8.0 (more like 0.8.1) I decided to merge it, as it will be useful in convincing people to update.

@MateuszKubuszok MateuszKubuszok changed the title [Not for 0.8.0] Create Java Collections module Create Java Collections module Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better Java standard libraries interop
1 participant