Some banter

[nojaf]

Hi Patrick, thanks again for the demo.

Some stuff, that came to mind (in random order):

• The untyped tree is used to represent both valid and invalid code. That is why certain trivia will be represented as optional.

let a

still gets parsed to a tree with errors:

trivia = {
    LeadingKeyword = SynLeadingKeyword.Let(R("(1,0--1,3)"))
    InlineKeyword = None
    EqualsRange = None
}

Incomplete structured construct at or before this point in binding. Expected '=' or other token.

We don't process any tree with errors (and certain warnings), so we can assume in ASTTransformer that stuff is just there. This means that you need to put in Some range0 in certain places. Just to tell Fantomas that the keyword is present.

We transform the ParsedInput to an Oak internally, to only print out the code again.
It is possible to print source code from an Oak, this is more convenient than the untyped tree.
I hoped that one day https://edgarfgp.github.io/Fabulous.AST/ would have taken off and become the recommended way to generate code.

• I think as a rule of thumb, every token or keyword that you want to see in the output code should be present (as Some range0). So, when generating a let binding, you know that let and = should be in the tree. The same with commas in tuples, = in record fields. These days, there are few tokens not represented in the tree.

• Myriad seems like a bit of a mixed bag. I think it isn't that hard to cook up some tool that is similar to how the analyzers work. Crack a fsproj, get type-check information of a file and use Fantomas to generate a new file.

Did you ever explore other options?

• It is ok to take some shortcuts when generating code. For example:

#r "nuget: Fantomas.Core, 6.3.0-alpha-005"

open Fantomas.FCS.Syntax

let cheatABit (e: SynExpr) : SynBinding =
    let source = "let a b = ()"

    let parsedInput =
        CodeFormatter.ParseAsync(false, source)
        |> Async.RunSynchronously
        |> Array.head
        |> fst

    match parsedInput with
    | ParsedInput.ImplFile(ParsedImplFileInput(
        contents = [ SynModuleOrNamespace(decls = [ SynModuleDecl.Let(bindings = [ b ]) ]) ])) ->
        match b with
        | SynBinding(accessibility,
                     kind,
                     isInline,
                     isMutable,
                     attributes,
                     xmlDoc,
                     valData,
                     headPat,
                     returnInfo,
                     _,
                     range,
                     debugPoint,
                     trivia) ->
            SynBinding(
                accessibility,
                kind,
                isInline,
                isMutable,
                attributes,
                xmlDoc,
                valData,
                headPat,
                returnInfo,
                e,
                range,
                debugPoint,
                trivia
            )
    | _ -> failwith "unexpected"

Parsing small strings to grab the AST nodes is a trick I do in Telplin from time to time.

• Myriad really misses some unit tests to cover https://github.com/MoiraeSoftware/myriad/blob/master/src/Myriad.Core/AstExtensions.fs. Every year or so, they upgrade Fantomas, tweak until things compile and then complain when stuff no longer is generated at runtime.

[Smaug123]

