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

Better support for collections outside Scala's std lib #444

Closed
wants to merge 1 commit into from

Conversation

MateuszKubuszok
Copy link
Member

Java collections module introduced on 0.8.0 solved some problems:

  • provided support for Java optional type (java.util.Optional) similar to support for scala.Option
  • provided support for Java collections

It has however a few drawbacks:

  • it uses TransformerOrUpcast type to avoid issues with automatic vs semiautomatic derivation (prevents having to pick between 2 imports, one for automatically derived internal types and one for only semiautomatically derived)
  • it only handles Scala StdLib <=> Java Collections, or Java Collections <=> Java Collections
  • if we introduced support for e.g. Cata NonEmpty collection we could convert Cats' NonEmptyList to Scala's List and Scala's List to Java's List but not Cats' NonEmptyList to Java's list which might be counterintuitive
  • problem would got even worse if someone wanted to add support for more collections from some other library

To handle it, the best solution would be to:

  • create a typeclass for optional types
  • create a typeclass for iterating over the collection
  • create a typeclass for creating the collection
  • define implicits using these typeclasses in core module (allowing to select automatic or semiautomatic support)
  • make Java Collections module only a collections of instances for iterating and building collections
  • allow creating instances for NonEmpty* Cats collection in some sane manner

This would be a breaking change, so it should not be merged as a part of 0.8.x but it can become a part of future 1.0.0-RC line.

TODO:

  • define new type classes
  • define new implicits (both automatic and semiautomatic) based on these type classes
    • test that these implicits work and do not create conflicts!
  • define instances in Java Collections module
    • test them
  • remove old solution
    • optionally make some "old" types deprecated aliases to the new types
  • document changes in docs
    • document how to provide support for custom Option (update existing section) or custom collection (create a new section)

@MateuszKubuszok MateuszKubuszok added breaking change Change resulting in compilation errors of code that used to be considered correct, or linking errors blocked Ticket cannot be implemented because it depends on another ticker or external factor labels Dec 13, 2023
Copy link

codecov bot commented Dec 13, 2023

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 90.08%. Comparing base (a85aec3) to head (d070c16).
Report is 10 commits behind head on master.

Files Patch % Lines
...calaland/chimney/integrations/TotalFactoryOf.scala 0.00% 4 Missing ⚠️
...io/scalaland/chimney/integrations/IteratorOf.scala 0.00% 1 Missing ⚠️
...laland/chimney/integrations/PartialFactoryOf.scala 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #444      +/-   ##
==========================================
+ Coverage   90.06%   90.08%   +0.01%     
==========================================
  Files         125      128       +3     
  Lines        4763     4761       -2     
  Branches      430      426       -4     
==========================================
- Hits         4290     4289       -1     
+ Misses        473      472       -1     

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

@MateuszKubuszok MateuszKubuszok added this to the 1.0.0-RC milestone Dec 19, 2023
@MateuszKubuszok MateuszKubuszok removed the blocked Ticket cannot be implemented because it depends on another ticker or external factor label Jan 11, 2024
@MateuszKubuszok
Copy link
Member Author

I have a better idea now:

  • define new type classes (iteration, total builder, partial builder)
  • define something similar to IterableOrArray in macros logic which would fetch these type classes
  • handle code generation inside a macro

This collection-awareness would let us define something like withFieldComputed(_.foo.eachIndex.bar.eachMapValue.baz, a => ...) in the future.

@MateuszKubuszok
Copy link
Member Author

This one will be much easier to implement one #490 lands

@MateuszKubuszok MateuszKubuszok added the blocked Ticket cannot be implemented because it depends on another ticker or external factor label Apr 4, 2024
@MateuszKubuszok
Copy link
Member Author

Postponed until #493 lands - then providing integrations would not require providing nested path improvements in the same PR.

@MateuszKubuszok
Copy link
Member Author

Closed in favor of #505

@MateuszKubuszok MateuszKubuszok deleted the collections-cleanups branch April 12, 2024 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Ticket cannot be implemented because it depends on another ticker or external factor breaking change Change resulting in compilation errors of code that used to be considered correct, or linking errors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant