fsprojects / fantomas

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

Extra space throws exception #1476

Closed theothornhill closed 3 years ago

theothornhill commented 3 years ago

Issue created from fantomas-online

Hi there, and thanks for 4.4.0

Code

f x = match x with
      | 5 ->
          { actual =
                x expectedOneOf = [  99 // extra space here
                                    6
                                    8
                                    10
                                    12
                                    14
                                    16
                                    18
                                    20
                                    25
                                    30
                                    40
                                    50 ] }

Result

System.Exception: cannot determine if synExpr Const (Int32 99, tmp.fsx (4,37--4,39) IsSynthetic=false) is uppercase or lowercase
   at Microsoft.FSharp.Core.PrintfModule.PrintFormatToStringThenFail@1433.Invoke(String message) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\printf.fs:line 1433
   at Fantomas.CodePrinter.addSpaceBeforeParensInFunCall(SynExpr functionOrMethod, SynExpr arg, Context ctx)
   at Fantomas.CodePrinter.addFirstSpace@3123.Invoke(Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 3127
   at Fantomas.CodePrinter.addFirstSpace@3122-1.Invoke(Context ctx)
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.atIndentLevel@426-6.Invoke(Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 426
   at Fantomas.Context.atIndentLevel(Boolean alsoSetIndent, Int32 level, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 430
   at Fantomas.Context.shortExpressionWithFallback(FSharpFunc`2 shortExpression, FSharpFunc`2 fallbackExpression, Int32 maxWidth, FSharpOption`1 startColumn, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 803
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.atIndentLevel@426-6.Invoke(Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 426
   at Fantomas.Context.atIndentLevel(Boolean alsoSetIndent, Int32 level, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 430
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.shortExpressionWithFallback(FSharpFunc`2 shortExpression, FSharpFunc`2 fallbackExpression, Int32 maxWidth, FSharpOption`1 startColumn, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 803
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.shortExpressionWithFallback(FSharpFunc`2 shortExpression, FSharpFunc`2 fallbackExpression, Int32 maxWidth, FSharpOption`1 startColumn, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 803
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.opt[a](FSharpFunc`2 f', FSharpOption`1 o, FSharpFunc`2 f, Context ctx)
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.col[T](FSharpFunc`2 f', IEnumerable`1 c, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 546
   at Fantomas.CodePrinter.smallRecordExpr@1343-19.Invoke(Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1343
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.shortExpressionWithFallback(FSharpFunc`2 shortExpression, FSharpFunc`2 fallbackExpression, Int32 maxWidth, FSharpOption`1 startColumn, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 803
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.expressionExceedsPageWidth(FSharpFunc`2 beforeShort, FSharpFunc`2 afterShort, FSharpFunc`2 beforeLong, FSharpFunc`2 afterLong, FSharpFunc`2 expr, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 914
   at Fantomas.Context.atIndentLevel@426-6.Invoke(Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 426
   at Fantomas.Context.atIndentLevel(Boolean alsoSetIndent, Int32 level, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 430
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.col[T](FSharpFunc`2 f', IEnumerable`1 c, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 546
   at Fantomas.CodePrinter.genExpr@1622-225.Invoke(Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 1622
   at Fantomas.Context.atIndentLevel@426-6.Invoke(Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 426
   at Fantomas.Context.atIndentLevel(Boolean alsoSetIndent, Int32 level, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 430
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.Context.col[T](FSharpFunc`2 f', IEnumerable`1 c, FSharpFunc`2 f, Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 546
   at Fantomas.CodePrinter.genImpFile@86-6.Invoke(Context ctx) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/CodePrinter.fs:line 86
   at Fantomas.Context.op_PlusGreater(FSharpFunc`2 ctx, FSharpFunc`2 f, Context x) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/Context.fs:line 461
   at Fantomas.CodeFormatterImpl.formatWith(ParsedInput ast, FSharpList`1 defines, FSharpList`1 hashTokens, FormatContext formatContext, FormatConfig config) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/CodeFormatterImpl.fs:line 395
   at Fantomas.CodeFormatterImpl.format@409-1.Invoke(Tuple`3[] _arg1) in /home/runner/work/fantomas-tools/fantomas-tools/.deps/fantomas/src/Fantomas/CodeFormatterImpl.fs:line 411
   at Microsoft.FSharp.Control.AsyncPrimitives.CallThenInvokeNoHijackCheck[a,b](AsyncActivation`1 ctxt, FSharpFunc`2 userCode, b result1) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 404
   at Microsoft.FSharp.Control.Trampoline.Execute(FSharpFunc`2 firstAction) in F:\workspace\_work\1\s\src\fsharp\FSharp.Core\async.fs:line 104

Problem description

Fantomas throws an exception with an error message that looks unrelated to me. I would expect fantomas to remove the extra space. I get a similar exception with

f x = match x with
      | 5 ->
          { actual = 6
             y = x }

Extra information

Options

Fantomas Master at 02/25/2021 08:33:56 - 9bc785c38d316c5e5b1bbdaeae04ed05b5ee8088

Default Fantomas configuration

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?

nojaf commented 3 years ago

Hello Theodor, thank you for reporting this issue.

The extra space seems to lead to a function call. Consider the following example:

let x =
      { actual = 6
         y = x }

let y =
      { actual = 6
        y = x }

matching AST

ImplFile
  (ParsedImplFileInput
     ("tmp.fsx", true, QualifiedNameOfFile Tmp$fsx, [], [],
      [SynModuleOrNamespace
         ([Tmp], false, AnonModule,
          [Let
             (false,
              [Binding
                 (None, NormalBinding, false, false, [],
                  PreXmlDoc ((1,4), FSharp.Compiler.XmlDoc+XmlDocCollector),
                  SynValData
                    (None, SynValInfo ([], SynArgInfo ([], false, None)), None),
                  Named
                    (Wild tmp.fsx (1,4--1,5) IsSynthetic=false, x, false, None,
                     tmp.fsx (1,4--1,5) IsSynthetic=false), None,
                  Record
                    (None, None,
                     [((LongIdentWithDots ([actual], []), true),
                       Some
                         (App
                            (NonAtomic, false,
                             App
                               (NonAtomic, true, Ident op_Equality,
                                App
                                  (NonAtomic, false,
                                   Const
                                     (Int32 6,
                                      tmp.fsx (2,17--2,18) IsSynthetic=false),
                                   Ident y,
                                   tmp.fsx (2,17--3,10) IsSynthetic=false),
                                tmp.fsx (2,17--3,12) IsSynthetic=false), Ident x,
                             tmp.fsx (2,17--3,14) IsSynthetic=false)), None)],
                     tmp.fsx (2,6--3,16) IsSynthetic=false),
                  tmp.fsx (1,4--1,5) IsSynthetic=false,
                  DebugPointAtBinding tmp.fsx (1,0--3,16) IsSynthetic=false)],
              tmp.fsx (1,0--3,16) IsSynthetic=false);
           Let
             (false,
              [Binding
                 (None, NormalBinding, false, false, [],
                  PreXmlDoc ((5,4), FSharp.Compiler.XmlDoc+XmlDocCollector),
                  SynValData
                    (None, SynValInfo ([], SynArgInfo ([], false, None)), None),
                  Named
                    (Wild tmp.fsx (5,4--5,5) IsSynthetic=false, y, false, None,
                     tmp.fsx (5,4--5,5) IsSynthetic=false), None,
                  Record
                    (None, None,
                     [((LongIdentWithDots ([actual], []), true),
                       Some
                         (Const
                            (Int32 6, tmp.fsx (6,17--6,18) IsSynthetic=false)),
                       Some (tmp.fsx (6,19--7,8) IsSynthetic=false, None));
                      ((LongIdentWithDots ([y], []), true), Some (Ident x), None)],
                     tmp.fsx (6,6--7,15) IsSynthetic=false),
                  tmp.fsx (5,4--5,5) IsSynthetic=false,
                  DebugPointAtBinding tmp.fsx (5,0--7,15) IsSynthetic=false)],
              tmp.fsx (5,0--7,15) IsSynthetic=false)], PreXmlDocEmpty, [], None,
          tmp.fsx (1,0--7,15) IsSynthetic=false)], (true, true)))

So

actual = 6
 y = x // with space

appears to be a function call and there Fantomas is trying to figure out if it needs a space or not. An extra pattern match clause in https://github.com/fsprojects/fantomas/blob/9bc785c38d316c5e5b1bbdaeae04ed05b5ee8088/src/Fantomas/CodePrinter.fs#L48-L57

before UppercaseSynExpr could solve the problem I think. Please note that original extra space might end up to something different then you indented.

Are you interested in submitting a PR?

theothornhill commented 3 years ago

Thanks for the info. I'll give it a go :)

theothornhill commented 3 years ago

Do you have an idea of where to put the tests?

In addition, it is getting close, however it is missing one part:

  Error Message:
     Expected string length 62 but was 61. Strings differ at index 21.
  Expected: "\nlet x = { actual = 6; y = x }\n\nlet y = { actual = 6; y = x }\n"
  But was:  "\nlet x = { actual = 6 y = x }\n\nlet y = { actual = 6; y = x }\n"
  ---------------------------------^

  Stack Trace:
     at FsUnit.TopLevelOperators.should[a,a](FSharpFunc`2 f, a x, Object y) in c:\Users\104170thth\Git\fantomas\src\Fantomas.Tests\FsUnit.fs:line 33
   at Fantomas.Tests.ListTests.testodor() in c:\Users\104170thth\Git\fantomas\src\Fantomas.Tests\ListTests.fs:line 2227

How do I add that semi?

let rec addSpaceBeforeParensInFunCall functionOrMethod arg (ctx: Context) =
    match functionOrMethod, arg with
    | SynExpr.TypeApp (e, _, _, _, _, _, _), _ -> addSpaceBeforeParensInFunCall e arg ctx
    | SynExpr.Paren _, _ -> true
    | SynExpr.Const _, _ -> true
    | UppercaseSynExpr, ConstExpr (Const "()", _) -> ctx.Config.SpaceBeforeUppercaseInvocation
    | LowercaseSynExpr, ConstExpr (Const "()", _) -> ctx.Config.SpaceBeforeLowercaseInvocation
    | SynExpr.Ident _, SynExpr.Ident _ -> true
    | UppercaseSynExpr, Paren _ -> ctx.Config.SpaceBeforeUppercaseInvocation
    | LowercaseSynExpr, Paren _ -> ctx.Config.SpaceBeforeLowercaseInvocation
    | _ -> true
nojaf commented 3 years ago

Thank you for your contribution!