Skip to content

Commit

Permalink
Preserve in keyword. Fixes fsprojects#340.
Browse files Browse the repository at this point in the history
  • Loading branch information
nojaf committed Sep 25, 2020
1 parent bd55433 commit d6b755c
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 58 deletions.
24 changes: 13 additions & 11 deletions src/Fantomas.Tests/LetBindingTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ let ``let in should be preserved`` () =
"""

[<Test>]
let ``multiple let in lines, should remove in`` () =
let ``multiple let in lines, should keep in`` () =
let codeSnippet = """
let f () =
let x = 1 in // the "in" keyword is available in F#
Expand All @@ -21,13 +21,13 @@ let f () =

formatSourceString false codeSnippet config
|> should equal """let f () =
let x = 1 // the "in" keyword is available in F#
let y = 2
let x = 1 in // the "in" keyword is available in F#
let y = 2 in
x + y
"""

[<Test>]
let ``multiple let in lines, should remove in, block comment`` () =
let ``multiple let in lines, should keep in, block comment`` () =
let codeSnippet = """
let f () =
let x = 1 in (* the "in" keyword is available in F# *)
Expand All @@ -41,13 +41,13 @@ let f () =
({ config with
MaxValueBindingWidth = 50 })
|> should equal """let f () =
let x = 1 (* the "in" keyword is available in F# *)
let y = 2
let x = 1 in (* the "in" keyword is available in F# *)
let y = 2 in
x + y
"""

[<Test>]
let ``multiline let in, should remove in`` () =
let ``multiline let in, should keep in`` () =
let codeSnippet = """
let f () =
let x = 1 in if longIdentifierThatWillForceThisConstructToBeMultiline
Expand All @@ -59,7 +59,7 @@ let f () =
|> prepend newline
|> should equal """
let f () =
let x = 1
let x = 1 in
if longIdentifierThatWillForceThisConstructToBeMultiline
then x
Expand All @@ -78,7 +78,7 @@ let f () =
|> prepend newline
|> should equal """
let f () =
let x = 1
let x = 1 in
(while true do
()
Expand Down Expand Up @@ -552,7 +552,9 @@ let ``handle hash directives before equals, 728`` () =
()
""" config
|> should equal """let Baz (firstParam: string)
|> prepend newline
|> should equal """
let Baz (firstParam: string)
#if DEBUG
(_: int)
#else
Expand Down Expand Up @@ -972,7 +974,7 @@ let x =
let ``preserve in keyword via trivia, 340`` () =
formatSourceString false """
let x = List.singleton <|
let item = "text"
let item = "text" in
item
""" config
|> prepend newline
Expand Down
2 changes: 0 additions & 2 deletions src/Fantomas.Tests/TypeProviderTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ let ``should throw FormatException on unparsed input`` () =
|> ignore)
|> ignore



[<Test>]
let ``should handle lines with more than 512 characters`` () =
formatSourceString false """
Expand Down
2 changes: 1 addition & 1 deletion src/Fantomas.Tests/VerboseSyntaxConversionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ let ``verbose syntax`` () =
let div2 = 2
let f x =
let r = x % div2
let r = x % div2 in
if r = 1 then ("Odd") else ("Even")
"""
10 changes: 8 additions & 2 deletions src/Fantomas/AstTransformer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -985,8 +985,14 @@ module private Ast =
and visitSynBinding (binding: SynBinding): Node =
match binding with
| Binding (access, kind, mustInline, isMutable, attrs, _, valData, headPat, returnInfo, expr, range, _) ->
{ Type = Binding_
Range = r range
let t =
match kind with
| SynBindingKind.StandaloneExpression -> StandaloneExpression_
| SynBindingKind.NormalBinding -> NormalBinding_
| SynBindingKind.DoBinding -> DoBinding_

{ Type = t
Range = r binding.RangeOfBindingAndRhs
Properties =
p [ yield "mustInline" ==> mustInline
yield "isMutable" ==> isMutable
Expand Down
77 changes: 44 additions & 33 deletions src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -404,20 +404,20 @@ and genModuleDecl astContext (node: SynModuleDecl) =
let sepBAndBs =
match List.tryHead bs with
| Some b' ->
let r = b'.RangeOfBindingSansRhs
let r = b'.RangeOfBindingAndRhs

sepNln
+> sepNlnConsideringTriviaContentBeforeForMainNode Binding_ r
+> sepNlnConsideringTriviaContentBeforeForMainNode (synBindingToFsAstType b) r
| None -> id

genLetBinding { astContext with IsFirstChild = true } "let rec " b
+> sepBAndBs
+> colEx (fun (b': SynBinding) ->
let r = b'.RangeOfBindingSansRhs
let r = b'.RangeOfBindingAndRhs

sepNln
+> sepNlnConsideringTriviaContentBeforeForMainNode Binding_ r) bs (fun andBinding ->
enterNodeFor Binding_ andBinding.RangeOfBindingSansRhs
+> sepNlnConsideringTriviaContentBeforeForMainNode (synBindingToFsAstType b) r) bs (fun andBinding ->
enterNodeFor (synBindingToFsAstType b) andBinding.RangeOfBindingAndRhs
+> genLetBinding { astContext with IsFirstChild = false } "and " andBinding)

| ModuleAbbrev (s1, s2) -> !- "module " -- s1 +> sepEq +> sepSpace -- s2
Expand Down Expand Up @@ -715,7 +715,6 @@ and genLetBinding astContext pref b =
+> leadingExpressionIsMultiline
(afterLetKeyword
+> genPat
+> dumpAndContinue
+> enterNodeTokenByName rangeBetweenBindingPatternAndExpression EQUALS)
(genExprSepEqPrependType astContext p e (Some(valInfo)))

Expand All @@ -729,7 +728,7 @@ and genLetBinding astContext pref b =
+> autoIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e)

| b -> failwithf "%O isn't a let binding" b
+> leaveNodeFor Binding_ b.RangeOfBindingSansRhs
+> leaveNodeFor (synBindingToFsAstType b) b.RangeOfBindingAndRhs

and genShortGetProperty astContext (pat: SynPat) e =
genExprSepEqPrependType astContext pat e None false
Expand Down Expand Up @@ -803,10 +802,10 @@ and genMemberBindingList astContext node =
| [] -> []
| mb :: rest ->
let expr = genMemberBinding astContext mb
let r = mb.RangeOfBindingSansRhs
let r = mb.RangeOfBindingAndRhs

let sepNln =
sepNlnConsideringTriviaContentBeforeForMainNode Binding_ r
sepNlnConsideringTriviaContentBeforeForMainNode (synBindingToFsAstType mb) r

(expr, sepNln, r) :: (collectItems rest)

Expand Down Expand Up @@ -1051,7 +1050,7 @@ and genMemberBinding astContext b =
+> sepSpaceOrIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e)

| b -> failwithf "%O isn't a member binding" b
|> genTriviaFor Binding_ b.RangeOfBindingSansRhs
|> genTriviaFor (synBindingToFsAstType b) b.RangeOfBindingAndRhs

and genMemberFlags astContext (mf: MemberFlags) =
match mf with
Expand Down Expand Up @@ -1523,7 +1522,7 @@ and genExpr astContext synExpr =
let prefix =
sprintf "%s%s" (if isUse then "use " else "let ") (if isRecursive then "rec " else "")

enterNodeFor Binding_ binding.RangeOfBindingSansRhs
enterNodeFor (synBindingToFsAstType binding) binding.RangeOfBindingAndRhs
+> genLetBinding astContext prefix binding
| LetOrUseBangStatement (isUse, pat, expr, r) ->
enterNodeFor SynExpr_LetOrUseBang r // print Trivia before entire LetBang expression
Expand All @@ -1541,14 +1540,14 @@ and genExpr astContext synExpr =

let getRangeOfCompExprStatement ces =
match ces with
| LetOrUseStatement (_, _, binding) -> binding.RangeOfBindingSansRhs
| LetOrUseStatement (_, _, binding) -> binding.RangeOfBindingAndRhs
| LetOrUseBangStatement (_, _, _, r) -> r
| AndBangStatement (_, _, r) -> r
| OtherStatement expr -> expr.Range

let getSepNln ces r =
match ces with
| LetOrUseStatement _ -> sepNlnConsideringTriviaContentBeforeForMainNode Binding_ r
| LetOrUseStatement (_, _, b) -> sepNlnConsideringTriviaContentBeforeForMainNode (synBindingToFsAstType b) r
| LetOrUseBangStatement _ -> sepNlnConsideringTriviaContentBeforeForMainNode SynExpr_LetOrUseBang r
| AndBangStatement _ -> sepNlnConsideringTriviaContentBeforeForToken AND_BANG r
| OtherStatement e -> sepNlnConsideringTriviaContentBeforeForMainNode (synExprToFsAstType e) r
Expand Down Expand Up @@ -2005,45 +2004,61 @@ and genExpr astContext synExpr =
genExpr astContext e
+> genGenericTypeParameters astContext ts
| LetOrUses (bs, e) ->
let isFromAst (ctx: Context) = ctx.Content = String.Empty
let inKeyWordTrivia (binding: SynBinding) ctx =
let inRange =
mkRange "IN" binding.RangeOfBindingAndRhs.End e.Range.Start

Map.tryFindOrEmptyList IN ctx.TriviaTokenNodes
|> TriviaHelpers.``keyword token inside range`` inRange
|> List.tryHead

let isInSameLine ctx =
match bs with
| [ _, LetBinding (_, _, _, _, _, p, _, _) ] ->
// the `` keyword should be a trivia thing
not (isFromAst ctx)
| [ _, (LetBinding (_, _, _, _, _, p, _, _) as binding) ] ->
Option.isSome (inKeyWordTrivia binding ctx)
&& p.Range.EndLine = e.Range.StartLine
&& not (futureNlnCheck (genExpr astContext e) ctx)
| _ -> false

let genInKeyword (binding: SynBinding) (ctx: Context) =
match inKeyWordTrivia binding ctx with
| Some (_, tn) ->
(printContentBefore tn
+> !- " in "
+> printContentAfter tn) ctx
| None -> sepNone ctx

fun ctx ->
if isInSameLine ctx then
// short expression with in keyword
// f.ex. let a in ()
atCurrentColumn
(col sepNone bs (fun (p, x) ->
genLetBinding
{ astContext with
IsFirstChild = p <> "and" }
p
x)
-- " in "
(optSingle (fun (p, x) ->
genLetBinding
{ astContext with
IsFirstChild = p <> "and" }
p
x
+> genInKeyword x) (List.tryHead bs)
+> genExpr astContext e)
ctx
else
let letBindings (bs: (string * SynBinding) list) =
bs
|> List.map (fun (p, x) ->
let expr =
enterNodeFor Binding_ x.RangeOfBindingSansRhs
enterNodeFor (synBindingToFsAstType x) x.RangeOfBindingAndRhs
+> genLetBinding
{ astContext with
IsFirstChild = p <> "and" }
p
x
+> genInKeyword x

let range = x.RangeOfBindingSansRhs
let range = x.RangeOfBindingAndRhs

let sepNln =
sepNlnConsideringTriviaContentBeforeForMainNode Binding_ range
sepNlnConsideringTriviaContentBeforeForMainNode (synBindingToFsAstType x) range

expr, sepNln, range)

Expand Down Expand Up @@ -2110,7 +2125,7 @@ and genExpr astContext synExpr =

let fsAstType, r =
match e with
| LetOrUses ((_, fb) :: _, _) -> Binding_, fb.RangeOfBindingSansRhs
| LetOrUses ((_, fb) :: _, _) -> (synBindingToFsAstType fb), fb.RangeOfBindingAndRhs
| _ -> synExprToFsAstType e, e.Range

let sepNln =
Expand Down Expand Up @@ -4560,11 +4575,7 @@ and infixOperatorFromTrivia range fallback (ctx: Context) =

let isValidIdent x = Regex.Match(x, validIdentRegex).Success

TriviaHelpers.getNodesForTypes
[ SynPat_LongIdent
SynPat_Named
Binding_ ]
ctx.TriviaMainNodes
TriviaHelpers.getNodesForTypes [ SynPat_LongIdent; SynPat_Named ] ctx.TriviaMainNodes
|> List.choose (fun t ->
match t.Range = range with
| true ->
Expand Down
6 changes: 6 additions & 0 deletions src/Fantomas/SourceTransformer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -306,3 +306,9 @@ let synModuleSigDeclToFsAstType =
| SynModuleSigDecl.HashDirective _ -> SynModuleSigDecl_HashDirective
| SynModuleSigDecl.NamespaceFragment _ -> SynModuleSigDecl_NamespaceFragment
| SynModuleSigDecl.ModuleAbbrev _ -> SynModuleSigDecl_ModuleAbbrev

let synBindingToFsAstType (Binding (_, kind, _, _, _, _, _, _, _, _, _, _)) =
match kind with
| SynBindingKind.StandaloneExpression -> StandaloneExpression_
| SynBindingKind.NormalBinding -> NormalBinding_
| SynBindingKind.DoBinding -> DoBinding_
4 changes: 3 additions & 1 deletion src/Fantomas/TokenParser.fs
Original file line number Diff line number Diff line change
Expand Up @@ -666,7 +666,8 @@ let private tokenNames =
"WITH"
"MEMBER"
"AND_BANG"
"FUNCTION" ]
"FUNCTION"
"IN" ]

let private tokenKinds = [ FSharpTokenCharKind.Operator ]

Expand Down Expand Up @@ -718,6 +719,7 @@ let internal getFsToken tokenName =
| "INFIX_STAR_STAR_OP" -> INFIX_STAR_STAR_OP
| "FUNCTION" -> FUNCTION
| "LPAREN_STAR_RPAREN" -> LPAREN_STAR_RPAREN
| "IN" -> IN
| _ -> failwithf "was not expecting token %s" tokenName

let getTriviaNodesFromTokens (tokens: Token list) =
Expand Down
18 changes: 11 additions & 7 deletions src/Fantomas/Trivia.fs
Original file line number Diff line number Diff line change
Expand Up @@ -239,14 +239,14 @@ let private updateTriviaNode (lens: TriviaNodeAssigner -> unit) (triviaNodes: Tr
// |> List.mapi (fun idx tn -> if idx = index then lens tn else tn)
| None -> triviaNodes

let private findBindingThatStartsWith (triviaNodes: TriviaNodeAssigner list) column line =
let private findNamedPatThatStartsWith (triviaNodes: TriviaNodeAssigner list) column line =
triviaNodes
|> List.tryFind (fun t ->
match t.Type with
| MainNode (Binding_) when (t.Range.StartColumn = column
&& t.Range.StartLine = line) -> true
| MainNode (SynPat_Named) when (t.Range.StartColumn = column
&& t.Range.StartLine = line) -> true
| MainNode (SynPat_LongIdent) when (t.Range.StartColumn = column
&& t.Range.StartLine = line) -> true
| _ -> false)

let private findParsedHashOnLineAndEndswith (triviaNodes: TriviaNodeAssigner list) startLine endColumn =
Expand Down Expand Up @@ -388,9 +388,13 @@ let private addTriviaToTriviaNode triviaBetweenAttributeAndParentBinding
|> updateTriviaNode (fun tn -> tn.ContentBefore.Add(Keyword(kw))) triviaNodes

| { Item = Keyword ({ Content = keyword }); Range = range } when (keyword = "in") ->
// find largest SynExpr right before in range
findNodeBeforeLineAndColumn triviaNodes range.StartLine range.StartColumn
|> updateTriviaNode (fun tn -> tn.ContentAfter.Add trivia.Item) triviaNodes
// find In keyword TriviaNode
triviaNodes
|> List.tryFind (fun tn ->
match tn.Type with
| Token (IN, _) -> RangeHelpers.rangeEq range tn.Range
| _ -> false)
|> updateTriviaNode (fun tn -> tn.ContentItself <- Some trivia.Item) triviaNodes

| { Item = Keyword ({ Content = keyword }); Range = range } when (keyword = "if"
|| keyword = "then"
Expand Down Expand Up @@ -439,7 +443,7 @@ let private addTriviaToTriviaNode triviaBetweenAttributeAndParentBinding
|> updateTriviaNode (fun tn -> tn.ContentItself <- Some chNode) triviaNodes

| { Item = IdentOperatorAsWord (_) as ifw; Range = range } ->
findBindingThatStartsWith triviaNodes range.StartColumn range.StartLine
findNamedPatThatStartsWith triviaNodes range.StartColumn range.StartLine
|> updateTriviaNode (fun tn -> tn.ContentItself <- Some ifw) triviaNodes

| { Item = IdentBetweenTicks (_) as iNode; Range = range } ->
Expand Down
5 changes: 4 additions & 1 deletion src/Fantomas/TriviaTypes.fs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ type FsTokenType =
| INFIX_STAR_STAR_OP
| FUNCTION
| LPAREN_STAR_RPAREN
| IN

type Token =
{ TokenInfo: FSharpTokenInfo
Expand Down Expand Up @@ -207,7 +208,9 @@ type FsAstType =
| SynSimplePat_Attrib
| SynSimplePats_SimplePats
| SynSimplePats_Typed
| Binding_
| StandaloneExpression_
| NormalBinding_
| DoBinding_
| SynBindingReturnInfo_
| SynValTyparDecls_
| TyparDecl_
Expand Down

0 comments on commit d6b755c

Please sign in to comment.