How to interpret inheritance with and without @discriminator

[MaryGao]

Say we have inheritance case without discriminator decorator(playground). We have Pet as request body in create operation. I was wondering how to interpret this in the semantic:

• Do we have inheritance releationship between Pet and Cat or Dog? Or just the composization relationship due Cat contains all properties of Pet?
• Do we allow Cat or Dog as valid parameter in operation create?

The same questions are for enabling the decorator @discriminator("kind"). Are Cat or Dog valid parameters in operation create?

// @discriminator("kind")
model Pet { 
  name: string; 
  weight?: float32; 
} 
model Cat extends Pet { 
  kind: "cat"; 
  meow: int32; 
} 
model Dog extends Pet { 
  kind: "dog"; 
  bark: string; 
} 
op create(@body body: Pet): void; 

[timotheeguerin]

extends is meant to describe inheritance relationship between models and I believe emitters should try to convey that if they can. Now it is true that without a discriminator you cannot deserialize to the right model.
There "could" be some valid reason to use inheritance without a discriminator but if you reference the base model you probably made an error and maybe you just meant to use composition via spread(... or model is) . We have an issue to lint against that scenario for azure specs https://github.com/Azure/typespec-azure/issues/3510.

[MaryGao]

Let me ask more directly - Do we have any implicit information the Cat is not allowed for that method in above example from TypeSpec point of view?

[timotheeguerin]

After talking with @MaryGao offline, I'll clarify a few things.

From a pure typespec point of view the example above is totally correct and an operation would allow a Cat model or Dog model to be passed to create.

If we think for example of typespec as a way to generate typescript interfaces(Or any other language) but that doesn't involve any serialization then there is no reason this shouldn't be allowed. An example of this use case is generation of the decorators typescript types form the typespec definition.

But as pointed in previous reply as soon as serialization is involved you are not able to deserialize the Cat or Dog model as you can't know which one it was without some discriminator. So in the case of an http library it definitely has little to no value to write that. If you want to support it, it is fine but this is also no something I would ever expect to be used correctly.

[tadelesh]

For tsp, it is valid. But I don't think it is valid for http. There is no good to write this but will involve lots of problems including ser/deser problem.

[bterlson]

I have what is possibly a comparable interpretation, though I'm not entirely sure. Sharing in case it helps.

From my perspective, ignoring decorators, I would not expect an emitter or serializer to handle subclasses of a base class given an operation like op create(@body body: Pet): void;. To me, this says this operation takes a Pet and will serialize a Pet. Whether subclasses of Pet could be passed in to a generated operation or not would depend on the language, but as far as the serializer is concerned, the expectation is that the endpoint wants a Pet and a Pet only.

The fix to this interpretation is to be specific. Again in the absence of any decorators, that would mean doing something like: op create(@body body: Cat | Dog): void;. Now this clearly means you can pass one of these pets. We generate the corresponding OpenAPI from such a definition, but as noted, deserialization becomes problematic for typed languages. There are potential strategies for handling this, including writing a linter flagging such usages of non-discriminated unions.

As far as I'm concerned, the fact that adding @discriminator to the base type makes references to the base type behave as if its a union of all of its subtypes is a strange thing™️, and have on numerous occasions wished I could delete it in favor of just allowing @discriminator on union declarations. This approach is more clear and more flexible, e.g. works in cases where you want to allow a Dog and a Cat but not a Lizard for some endpoints, though has the (IMO minor) cost of having to declare an additional union of all the base types if that's something you need.

[qiaozha]

Maybe I misunderstood, but can we treat inheritance and polymorphism two different things?

Inheritance is just reuse some of the the base class stuff like properties methods, which means there's no way to call a subclass method via a baseclass pointers, in those scenarios, base class are just normal class. In this case, when we get an object of subclass, it's depends on whether we can accept additional properties for this base class instead of from an inheritance perspective to serialize or deserialize it.

but if we want to change the runtime or compile behavior according to the type. in this case, we could use a base class pointers to call a sub class method, we have to specify discriminator to indicate there's a polymorphism here,

which means in the above case,

@discriminator("kind")
model Pet { 
  name: string; 
  weight?: float32; 
}  

we got an operation here op create(@body body: Pet): void; any places Pet exists can be replaced by anyof its subclass and fallback to itself if no subclass matches. In that case, they don't need a union to specify there's a polymorphism here.

[MaryGao]

Whether subclasses of Pet could be passed in to a generated operation or not would depend on the language.

Service team leverages typespec to express their backend contract, regarding the above statement op create(@body body: Pet): void; do we have a clear answer for them - the body is open for Pet and its subclasses OR the body is in a closed state and only open to accept Pet objects?

If this is a general question for all services then emitter could follow that, if it is a case-by-case decision then emitter could do nothing.

The fact that adding @Discriminator to the base type makes references to the base type behave as if its a union of all of its subtypes is a strange thing

It looks like magic to me. @Discriminator is not just pointing discriminator property but like a syntax sugar. It seems that below two cases are equivalent in typespec.

// Case 1: with discrinminator
@discriminator("kind")
model Pet { 
  name: string; 
  weight?: float32; 
} 
model Cat extends Pet { 
  kind: "cat"; 
  meow: int32; 
} 
model Dog extends Pet { 
  kind: "dog"; 
  bark: string; 
} 
op create(@body body: Pet): void; 

// Case 2: without discrinminator
model Pet { 
  name: string; 
  kind: string; // adding discriminator property if missing
  weight?: float32; 
} 
model Cat extends Pet { 
  kind: "cat"; 
  meow: int32; 
} 
model Dog extends Pet { 
  kind: "dog"; 
  bark: string; 
} 
op create(@body body: Cat | Dog // replace the base class with union of subs): void; 

[bterlson]

Maybe I misunderstood, but can we treat inheritance and polymorphism two different things?

I consider them separate from an API definition standpoint, but they cannot be separated for all emit targets where inheritance implies polymorphism. But I think this is fine.

but if we want to change the runtime or compile behavior according to the type. in this case, we could use a base class pointers to call a sub class method, we have to specify discriminator to indicate there's a polymorphism here,

I believe that is the intention of the @discriminator-on-base-class, but is the behavior I'm skeptical about and would prefer being explicit with unions.

Service team leverages typespec to express their backend contract, regarding the above statement op create(@Body body: Pet): void; do we have a clear answer for them - the body is open for Pet and its subclasses OR the body is in a closed state and only open to accept Pet objects?

IMO the contract is simply that the service will take a Pet and may or may not support additional properties depending on convention we prefer. The default position would be that subtypes are allowed as that aligns with the TypeSpec type system's semantics, and in general, I believe services often do accept additional properties, so any subtype of Pet would be allowed, not just those that happen to be declared in the TypeSpec (@johanste has talked to me about this before, maybe he knows more). But in either case, I don't think the presence of subclasses should affect what this code means (in the absence of @Discriminator at least).

It seems that below two cases are equivalent in typespec.

They are equivalent from a type system perspective yes (and I strongly prefer the latter), however @Discriminator on the union is still useful for codegen because we don't need to infer what the discriminator is and we can ensure the discriminator is present on all union members as the API evolves. So I prefer this pattern:

model PetBase { 
  name: string; 
  kind: string; // adding discriminator property if missing
  weight?: float32; 
} 
model Cat extends PetBase { 
  kind: "cat"; 
  meow: int32; 
} 
model Dog extends PetBase { 
  kind: "dog"; 
  bark: string; 
}

@discriminator("kind")
union Pet {
  Cat;
  Dog;
}
op create(@body body: Pet): void; 

We have also discussed allowing union to express a subtyping relationship such that all union variants must be a subtype of the specified type, so Pet could be defined as:

@discriminator("kind")
union Pet extends PetBase {
  Cat;
  Dog;
}

This effectively prevents random other types that happen to have a "kind" field from being added to the union.

[markcowl]

Note that there is an issue around how we should encourage types using extends in Azure APIs to use discriminators here: https://github.com/Azure/typespec-azure/issues/3510

[markcowl]

closing a sduplicate