fsprojects / fantomas

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

Content of #if block removed - Breaking Code on reformat #801

Closed sadgit closed 2 years ago

sadgit commented 4 years ago

Issue created from fantomas-online

Please describe here fantomas problem you encountered

Extra lines added - minor

Content of #if block removed - Breaking

Code

#if !COMPILED

#I "C:/devuser/AppData/Roaming/npm/node_modules/azure-functions-core-tools/bin"
#r "Microsoft.Azure.WebJobs.Host.dll"
#r "System.Net.Http.Formatting.dll"

open Microsoft.Azure.WebJobs
open Microsoft.Azure.WebJobs.Host

#endif

#load "../shared/utilities.fsx"

open Common.Utilities

#load "Function.fsx"

open Domain.Functions

let Run(message: string, executionContext: ExecutionContext, log: TraceWriter) =
    let logInfo = log.Info
    let getSetting = System.Environment.GetEnvironmentVariable

    executionContext
    |> FunctionGuid logInfo
    |> ignore

    message |> Function.Accept logInfo getSetting

Result

#if !COMPILED
#endif
#load "../shared/utilities.fsx"

open Common.Utilities

#load "Function.fsx"

open Domain.Functions

let Run (message: string, executionContext: ExecutionContext, log: TraceWriter) =
    let logInfo = log.Info

    let getSetting =
        System.Environment.GetEnvironmentVariable

    executionContext |> FunctionGuid logInfo |> ignore

    message
    |> Function.Accept logInfo getSetting

Options

Fantomas Next - 4.0.0-alpha-001-1/1/1990

Name Value
IndentSpaceNum 4
PageWidth 120
SemicolonAtEndOfLine false
SpaceBeforeParameter true
SpaceBeforeLowercaseInvocation true
SpaceBeforeUppercaseInvocation false
SpaceBeforeClassConstructor false
SpaceBeforeMember false
SpaceBeforeColon false
SpaceAfterComma true
SpaceBeforeSemicolon false
SpaceAfterSemicolon true
IndentOnTryWith false
SpaceAroundDelimiter true
MaxIfThenElseShortWidth 40
MaxInfixOperatorExpression 50
MaxRecordWidth 40
MaxArrayOrListWidth 40
MaxLetBindingWidth 40
MultilineBlockBracketsOnSameColumn false
NewlineBetweenTypeDefinitionAndMembers false
StrictMode false
nojaf commented 4 years ago

@jindraivanek this is a weird bug as the compiler is returning the same AST in both code paths. So the define solver parses correctly that it should generate AST with both [] and ["COMPILED"]. However, change COMPILED to COMPILEDX and the result is way better.

I'm guessing passing "COMPILED" does weird things when we create the AST. The AST viewer tab does get it right though, however, that creates the AST slightly different.

jindraivanek commented 4 years ago

@nojaf Related part of specs (https://fsharp.org/specs/language-spec/4.1/FSharpSpec-4.1-latest.pdf#page=21&zoom=auto,-137,799) :

The COMPILED compilation symbol is defined for input that the F# compiler has processed. The INTERACTIVE compilation symbol is defined for input that F# Interactive has processed.

I'm quite sure that when parsing to AST, F# compiler add COMPILED or INTERACTIVE (for script files) to defines. Looks like it is based on FSharpParsingOptions.IsInteractive not on filename. Looks like AST viewer parse file as interactive, Fantomas don't.

To fix this, I propose to change this special defines (COMPILED, INTERACTIVE) to something different (that is not used in source code), and then change it back when outputting formatted code.

nojaf commented 4 years ago

Good thinking, thanks for checking that out. I'll see if I can come up with something.

WalternativE commented 3 years ago

Maybe already covered with this issue but just for completeness sake:

When I load multiple files with one #load statement like

#load "file1.fsx" "file2.fsx"

....

reformatting will translate this to

#load "file1.fsx"
#load "file2.fsx"

...

Afaik this doesn't translate to the exact same behavior in FSI :/

nojaf commented 3 years ago

@WalternativE not sure that case is related here. Could you reproduce this using our online-tool? That might be a separate issue.

WalternativE commented 3 years ago

@nojaf apparently I can't reproduce it anyway. I swear to god, that it happened to me but apparently it mus have been the ghosts in the machine again 🙈

nojaf commented 3 years ago

@WalternativE nah, this could very much have been a thing at your PC at one time. There are a couple of versions out there in the wild. It is totally possible to find a bug locally and see that the online tool (linked to the latest and possibly unreleased code) works out just fine.

WalternativE commented 3 years ago

Could very well be the case. I use Fantomas mainly from the Ionide integration (just as an aside: the progress y'all have made just in the last year is super awesome ❤). There have been updates since I've seen the odd behavior.

nojaf commented 2 years ago

This is no longer a problem because we don't have the same directive restrictions in Fantomas.FCS than we had with the regular FCS. A regression test can close this issue.