[Bug]: Issues using Versioning decorators with operations

tjprescott commented 3 months ago

Describe the bug

A service team is trying to use version to describe changes to an operation signature. The following arguable should work, but throws errors:

@added(Versions.v2)
model MyOptions {
  blah: string;
}

@removed(Versions.v2)
@put
op foo(): Foo;

@added(Versions.v2)
@put
op foo(...MyOptions): Foo;

1) The first issue is the re-use of the name foo. It seems intuitive that the name is not actually duplicated, because the first foo exists only in v1 and the second only in v2. But maybe there's a good technical reason why we can't allow that, so you can just change them to fooOld and fooNew... but THEN... 2) You get a "duplicate operation routed at" error and are blocked. You can't realistically make them point to different routes because the whole point is it is the same operation.

So it seems like the errors around routes and maybe around names needs to take versioning into account.

Reproduction

https://cadlplayground.z22.web.core.windows.net/?c=aW1wb3J0ICJAdHlwZXNwZWMvdmVyc2lvbmluZyI7DQrSIGh0dHDEGg0KdXNpbmcgVskwO8gTSHR0cMUiQHNlcnZpY2Uoew0KICBuYW1lOiAiRGVtbyBTxhkiLA0KfSkNCkDHdmVkKMdQcykNCsQxc3BhY2UgxDTFUmVudW3Ic3MgxVd2MSzFBzLETMQkbW9kZWwgRm9vxiDGd3N0cuYAo8UjQGFkZMtsLnYyKcg4TXlPcHTKXmJsYWjRPnJlbW920UBAcHV0DQpvcCBmb28oKTrkAILmAQbUbc0tLi4uyXfJOQ%3D%3D&e=%40typespec%2Fopenapi3&options=%7B%7D

Checklist

[X] Follow our Code of Conduct
[X] Check that there isn't already an issue that request the same bug to avoid creating a duplicate.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion.
[X] The provided reproduction is a minimal reproducible example of the bug.

bterlson commented 3 months ago

I don't think we want to allow name conflicts (otherwise references need to become version-aware, or references would need to resolve to some kind of version record instead of a type which must be handled by the referencee, both are very big changes with severe tradeoffs I think), but making the duplicate route error go away in this case feels doable at first glance.

tjprescott commented 3 months ago

Here's the workaround for this scenario Playground:

@added(Versions.v2)
model MyOptions {
  blah: string;
}

@sharedRoute
@removed(Versions.v2)
@renamedFrom(Versions.v2, "foo")
@put
op fooOld(): Foo;

@sharedRoute
@added(Versions.v2)
@put
op foo(...MyOptions): Foo;

microsoft / typespec