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.

Sign up for GitHub

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

Place XML doc lines before any attribute lists #1267

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Place XML doc lines before any attribute lists #1267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 42 additions & 27 deletions src/FsAutoComplete.Core/Commands.fs
View file
Open in desktop
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
Open in desktop
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
Loading