search

fsprojects / fantomas

FSharp source code formatter
https://fsprojects.github.io/fantomas
Other
772 stars 194 forks source link

Exception: Unexpected scenario when formatting else if / elif #713

Closed tatumalenko closed 4 years ago

tatumalenko commented 4 years ago

Issue created from fantomas-ui

Exception throws when formatting certain patterns of nested else if / elif blocks. Likely related to [#675] which was recently closed and fixed in [#678].

Code Example 1

if v1 < v2 then
    -1
elif v1 > v2 then
    1
else
    if t1 < t2 then
        -1
    elif t1 > t2 then
        1
    else
        0

Code Example 2

Another example slightly tweaked taken from [#675]:

module String =
    let merge a b =
            if la <> lb then 
                if la > lb then a' else b'
            elif la = lb then a'
            else
                if String.length a' < String.length b' then a' else b'

Changing the elif into an else if in the previous example will format successfully.

Code Example 3

Yet another example, again similar to the previous one, here the issue still arises but no elif were involved:

module String =
    let merge a b =
            if la <> lb then 
                if la > lb then a' else b'
            else
                if String.length a' < String.length b' then a' else if String.length a' > String.length b' then b' else b'

The above still throws even if if la > lb then a' else b' is replaced simply with a' (i.e. nesting only inside the else).

Note

In all cases, if strict mode is enabled, the format succeeds.

Error

Exception: Unexpected scenario when formatting else if / elif, please open an issue via https://jindraivanek.gitlab.io/fantomas-ui Stack Trace:    at Fantomas.TriviaContext.else if / elif(range rangeOfIfThenElse, Context ctx) in /build/.deps/fantomas/src/Fantomas/TriviaContext.fs:line 83
   at Fantomas.CodePrinter.genElifOneliner@1259-2.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1259
   at Fantomas.CodePrinter.genElifOneliner@1259-3.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1259
   at Fantomas.CodePrinter.genElifOneliner@1260-5.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1260
   at Fantomas.CodePrinter.genElifOneliner@1261-6.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1261
   at Fantomas.CodePrinter.genElifOneliner@1262-8.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1262
   at Fantomas.CodePrinter.genElifOneliner@1263-9.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1263
   at Fantomas.CodePrinter.genTrivia@2468-1.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 2468
   at Fantomas.Context.futureNlnCheckMem(FSharpFunc`2 f, Context ctx) in /build/.deps/fantomas/src/Fantomas/Context.fs:line 429
   at Fantomas.Context.futureNlnCheck(FSharpFunc`2 f, Context ctx) in /build/.deps/fantomas/src/Fantomas/Context.fs:line 433
   at Fantomas.CodePrinter.isAnyElifBranchMultiline@1279.Invoke(Tuple`3 elf) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1279
   at Microsoft.FSharp.Primitives.Basics.List.exists[T](FSharpFunc`2 predicate, FSharpList`1 xs1)
   at Fantomas.CodePrinter.genExpr@1153-309.Invoke(Context ctx) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1277
   at Fantomas.CodePrinter.genTrivia@2468-1.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 2468
   at Fantomas.CodePrinter.genTrivia@2468-1.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 2468
   at Fantomas.CodePrinter.genTrivia@2468-1.Invoke(Context x) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 2468
   at Fantomas.Context.col[T](FSharpFunc`2 f', IEnumerable`1 c, FSharpFunc`2 f, Context ctx) in /build/.deps/fantomas/src/Fantomas/Context.fs:line 289
   at Fantomas.CodePrinter.genImpFile@82-6.Invoke(Context ctx) in /build/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 82
   at Fantomas.CodeFormatterImpl.formatWith(ParsedInput ast, FSharpList`1 defines, FormatContext formatContext, FormatConfig config) in /build/.deps/fantomas/src/Fantomas/CodeFormatterImpl.fs:line 392
   at Fantomas.CodeFormatterImpl.format@408-1.Invoke(Tuple`2[] _arg1) in /build/.deps/fantomas/src/Fantomas/CodeFormatterImpl.fs:line 410
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, FSharpFunc`2 userCode, b result1) in E:\A\_work\130\s\src\fsharp\FSharp.Core\async.fs:line 416
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in E:\A\_work\130\s\src\fsharp\FSharp.Core\async.fs:line 109
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.FSharp.Control.AsyncResult`1.Commit() in E:\A\_work\130\s\src\fsharp\FSharp.Core\async.fs:line 349
   at Microsoft.FSharp.Control.AsyncPrimitives.RunSynchronouslyInCurrentThread[a](CancellationToken cancellationToken, FSharpAsync`1 computation) in E:\A\_work\130\s\src\fsharp\FSharp.Core\async.fs:line 870
   at Microsoft.FSharp.Control.AsyncPrimitives.RunSynchronously[T](CancellationToken cancellationToken, FSharpAsync`1 computation, FSharpOption`1 timeout) in E:\A\_work\130\s\src\fsharp\FSharp.Core\async.fs:line 890
   at Microsoft.FSharp.Control.FSharpAsync.RunSynchronously[T](FSharpAsync`1 computation, FSharpOption`1 timeout, FSharpOption`1 cancellationToken) in E:\A\_work\130\s\src\fsharp\FSharp.Core\async.fs:line 1153
   at Server.Result.attempt[a](FSharpFunc`2 f) in /build/src/Server/Server.fs:line 24

Options

Fantomas Next - 3.3.0-3/2/2020

Name Value
IndentOnTryWith false
IndentSpaceNum 2
KeepNewlineAfter false
MaxIfThenElseShortWidth 40
PageWidth 90
ReorderOpenDeclaration false
SemicolonAtEndOfLine false
SpaceAfterComma true
SpaceAfterSemicolon true
SpaceAroundDelimiter true
SpaceBeforeArgument true
SpaceBeforeColon false
SpaceBeforeSemicolon false
StrictMode false
nojaf commented 4 years ago

Hey @tatumalenko, thanks for reporting this. Formatting these nested if/elif/else constructions can be pretty tricky. Because the AST doesn't always tell you where the keyword reside.

I should debug these examples to find what is going wrong. And thank you for taking the time to find multiple examples.

nojaf commented 4 years ago

After some debugging, I found out I made a wrong assumption that only the last else if could have a wrong range.

Turns out every SynExpr.IfThenElse can potentially have a wrong range (ignoring the else keyword).

ImplFile
  (ParsedImplFileInput
     ("Script.fsx",true,QualifiedNameOfFile Script$fsx,[],[],
      [SynModuleOrNamespace
         ([Script],false,AnonModule,
          [DoExpr
             (NoSequencePointAtDoBinding,
              IfThenElse
                (App
                   (NonAtomic,false,
                    App
                      (NonAtomic,true,Ident op_LessThan,Ident v1,
                       Script.fsx (1,3--1,7) IsSynthetic=false),Ident v2,
                    Script.fsx (1,3--1,10) IsSynthetic=false),
                 Const (Int32 -1,Script.fsx (2,4--2,6) IsSynthetic=false),
                 Some
                   (IfThenElse
                      (App
                         (NonAtomic,false,
                          App
                            (NonAtomic,true,Ident op_GreaterThan,Ident v1,
                             Script.fsx (3,5--3,9) IsSynthetic=false),Ident v2,
                          Script.fsx (3,5--3,12) IsSynthetic=false),
                       Const (Int32 1,Script.fsx (4,4--4,5) IsSynthetic=false),
                       Some
                         (IfThenElse
                            (App
                               (NonAtomic,false,
                                App
                                  (NonAtomic,true,Ident op_LessThan,Ident t1,
                                   Script.fsx (6,7--6,11) IsSynthetic=false),
                                Ident t2,
                                Script.fsx (6,7--6,14) IsSynthetic=false),
                             Const
                               (Int32 -1,
                                Script.fsx (7,8--7,10) IsSynthetic=false),
                             Some
                               (IfThenElse
                                  (App
                                     (NonAtomic,false,
                                      App
                                        (NonAtomic,true,Ident op_GreaterThan,
                                         Ident t1,
                                         Script.fsx (8,9--8,13) IsSynthetic=false),
                                      Ident t2,
                                      Script.fsx (8,9--8,16) IsSynthetic=false),
                                   Const
                                     (Int32 1,
                                      Script.fsx (9,8--9,9) IsSynthetic=false),
                                   Some
                                     (Const
                                        (Int32 0,
                                         Script.fsx (11,8--11,9) IsSynthetic=false)),
                                   SequencePointAtBinding
                                     Script.fsx (8,4--8,21) IsSynthetic=false,
                                   false,
                                   Script.fsx (8,4--8,21) IsSynthetic=false,
                                   Script.fsx (8,4--11,9) IsSynthetic=false)),
                             SequencePointAtBinding
                               Script.fsx (6,4--6,19) IsSynthetic=false,false,
                             Script.fsx (6,4--6,19) IsSynthetic=false,
                             Script.fsx (6,4--11,9) IsSynthetic=false)),
                       SequencePointAtBinding
                         Script.fsx (3,0--3,17) IsSynthetic=false,false,
                       Script.fsx (3,0--3,17) IsSynthetic=false,
                       Script.fsx (3,0--11,9) IsSynthetic=false)),
                 SequencePointAtBinding Script.fsx (1,0--1,15) IsSynthetic=false,
                 false,Script.fsx (1,0--1,15) IsSynthetic=false,
                 Script.fsx (1,0--11,9) IsSynthetic=false),
              Script.fsx (1,0--11,9) IsSynthetic=false)],PreXmlDocEmpty,[],None,
          Script.fsx (1,0--11,9) IsSynthetic=false)],(true, true)))

Hmm, not sure how to tackle this one yet. Ongoing investigation.

nojaf commented 4 years ago

I was able to find a way to correct all elif ranges.