Use .editorconfig to define Fantomas settings

[xperiandri]

.editorconfig is a common solution to define all code related rules including formatting and code analysis.
It would be the right option for Fantomas to read settings from it.

[nojaf]

Hey @xperiandri , this issue came just in time ;)
I'm about to release to 3.2 stable and that would mark fantomas-config.json as public api so glad we can discuss other options before that.

I'm a bit less familiar with .editorconfig so my main question would be: how does this work with custom settings? Can we just write anything we want in the file and parse it as we want?

How would our current default look like in .editorconfig?

{
  "IndentSpaceNum":4,
  "PageWidth":120,
  "SemicolonAtEndOfLine":false,
  "SpaceBeforeArgument":true ,
  "SpaceBeforeColon":false,
  "SpaceAfterComma":true ,
  "SpaceAfterSemicolon":true ,
  "IndentOnTryWith":false,
  "ReorderOpenDeclaration":false,
  "SpaceAroundDelimiter":true ,
  "KeepNewlineAfter":false,
  "MaxIfThenElseShortWidth":40,
  "StrictMode":false
}

One advantage of using a JSON file is that we have listed a JSON schema. Would there be something similar for editorconfig?

[xperiandri]

@madskristensen could you explain what is the source of Editor Config Language autocomplete? How can Fantomas provide F# specific settings to a service?

[xperiandri]

# All files
[*]
indent_style = space
# Code files
[*.{cs,csx,fs,fsx,vb,vbx}]
indent_size          = 4         #"IndentSpaceNum":4
insert_final_newline = true
charset              = utf-8-bom
max_line_length      = 120       #"PageWidth":120
###############################
# .NET Coding Conventions     #
###############################
[*.{cs,fs,vb}]
# Organize usings
dotnet_sort_system_directives_first = false            #"ReorderOpenDeclaration" :false
###############################
# F# Formatting Rules         #
###############################
fsharp_strict_mode                  = false            #"StrictMode"             :false
fsharp_semicolon_at_end_of_line     = true             #"SemicolonAtEndOfLine"   :false
fsharp_space_before_argument        = true             #"SpaceBeforeArgument"    :true
fsharp_space_around_coma            = after            #"SpaceAfterComma"        :true
fsharp_space_around_colon           = none             #"SpaceBeforeColon"       :false
fsharp_space_around_semicolon       = after            #"SpaceAfterSemicolon"    :true
fsharp_space_around_delimiter       = before_and_after #"SpaceAroundDelimiter"   :true
fsharp_ident_on_try_with            = false            #"IndentOnTryWith"        :false
fsharp_keep_new_line_after          = false            #"KeepNewlineAfter"       :false
fsharp_max_if_then_else_short_width = 40               #"MaxIfThenElseShortWidth":40

[nojaf]

Why are you changing the names of settings?

fsharp_space_around_coma            = after            #"SpaceAfterComma"        :true
fsharp_space_around_colon           = none             #"SpaceBeforeColon"       :false
fsharp_space_around_semicolon       = after            #"SpaceAfterSemicolon"    :true

And why not use boolean for

fsharp_space_around_coma            = after            #"SpaceAfterComma"        :true
fsharp_space_around_semicolon       = after            #"SpaceAfterSemicolon"    :true
fsharp_space_around_delimiter       = before_and_after #"SpaceAroundDelimiter"   :true

To be clear, the only thing I'm considering here is swapping the JSON config file to .editorconfig format.

Different names for the settings might eventually be being discussed but for now, I would like to keep the same names in the CLI tool as in the configuration file. Changing names is something that I would only do in the next major version.

[xperiandri]

@nojaf, could you show me an excerpt of the code, that matches your settings that I have changed?
Or show them on my excerpt

module Waggrx.Store.Cache

open Waggrx.Models
open System.Collections.Generic
open System.Net.Http
open FSharp.Collections.Immutable
open Blazor.Fluxor

type State =
    {
        Products : HashMap<ProductId, Product>
        ProductTypes : HashMap<ProductTypeId, ProductType>
        Dosages : HashMap<DosageId, Dosage>
        Quantities : HashMap<QuantityId, Quantity>
        Flavors : HashMap<FlavorId, Flavor>
    }

type Action =
    | LoadProducts
    | LoadProductTypes of Id : ProductId
    | LoadProductType of ProductId : ProductId * ProductTypeId : ProductTypeId

let inline private merge< ^K, ^E when ^E : (member Id: ^K) > existing (loaded : seq< ^E>) =
    let keys = loaded |> Seq.map (fun e -> (^E: (member Id: ^K) (e)))
    let cached = (keys, existing) ||> HashMap.except
    let loaded = loaded |> Seq.map Catalog.toKVP
    (cached, loaded) ||> HashMap.append

