Skip to content

Commit

Permalink
Use ranges for parenthesis found in SynExpr.Paren to print trivia. Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
nojaf committed Sep 4, 2020
1 parent 6dd19cf commit 636a09d
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 23 deletions.
35 changes: 35 additions & 0 deletions src/Fantomas.Tests/LambdaTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -394,4 +394,39 @@ let options =
| Graph.HierarchicalUpDown -> createObj [ "hierarchical" ==> hierOpts "UD" ]
o.layout <- Some layout)
"""

[<Test>]
let ``don't print unrelated trivia after closing parenthesis of lambda, 1084`` () =
formatSourceString false """
let private tokenizeLines (sourceTokenizer: FSharpSourceTokenizer) allLines state =
allLines
|> List.mapi (fun index line -> line, (index + 1)) // line number is needed in tokenizeLine
|> List.fold (fun (state, tokens) (line, lineNumber) ->
let tokenizer = sourceTokenizer.CreateLineTokenizer(line)
let nextState, tokensOfLine =
tokenizeLine tokenizer allLines state lineNumber []
let allTokens = List.append tokens (List.rev tokensOfLine) // tokens of line are add in reversed order
(nextState, allTokens)
) (state, []) // empty tokens to start with
|> snd // ignore the state
""" config
|> prepend newline
|> should equal """
let private tokenizeLines (sourceTokenizer: FSharpSourceTokenizer) allLines state =
allLines
|> List.mapi (fun index line -> line, (index + 1)) // line number is needed in tokenizeLine
|> List.fold (fun (state, tokens) (line, lineNumber) ->
let tokenizer =
sourceTokenizer.CreateLineTokenizer(line)
let nextState, tokensOfLine =
tokenizeLine tokenizer allLines state lineNumber []
let allTokens =
List.append tokens (List.rev tokensOfLine) // tokens of line are add in reversed order
(nextState, allTokens)) (state, []) // empty tokens to start with
|> snd // ignore the state
"""
44 changes: 23 additions & 21 deletions src/Fantomas/CodePrinter.fs
Original file line number Diff line number Diff line change
Expand Up @@ -958,13 +958,13 @@ and genExpr astContext synExpr =
match e with
| MultilineString _
| Lambda _
| Paren (Lambda _)
| Paren (MatchLambda _) -> id
| Paren (_, Lambda _, _)
| Paren (_, MatchLambda _, _) -> id
| _ -> autoNlnIfExpressionExceedsPageWidth

let kw tokenName f = tokN synExpr.Range tokenName f
let sepOpenT = tokN synExpr.Range LPAREN sepOpenT
let sepCloseT = tokN synExpr.Range RPAREN sepCloseT
let sepOpenTFor r = tokN (Option.defaultValue synExpr.Range r) LPAREN sepOpenT
let sepCloseTFor r = tokN (Option.defaultValue synExpr.Range r) RPAREN sepCloseT

match synExpr with
| ElmishReactWithoutChildren(identifier, isArray, children) ->
Expand Down Expand Up @@ -1309,7 +1309,7 @@ and genExpr astContext synExpr =
fun ctx -> isShortExpression ctx.Config.MaxArrayOrListWidth shortExpression multilineExpression ctx

| JoinIn(e1, e2) -> genExpr astContext e1 -- " in " +> genExpr astContext e2
| Paren(DesugaredLambda(cps, e)) ->
| Paren(_, DesugaredLambda(cps, e), _) ->
fun (ctx: Context) ->
let lastLineOnlyContainsParenthesis = lastLineOnlyContains [| ' ';'('|] ctx

Expand Down Expand Up @@ -1338,22 +1338,22 @@ and genExpr astContext synExpr =
| DesugaredLambda(cps, e) ->
!- "fun " +> col sepSpace cps (fst >> genComplexPats astContext) +> sepArrow
+> autoIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e)
| Paren(Lambda(e, sps)) ->
| Paren(lpr, Lambda(e, sps), rpr) ->
fun (ctx: Context) ->
let lastLineOnlyContainsParenthesis = lastLineOnlyContains [| ' ';'('|] ctx
let hasLineCommentAfterArrow =
findTriviaTokenFromName RARROW synExpr.Range ctx
|> Option.isSome

let expr =
sepOpenT
sepOpenTFor (Some lpr)
-- "fun "
+> col sepSpace sps (genSimplePats astContext)
+> triviaAfterArrow synExpr.Range
+> ifElse hasLineCommentAfterArrow (genExpr astContext e)
(ifElse lastLineOnlyContainsParenthesis (autoIndentAndNlnIfExpressionExceedsPageWidth (genExpr astContext e))
(autoNlnIfExpressionExceedsPageWidth (genExpr astContext e)))
+> sepCloseT
+> sepCloseTFor rpr

expr ctx

Expand Down Expand Up @@ -1384,18 +1384,20 @@ and genExpr astContext synExpr =
genTyparList astContext tps +> sepColon +> sepOpenT +> genMemberSig astContext msg +> sepCloseT
+> sepSpace +> genExpr astContext e

| Paren (ILEmbedded r) ->
| Paren (_, ILEmbedded r, _) ->
// Just write out original code inside (# ... #)
fun ctx -> !- (defaultArg (lookup r ctx) "") ctx
| Paren e ->
| Paren (lpr ,e, rpr) ->
match e with
| MultilineString _ ->
sepOpenT
sepOpenTFor (Some lpr)
+> atCurrentColumn (genExpr { astContext with IsInsideDotGet = false } e +> indentIfNeeded sepNone)
+> sepCloseT
+> sepCloseTFor rpr
| _ ->
// Parentheses nullify effects of no space inside DotGet
sepOpenT +> genExpr { astContext with IsInsideDotGet = false } e +> sepCloseT
sepOpenTFor (Some lpr)
+> genExpr { astContext with IsInsideDotGet = false } e
+> sepCloseTFor rpr
| CompApp(s, e) ->
!- s +> sepSpace +> sepOpenS +> genExpr { astContext with IsNakedRange = true } e +> sepCloseS
// This supposes to be an infix function, but for some reason it isn't picked up by InfixApps
Expand Down Expand Up @@ -1499,9 +1501,9 @@ and genExpr astContext synExpr =

let genMultilineExpr =
match e with
| Paren(Lambda(_)) -> atCurrentColumnIndent(genExpr astContext e)
| Paren(App(_))
| Paren(Tuple(_)) ->
| Paren(_, Lambda(_), _) -> atCurrentColumnIndent(genExpr astContext e)
| Paren(_, App(_), _)
| Paren(_, Tuple(_), _) ->
atCurrentColumn(genExpr astContext e)
| _ ->
ifElse hasParenthesis
Expand Down Expand Up @@ -1542,8 +1544,8 @@ and genExpr astContext synExpr =
+> (ifElse (not hasPar && addSpaceBefore) sepSpace sepNone)
+> (fun ctx ->
match e2 with
| Paren (App (_)) when astContext.IsInsideDotGet -> genExpr astContext e2 ctx
| Paren (DotGet (App(_), _)) when astContext.IsInsideDotGet -> genExpr astContext e2 ctx
| Paren (_, App (_), _) when astContext.IsInsideDotGet -> genExpr astContext e2 ctx
| Paren (_, DotGet (App(_), _), _) when astContext.IsInsideDotGet -> genExpr astContext e2 ctx
| ConstExpr(SynConst.Unit, _) when astContext.IsInsideDotGet -> genExpr astContext e2 ctx
| _ -> appNlnFun e2 (genExpr astContext e2) ctx)
+> unindent)
Expand Down Expand Up @@ -1579,16 +1581,16 @@ and genExpr astContext synExpr =

let hasThreeOrMoreLambdas =
List.filter (function
| Paren (Lambda (_)) -> true
| Paren (_, Lambda (_), _) -> true
| _ -> false) es
|> List.length
|> fun l -> l >= 3

if List.exists (function
| Lambda _
| MatchLambda _
| Paren (Lambda (_))
| Paren (MatchLambda (_))
| Paren (_, Lambda (_), _)
| Paren (_, MatchLambda (_), _)
| MultilineString _
| CompExpr _ -> true
| _ -> false) es && not hasThreeOrMoreLambdas then
Expand Down
4 changes: 2 additions & 2 deletions src/Fantomas/SourceParser.fs
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,8 @@ let (|Quote|_|) = function
| _ -> None

let (|Paren|_|) = function
| SynExpr.Paren(e, _, _, _) ->
Some e
| SynExpr.Paren(e, lpr, rpr, _) ->
Some (lpr, e, rpr)
| _ -> None

type ExprKind =
Expand Down

0 comments on commit 636a09d

Please sign in to comment.