[Bug]: namespace for template instance

tadelesh commented 2 weeks ago

Describe the bug

currently, the namespace for template operation or model instance will follow the original template instance. but imo, it should follow the interface or operation that contains the operation or model instance.

for this playground example, i want the return type of test, the inner operation in the inner template and its return type should all have namespace of User.

Reproduction

playground

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.

tadelesh commented 1 week ago

@timotheeguerin this will block emitter's work on namespace. could you prioritize this?

timotheeguerin commented 1 week ago

@tadelesh I don't think this is unexpected, that operation is still a template and so still belongnig to the previous namespace isn't wrong. Why are are you navigating to this type you really shouldn't ever see it?

tadelesh commented 1 week ago

@timotheeguerin the User,inner interface is the instance of Template.Inner and it will have the instance operation op inner(): Test<string>, here the inner and Test<string> are not template, right? but they all have the namespace Template which i think is wrong. i have added another operation test2, it uses template instance Template.Test<int32> directly as the return type, then this return type will also have the namespace of Template which i also think is not right.

timotheeguerin commented 1 week ago

the return type having namespace of Template is 100% correct. It is still the template declaration of Test. For the operation in the template I guess it could be debatable but I think its just better we are consistent and we don't reassign namespace for non fully instantiated templates.

Goes back to the question why do you even check this this should never be used in regular emitter(TypeSpec ref doc generation would be special case which would want to get the orignal templates declaration). If you are getting this because the user wrote that, yoiu should be filtering types to make sure they are not template declaration if you are not using the semantic navigator which already does that for you

timotheeguerin commented 1 week ago

sorry for some reason thought that Inner was also a templated operation. Still for the model its doesn't make sense ot change the namespec for the operation I guess it probably should

tadelesh commented 1 week ago

so, in my example, the return type for both test, test2, userInner.test should all be the same type Test<string> (iow, the type we get from compiler is the same object) in the template namespace, right? if so, then is the userInner.test and userInner2.test the same operation type?

tadelesh commented 1 week ago

and if the Test<T> changed to alias, then will it be the same type?

timotheeguerin commented 1 week ago

the operations are not the same because you an op is or interface extends so it creates a different one that isn't a template instance (and you can augment for example) but if you had this

model ThatRefOps {
   a: inner<string>
 b: inner<string>
}

then both of those would be the same op yeah

and if the Test changed to alias, then will it be the same type?

that wouldn;t change but not sure I understand what you meant

tadelesh commented 1 week ago

oh, model property could also ref operations, amazing. i mean for the following tsp, both test1 and test2 return type is the same type?

alias Test<T> = {
  prop: T;
}

op test1(): Test<string>;
op test2(): Test<string>;

timotheeguerin commented 1 week ago

Yeah

microsoft / typespec