let loadProducts (loadFunction : HttpClient -> Async<FlatList<Product>>)
                 httpClient (state : State) =
    async {
        let! loadedProducts = loadFunction httpClient
        return merge state.Products loadedProducts
    }

let loadAllProducts httpClient state =
    async {
        let! mergedProducts = loadProducts Catalog.loadProducts httpClient state
        return { state with Products = mergedProducts }
    }

let loadProductTypes httpClient (state : State) id =
    async {
        let! loadedProductTypes = Catalog.loadProductTypes httpClient id
        let keys = loadedProductTypes |> Seq.map (fun pt -> pt.Id)
        let cachedProducts = (keys, state.ProductTypes) ||> HashMap.except
        let loadedProductTypes = loadedProductTypes |> Seq.map Catalog.toKVP
        let mergedProducts = (cachedProducts, loadedProductTypes) ||> HashMap.append
        return { state with ProductTypes = mergedProducts }
    }

let loadProductsPage httpClient (state : State) pageSize pageNumber =
    async {
        let loadFunction httpClient = Catalog.loadProductsPage httpClient pageSize pageNumber
        let! mergedProducts = loadProducts loadFunction httpClient state
        let! productIdsWithTypes =
            async {
                let! productIdsWithTypes =
                    query {
                        for productId in mergedProducts.Keys do
                            let productIdsWithTypes = async {
                                let! productTypes =
                                    Catalog.loadProductTypes httpClient productId
                                return productTypes
                                    |> Seq.map (fun productType -> KeyValuePair(productId, productType))
                            }
                            select productIdsWithTypes
                    } |> Async.Parallel
                return productIdsWithTypes
                    |> Seq.collect id
                    |> HashMap.ofSeq
            }
        let! productIdsWithDosages =
            async {
                let! productIdsWithDosages =
                    query {
                        let idsPairs =
                            productIdsWithTypes
                            |> Seq.map (fun kvp -> struct (kvp.Key, kvp.Value.Id))
                        for ids in idsPairs do
                            let struct (productId, typeId) = ids
                            let productIdsWithDosages = async {
                                let! dosages = Catalog.loadDosages httpClient productId typeId
                                return dosages
                                    |> Seq.map (fun dosage -> KeyValuePair(ids, dosage))
                            }
                            select productIdsWithDosages
                    } |> Async.Parallel
                return productIdsWithDosages
                    |> Seq.collect id
                    |> HashMap.ofSeq
            }
        let! quantities =
            async {
                let! productIdsWithQuantities =
                    query {
                        let idsTriples =
                            productIdsWithDosages
                            |> Seq.map (fun kvp ->
                                let struct (productId, typeId) = kvp.Key
                                struct (productId, typeId, kvp.Value.Id))
                        for ids in idsTriples do
                            let struct (productId, typeId, dosageId) = ids
                            select (Catalog.loadQuantities httpClient productId typeId dosageId)
                    } |> Async.Parallel
                return productIdsWithQuantities
                    |> Seq.collect id
            }
        let! productIdsWithFlavors =
            async {
                let! productIdsWithFlavors =
                    query {
                        let idsPairs =
                            productIdsWithTypes
                            |> Seq.where (fun kvp -> kvp.Value.HasFlavors)
                            |> Seq.map (fun kvp -> struct (kvp.Key, kvp.Value.Id))
                        for ids in idsPairs do
                            let struct (productId, typeId) = ids
                            let productIdsWithFlavors = async {
                                let! flavors = Catalog.loadFlavors httpClient productId typeId
                                return flavors
                                    |> Seq.map (fun dosage -> KeyValuePair(ids, dosage))
                            }
                            select productIdsWithFlavors
                    } |> Async.Parallel
                return productIdsWithFlavors
                    |> Seq.collect id
                    |> HashMap.ofSeq
            }

        return {
            Products = mergedProducts
            ProductTypes = merge state.ProductTypes productIdsWithTypes.Values
            Dosages = merge state.Dosages productIdsWithDosages.Values
            Quantities = merge state.Quantities quantities
            Flavors = merge state.Flavors productIdsWithFlavors.Values
        }
    }

type Feature () =
    inherit Feature<State> ()
    override __.GetName () = "Cache"
    override __.GetInitialState () =
        {
            Products = HashMap.empty
            ProductTypes = HashMap.empty
            Dosages = HashMap.empty
            Quantities = HashMap.empty
            Flavors = HashMap.empty
        }

Or any from here https://github.com/SfSync/Bayeux/tree/master/src/Bayeux.Client

These are how I prefer to format my code.

[xperiandri]

Why are you changing the names of settings?
And why not use boolean for

