nickel-lang / json-schema-to-nickel

Convert JSON schemas into Nickel contracts
Apache License 2.0
32 stars 0 forks source link

As{Contract,Predicate,...} traits #10

Closed Radvendii closed 1 year ago

Radvendii commented 1 year ago

We had a lot of functions that converted from various types to RichTerm, either producing a contract or a predicate. This change moves them all under a few traits.

Radvendii commented 1 year ago

cargo clippy doesn't like as_foo() functions that take self. It either wants them to take &self, or be called into_foo(). I think this is what I was getting at but couldn't express well when we were working on this.

But is it kind of weird to impl into_foo() for &Bar rather than as_foo() for Bar? I've done this for now, but I don't know the conventions.

But also it's already kind of overkill the number of traits we're creating, so I would feel silly having AsFoo and IntoFoo for everything.

Thoughts? If we were implementing these traits for public consumption rather than on a by-need basis for our own code, what should we include?

vkleen commented 1 year ago

I think the convention would be to implement AsFoo on Type instead of IntoFoo on &Type. The only type that we take ownership of during the conversion is InstanceType, if I recall correctly, and we do that only because InstanceType is Copy. So, I think we should use AsFoo traits throughout.

Radvendii commented 1 year ago

Okay, that's what i thought the convention was. There is a single place we take ownership:

impl IntoLabeledType for RichTerm {
    fn into_labeled_type(self) -> LabeledType {
        LabeledType {
            types: TypeF::Flat(self).into(),
            label: Label::dummy(),
        }
    }
}