Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: namespace for template instance #4835

Open
4 tasks done
tadelesh opened this issue Oct 23, 2024 · 10 comments
Open
4 tasks done

[Bug]: namespace for template instance #4835

tadelesh opened this issue Oct 23, 2024 · 10 comments
Assignees
Labels
bug Something isn't working needs-area

Comments

@tadelesh
Copy link
Member

tadelesh commented Oct 23, 2024

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

@tadelesh tadelesh added the bug Something isn't working label Oct 23, 2024
@tadelesh
Copy link
Member Author

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

@timotheeguerin
Copy link
Member

@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?

@timotheeguerin timotheeguerin added needs-info Mark an issue that needs reply from the author or it will be closed automatically and removed needs-area labels Oct 28, 2024
@tadelesh
Copy link
Member Author

tadelesh commented Oct 30, 2024

@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.

@microsoft-github-policy-service microsoft-github-policy-service bot added needs-area and removed needs-info Mark an issue that needs reply from the author or it will be closed automatically labels Oct 30, 2024
@timotheeguerin
Copy link
Member

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 timotheeguerin added the needs-info Mark an issue that needs reply from the author or it will be closed automatically label Oct 30, 2024
@timotheeguerin
Copy link
Member

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
Copy link
Member Author

tadelesh commented Oct 31, 2024

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?

@microsoft-github-policy-service microsoft-github-policy-service bot removed the needs-info Mark an issue that needs reply from the author or it will be closed automatically label Oct 31, 2024
@tadelesh
Copy link
Member Author

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

@timotheeguerin
Copy link
Member

timotheeguerin commented Oct 31, 2024

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
Copy link
Member Author

tadelesh commented Nov 1, 2024

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
Copy link
Member

Yeah

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-area
Projects
None yet
Development

No branches or pull requests

2 participants