Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make F# Analyzer API independent of FCS? #28

Open
dsyme opened this issue Oct 26, 2020 · 7 comments
Open

Make F# Analyzer API independent of FCS? #28

dsyme opened this issue Oct 26, 2020 · 7 comments

Comments

@dsyme
Copy link

dsyme commented Oct 26, 2020

I've been thinking about the problem of binary compatibility for F# analyzers, in the context of trying to get F# analyzers available in all F# tooling.

This is a blocking problem for incorporation of analyzers into the Visual F# Tools. This applies even if those tools switch to FsAutoComplete - all analyzers must still be loadable into all future iterations of F# tooling.

The main issue is binary compatibility for the information being drawn from the FCS API (FSharpSymbol etc.). I am wondering if we might consider making this F# Analyzer API completely independent of FCS. When hosted, FSharpAutoComplete would provide necessary shims.

This would mean the F# Analyzer API would include abstract types like IFSharpSymbol, ISyntaxTree and so on. It would be a complete self-contained, zero-dependency facade over all the information analyzers typically require.

FCS could then take a dependency on this component (which I called FSharp.Compiler.Analyzers in the link above).

@dsyme dsyme changed the title Make F# Analyzer API self contained Make F# Analyzer API independent of FCS? Oct 26, 2020
@baronfel
Copy link
Collaborator

Something like this would be great for another reason: incorporating some kind of stable view of the syntax nodes into an interface would make it easier for tools like sharp lab to bind to multiple versions of the compiler, which would make before and after analysis and comparisons much easier. It would potentially also support work in progress comparisons for interesting feature branches of compiler work.

@dsyme
Copy link
Author

dsyme commented Oct 26, 2020

It might, yes - it partly depends how often we revise the API, whether an analysis application can host mutiple versions (do we use naming FSharp.Compiler.Analysis.v1.0, FSharp.Compiler.Analysis.v2.0 etc.), and who provides the implementations of the shims.

It's really a hard problem - the need for binary compat means the API would have to be really, really stable. But the nature of the FCS API means it gets enriched on every language update, even without taking into account bug fixes. Even yesterday I added more information for witness passing, for example.

  • If information is missing from the API (e.g. new language features are added or the SyntaxTree changes) then the API would also need to be revamped.

  • If we got it right I'd be ok with FCS taking a dependency on this and promising to always provide shims going forward, including progressing forward as the API is revised and providing backwards compat shims.

  • The API must serve as a binary-compatible view into FCS - there's no point having it if it's not going to be 100% binary compat forever - we may as well just depend on FCS in that case.

  • From an engineering point of view something like this should likely sit in dotnet/fsharp, at least if FCS takes a dependency on it and implements the shims.

@dsyme
Copy link
Author

dsyme commented Oct 26, 2020

I'll link this discussion in the RFC (am happy to keep discussing here)

@Zaid-Ajaj
Copy link
Contributor

all analyzers must still be loadable into all future iterations of F# tooling.

Finally 😍

@dsyme
Copy link
Author

dsyme commented Oct 26, 2020

It's curious that one way to make the Analyzer API both binary compatible and simple and stable and extensible would be to transact all context information via an untyped format like JSON. Perhaps it would even be fast enough. (I'm not saying we should do that, I'm just thinking aloud)

type AnalyzerAttribute

type JsonResponse = string

// implemented by FCS
type ISourceText =
    abstract ... (same as ISourceText in F# compiler)

// implemented by FCS
type IAnalyzerContext =
    abstract ProjectFile: string
    abstract ProjectOptions: string[]
    abstract FileName: string
    abstract Source: ISourceText
    abstract GetSyntaxTree: unit -> Async<JsonResponse>
    abstract GetTypedTree: unit -> Async<JsonResponse>

type postion = string * int * int
type range = string * int * int * int * int

// implemented by analyzer
type IAnalyzer =
    abstract GetDiagnostics: IAnalyzerContext -> Async<JsonResponse>
    abstract GetDiagnosticFixes: IAnalyzerContext * position -> Async<JsonResponse>
    abstract GetQuickInfo: IAnalyzerContext * position -> Async<JsonResponse>

@Zaid-Ajaj
Copy link
Contributor

I know people don't like that idea but JSON makes a lot of sense, especially when there is a version and a schema about what to expect and what to send. Each time a JsonResponse is sent back from the concrete analyzer instance, it would be something like:

{
  "$schema": "https://fsharp.org/analyzer-schemas/v1.json", 
  "data": { 
      "SyntaxTree": { }
   } 
}

Analyzers receiving this JSON would have to check which version of the schema they are receiving and parse the data accordingly. Similarly, when the analyzer sends code fixes and error message, it would send JSON that follows an expected schema by the analyzer SDK.

I would take JSON-compat everyday instead of having to make sure assemblies can load each other which is a giant rabbit hole of issues.

@dsyme
Copy link
Author

dsyme commented Oct 27, 2020

My interest in this is partly for the shape checking tooling for DiffSharp, which roughly speaking needs nothing more than the compilation arguments and ISourceText - it's happy to do its own recompilation (though would be marginally more efficient if it sits directly on the FSharpExpr/FSharpSymbols)

The annoying thing here is that this kind of tooling addin only requires an "ultra simple" version of analyzers that do their own recompilation (presumably using a private copy of FCS or a tool out of process).

And then there is the "ultra hard" version that exposes the whole of SyntaxTree/Symbols/Exprs in a binary compatible way.

And not a lot in between

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants