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

Add "Add missing XML documentation" codefix #1108

Merged
merged 9 commits into from
May 1, 2023

Conversation

dawedawe
Copy link
Contributor

@dawedawe dawedawe commented Apr 23, 2023

WHAT

🤖 Generated by Copilot at c1d8742

This pull request adds a new code fix feature for adding missing XML documentation comments to F# code. It implements the logic for the code fix in a new module AddMissingXmlDocumentation.fs and adds tests for it in Tests.fs. It also registers the code fix function in the two language server types AdaptiveFSharpLspServer and FSharpLspServer so that it can be invoked by the code action request.

🤖 Generated by Copilot at c1d8742

AddMissingXmlDocumentation
A code fix for F# / kireji
Autumn leaves no gaps / kigo

📝🛠️🧪

WHY

As discussed here, another code fix should handle the insertion of missing xml tags. It's applicable if the cursor is inside of an xml comment with a summary tag above an AST element for which the FCS gives a signature.

Recording.2023-04-23.191411.mp4

HOW

🤖 Generated by Copilot at c1d8742

  • Add a new module AddMissingXmlDocumentation that implements a code fix for adding missing XML documentation to F# symbols (link)
  • Register the AddMissingXmlDocumentation.fix function as a code fix for the AdaptiveFSharpLspServer and FSharpLspServer types, which handle code action requests from the language server protocol (link, link)
  • Define a list of test cases for the AddMissingXmlDocumentation code fix in the CodeFixTests.Tests module, using the CodeFix.check and CodeFix.checkNotApplicable functions to verify the expected behavior (link)
  • Run the addMissingXmlDocumentationTests function as part of the tests function, which is the main entry point for the code fix tests (link)

@dawedawe dawedawe changed the title Addmissingxmldoc codefix Add "Add missing XML documentation" codefix Apr 23, 2023

let private tryGetExistingXmlDoc (pos: FSharp.Compiler.Text.Position) (xmlDoc: PreXmlDoc) =
let containsPosAndSummaryPresent (xd: PreXmlDoc) =
let d = xd.ToXmlDoc(false, None)
Copy link
Contributor

Choose a reason for hiding this comment

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

I was going to ask about doing this work, but then I remembered I raised dotnet/fsharp#14882 already to track it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've put it on my list and will have a go at it in the next couple of weeks.

Comment on lines 1725 to 1733
testCaseAsync "not applicable for function without summary"
<| CodeFix.checkNotApplicable
server
"""
/// some comment$0
let f x y = x + y
"""
Diagnostics.acceptAll
selectCodeFix
Copy link
Contributor

Choose a reason for hiding this comment

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

How hard would it be to make this case work? It'd be really nice to have one gesture to go from 'simple' xmldoc to 'full' xmldoc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I'm afraid there was some misunderstanding on my side. I thought your goal was to have a two step approach like:

  1. convert tripple slash comment to xml comment
  2. add missing xml tags

I'll take a look. Shouldn't be too much work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's in.

Recording.2023-04-26.233807.mp4

@baronfel
Copy link
Contributor

baronfel commented May 1, 2023

Nicely done, @dawedawe - thank again for pushing on this area :)

@baronfel baronfel merged commit 1590ca0 into ionide:main May 1, 2023
@dawedawe dawedawe deleted the addmissingxmldoc_codefix branch May 1, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants