Simplify enums

[heaths]

While reviewing a Cadl for a service team today, I was surprised by the verbosity of extensible enums:

enum ThingKindValues {
  A;
  B;
}
@knownValues(ThingKindValues)
model ThingKind is string {};
model Thing {
  kind: ThingKind;
}

Seems that could be simplified, but how really depends on whether Cadl wants to follow Azure guidelines by default or not (seems not).

If we say that enums are extensible by default, for example, we could simply declare the enum as, say, @fixed and just use it:

@fixed
enum ThingKind {
  A;
  B;
}
model Thing {
  kind: ThingKind;
}

Conversely, if enums are fixed by default, we could come up with something like @extensible and treat them, effectively as is string {} now.

The syntax is more concise for authors, and this also seems like less guesswork for generators. Take the C# generator, since .NET doesn't support unions nor string-based enums - either of which other languages do: we have to now look at two different models to determine if something should be a regular, fixed enum or an enum-like readonly struct per our current guidelines.

[bterlson]

I think there are two parallel questions here:

1. What should Cadl's opinions about enums be, if any?
2. How should you specify the other way around?

Tackling 2 first, one nice aspect of the present solution is that it is much less error prone. Emitters are not required to interrogate enums to see if they are extensible or not. If they do the simple thing, an extensible enum will just appear as a string or number to them, and they will generate a client that works (albeit with less type system help). I believe that's why we went with the present approach over the @extensible annotation, but perhaps others remember more of the discussion.

Regarding 1, pinging @KrzysztofCwalina who I know also has opinions! While Cadl doesn't presently expose enum strictness in any way you can detect, my opinion is that Cadl enums are fixed because this is the stricter semantics that I think aligns most conceptually with how folks around industry would want to use Cadl. Calling something an enum and then making it just a hint over its base type feels surprising for an API description language, to me.

KC pointed out that, in C#, enums are "by default" extensible in the sense that a method which takes an enum parameter can take any int. It is likely very common for integer enums to behave this way across languages for performance reasons and because it makes using bitflags more ergonomic. But this feels much less justifiable with string enums, and indeed in TypeScript at least string enums are "fixed" and do not allow any string, and I wouldn't be surprised if C# string enums follow a similar pattern. Being fixed is perhaps better for performance for string enums as well.

[heaths]

Yes, it's true that enums in .NET can take any int, but the predominant use case is for strings. In fact, of all the swaggers I've reviewed the past few years, I don't think I've once seen any use besides strings. It's certainly possible and should be allowed, but seems better to focus on the main use case.

I appreciate that Cadl starts off with a strict interpretation and don't argue it's wrong, but I do think that OpenAPI has enums wrong. It doesn't seem version compatibility has been - or is - top of mind. At the very list, enums as a "hint" of possible values provide a better UX for both REST and client libraries. As merely a string - if generators didn't interrogate for more information - it's a worse UX. Besides, couldn't generators just as easily ignore any defined enum values and treat it as a string, while not making generators that want to do the better UX figure out what types to define and which not? As written today, I wouldn't want to see ThingKindValues defined at all and ThingKind to be an enum instead of a model class (or struct), which doesn't really model the Cadl as written.

[nguerrera]

KC pointed out that, in C#, enums are "by default" extensible in the sense that a method which takes an enum parameter can take any int.

This is true, but it's also a bit like saying every TypeScript type is extensible because you can force the compiler to let you pass any value in its place.

[timotheeguerin]

A few things that would/could simplify this

Inline enums

We have plan to allow specifying enum inline which would make the known value pattern simpler

@knownValues(enum {
  A,
  B,
})
model ThingKind is string;

Also looks pretty good with decorators after (Not something you can do now but we have discussed it)

model ThingKind is string @knownValues(enum {
  A,
  B,
})

String union

Should we allow

@knownValues("A" | "B")
model ThingKind is string 

[heaths]

How would authors document string unions in this case? I think that's somewhat important. Too many swaggers already don't do a great job of documenting enum values.

[timotheeguerin]

I knew there was a reason string union wasn't that great of an alternative :) but couldn't remember, you cannot document options.

However, if we forget documentation from a language completeness this feels like we should allow it.

[heaths]

However, if we forget documentation from a language completeness this feels like we should allow it.

I don't think we should. We should be encouraging better documentation, which is what the stewardship board wants to do by asking that default documentation not be generated and have validation to make sure everything is @doc'd.

[timotheeguerin]

Though this is an Azure opinion for Azure SDK and is enforced by the cadl-azure-core linter, the Cadl language doesn't force and shouldn't force documentation.
If we add @knownValues("one"|"two") then the azure core linter will complain that you shouldn't use that anyway. But we shouldn't prevent someone from being to do that outside.

[KrzysztofCwalina]

I think the concept (a union of known values and unknown values) exists in the modeling space. We are now trying to decide how to express this concept in CADL. We can express it as a string with annotations (as it's done today) or as some first-class syntax. I think the latter would be more readable without concrete negatives I can see. If there are concrete negatives, I would love to understand them. I have to say the arguments above read a bit too philosophical to me and I am having problems distilling it to concrete negative implications.

[nguerrera]

We can express it as a string with annotations (as it's done today) or as some first-class syntax.

Or as an enum with annotations as is proposed at the top of the issue. Without expressing a very strong opinion in either direction, I don't think we should add more syntax to the language for this scenario.

[nguerrera]

I think @extensible enum { A, B, C } is OK if people feel really strongly against the string + knownValues approach. I think it is perfectly concrete that an emitter that doesn't do anything with @extensible will emit something more restricitive than it should, but I think it is probably OK to trade that small negative for the gained readability in what is a very common scenario in our own API.

[KrzysztofCwalina]

I think it is probably OK to trade that small negative for the gained readability in what is a very common scenario in our own API.

I think this philosophy should be applied to other concepts in services/libraries. i.e. I love the extensibility CADL offers, but I think we should use these extensibility mechanisms for non-common concepts/scenarios. Core scenarios should be expressible using super readable built-in syntax. I really think there is an opportunity to obsolete API view (and do reviews of CADL files in github) if we simplify the syntax.

[nguerrera]

@extensible enum would still be using Cadl's extensibility via decorators. It's just a more readable thing. I think we often debate first-class vs. via-extension too much instead of just looking for good readability, which doesn't have to exclude decoration.

[heaths]

I really think there is an opportunity to obsolete API view (and do reviews of CADL files in github) if we simplify the syntax.

I'm behind this 100%. I don't want to have to switch context to APIView and not have the back and forth experience of conversing over a PR. I'm already in the notification page reviewing notifications, why have to switch to something separate? And the more concise the language, the easier that is to do. Already, I find reviewing Cadl files so much easier than swagger because they are concise and you can get a better overview of the API.