Discussion FST-1032 - F# analyzers and binary compat

[dsyme]

Discussion thread for this tooling RFC/discussion - https://github.com/fsharp/fslang-design/blob/master/tooling/FST-1033-analyzers.md

[moloneymb]

+1 source code distribution method. In general not just for Analyzers. Similar to the nuget references but maybe pointing it to a github, gist, or any webpage of your choosing.

Thinking aloud for Part 1; Perhaps a binary format such as Protobuf? With the use of Span it should be incredibly fast. The representation could be used re-used for caching as well as dispatching to an out of process analyzer.

[smoothdeveloper]

+1 for source code distribution method, it settles the design decisions and work involved for binary compat. to a later stage.

It will help a lot to thinker on said analyzers "in the IDE" / interesting synergies with interactive / REPL approach that F# embodies.

I'm trying to see if it causes problems with different versions of IDEs compared to going with binary distribution, but nothing comes to mind.

@dsyme, do you see any connection with what sits underlying https://github.com/fsprojects/fsharp-ai-tools live checking ? should it be mentioned in the RFC as a related area to investigate at some point?

[dsyme]

@dsyme, do you see any connection with what sits underlying https://github.com/fsprojects/fsharp-ai-tools live checking ?

Yes, I've rejigged that checking to use this F# analyzers prototype, and it's a major stress test for the mechanism

[baronfel]

One thing that jumps out with this effort is the difficulty of using these analyzers in editor tooling that also supports the enablePartialTypeChecking flag in FCS , which enables using signature files to skip certain pieces of work.

