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

Feature: Add iron's RefinedTypeOps support #3245

Merged
merged 3 commits into from
Oct 18, 2023

Conversation

vbergeron
Copy link

Using the Iron library, a common pattern is to hide the constrained type behind an opaque alias, to disallow similarly constrained types (The same result can be obtained using DescribedAs, but I prefer not messing description with identity).

type RefinedIntConstraint = Interval.ClosedOpen[0, 10]
opaque type RefinedInt <: Int = Int :| RefinedIntConstraint

object RefinedInt extends RefinedTypeOpsImpl[Int, RefinedIntConstraint, RefinedInt]

Once the alias is opaque, it is not possible anymore to derive instances with the Value :| Predicate type pattern.

This PR provides instances relying on the given RefinedTypeOps.Mirror[T] instance that is provided by RefinedTypeOpsImpl.

I wonder if the provided tests are enough or if there is corner cases I need to be aware of.

@vbergeron vbergeron force-pushed the tapir-iron-refined-type-support branch from d0820af to cfcf325 Compare October 16, 2023 10:22
@@ -18,6 +18,11 @@ import io.github.iltotore.iron.constraint.all.*
import sttp.tapir.Validator
import sttp.tapir.ValidationError

// Opaque aliases can't be included into class bodies
type RefinedIntConstraint = Interval.ClosedOpen[0, 10]
opaque type RefinedInt <: Int = Int :| RefinedIntConstraint
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't the opaque type alias be defined in a separate file, so that it's alias is not visible inside this file (and hence inside the test)? Otherwise we might be testing behaviour similar to a normal type alias?

Copy link
Author

Choose a reason for hiding this comment

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

Right, I moved the opaque definition to its own file.

@adamw adamw merged commit 60870fe into softwaremill:master Oct 18, 2023
10 checks passed
@adamw
Copy link
Member

adamw commented Oct 18, 2023

Thanks! :)

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.

3 participants