Support assigning enum members in compatible scalar types (#4302)

[lorenzodejong]

This PR introduces support for assigning compatible enum members to a scalar type. Related issue: #4302.

As mentioned in the documentation, Enums support the following types:

• string
• integer
• float

The implementation of the emitter framework already allowed the assignment of these values, however the assignment was blocked by the type-relation-checker in the compiler package.

The following assignments are supported:

enum Constants {
  STRING: "foo",
  INT: 123,
  FLOAT: 1.23,
}

model A {
  stringAssignment: string = Constants.STRING;
  intAssignment: int32 = Constants.INT;
  floatAssignment: float32 = Constants.FLOAT,
  floatWithIntAssignment: float32 = Constants.INT
}

The following assignments will raise an error:

enum Constants {
  STRING: "foo",
  INT: 123,
  FLOAT: 1.23,
}

model A {
  stringWithIntAssignment: string = Constants.INT; // Type 'Constants.INT' is not assignable to type 'string'
  stringWithFloatAssignment: string = Constants.FLOAT; // Type 'Constants.FLOAT' is not assignable to type 'string'
  intWithStringAssignment: int32 = Constants.STRING; // Type 'Constants.STRING' is not assignable to type 'int32'
  intWithFloatAssignment: int32 = Constants.FLOAT; // Type 'Constants.FLOAT' is not assignable to type 'int32'
  floatWithStringAssignment: float32 = Constants.STRING // Type 'Constants.STRING' is not assignable to type 'float32'
}

The implementation works the same for all bit types of integers and floats.

[lorenzodejong]

@microsoft-github-policy-service agree

[mario-guerra]

Thanks for the PR @lorenzodejong!

[timotheeguerin]

/azp run typespec - PR Tools

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[timotheeguerin]

Thanks a lot for the contribution, however I do think we need to discuss this a little further before allowing it as it has other consequences. As much as it could make sense for default it might be a bit more tricky in other scenarios and we need to be sure we want that in the type system.

Potential issues(will only take string as example but apply to numeric enums too):

1. A decorator that expect string value will now be able to get an enum member pass to it.
   a. Do we auto convert to the string or now every decorator also need to start handling enum members in those cases? . Previous discussion on this showed some concern that if there is extra info on the enum it could then be silently lost.
   b. What if the decorator accept both valueof string | EnumMember, this one is probably an easy choice it should just be keeping it as an EnumMember as we have precedent for such behavior.

2. This also then apply to type relation in general. If you expect a certain type to be a string it could be an enum member now

3. Do we only count enums that actually have an explicit value defined or enum Foo {a, b} would have the string value a or b like it has over json

[timotheeguerin]

Example of a crash that this now reveal:
PR Playground vs Latest playground

[mario-guerra]

I think we should do what TypeScript does in each case. Here's what I think we should do, with the caveat that my understanding of TypeScript is limited.

• Decorators: Explicit handling of enum members to avoid silent loss of information. If a decorator can accept both string | EnumMember, handle the enum member as is.
• Type Relations: TypeScript ensures type safety when using enum members where a specific type is expected. We should do the same in TypeSpec.
• Serialization: Explicit enum values are serialized as their string values, while implicit values are serialized as numbers.

[timotheeguerin]

I would say that TypeScript is a little different here. Typescript enum are this weird thing that have no direct representation in JS and just map to an object with those keys/values (and are quite hated for that reason as well) so I think its easier in typescript to justify that you can pass an enum of certain value to anything that accept that kind.
For us we do treat enums members as this unique symbols. I don';t think explicit handling is a good solution. Taking in a string is the most common arg a decorator takes and it would involve breaking any decororator that does it on top of being extremely error prone.

My opinion is if you do give an explicit value we could consider using that value in places that expect that type and silently loose any additional information that might be on the member.

Regarding this

Serialization: Explicit enum values are serialized as their string values, while implicit values are serialized as numbers.
this is not what we have been doing

enum Foo {a, b}

will serialize as "a" or "b" for json

[mario-guerra]

and silently loose any additional information that might be on the member.

Would it be worthwhile and possible to emit a suppressible warning so the user is aware of the potential loss and can decide if that's what they want?

[lorenzodejong]

Thanks for the thorough review! I had expected some concerns to pop up, especially taking my limited knowledge of the TypeSpec codebase into account.

As you can see in the proposed code, I'm explicitly converting the EnumMember value to a string/numeric literal type. This information was actually missing in the EnumMember symbol, however is (in my opinion) relevant to the type system in general.

Would explicitly supporting primitive (literal) types in the EnumMember.value property perhaps easen the solution to implement this logic?

Feels like this would be a more fundamental change throughout the type system, however that would open up for more graceful handling of enum members (like the intention of this PR).

[timotheeguerin]

That does sound like a very interesting idea. If we do want to start treating some enum with values as the value it would definitely make sense to keep a reference of the literal.

I think it still would need some special handling in decorator calls to actually retrieve the value but that would be cleaner for sure.

[timotheeguerin]

@lorenzodejong just a little update, we talked about this yersteday in the design meeting and I think everyone was ok about doing the implicit coercion however we are not sure that the defaultValue should remain at this point the EnumValue and not be the string/numeric value itself as this is what we are doing here.
However doing so I believe wouldn't help your use case of trying to use enum members as reusable constant in the generated code.

If we went that way we would probably need a way to trace back if that value was coerced or not. Either with an helper or an additional property originalDefaultValue

[timotheeguerin]

I updated the issue proposal with what we should do. Need to coerce the value to numeric/string value and provide a new helper to retrieve the original if needed.

Let us know if 1. that will still solve your problem and 2. if this is too much for you to keep implementing and we'll take over.

[lorenzodejong]

Hi @timotheeguerin, i think the proposed solution will indeed solve my problem! With my limited knowledge of the codebase, I'm not entirely sure about the implementation strategy so I think the best path forward is for you guys to take over the implementation part.

I will definitely stay involved with the implementation to learn more about the codebase 😄

[azure-sdk]

❌ There is undocummented changes. Run chronus add to add a changeset or click here.

The following packages have changes but are not documented.

• ❌@typespec/compiler

Show changes

[azure-sdk]

You can try these changes here

🛝 Playground	🌐 Website	📚 Next docs