Because I would prefer having option to set spaces not only after colons and semicolons but before them too.

Different names for the settings might eventually be being discussed but for now, I would like to keep the same names in the CLI tool as in the configuration file. Changing names is something that I would only do in the next major version.

I'm OK with that.
Anyway these features are not implemented as far as I know.

[nojaf]

Because I would prefer having option to set spaces not only after colons and semicolons but before them too.

Please create another issue for these kinds of requests 😉, otherwise, this issue is getting a bit derailed.

Check out the current options at our documentation.

So to get back on track, I will see if I can come up with a PR to get the settings from an .editorconfig file instead of our JSON format.

[madskristensen]

@xperiandri See these files: https://github.com/madskristensen/EditorConfigLanguage/tree/master/src/Schema

This is where you specify the properties in the .json file and you also have to add a few things in some of the other files since you're introducing a new language. It should be simple enough when you see the code in those files

[nojaf]

Hello @xperiandri , I've refactored the code a bit so that the only thing remaining here is the implementation of parsing the file to settings.

I won't pursuit this myself for the time being, there are other priorities and this can easily be picked up by someone else it doesn't really involve any knowledge about Fantomas.

To whoever that wants to pick this up, keep the following rules in mind:

• Don't introduce any new NuGet or other dependencies. Fantomas should only have the FSharp.Compiler.Service package as dependency.
• Write an extensive set of unit tests.
• Each Fantomas specific setting should snake-cased and prefixed with fsharp-.
  Example: SpaceAfterComma becomes fsharp_space_after_comma. These things should be transformed to back to the Pascalcase style when returning them as key of the return value of parseOptionsFromEditorConfig.

[xperiandri]

Why don't got want to reference Roslyn package that already has that logic?

[nojaf]

Don't introduce any new NuGet or other dependencies.

[nojaf]

Fantomas is being used by Rider, FSAutocomplete , FSAst and maybe even other projects.

I don't want to introduce an additional dependency for the consumers of Fantomas.

[xperiandri]

I don't see any benefit copying already implemented code even as source especially if it is from official MS package used by all IDEs

[Mpdreamz]

There is also: https://github.com/editorconfig/editorconfig-core-net which I maintain and is itself dependency free. Rider already uses it as well.

[nojaf]

Hey @Mpdreamz , thanks for the suggestion.
Any advice you could give on .editorconfig in general? I have some questions here:

• Should Fantomas parse the editorconfig or is that more the responsibility of the editor?
  In case of our CLI I feel that it makes sense to process it ourself. But in editor integration scenario's I feel like the editor will already have parsed the file for other reasons. Should it not just take that result and construct our configuration object here?
• What about custom settings? I feel like the list of 'agreed' settings is very limited so to support Fantomas we would need special sharp specific settings. Are we allowed to invent these settings? What prefix do we use here? How could we best align with other F# tools like F# lint?
• Any idea how editorconfig and language server work together? Might be less relevant here but just out of curiosity ;)

[josefblaha]

I thought about finishing the feature as prepared by @nojaf. However I found out that the code is not quite ready yet. The editorconfig format supports two features, that fantomas-config does not:

• root property. It's a special property that, when set to true, prevents searching for more .editorconfig files in parent directories.
• Different properties for different files. While Fantomas applies the same configuration to all F# files, editorconfig allows to set a global configuration for all files, and also specific configurations based on file wildcards (called globs). For example I may create this .editorconfig file:

[*]
indent_size = 4

[*.fs]
indent_size = 2

This sets indentation for .fs files to 2, but for other files (including .fsx and .fsi) the indentation is 4.

Now how to implement that? The root property should be easy; it may be covered by tweaking the CodeFormatterImpl.readConfiguration function. On the other hand, the different file configurations are going to require some design changes.

I see three possible solutions:

1. Fully implement the file configurations. This would be a breaking change to the Fantomas API. The configuration type would change from FormatConfig to something like Map<Glob, FormatConfig>.
2. Assume that there is only one configuration for F# in a .editorconfig file and merge any * and *.fs* sections into one. So for the example above, the resulting FormatConfig would have just IndentSpaceNum = 2. This would
3. Implement solution 2 now and solution 1 with in the next major version. Speaking of phased changes, we could also now read just the common properties like the tab size, and add custom F#-specific properties later.

What do you think?

---

Ad custom settings in editorconfig. Currently the editorconfig format defines only a limited set of universal properties (like tab settings, charset, newlines etc.). The file can contain any custom properties. There is a prefix-based convention for naming the file format specific properties. E.g. Visual Studio uses properties with dotnet_, csharp_, visual_basic_ prefixes to configure its code analyzers.

This means we are free to come up with any fsharp_ properties, preferably synced with other related F# projects (like the mentioned FSharpLint).

