Skip to content

Commit

Permalink
Improve xml doc command:
Browse files Browse the repository at this point in the history 
- Don't add xml doc lines if there are already some doc lines present.
- Work on all possible AST elements, not just the ones for which we get a result from TryGetSignatureData.

Adjust tests accordingly
  • Loading branch information
@dawedawe
dawedawe committed Apr 21, 2023
1 parent 4edbebd commit ff2ee3d
Show file tree
Hide file tree
Showing 3 changed files with 214 additions and 168 deletions.
158 changes: 143 additions & 15 deletions src/FsAutoComplete.Core/Commands.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ open System.Collections.Immutable
open System.Collections.Generic
open Ionide.ProjInfo.ProjectSystem
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text.Range


[<RequireQualifiedAccess>]
Expand Down Expand Up @@ -1978,14 +1979,138 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
/// calculates the required indent and gives the position to insert the text.
static member GenerateXmlDocumentation(tyRes: ParseAndCheckResults, triggerPosition: Position, lineStr: LineStr) =
asyncResult {
let longIdentContainsPos (longIdent: LongIdent) (pos: FSharp.Compiler.Text.pos) =
longIdent
|> List.tryFind (fun i -> rangeContainsPos i.idRange pos)
|> Option.isSome

let isLowerAstElemWithEmptyPreXmlDoc input pos =
SyntaxTraversal.Traverse(
pos,
input,
{ new SyntaxVisitorBase<_>() with
member _.VisitBinding(_, defaultTraverse, synBinding) =
match synBinding with
| SynBinding(xmlDoc = xmlDoc) as s when
rangeContainsPos s.RangeOfBindingWithoutRhs pos && xmlDoc.IsEmpty
->
Some()
| _ -> defaultTraverse synBinding

member _.VisitComponentInfo(_, synComponentInfo) =
match synComponentInfo with
| SynComponentInfo(longId = longId; xmlDoc = xmlDoc) when
longIdentContainsPos longId pos && xmlDoc.IsEmpty
->
Some()
| _ -> None

member _.VisitRecordDefn(_, fields, _) =
let isInLine c =
match c with
| SynField(xmlDoc = xmlDoc; idOpt = Some ident) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None

fields |> List.tryPick isInLine

member _.VisitUnionDefn(_, cases, _) =
let isInLine c =
match c with
| SynUnionCase(xmlDoc = xmlDoc; ident = (SynIdent(ident = ident))) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None

cases |> List.tryPick isInLine

member _.VisitEnumDefn(_, cases, _) =
let isInLine b =
match b with
| SynEnumCase(xmlDoc = xmlDoc; ident = (SynIdent(ident = ident))) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None

cases |> List.tryPick isInLine

member _.VisitLetOrUse(_, _, defaultTraverse, bindings, _) =
let isInLine b =
match b with
| SynBinding(xmlDoc = xmlDoc) as s when
rangeContainsPos s.RangeOfBindingWithoutRhs pos && xmlDoc.IsEmpty
->
Some()
| _ -> defaultTraverse b

bindings |> List.tryPick isInLine

member _.VisitExpr(_, _, defaultTraverse, expr) = defaultTraverse expr } // needed for nested let bindings
)

let isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos =
SyntaxTraversal.Traverse(
pos,
input,
{ new SyntaxVisitorBase<_>() with

member _.VisitModuleOrNamespace(_, synModuleOrNamespace) =
match synModuleOrNamespace with
| SynModuleOrNamespace(longId = longId; xmlDoc = xmlDoc) when
longIdentContainsPos longId pos && xmlDoc.IsEmpty
->
Some()
| SynModuleOrNamespace(decls = decls) ->

let rec findNested decls =
decls
|> List.tryPick (fun d ->
match d with
| SynModuleDecl.NestedModule(moduleInfo = moduleInfo; decls = decls) ->
match moduleInfo with
| SynComponentInfo(longId = longId; xmlDoc = xmlDoc) when
longIdentContainsPos longId pos && xmlDoc.IsEmpty
->
Some()
| _ -> findNested decls
| SynModuleDecl.Types(typeDefns = typeDefns) ->
typeDefns
|> List.tryPick (fun td ->
match td with
| SynTypeDefn(typeRepr = SynTypeDefnRepr.ObjectModel(_, members, _)) ->
members
|> List.tryPick (fun m ->
match m with
| SynMemberDefn.AutoProperty(ident = ident; xmlDoc = xmlDoc) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None)
| _ -> None)
| _ -> None)

findNested decls }
)

let isAstElemWithEmptyPreXmlDoc input pos =
match isLowerAstElemWithEmptyPreXmlDoc input pos with
| Some xml -> Some xml
| _ -> isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos

let trimmed = lineStr.TrimStart(' ')
let indentLength = lineStr.Length - trimmed.Length
let indentString = String.replicate indentLength " "

match! Commands.SignatureData tyRes triggerPosition lineStr |> Result.ofCoreResponse with
match isAstElemWithEmptyPreXmlDoc tyRes.GetAST triggerPosition with
| None -> return None
| Some(_, memberParameters, genericParameters) ->
| Some() ->

let signatureData =
Commands.SignatureData tyRes triggerPosition lineStr |> Result.ofCoreResponse

let summarySection = "/// <summary></summary>"

Expand All @@ -2001,19 +2126,22 @@ type Commands(checker: FSharpCompilerServiceChecker, state: State, hasAnalyzers:
seq {
yield summarySection

match memberParameters with
| [] -> ()
| parameters ->
yield!
parameters
|> List.concat
|> List.mapi (fun _index parameter -> parameterSection parameter)

match genericParameters with
| [] -> ()
| generics -> yield! generics |> List.mapi (fun _index generic -> genericArg generic)

yield returnsSection
match signatureData with
| Ok(Some(_, memberParameters, genericParameters)) ->
match memberParameters with
| [] -> ()
| parameters ->
yield!
parameters
|> List.concat
|> List.mapi (fun _index parameter -> parameterSection parameter)

match genericParameters with
| [] -> ()
| generics -> yield! generics |> List.mapi (fun _index generic -> genericArg generic)

yield returnsSection
| _ -> ()
}
|> Seq.map (fun s -> indentString + s)
|> String.concat Environment.NewLine
Expand Down
152 changes: 16 additions & 136 deletions src/FsAutoComplete/CodeFixes/GenerateXmlDocumentation.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,154 +5,34 @@ open FsAutoComplete.CodeFix.Types
open Ionide.LanguageServerProtocol.Types
open FsAutoComplete
open FsAutoComplete.LspHelpers
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text.Range

let title = "Generate placeholder XML documentation"

let private longIdentContainsPos (longIdent: LongIdent) (pos: FSharp.Compiler.Text.pos) =
longIdent
|> List.tryFind (fun i -> rangeContainsPos i.idRange pos)
|> Option.isSome

let private isLowerAstElemWithEmptyPreXmlDoc input pos =
SyntaxTraversal.Traverse(
pos,
input,
{ new SyntaxVisitorBase<_>() with
member _.VisitBinding(_, defaultTraverse, synBinding) =
match synBinding with
| SynBinding(xmlDoc = xmlDoc) as s when rangeContainsPos s.RangeOfBindingWithoutRhs pos && xmlDoc.IsEmpty ->
Some()
| _ -> defaultTraverse synBinding

member _.VisitComponentInfo(_, synComponentInfo) =
match synComponentInfo with
| SynComponentInfo(longId = longId; xmlDoc = xmlDoc) when longIdentContainsPos longId pos && xmlDoc.IsEmpty ->
Some()
| _ -> None

member _.VisitRecordDefn(_, fields, _) =
let isInLine c =
match c with
| SynField(xmlDoc = xmlDoc; idOpt = Some ident) when rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty ->
Some()
| _ -> None

fields |> List.tryPick isInLine

member _.VisitUnionDefn(_, cases, _) =
let isInLine c =
match c with
| SynUnionCase(xmlDoc = xmlDoc; ident = (SynIdent(ident = ident))) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None

cases |> List.tryPick isInLine

member _.VisitEnumDefn(_, cases, _) =
let isInLine b =
match b with
| SynEnumCase(xmlDoc = xmlDoc; ident = (SynIdent(ident = ident))) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None

cases |> List.tryPick isInLine

member _.VisitLetOrUse(_, _, defaultTraverse, bindings, _) =
let isInLine b =
match b with
| SynBinding(xmlDoc = xmlDoc) as s when rangeContainsPos s.RangeOfBindingWithoutRhs pos && xmlDoc.IsEmpty ->
Some()
| _ -> defaultTraverse b

bindings |> List.tryPick isInLine

member _.VisitExpr(_, _, defaultTraverse, expr) = defaultTraverse expr } // needed for nested let bindings
)

let private isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos =
SyntaxTraversal.Traverse(
pos,
input,
{ new SyntaxVisitorBase<_>() with

member _.VisitModuleOrNamespace(_, synModuleOrNamespace) =
match synModuleOrNamespace with
| SynModuleOrNamespace(longId = longId; xmlDoc = xmlDoc) when
longIdentContainsPos longId pos && xmlDoc.IsEmpty
->
Some()
| SynModuleOrNamespace(decls = decls) ->

let rec findNested decls =
decls
|> List.tryPick (fun d ->
match d with
| SynModuleDecl.NestedModule(moduleInfo = moduleInfo; decls = decls) ->
match moduleInfo with
| SynComponentInfo(longId = longId; xmlDoc = xmlDoc) when
longIdentContainsPos longId pos && xmlDoc.IsEmpty
->
Some()
| _ -> findNested decls
| SynModuleDecl.Types(typeDefns = typeDefns) ->
typeDefns
|> List.tryPick (fun td ->
match td with
| SynTypeDefn(typeRepr = SynTypeDefnRepr.ObjectModel(_, members, _)) ->
members
|> List.tryPick (fun m ->
match m with
| SynMemberDefn.AutoProperty(ident = ident; xmlDoc = xmlDoc) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some()
| _ -> None)
| _ -> None)
| _ -> None)

findNested decls }
)

let private isAstElemWithEmptyPreXmlDoc input pos =
match isLowerAstElemWithEmptyPreXmlDoc input pos with
| Some xml -> Some xml
| _ -> isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos

let fix (getParseResultsForFile: GetParseResultsForFile) : CodeFix =
fun codeActionParams ->
asyncResult {
let filePath = codeActionParams.TextDocument.GetFilePath() |> Utils.normalizePath
let fcsPos = protocolPosToPos codeActionParams.Range.Start
let! (parseAndCheck, lineStr, _sourceText) = getParseResultsForFile filePath fcsPos
let showFix = isAstElemWithEmptyPreXmlDoc parseAndCheck.GetAST fcsPos

match showFix with
| Some _ ->
let! docEdit = Commands.GenerateXmlDocumentation(parseAndCheck, fcsPos, lineStr)
let! docEdit = Commands.GenerateXmlDocumentation(parseAndCheck, fcsPos, lineStr)

match docEdit with
| Some({ InsertPosition = insertPosition
InsertText = formattedXmlDoc }) ->
let protocolPos = fcsPosToLsp insertPosition
match docEdit with
| Some({ InsertPosition = insertPosition
InsertText = formattedXmlDoc }) ->
let protocolPos = fcsPosToLsp insertPosition

let editRange =
{ Start = protocolPos
End = protocolPos }
let editRange =
{ Start = protocolPos
End = protocolPos }

let text = formattedXmlDoc
let text = formattedXmlDoc

return
[ { Edits = [| { Range = editRange; NewText = text } |]
File = codeActionParams.TextDocument
Title = title
SourceDiagnostic = None
Kind = FixKind.Refactor } ]
| _ -> return []
| None -> return []
return
[ { Edits = [| { Range = editRange; NewText = text } |]
File = codeActionParams.TextDocument
Title = title
SourceDiagnostic = None
Kind = FixKind.Refactor } ]
| _ -> return []
}
Loading

0 comments on commit ff2ee3d

Please sign in to comment.