v2 - type upgrade from string to flag enum - easy catch for consumers

[baywet]

While I support the use of a flaggable enum for performance reasons, I believe this is going to lead to a regression that's going to be hard to catch for most people. And which I think our own code is a victim of.

v1 Type is a string
v2 Type is a flaggable enum (currently)

It's super easy to not notice the change in some scenarios, especially the ones involving string interpolation, implicit operators. In that case, overlooking the fact that multiple values are present will lead to cases with multiple entries falling off a cliff.

I think to start with, our to identifier method should throw if multiple values are present, instead of returning null to signal something was wrong in the logic of the application

OpenAPI.NET/src/Microsoft.OpenApi/Extensions/OpenApiTypeMapper.cs

Line 32 in cb5bb27

_ => null,

[darrelmiller]

@baywet We should talk.

[darrelmiller]

@baywet After discussing this with @MaggieKimani1 we were wondering if it would make sense to change the signature of ToIdentifier to return an array of strings and the caller can deal with simplifying the output to a simple string if the array length is one.

[baywet]

I think it's be a step forward, however it doesn't solve the string interporlation or implicit operator issue.

If today I'm doing

if (schema.Type == "string")
{
 // do something
}

I believe this will still compile with v2, but depending on the description, you might end up with string,number as a value at runtime, derailing the application logic.

[baywet]

The rule of thumb guidelines:

• if the method is named singular (no s), it should only ever return a single value or throw.
• if the method is named plural (ends with an s), it should always return an array, which can potentially contain a single entry.
• the naming should be explicit, i.e. using ToFirstIdentifier instead of ToIdentifier and not throw. ToSingleIdentifier and throws if more than one.

[baywet]

PR is already partially started at #2156