Should `type_of_restricted_expr()` mark all record attributes as optional?

[cdisselkoen]

Category

Internal refactors/changes

Describe the feature you'd like to request

This issue comes out of this discussion on #360. Currently, the function ValueParser::type_of_rexpr() (before #360) or type_of_restricted_expr() (after #360) returns SchemaTypes in which all record attributes are marked optional. This has some consequences when used in Core's SchemaType::is_consistent_with(). (note that all of the code and types just referenced are in Core, not referring to the validator's type system.) @khieta and @john-h-kastner-aws are confused why this is the case, so it merits investigation at a minimum, and at least some clarifying comments, if not a behavior change or a reorganization of Core's SchemaType or an adjustment to how Core gets schema information from the validator crate.

Describe alternatives you've considered

It's possible that the current behavior is correct, in which case we should just add comments clarifying why it is correct for future folks that might happen across this code.

Additional context

No response

Is this something that you'd be interested in working on?

• 👋 I may be able to implement this feature request
• ⚠️ This feature might incur a breaking change

[andrewmwells-amazon]

Marking attributes as required affects which sets are considered heterogeneous, which would be a breaking change. I think we can close this because that's not viable.

[cdisselkoen]

Should we update the comments in the code?

[cdisselkoen]

Oh I see that #1177 updates the relevant comments

[john-h-kastner-aws]

Resolved by #1177, #1190, #1194, and #1197 which together remove schematype_of_restricted_expr entirely