microsoft / typespec

https://typespec.io/
MIT License
4.51k stars 217 forks source link

`versioning/using-versioned-library` error thrown for imports, with no source information #1953

Open eddyashton opened 1 year ago

eddyashton commented 1 year ago

I'm trying to split a TypeSpec definition up into multiple files. A small repro is something like this:

// File: bar.tsp
import "@typespec/rest";

@doc(" ")
@TypeSpec.Rest.resource("bazs")
model Baz {
    @doc(" ")
    @visibility("read")
    @key
    id: string;
}

interface Bar {
    @doc(" ")
    getBaz is Azure.Core.StandardResourceOperations.ResourceRead<Baz>;
}
// File: main.tspimport "@typespec/versioning";
import "@azure-tools/typespec-azure-core";
import "@typespec/rest";

import "./bar.tsp";

@service({
  title: "My Service",
})
@TypeSpec.Versioning.versioned(MyService.Versions)
namespace MyService;

enum Versions {
  @TypeSpec.Versioning.useDependency(Azure.Core.Versions.v1_0_Preview_2)
  `0.0.1-preview`,
}

@TypeSpec.Http.route("/foo")
interface Foo extends Bar {}

This throws the following error during compilation:

TypeSpec compiler v0.44.0

Diagnostics were reported during compilation:

<unknown location>:1:1 - error @typespec/versioning/using-versioned-library: Namespace '' is referencing types from versioned namespace 'Azure.Core' but didn't specify which versions with @useDependency.
> 1 |
    | ^

Found 1 error.

Note the complete lack of source information on the error, making it extremely hard to track down.

It seems the fix is to add namespace MyService to ./bar.tsp as well. EDIT - that's not a viable fix, since it results in all of the imported interfaces also being listed directly at the top-level of the API. So how should we be splitting service definitions up between multiple files?

I think this broke recently? But I could be wrong, maybe I've juggled my files somehow recently.

Regardless, the error message should be improved, as I imagine this is an extremely common error when refactoring code.

eddyashton commented 1 year ago

I've discovered this is only occurring with @typespec/compiler == 0.44.0 (using types from @azure-tools/typespec-azure-core == 0.30.0, in case it matters). If I pin back to 0.43.0 (and azure-core at 0.29.0 for compatibility), then this pattern compiles with no error.

timotheeguerin commented 1 year ago

your types in bar.tsp needs to be in an namespace that is specifying which version of azure.core it is using.

You probably just want to add namespace MyService; at the top to be in the same namespace as the rest.

At that point you don't need to specify again which version of azure.core to use.

(there was some change in the latest version to catch more offending use case)

eddyashton commented 1 year ago

your types in bar.tsp needs to be in an namespace that is specifying which version of azure.core it is using.

You probably just want to add namespace MyService; at the top to be in the same namespace as the rest.

At that point you don't need to specify again which version of azure.core to use.

(there was some change in the latest version to catch more offending use case)

Adding namespace MyService; to bar.tsp removes the compile error, but results in interface Bar emitting output which duplicates the existing paths.

So the code above, in 0.43.0, would produce a single operation:

{
  ...
  "paths": {
    "/foo/bazs/{id}": {
      "get": {
        "operationId": "Foo_GetBaz"
  ...
}

Adding namespace MyService;, in either 0.43.0 or 0.44.0, produces 2 operations:

  ...
  "paths": {
    "/bazs/{id}": {
      "get": {
        "operationId": "Bar_GetBaz",
        ...
      }
    },
    "/foo/bazs/{id}": {
      "get": {
        "operationId": "Foo_GetBaz",
  ...

It seems like the behaviour is that only things in a namespace get emitted, but everything (even in a global namespace which could be unemitted), must now be versioned. That forbids having "abstract" interfaces in the global namespace which are unversioned, but are only emitted in derived, properly versioned interfaces. Is it possible to only apply the versioning at that point? That's the pattern that was supported previously.

timotheeguerin commented 1 year ago

The rule for what gets emitted is the service namespace(and it’s subnamespace) so any other namespace and the global will not. Any template declaration will also not get included. Same for aliases.

now if you are using types from a versioned library you do need to say which version of that namespace you are using in each different namespace you might have(sub namespace respect the parent by default)

if you want to have types like that you can move them to another namespace and still add the use dependency on azure core.

The pattern you were doing before was not supported it was a bug. It meant that your usage of the types from azure core couldn’t decide which version and would include everything(including removed properties)

eddyashton commented 1 year ago

@timotheeguerin Thanks for the explanation, that's a big help!

Can confirm that adding a separate namespace, with explicit useDependency, for the imported files works as expected - no compile error, and only emits a single path (using the names from the service-decorated namespace).

So my only outstanding question is about the error message in the original case. Do you know why that's pointing at an unknown location, rather than somewhere in bar.tsp?

timotheeguerin commented 1 year ago

hhm, yeah I think is is interesting because the error is reported on the global namespace which doesn't have a declaration anywhere so just show at the top of the file.

Maybe the message should just be different if it is happening on the global namespace.

something like..

Types in the global namespace are referencing types from Azure.Core which is vesioned. Move those types into a versioned namespace

We could also make it that this message show the errors on each offending type instead but that might also be overwhelming and just as confusing(for the regular use case) as the fix is to go back up to the namespace level.

markcowl commented 1 year ago

est: 3