Skip to content

Comments

Fix the xml doc issue when the parameter's actual name is not the same as their variable name#5615

Merged
ArcturusZhang merged 13 commits intomicrosoft:mainfrom
ArcturusZhang:fix-parameter-name-issue
Jan 16, 2025
Merged

Fix the xml doc issue when the parameter's actual name is not the same as their variable name#5615
ArcturusZhang merged 13 commits intomicrosoft:mainfrom
ArcturusZhang:fix-parameter-name-issue

Conversation

@ArcturusZhang
Copy link
Member

@ArcturusZhang ArcturusZhang commented Jan 15, 2025

Fixes #5565
Fixes #5567
Fixes #5568

The root cause of the issue here is:
the ParameterProvider has a Name property, but it is actually the name of its input parameter, not its actual name in the code which we will never know until we actually write them into the code (because of the scope stuff and name collisions)

Therefore, this PR added a scope version of WriteXmlDocs and allows the parameters to be declared in the xml doc.
For a method, the parameters first appear in the xml docs, and then the method signature, therefore allowing the xml doc part to declare the parameter actually makes sense - since our code writer could only go forwards, no backwards.
With this change, if there is xml doc, when the code writer writes the xml doc for a parameter, if the parameter has a "not declared" CodeWriterDeclaration, it will declare it with the current scope (which is basically the same scope of the method), and then when it writes the method signature, it will find those parameters have been declared and it will just use the name, therefore everything works fine.
If there is no xml doc, when the code writer writes the method signature, it will find those parameters are not declared yet, and declares them. This is the same logic before this change.

To achieve this, this PR has to change the start tag and end tag in the XmlDocProvider to be FormattableStrings. This allows us to utilize the ability of CodeWriter to automatically resolve the declaration of the parameter inside the tag.

@microsoft-github-policy-service microsoft-github-policy-service bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label Jan 15, 2025
@ArcturusZhang ArcturusZhang force-pushed the fix-parameter-name-issue branch from 03ff293 to bad4f99 Compare January 15, 2025 02:24
@ArcturusZhang ArcturusZhang force-pushed the fix-parameter-name-issue branch from 63f7453 to 4a7047e Compare January 15, 2025 02:32
@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 15, 2025

API change check

API changes are not detected in this pull request.

@ArcturusZhang ArcturusZhang marked this pull request as ready for review January 15, 2025 09:45
Copy link
Contributor

@JoshLove-msft JoshLove-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach seems reasonable to me considering the alternative would need to involve either rewinding the writer and writing the xml docs after the method or otherwise some way of constructing scopes ahead of time before writing.

@ArcturusZhang ArcturusZhang added this pull request to the merge queue Jan 16, 2025
Merged via the queue into microsoft:main with commit 59c8375 Jan 16, 2025
21 checks passed
@ArcturusZhang ArcturusZhang deleted the fix-parameter-name-issue branch January 16, 2025 01:59
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

3 participants