Implement type-refinement #333
Conversation
lewisjkl
commented
Jul 22, 2022
lewisjkl
commented
- Add refined trait in smithy4s.meta
- Add example in sampleSpecs
- Add example type correlating to example in sampleSpecs
|
Still have some TODOs that I need to resolve and generally do more testing. Also need to get it working on aggregate shapes like lists, unions, structs, etc. Currently it is just targeting primitives |
| } | ||
| .headOption // TODO: Currently shapes could have multiple traits that have the refined trait. A validator should be added to prevent this |
| hints.get(constraint.tag) match { | ||
| case Some(hint) => | ||
| RefinementSchema(this, constraint.make(hint)) | ||
| case None => ??? // TODO: |
There was a problem hiding this comment.
hah, yeah ... so we have two choices: either we force a C value on this method (which might be a good idea), but implies that we duplicate the rendering. Or we create an "always failed refinement", which breaks typesafety. It might be a good idea to go with the former. It means that the hint value will be duplicated though. No biggie.
Unless we do not render the refinement/validation hints in addHints, but rather have refined (and validated) do : RefinementSchema(this.addHints(c), constraint.make(c))
| .some | ||
| } else Type.PrimitiveType(primitive).some | ||
|
|
||
| getExternalTypeInfo(shape) match { |
There was a problem hiding this comment.
we may want to allow it on collection types too, to enable things like NonEmptyList and whatnot.
| Age.underlyingSchema.optional[TestItOut]("age", _.age), | ||
| PersonAge.underlyingSchema.optional[TestItOut]("personAge", _.personAge), | ||
| FancyList.underlyingSchema.optional[TestItOut]("fancyList", _.fancyList), | ||
| Name.underlyingSchema.optional[TestItOut]("name", _.name), |
There was a problem hiding this comment.
Wasn't sure whether these should be using the underlyingSchema or the schema. I couldn't quite tell when it is appropriate to use one over the other.
There was a problem hiding this comment.
Right, so, I'd like for us to use newtypes by default, and therefore using schema. This implies that the fields in TestItOut would point to the newtypes instead of the wrapped types.
We typically use underlyingSchema when we don't want the newtype. Typically, most of the time for collections, we don't want the newtype, so it's a better default to not wrap them.
In a subsequent PR, we'll add a @unwrapped trait in a subsequent PR, that will target members, so that the corresponding fields and referenced types in Scala will be the underlying type (as opposed to the newtype)
sampleSpecs/refined.smithy
Outdated
| ) | ||
| structure ageFormat {} | ||
|
|
||
| @trait(selector: "list") | ||
| @refined( |
There was a problem hiding this comment.
let's call it "refinement", because it's not the trait that is refined, it's the type that the trait ends up being applied on.
