Skip to content

Commit

Permalink
Place XML doc lines before any attribute lists (#1267)
Browse files Browse the repository at this point in the history
  • Loading branch information
@dawedawe
dawedawe authored Apr 19, 2024
1 parent 5ad3861 commit 9c04d1e
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 27 deletions.
69 changes: 42 additions & 27 deletions src/FsAutoComplete.Core/Commands.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1266,95 +1266,105 @@ type Commands() =
/// calculates the required indent and gives the position to insert the text.
static member GenerateXmlDocumentation(tyRes: ParseAndCheckResults, triggerPosition: Position, lineStr: LineStr) =
asyncResult {
let tryGetFirstAttributeLine (synAttributes: SynAttributes) =
synAttributes
|> List.collect (fun a -> a.Attributes)
|> function
| [] -> None
| attributes ->
attributes
|> Seq.minBy (fun a -> a.Range.StartLine)
|> fun attr -> Some attr.Range.StartLine

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

let isLowerAstElemWithEmptyPreXmlDoc input pos =
let isLowerAstElemWithEmptyPreXmlDoc input pos : Option<bool * Option<int>> =
SyntaxTraversal.Traverse(
pos,
input,
{ new SyntaxVisitorBase<_>() with
member _.VisitBinding(_, defaultTraverse, synBinding) =
match synBinding with
| SynBinding(xmlDoc = xmlDoc; valData = valData) as s when
| SynBinding(attributes = attributes; xmlDoc = xmlDoc; valData = valData) as s when
rangeContainsPos s.RangeOfBindingWithoutRhs pos && xmlDoc.IsEmpty
->
match valData with
| SynValData(memberFlags = Some({ MemberKind = SynMemberKind.PropertyGet }))
| SynValData(memberFlags = Some({ MemberKind = SynMemberKind.PropertySet }))
| SynValData(memberFlags = Some({ MemberKind = SynMemberKind.PropertyGetSet })) -> None
| _ -> Some false
| _ -> Some(false, tryGetFirstAttributeLine attributes)
| _ -> defaultTraverse synBinding

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

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

fields |> List.tryPick isInLine

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

cases |> List.tryPick isInLine

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

cases |> List.tryPick isInLine

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

bindings |> List.tryPick isInLine

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

let isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos =
let isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos : Option<bool * Option<int>> =
SyntaxTraversal.Traverse(
pos,
input,
{ new SyntaxVisitorBase<_>() with

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

let rec findNested decls =
Expand All @@ -1363,10 +1373,10 @@ type Commands() =
match d with
| SynModuleDecl.NestedModule(moduleInfo = moduleInfo; decls = decls) ->
match moduleInfo with
| SynComponentInfo(longId = longId; xmlDoc = xmlDoc) when
| SynComponentInfo(attributes = attributes; longId = longId; xmlDoc = xmlDoc) when
longIdentContainsPos longId pos && xmlDoc.IsEmpty
->
Some false
Some(false, tryGetFirstAttributeLine attributes)
| _ -> findNested decls
| SynModuleDecl.Types(typeDefns = typeDefns) ->
typeDefns
Expand All @@ -1376,22 +1386,26 @@ type Commands() =
members
|> List.tryPick (fun m ->
match m with
| SynMemberDefn.AutoProperty(ident = ident; xmlDoc = xmlDoc) when
| SynMemberDefn.AutoProperty(attributes = attributes; ident = ident; xmlDoc = xmlDoc) when
rangeContainsPos ident.idRange pos && xmlDoc.IsEmpty
->
Some true
Some(true, tryGetFirstAttributeLine attributes)
| SynMemberDefn.GetSetMember(
memberDefnForSet = Some(SynBinding(
xmlDoc = xmlDoc; headPat = SynPat.LongIdent(longDotId = longDotId)))) when
attributes = attributes
xmlDoc = xmlDoc
headPat = SynPat.LongIdent(longDotId = longDotId)))) when
rangeContainsPos longDotId.Range pos && xmlDoc.IsEmpty
->
Some false
Some(false, tryGetFirstAttributeLine attributes)
| SynMemberDefn.GetSetMember(
memberDefnForGet = Some(SynBinding(
xmlDoc = xmlDoc; headPat = SynPat.LongIdent(longDotId = longDotId)))) when
attributes = attributes
xmlDoc = xmlDoc
headPat = SynPat.LongIdent(longDotId = longDotId)))) when
rangeContainsPos longDotId.Range pos && xmlDoc.IsEmpty
->
Some false
Some(false, tryGetFirstAttributeLine attributes)
| _ -> None)
| _ -> None)
| _ -> None)
Expand All @@ -1401,7 +1415,7 @@ type Commands() =

let isAstElemWithEmptyPreXmlDoc input pos =
match isLowerAstElemWithEmptyPreXmlDoc input pos with
| Some isAutoProperty -> Some isAutoProperty
| Some(isAutoProperty, firstAttrLine) -> Some(isAutoProperty, firstAttrLine)
| _ -> isModuleOrNamespaceOrAutoPropertyWithEmptyPreXmlDoc input pos

let trimmed = lineStr.TrimStart(' ')
Expand All @@ -1410,7 +1424,7 @@ type Commands() =

match isAstElemWithEmptyPreXmlDoc tyRes.GetAST triggerPosition with
| None -> return None
| Some(isAutoProperty) ->
| Some(isAutoProperty, firstAttrLine) ->

let signatureData =
Commands.SignatureData tyRes triggerPosition lineStr |> Result.ofCoreResponse
Expand Down Expand Up @@ -1450,7 +1464,8 @@ type Commands() =
|> fun s -> s + Environment.NewLine // need a newline at the very end

// always insert at the start of the line, because we've prepended the indent to the start of the summary section
let insertPosition = Position.mkPos triggerPosition.Line 0
let insertPosLine = firstAttrLine |> Option.defaultValue triggerPosition.Line
let insertPosition = Position.mkPos insertPosLine 0

return
Some
Expand Down
35 changes: 35 additions & 0 deletions test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1556,6 +1556,24 @@ let private generateXmlDocumentationTests state =
let f x y = x + y
"""

testCaseAsync "documentation for function with attribute"
<| CodeFix.check
server
"""
[<System.Obsolete("foo")>]
let $0f x y = x + y
"""
Diagnostics.acceptAll
selectCodeFix
"""
/// <summary></summary>
/// <param name="x"></param>
/// <param name="y"></param>
/// <returns></returns>
[<System.Obsolete("foo")>]
let f x y = x + y
"""

testCaseAsync "documentation for use"
<| CodeFix.check
server
Expand Down Expand Up @@ -1588,6 +1606,23 @@ let private generateXmlDocumentationTests state =
/// <summary></summary>
type MyRecord = { Foo: int }
"""

testCaseAsync "documentation for record type with multiple attribute lists"
<| CodeFix.check
server
"""
[<System.Obsolete("foo")>]
[<System.Serializable>]
type MyRec$0ord = { Foo: int }
"""
Diagnostics.acceptAll
selectCodeFix
"""
/// <summary></summary>
[<System.Obsolete("foo")>]
[<System.Serializable>]
type MyRecord = { Foo: int }
"""

testCaseAsync "documentation for discriminated union type"
<| CodeFix.check
Expand Down

0 comments on commit 9c04d1e

Please sign in to comment.