Skip to content

Commit

Permalink
Correct Elif expression range when there is only a single elif. (#1659)
Browse files Browse the repository at this point in the history
* Correct Elif expression range when there is only a single elif.

* Use end of previous elif body expression as start of correct elif range.

* Remove atCurrentColumnIndent for non special long condition expressions.

* Don't add extra space before long if expression.

* Use specific range for keywords in elif.
  • Loading branch information
nojaf authored Apr 17, 2021
1 parent 6314c08 commit 26674f9
Show file tree
Hide file tree
Showing 7 changed files with 302 additions and 166 deletions.
196 changes: 195 additions & 1 deletion src/Fantomas.Tests/IfThenElseTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ if // c1
else // c5
if // c6
c // c7
then // c8
then // c8
d // c9
else // c10
e // c11
Expand Down Expand Up @@ -2223,3 +2223,197 @@ type internal Foo2 private () =
else
failwith ""
"""

[<Test>]
let ``multiline else if expression with nested if/then/else in body`` () =
formatSourceString
false
"""
if result.LaunchSuccess && result.ExitCode = 0 then
Ok r
else if result.ExitCode = 1 then
let stdout, stderr =
output
|> List.map
(function
| StdErr e -> Error e
| StdOut l -> Ok l)
|> Result.partition
if not stderr.IsEmpty then
failwithf "Got stderr bad bad bad"
match stdout with
| [] -> failwithf "Got no stdout :("
| xs when
xs
|> List.exists (fun i -> i.Contains "magic string goes here!")
->
Error(Ok r)
| _ -> Error(Error r)
else
failwith ""
"""
config
|> prepend newline
|> should
equal
"""
if result.LaunchSuccess && result.ExitCode = 0 then
Ok r
else if result.ExitCode = 1 then
let stdout, stderr =
output
|> List.map
(function
| StdErr e -> Error e
| StdOut l -> Ok l)
|> Result.partition
if not stderr.IsEmpty then
failwithf "Got stderr bad bad bad"
match stdout with
| [] -> failwithf "Got no stdout :("
| xs when
xs
|> List.exists (fun i -> i.Contains "magic string goes here!")
->
Error(Ok r)
| _ -> Error(Error r)
else
failwith ""
"""

[<Test>]
let ``multiline elif expression with nested if/then/else in body`` () =
formatSourceString
false
"""
if result.LaunchSuccess && result.ExitCode = 0 then
Ok r
elif result.ExitCode = 1 then
let stdout, stderr =
output
|> List.map
(function
| StdErr e -> Error e
| StdOut l -> Ok l)
|> Result.partition
if not stderr.IsEmpty then
failwithf "Got stderr bad bad bad"
match stdout with
| [] -> failwithf "Got no stdout :("
| xs when
xs
|> List.exists (fun i -> i.Contains "magic string goes here!")
->
Error(Ok r)
| _ -> Error(Error r)
else
failwith ""
"""
config
|> prepend newline
|> should
equal
"""
if result.LaunchSuccess && result.ExitCode = 0 then
Ok r
elif result.ExitCode = 1 then
let stdout, stderr =
output
|> List.map
(function
| StdErr e -> Error e
| StdOut l -> Ok l)
|> Result.partition
if not stderr.IsEmpty then
failwithf "Got stderr bad bad bad"
match stdout with
| [] -> failwithf "Got no stdout :("
| xs when
xs
|> List.exists (fun i -> i.Contains "magic string goes here!")
->
Error(Ok r)
| _ -> Error(Error r)
else
failwith ""
"""

[<Test>]
let ``multiple multiline elifs`` () =
formatSourceString
false
"""
if startWithMember sel then
(String.Join(String.Empty, "type T = ", Environment.NewLine, String(' ', startCol), sel), TypeMember)
elif String.startsWithOrdinal "and" (sel.TrimStart()) then
// Replace "and" by "type" or "let rec"
if startLine = endLine then
(pattern.Replace(sel, replacement, 1), p)
else
(String(' ', startCol)
+ pattern.Replace(sel, replacement, 1),
p)
elif startLine = endLine then
(sel, Nothing)
else
failAndExit ()
"""
config
|> prepend newline
|> should
equal
"""
if startWithMember sel then
(String.Join(String.Empty, "type T = ", Environment.NewLine, String(' ', startCol), sel), TypeMember)
elif String.startsWithOrdinal "and" (sel.TrimStart()) then
// Replace "and" by "type" or "let rec"
if startLine = endLine then
(pattern.Replace(sel, replacement, 1), p)
else
(String(' ', startCol)
+ pattern.Replace(sel, replacement, 1),
p)
elif startLine = endLine then
(sel, Nothing)
else
failAndExit ()
"""

[<Test>]
let ``multiline but not that special if expression`` () =
formatSourceString
false
"""
if
List.exists
(function
| CompExpr _ -> true
| _ -> false)
es
then
shortExpression ctx
else
expressionFitsOnRestOfLine shortExpression longExpression ctx
"""
config
|> prepend newline
|> should
equal
"""
if List.exists
(function
| CompExpr _ -> true
| _ -> false)
es then
shortExpression ctx
else
expressionFitsOnRestOfLine shortExpression longExpression ctx
"""
20 changes: 20 additions & 0 deletions src/Fantomas/AstTransformer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,26 @@ module private Ast =
|> finalContinuation

Continuation.sequence continuations finalContinuation
// don't collect nested elif expression as nodes.
// the ranges are often incorrect and using the else if or elif token is more reliable.
| SourceParser.ElIf ((ifExpr, thenExpr, _, range, _) :: es, elseExpr) ->
let elifs =
es
|> List.collect (fun (e1, e2, _, _, _) -> [ visit e1; visit e2 ])

let continuations : ((TriviaNodeAssigner list -> TriviaNodeAssigner list) -> TriviaNodeAssigner list) list =
[ yield visit ifExpr
yield visit thenExpr
yield! elifs
yield! (Option.toList elseExpr |> List.map visit) ]

let finalContinuation (nodes: TriviaNodeAssigner list list) : TriviaNodeAssigner list =
mkNode SynExpr_IfThenElse range
:: (List.collect id nodes)
|> finalContinuation

Continuation.sequence continuations finalContinuation

| SynExpr.IfThenElse (ifExpr, thenExpr, elseExpr, _, _, _, range) ->
let continuations : ((TriviaNodeAssigner list -> TriviaNodeAssigner list) -> TriviaNodeAssigner list) list =
[ yield visit ifExpr
Expand Down
Loading

0 comments on commit 26674f9

Please sign in to comment.