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

Preserve return attribute #839

Merged
merged 10 commits into from
May 22, 2020
14 changes: 13 additions & 1 deletion src/Fantomas.Tests/AttributeTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -333,4 +333,16 @@ open System.Runtime.InteropServices
[<assembly:AssemblyDescription("")>]

do ()
"""
"""

[<Test>]
let ``should preserve single return type attribute`` () =
formatSourceString false """let f x : [<return: Attribute>] int = x""" config
|> should equal """let f x: [<return:Attribute>] int = x
"""

[<Test>]
let ``should preserve multiple return type attributes`` () =
formatSourceString false """let f x : [<return: AttributeOne;AttributeTwo;AttributeThree("foo")>] int = x""" config
|> should equal """let f x: [<return:AttributeOne; AttributeTwo; AttributeThree("foo")>] int = x
"""
20 changes: 13 additions & 7 deletions src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ and addSpaceAfterGenericConstructBeforeColon ctx =
sepNone
<| ctx

and genExprSepEqPrependType astContext (pat:SynPat) (e: SynExpr) (isPrefixMultiline: bool) ctx =
and genExprSepEqPrependType astContext (pat:SynPat) (e: SynExpr) (valInfo:SynValInfo option) (isPrefixMultiline: bool) ctx =
nojaf marked this conversation as resolved.
Show resolved Hide resolved
let hasTriviaContentAfterEqual =
ctx.Trivia
|> List.exists (fun tn ->
Expand Down Expand Up @@ -486,9 +486,15 @@ and genExprSepEqPrependType astContext (pat:SynPat) (e: SynExpr) (isPrefixMultil
+> enterNode t.Range
+> ifElse hasLineCommentBeforeColon unindent sepNone) ctx

let genMetadataAttributes =
match valInfo with
| Some(SynValInfo(_, SynArgInfo(attributes, _, _))) -> genOnelinerAttributes astContext attributes
| None -> sepNone

(addExtraSpaceBeforeGenericType
+> genCommentBeforeColon
+> sepColon
+> genMetadataAttributes
+> genType astContext false t
+> sepEqual (isPrefixMultiline || hasLineCommentBeforeColon)
+> ifElse (isPrefixMultiline || hasTriviaContentAfterEqual || hasLineCommentBeforeColon)
Expand Down Expand Up @@ -524,7 +530,7 @@ and genTypeParamPostfix astContext tds tcs = genTypeAndParam astContext "" tds t

and genLetBinding astContext pref b =
match b with
| LetBinding(ats, px, ao, isInline, isMutable, p, e) ->
| LetBinding(ats, px, ao, isInline, isMutable, p, e, valInfo) ->
let genPat =
match e, p with
| TypedExpr(Typed, _, t), PatLongIdent(ao, s, ps, tpso) when (List.length ps > 1)->
Expand All @@ -550,7 +556,7 @@ and genLetBinding astContext pref b =
+> genAttr // this already contains the `let` or `and` keyword
+> leadingExpressionIsMultiline
(afterLetKeyword +> genPat +> enterNodeTokenByName rangeBetweenBindingPatternAndExpression "EQUALS")
(genExprSepEqPrependType astContext p e)
(genExprSepEqPrependType astContext p e (Some(valInfo)))

| DoBinding(ats, px, e) ->
let prefix = if pref.Contains("let") then pref.Replace("let", "do") else "do "
Expand All @@ -563,7 +569,7 @@ and genLetBinding astContext pref b =
|> genTrivia b.RangeOfBindingSansRhs

and genShortGetProperty astContext (pat:SynPat) e =
genExprSepEqPrependType astContext pat e false
genExprSepEqPrependType astContext pat e None false

and genProperty astContext prefix ao propertyKind ps e =
let tuplerize ps =
Expand All @@ -579,12 +585,12 @@ and genProperty astContext prefix ao propertyKind ps e =
!- prefix +> opt sepSpace ao genAccess -- propertyKind
+> ifElse (List.atMostOne ps) (col sepComma ps (genPat astContext) +> sepSpace)
(sepOpenT +> col sepComma ps (genPat astContext) +> sepCloseT +> sepSpace)
+> genPat astContext p +> genExprSepEqPrependType astContext p e false
+> genPat astContext p +> genExprSepEqPrependType astContext p e None false

| ps ->
let (_,p) = tuplerize ps
!- prefix +> opt sepSpace ao genAccess -- propertyKind +> col sepSpace ps (genPat astContext)
+> genExprSepEqPrependType astContext p e false
+> genExprSepEqPrependType astContext p e None false
|> genTrivia e.Range

and genPropertyWithGetSet astContext (b1, b2) =
Expand Down Expand Up @@ -1243,7 +1249,7 @@ and genExpr astContext synExpr =
let isFromAst (ctx: Context) = ctx.Content = String.Empty
let isInSameLine ctx =
match bs with
| [_, LetBinding(_, _, _, _, _, p, _)] ->
| [_, LetBinding(_, _, _, _, _, p, _, _)] ->
// the `in` keyword should be a trivia thing
not (isFromAst ctx) && p.Range.EndLine = e.Range.StartLine && not (futureNlnCheck (genExpr astContext e) ctx)
| _ -> false
Expand Down
8 changes: 4 additions & 4 deletions src/Fantomas/SourceParser.fs
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ let (|SigModuleOrNamespace|) (SynModuleOrNamespaceSig.SynModuleOrNamespaceSig(Lo

let (|Attribute|) (a : SynAttribute) =
let (LongIdentWithDots s) = a.TypeName
(s, a.ArgExpr, Option.map (|Ident|) a.Target)
(s, a.ArgExpr, Option.map (fun (i:Ident) -> i.idText) a.Target)

// Access modifiers

Expand Down Expand Up @@ -463,8 +463,8 @@ let (|DoBinding|LetBinding|MemberBinding|PropertyBinding|ExplicitCtor|) = functi
MemberBinding(ats, px, ao, isInline, mf, pat, expr)
| SynBinding.Binding(_, DoBinding, _, _, ats, px, _, _, _, expr, _, _) ->
DoBinding(ats, px, expr)
| SynBinding.Binding(ao, _, isInline, isMutable, attrs, px, _, pat, _, expr, _, _) ->
LetBinding(attrs, px, ao, isInline, isMutable, pat, expr)
| SynBinding.Binding(ao, _, isInline, isMutable, attrs, px, SynValData(_, valInfo, _), pat, _, expr, _, _) ->
LetBinding(attrs, px, ao, isInline, isMutable, pat, expr, valInfo)

// Expressions (55 cases, lacking to handle 11 cases)

Expand Down Expand Up @@ -1315,7 +1315,7 @@ let (|FunType|) (t, ValInfo(argTypes, returnType)) =
/// A rudimentary recognizer for extern functions
/// Probably we should use lexing information to improve its accuracy
let (|Extern|_|) = function
| Let(LetBinding(ats, px, ao, _, _, PatLongIdent(_, s, [_, PatTuple ps], _), TypedExpr(Typed, _, t))) ->
| Let(LetBinding(ats, px, ao, _, _, PatLongIdent(_, s, [_, PatTuple ps], _), TypedExpr(Typed, _, t), _)) ->
let hasDllImportAttr =
ats
|> List.exists (fun { Attributes = attrs } ->
Expand Down