There is a discussion (see editorconfig/editorconfig#415) in the editorconfig project itself regarding its future extensions and custom namespaces. However there is no definite conclusion yet.

[nojaf]

Hello @josefblaha, thanks for this thorough remark.
We are currently working on the next major version of Fantomas so I feel like option 1 should be on the table.

As for the custom settings, if we can use fsharp_whatever_we_want_setting, we might only need to align with FSharpLint and write out how we can move forward. As mentioned, we are working on the next major version, so we can rename some settings if necessary.

As @Mpdreamz mentioned https://github.com/editorconfig/editorconfig-core-net also seems interesting. Perhaps we can combine this with some sort of assembly weaving. So from a Nuget point of view, we still don't have the extra dependency.

Lastly, thinking out loud at this point, perhaps we should consider of having only the .editorconfig solution.
The initial idea of the JSON schema was nice but ever since I implemented the copy your settings from the tool button, I feel like that is the way to go to configure Fantomas. The copy button would make up for the lack of intellisense in the editor I think.

[baronfel]

I would definitely be in favor of standardizing on .editorconfig, personally.

[nojaf]

I've talked with @jindraivanek about this behind the scenes and we would move forward on this.
I'm already working on this so expect it to drop in the weeks to come.
The idea would be to replace the JSON format with editor config.
Once I have something, I'll reach out to FSharpLint to see if we can consolidate some settings.

[josefblaha]

Oh :-/ Have you made much progress yet? I'm working on it too. I thought this would be a good starter issue for me.

[nojaf]

Hey, I've trying to port https://github.com/editorconfig/editorconfig-core-net to F#. The further I get there, the more obvious it becomes that this is a bad idea so I'm abandoning that train of thought.

Next attempt I had in mind was to use ILRepack and move the C# project to this repository and merge it into the Fantomas dll.
This works but well, pro's and cons there as well.

How is your attempt going @josefblaha? Biggest challenge seems the glob thing. Any fork I can take a look at maybe?

[josefblaha]

So far I've been playing with editorconfig-core-net. It provides not only parsing the .editorconfig itself, but resolving the right settings as well. There is EditorConfigParser.Parse (misleading name IMHO) method which, given source file name, returns the list of settings applicable to that file. With this, integration to Fantomas should be easy, as all the directory traversing and glob matching can be offloaded to the library.... if only we're able to merge the library in.

Why do you think it's necessary to move the library to this repository? Wouldn't referencing it as a regular NuGet package and merging it to Fantomas DLL as part of the build process be enough?

What are the cons of merging the library to Fantomas DLL? If there is a requirement for a single DLL output, it seems like a good solution.

[nojaf]

EditorConfigParser.Parse

Yes, I experimented there a bit as well. Using that method it is rather easy indeed.

Reason for going with a fork was to have an netstandard2.0 only thing. I didn't like the msbuild warnings I guess. Maybe I missed something in Paket configuration there.

What are the cons of merging the library to Fantomas DLL?

Never really used any ilmerge like thing, so has that black alchemy vibe to it.
I'm just a bit worried if that goes wrong in a couple of months, I have no idea what to do about it.

[baronfel]

I wouldn't worry too much about ilmerging, paket has been doing it from the beginning with several dependencies and it hasn't been an issue there.

[nojaf]

I released a new alpha to try this out: https://www.nuget.org/packages/fantomas-tool/4.0.0-alpha-012
Please give this a spin, it worked for me on fantomas-tools.

For the settings names, see the docs.
Also, I just noticed that the docs still mention --config, this is not necessary anymore.

[Mpdreamz]

• What about custom settings? I feel like the list of 'agreed' settings is very limited so to support Fantomas we would need special sharp specific settings. Are we allowed to invent these settings? What prefix do we use here? How could we best align with other F# tools like F# lint?

Custom settings are totally fine! Namespacing is an active discussion but given the prevalence of existing prefixes I reckon reserved prefixes will end up in the spec at some point.
fsharp_ makes the most sense to me too here. It'd be awesome if fslint picks these up as well.

• Any idea how editorconfig and language server work together? Might be less relevant here but just out of curiosity ;)

Some LSP's support it and some don't. I don't think the LSP protocol exposes it explicitly although there is this: https://microsoft.github.io/language-server-protocol/specifications/specification-3-14/#workspace_configuration

I'm keen to get y'all of the fork, I will look at getting the compiler errors to go away when building on a non Windows machine.

There is a dotnet tool I help maintain that could be run before IL merge:

https://github.com/nullean/assembly-rewriter

To rewrite the namespaces to Fantomas.EditorConfig.*.

Let me know how I can help further in any way. You should be able to find me on the fsharp slack too.