The compiler itself of course would not use this flag, but hosted compiler services would likely want to turn this flag on, but it is mutually exclusive with the keepAssemblyContents flag, which is required to access the TypedTree: FSharpImplementationFileContents property on the proposed structures (and indeed the existing API on the Ionide F# Analyzers).

How should this dichotomy be addressed by hosted tooling? Multiple FSharpCheckers that are kept in sync somehow?

[dsyme]

Good point. The current prototype implicitly turns off enablePartialTypeChecking for a project if any analyzers referenced by that project declare RequiresAssemblyContents. I think this is an adequate solution

https://github.com/dotnet/fsharp/pull/11057/files#diff-4cf589945def81188bed84b5466d6c2c6eca933e91427e54ed059f018fb9cec9R1487

[baronfel]

That seems good at preventing crashes due to a misconfigured FSharpChecker, but is not ideal for hosted scenarios. Is there some notification or log event that can be observed to know when/if the user has added an analyzer that requests that capability? I'm thinking of working on a large project and getting used to a certain level of performance in tooling due to the use of signature files, and then not being able to correlate a tooling regression with the act of referencing an analyzer.

[dsyme]

That seems good at preventing crashes due to a misconfigured FSharpChecker, but is not ideal for hosted scenarios.

I'm not quite sure what you mean "for hosted scenarios"? The disabling of enablePartialTypeChecking is only for the actual project that uses the RequiresAssemblyContents analyzer, and not global.

I mean, the implicit disabling is not perfect, but using analyzers will have a cost, it's part of the territory. And the analyzer might do anything, it might inflict arbitrary extra costs. So (like type providers) it seems inherent that the author of the analyzer be responsible for communicating the costs of the analyzer. We can document this "need to communicate" as part of RequiresAssemblyContents documentation.

[baronfel]

By Hosted Scenarios I mean long-lived Checkers like FSAC, that's all. Your prototype handles this switch much more gracefully than FSAC does at the moment for Ionide Analyzers, so it's probably ok on balance.

[baronfel]

Came across this tweet by Jared Parsons talking about diagnostics, and was wondering if we had the capacity to do similar in the current implementation. I can definitely see a benefit to making this information visible to end users via normal MSBuild reporting, and the binarylog is my go-to these days for diagnosing build issues.

[dsyme]

There's no such capability in the prototype. It's an important issue yes

[baronfel]

I've been discussing with a few current Ionide Analyzer authors and there's a significant amount of support for altering the Ionide Analyzer API shape to be very similar to the proposed/implemented shape here, to make the migration path for this functionality much smoother as it gets closer to landing. The path would be something like:

• update the API Contracts in the SDK and SDK client library with the new shapes
• update FSAC with the new shape and make use of the new APIs to handle dynamic toggling between fast-compilation FSharpCheckers and Full-Implementation-Contents FSharpCheckers, instead of the heuristics we now use
• when this feature releases, it should be minimal code churn (ideally a package reference change/removal + potentially a nuget packaging change for the library) to support the new mechanism.

[dsyme]

That would be great! And it would certainly help validate this API

[T-Gro]

For start, I am sorry for invading an older and probably even closed discussion.

INTRO
I have been thinking about the FCS binary compatibility aspect of the analyzer roadmap and cross-checked it against of a bulk of relatively recent PRs ;; since similar decision also affects other ideas on how compiler and "user code" could interact (passing types to TPs or source-gen interop for instance). Obviously, even the smallest additions in the PRs I have scanned would break binary compat as of now:

• Adding new trivia data to parsed nodes
• Changing (n)-tuple to (n+1) tuple
• Adding or removing members to FCS's discriminated unions
• ...

There would be solutions to overcome this at the cost of more complex FSC code and more stagnation, possibly also disabling certain refactoring/redesign options in the future.

What is worse, even when binary compatibility could be maintained at some cost, semantic compatibility would be a lot harder due to tight coupling of tree traversal code at analyzer side. Imagine a scenario of a new language construct that would suddenly appear in the middle of the tree, let's pretend TryFinally would not exist before and was to be introduced. Older analyzers would not be capable of traversing into the child nodes of it, even though they could do useful work inside.

Disclaimer:
Since I am very new to all of compiler + tooling development, I am quite sure I am on the wrong side of Dunning-Kruger , and would love to read where my thinking is wrong. I tried to search if the following has been proposed around analyzers, but did not find any mentions.

THOUGHT

It is a good industry practice to decouple data used internally from the types shared with the outside via interfaces. At the same time, the interface description should be ideally an independent schema enable evolution of code/language used by both sides , and the data+operations driven by the needs of client ( => NOT being a 1:1 mapping between domain code used in the compiler and isomorphic DTOs, since this will eventually hit compat issues too).

Thinking about an API to enable semantic compatibility for the analyzers and non-breaking ways of evolving compiler code at the same time, surprisingly a lot of similarities with DOM/XML and it's operations come to mind:

• Describing the contract via an independent schema
• Ability to jump over unknown elements in the tree via "Descendants"-like operations or full querying capability to find specific constructs deep in the tree
• Multidirectional pointers (parents, children, siblings)
• Intentionally designed to be extensible in both elements (new nodes) as well as their attributes
• Support for unique ID which analyzers would use for reporting results back (instead of being coupled to ranges)

I obviously do know that xml is not the hottest technology of the day and defining the interface (data and querying operations) to be "XML-like, but fully developed and maintained by F# repo" would sound better, and API of Roslyn seems to be inspired that way too.

At the same time, it is difficult to ignore the benefits of existing technologies around XML helping here (XML Schema, XPath, client-side generators/providers to work with semi-structured data ) and not having to write them from scratch.
Especially a generic tree-querying ability is hard to beat compared to manually written API operations trying to cover the same range of use cases.

If analyzers are restricted to be used in-process, it could still be an in-memory object model without allocating the full xml document as a string. Given a few standard navigation operations, there is the potential to generate nodes for the interface lazily at the cost of a less generic (and less leaning on existing tools) approach.

A more performant (conversion, allocations) and less known alternative could be protobuf v3 and it's field masks, but I do not consider it to be a technology wide-spread enough to base an API on. Also the simple JSON path used within it would very likely not deliver the desired semantic compatibility advantages.

[moloneymb]

It's up to whoever is building it, I'm just giving a heads up that serialization/deserialization can get expensive surprisingly quickly.

[dsyme]

I think it's good to think outside the box with regard to how we handover the syntax tree to the analyzers. I've previously suggested just handing over JSON to the analyzers, for example, and documenting that JSON schema separately.

It's likely that analyzers will be very performance sensitive

Currently the perf expectation is

• An analyzer would only get called after at least a parse of an entire F# source code file and the full creation of the entire SyntaxTree for that file (given incremental F# file parsing is not on the horizon any time soon). That's already O(source-file-length). If any analyzer also requires semantic/checked information then we have the full typecheck of the F# file too, plus analyzers may also require the full symbol-use graph or semantic classifications of tokens etc.

So creating a DOM/JSON/XML/StronglyTypedButBinaryCompatible thing to interact with is about the same order of complexity - no matter what information representation technique is used - sure we may double the allocations but it's not a vast expense. Indeed the thing might be quite shallow veneer over SyntaxTree nodes, or perhaps a set of public interfaces implemented by the (eventually private) syntax tree nodes.

My understanding is that the analyzers would run in-process with the language service (which may be out of process from the editor). However it could be analyzers themselves run out-of-process from the language service at some point in the future.

Anyway I'm happy to see alternatives being discussed. Equally my intuition is not to rely on XML as the delivery mechanism simply because it's so "ugh" to many people. But it's good to have all the options on the table.

[T-Gro]

Protobuf is used extensively with graphs in Machine Learning; see Onnx and Tensorflow for very well engineered examples and both of these are extensible and have been evolved over time. It's hard for me to see how protobuf would be bad for this but XML/JSON would be good. This is obviously for the out-of-process option. There are some reasons to do things out of process but given the surface area of the problem it may best to do things in process.

I am still failing to find a robust solution for forward-compatible tree querying here. Anything in a protobuf model with 'oneof' choices for tree nodes that would be analogous to what XPath/JSONPath (in text-language world) or DescendantNodes (in Roslyn APIs world) is. Main thing being ability to work even when previously unknown items start to appear in the middle of the tree, between root and the nodes of interest.

Taking the tensorflow example, it's guarantees about compatibility between different versions are not that strong - not across major versions of TF, and only supporting a range of graph versions to cater for producer<>consumer version mismatch https://www.tensorflow.org/guide/versions .

[T-Gro]

I think it's good to think outside the box with regard to how we handover the syntax tree to the analyzers. I've previously suggested just handing over JSON to the analyzers, for example, and documenting that JSON schema separately.
...
Anyway I'm happy to see alternatives being discussed. Equally my intuition is not to rely on XML as the delivery mechanism simply because it's so "ugh" to many people. But it's good to have all the options on the table.

I think that would work equally well for the producing side (compiler), yes.
The Roslyn API for sure feels more XML-inspired, and the querying technologies might not be on par, but that's more for some prototype-analyzers to tell and evaluate.

[moloneymb]

Graph representations in protobuf are not typed that way in Onnx and Tensorflow; they have the concept of operations and edges separate and you can traverse edges without knowing what the operations are until you get to a operation that you are interested in. Does F# Analyzers need stronger guarantees of compatibility than Tensorflow? On the spectrum of no-guarantees to total guarantees there must be a productive middle ground.

[cmeeren]

I can't see configuration of analyzers being mentioned in the RFC. This is an important feature. For example, setting the cyclomatic complexity limit for an analyzer, or turning on and off arbitrary analyzers.

There is some ongoing discussion of analyzer config in ionide/FSharp.Analyzers.SDK#8.

[cmeeren]

Nevermind, I missed that the RFC does in fact link to that issue near the bottom.