Thanks for this! (In theory I'm testing everything I have in this repo; I did actually consider just making my own AstExtensions entirely. If necessary I can just make my own versions of everything I currently use from AstExtensions.)

A tool that gives access to type-checking information would be super cool and would fix so many of my hacks. I probably don't have much spare energy soon, but I will definitely keep that in the back of my mind for when I next become inspired. I need to work out how the analysers actually work. I didn't really bother exploring anything other than Myriad.

Neat idea with parsing strings. That's definitely cheating ;)

---

I guess there's a possible world where the AST had a tighter set of types, so that an "incomplete" AST had the different possible incompletenesses modelled precisely (in such a way that the default were valid, and you had to add extra information to represent an invalid node), and so that a "complete" AST were always valid if you managed to construct a node of the right type. Like, instead of the current optional type, trivia is instead a three-case DU:

type Trivia =
    /// this is "strict mode", currently not really modelled in the AST
    | Absent
    /// this is a subset of the current `Some trivia` cases
    | Present of (data which specifies node layout)
    /// currently implicit as "anything that isn't Present"
    | Incomplete of (data which specifies the fragment)

type SynExpr =
    | SynPat of (_ * {anything which is currently in trivia that is necessary for semantics, e.g. "inline"} * Trivia)

That way, tools such as Fantomas could know whether a node was intended to carry layout information or not.

This would, of course, be a bunch of work in both FCS and Fantomas, and would be a very API-breaking change; I don't have the intuition of working with the compiler to know whether it would be worthwhile, but I bet it would be too much work. It has the nice property that it makes explicit some stuff which is currently implicit in the treatment of range0 as a sort of Some(None) whose interpretation is based on context.

[Smaug123]

I started looking at this, using the analyzers as a base, but immediately entered NuGet hell while trying to load a project file. This is the kind of work that is literally physically painful to me, so I will probably not proceed with it :P

The "ProcessFrameworkReferences" task failed unexpectedly.
System.IO.FileLoadException: Could not load file or assembly 'NuGet.Frameworks, Version=6.8.0.122, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. Could not find or load a specific file. (0x80131621)

https://github.com/ionide/FSharp.Analyzers.SDK/compare/main...Smaug123:FSharp.Analyzers.SDK:blah?expand=1

[nojaf]

Hi Patrick, I totally hear you that NuGet hell is no fun.
I was actually able to run your test project with this small change:

The clash seems to come from using ManagePackageVersionsCentrally I would suspect.

[Smaug123]

So that didn't work for me, but an explicit dep on NuGet.Frameworks v6.8.0 did work 🤷

[nojaf]

Hmm, yes, that is weird.
What SDK and OS are you on?

[Smaug123]

net8.0.1, osx-arm64. I'm not hugely interested in working out the problem here though because I do now have a TAST!

[nojaf]

Yeah fair enough. Nothing comes to mind anyway.

[Smaug123]

Things of this level of cursedness happen every time I try and host the compiler - with the debugger attached, typechecking consistently passes; without the debugger attached, typechecking consistently aborts.

[Smaug123]

TASTCollecting in Analyzers.SDK currently seems to lack what I need, namely "visit the declarations directly" - it currently only visits right-hand sides of expressions? (For example, I can't visit a simple declaration of a record type, I think.) But I'm a bit surprised that nobody's needed to visit left-hand sides for analysers before, so maybe I've misunderstood something.

[nojaf]

Yes, you would probably be the first to attempt to find that information via the type tree collector.
Most tooling will likely attempt to use the symbols API.
I've hacked a quick example of how you could do it: G-Research/fsharp-analyzers@main...record-type

Or a more direct approach would be to filter all the symbols of a file: ctx.CheckFileResults.GetAllUsesOfAllSymbolsInFile() |> // ... filter them out

[Smaug123]

Do you happen to know where the attributes have gone on the FSharpImplementationFileDeclaration list in the TypedTree? I can't find any declaration that contains a nonempty Attributes. Or did they get thrown away at some point before now? (I've updated https://github.com/ionide/FSharp.Analyzers.SDK/compare/main...Smaug123:blah?expand=1 if interested.)

[nojaf]

Hmm, not sure. This comes to mind: dotnet/fsharp#13786

[Smaug123]

@nojaf I've started getting a little annoyed with how many versions behind Myriad is with its FCS dependency, so I'm now more interested than I was in rearchitecting things. Do you know if there's a tool which I can reference in an fsproj file like I can with Myriad, which uses Fabulous.AST already?

[nojaf]

I'm not entirely sure if I understand what you are looking for.
An example project of Fabulous.AST would be ionide/LanguageServerProtocol#49 maybe. Could you briefly describe again what you are after?

[Smaug123]

Just a reasonably drop-in "here (WoofWare.Myriad.Plugins) is some F# assembly which exposes methods to construct a syntax tree; here (MyNewProject.fsproj) is an fsproj file which contains some inputs (MyInput.fs) to that construction; run the Plugins on MyInput.fs, creating this resulting output MyOutput.fs file which gets compiled into MyNewProject.fsproj".

[Smaug123]

Thanks for that PR link; there's rather more manual faff in there than I'd like (which Myriad exists to abstract away from us). I can probably write something Myriad-like which accepts arbitrary plugins.

[Smaug123]

OK, it's happening (https://github.com/Smaug123/WoofWare.Whippet). I probably won't get time to work on this very much over the next few weeks, due to life happening, but hopefully this will become the future direction of this source generation repo.

[Smaug123]

All the useful generators in WoofWare.Myriad are now implemented in Whippet, published into NuGet, and usable by consumers if they want. The Whippet setup is fully customisable and would (for example) accept a plugin written against Fabulous.AST. This customisation arises because Whippet does not take a dependency on Fantomas: at its heart it requires just a byte[] -> string? function, and it expects plugins themselves to take a dependency on a client library like WoofWare.Whippet.Fantomas or Fabulous.AST to provide Fantomas's capabilities.

In particular, it is now feasible to upgrade Fantomas!

I haven't yet worked out an appropriate interface for getting typing information into the plugin. It would make some sense for Whippet to handle that and pass it in somehow (because it's already using proj-info to obtain msbuild information), but that would probably be an absolute nightmare for getting dependencies matched up correctly between Whippet and the plugin.

[nojaf]

Very interesting approach! Myriad has always lacked the discipline to keep up with Fantomas, so avoiding that dependency is a great move!