Skip to content

[http-client-csharp] Adopt TCGC changes on client hierarchy #5924

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

Closed
ArcturusZhang opened this issue Feb 10, 2025 · 2 comments · Fixed by #6179
Closed

[http-client-csharp] Adopt TCGC changes on client hierarchy #5924

ArcturusZhang opened this issue Feb 10, 2025 · 2 comments · Fixed by #6179
Assignees
Labels
emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp

Comments

@ArcturusZhang
Copy link
Member

ArcturusZhang commented Feb 10, 2025

TCGC changes its design on client hierarchies, now we should be able to adopt the change and simplify our implementation a lot.
TCGC changes here: Azure/typespec-azure#2027

NOTE: this will change the schema of our output json of the emitter, which potentially impacts autorest.csharp and 3rd usages.

As a general principle, we would like our emitter (ts part) to do less by just taking the output from TCGC with minimum changes/conversions. Now TCGC changes its format in the output, therefore we should change the schema of our "input" to align with their output to keep in a parity.

@ArcturusZhang ArcturusZhang added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Feb 10, 2025
@JoshLove-msft
Copy link
Contributor

@ArcturusZhang can you please add more details on what is needed here?

@ArcturusZhang
Copy link
Member Author

@ArcturusZhang can you please add more details on what is needed here?

I updated some more in the description.

@ArcturusZhang ArcturusZhang self-assigned this Mar 3, 2025
github-merge-queue bot pushed a commit that referenced this issue Mar 26, 2025
Fixes #5924
Going forward to the ultimate goal of "only keep minimum code in our
emitter, which should only parse and spit out the data from TCGC", this
PR removes a few unnecessary code in our emitter, and align the
structure of client in our emitter with the client from TCGC.

Client initialization stuff has not been started therefore I still keep
the property `parameters` on the client type to keep maximum
compatibility and minimum the code change. I think the `parameters`
could be gone once our client initialization work item was done.

Operation stuff is also not changed, because the way TCGC represent
operations/methods is different from ours. I also keep that part as a
future work to align.

About client names:
Before, we did client name calculations in the emitter, to assign
verbose name to subclients, such as:
```
namespace Service;

namespace Foo {
	op one(): void;
	
	namespace Bar {
		op two(): void;
		
		namespace Qux {
			op three(): void;
		}
	}
}
```
We will have 4 clients. The hierarchy looks like this:
```
ServiceClient
- Foo
	- Bar
		- Qux
```
As a subclient, for instance, their full name could be
`ServiceClient.Foo.Bar` in this case.
Those second level (or deeper) subclients have a chance to have name
collisions because we would put them in the same namespace previously,
and we could have another subclient like `ServiceClient.Foo2.Bar`.

Therefore previously we are making name changes to them automatically
for those 2nd-level or deeper subclients by prepending all their
parents' name to it.

Now in this PR, it is not the goal here that we change that rule - this
PR just wants them all to unchange - therefore I put this logic at the
place that we have serialized the whole namespace to keep things in
parity.
mario-guerra pushed a commit that referenced this issue Apr 2, 2025
Fixes #5924
Going forward to the ultimate goal of "only keep minimum code in our
emitter, which should only parse and spit out the data from TCGC", this
PR removes a few unnecessary code in our emitter, and align the
structure of client in our emitter with the client from TCGC.

Client initialization stuff has not been started therefore I still keep
the property `parameters` on the client type to keep maximum
compatibility and minimum the code change. I think the `parameters`
could be gone once our client initialization work item was done.

Operation stuff is also not changed, because the way TCGC represent
operations/methods is different from ours. I also keep that part as a
future work to align.

About client names:
Before, we did client name calculations in the emitter, to assign
verbose name to subclients, such as:
```
namespace Service;

namespace Foo {
	op one(): void;
	
	namespace Bar {
		op two(): void;
		
		namespace Qux {
			op three(): void;
		}
	}
}
```
We will have 4 clients. The hierarchy looks like this:
```
ServiceClient
- Foo
	- Bar
		- Qux
```
As a subclient, for instance, their full name could be
`ServiceClient.Foo.Bar` in this case.
Those second level (or deeper) subclients have a chance to have name
collisions because we would put them in the same namespace previously,
and we could have another subclient like `ServiceClient.Foo2.Bar`.

Therefore previously we are making name changes to them automatically
for those 2nd-level or deeper subclients by prepending all their
parents' name to it.

Now in this PR, it is not the goal here that we change that rule - this
PR just wants them all to unchange - therefore I put this logic at the
place that we have serialized the whole namespace to keep things in
parity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants