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

fix: enable UUID field overrides with patch #687

Merged
merged 3 commits into from
Feb 4, 2025

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Feb 1, 2025

There seems to have been a regression in the 1.7.x release, where patching a UUID field failed compilation. The problem is not limited to UUIDs, but applies to patches where a field is any pojo class with a single constructor and no matching accessors.

The fix I've proposed here is not an elegant one, but it does seem to work; the basic approach is just to forbid TransformProductToProductRule from applying where From <:< To actually pretty elegant, we just perform the last subtype rule on patches before the product rule

@MateuszKubuszok
Copy link
Member

MateuszKubuszok commented Feb 1, 2025

Probably the same problem as with #683 and would require similar fix to #684 - since Patchers are reimplemented as specialized withFallbacks, fallback issues propagate to Patchers:

  • something prevented subtyping rule to apply
  • subtyping and implicits are skipped without trying only by non-empty overrides
  • so in this case overrides should have been empty but weren't
  • Patchers are without overrides -> non-empty overrides had to be some fallbacks in TransformationContext

I would have to be debugged with enableMacrosLogging, but I probably won't have the time for it anytime soon. If you are interested I'd start with https://github.com/scalalandio/chimney/blob/master/chimney/src/main/scala/io/scalaland/chimney/internal/compiletime/derivation/transformer/rules/TransformProductToProductRuleModule.scala and see how fallbacks are propagated there. Maybe they shouldn't be if:

  • we're using fallback
  • FallbackFrom <:< To
  • other overrides are empty

but that's just a quick guess. Other option might be that Patcher's Implicit/Subtyping rule should be special cased for this example, like a few other rules.

@hughsimpson
Copy link
Contributor Author

Cheers. I'll try to look into this myself in the next couple of days

@hughsimpson hughsimpson changed the title add failing test fix: Don't attempt to apply TransformProductToProductRule when From <:< To Feb 3, 2025
@hughsimpson hughsimpson marked this pull request as ready for review February 3, 2025 17:12
@hughsimpson
Copy link
Contributor Author

@MateuszKubuszok this is the best working fix that I could manage on the issue... let me know what you think, or if there's a better solution that strikes you.

@hughsimpson hughsimpson changed the title fix: Don't attempt to apply TransformProductToProductRule when From <:< To fix: enable UUID field overrides with patch Feb 3, 2025
@MateuszKubuszok MateuszKubuszok merged commit e800b43 into scalalandio:master Feb 4, 2025
22 checks passed
@MateuszKubuszok
Copy link
Member

If it pass all tests, then it's good. :) Thank you!

@hughsimpson hughsimpson deleted the new_test branch February 4, 2025 07:02
@hughsimpson
Copy link
Contributor Author

Brilliant! Thanks for the prompt review and release! 🙏

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.

2 participants