[WIP - VFT vNext? ] Check against Document/Project snapshots in VS2017

[vasily-kirichenko]

wip

[dsyme]

@vasily-kirichenko If we're going to go down this route we may want to rearchitect the whole way we process documents to be much more Roslyn like.

[dsyme]

@vasily-kirichenko Could you merge with master when you get a chance?

[vladima]

I don't think this is right thing to do in Roslyn world. In Roslyn request is made against the snapshot of the world (represented via Document or Project) and this snapshot in theory can diverge from the current content of the buffer. This might lead to unexpected consequences, i.e. when after parsing content of the buffer some request returns span that is located past the current length of the snapshot.

We've already walked that path in in-proc version of TypeScript language service which btw has some similarities with its F# counterpart - i.e. all semantic requests are processed by dedicated thread that marshals requests into hosted Chakra instance. With this in mind we never read anything from disk or from editor buffers ourselves - everything was handled by Roslyn and before making any requests we've synchronized our internal state with current snapshot of the world.

I can imagine something having something similar in F# language service: implementation of IFileSystem that can be powered by snapshot of the project. Versions of text snapshots can be used instead of last write times to determine if file needs to be reparsed. Also it is possible to query change ranges between different versions of source text to do incremental parsing (if F# language service supports this).

[vasily-kirichenko]

@dsyme Could you merge with master when you get a chance?

Done.

I agree with @vladima about using Roslyn workspaces for it instead. I think @cloudRoutine is an expert in this area :)

[cloudRoutine]

@vladima so something like this would get us the content of the dirty buffers?

    let workspace = 
        serviceProvider.GetService(typeof<VisualStudioWorkspaceImpl>) 
        :?> VisualStudioWorkspaceImpl

    let getOpenDocuments () =
        workspace.CurrentSolution.ProjectIds
        |> Seq.collect (fun projId ->
            workspace.GetOpenDocumentIds projId |> Seq.map (fun docId ->
                workspace.CurrentSolution.GetDocument docId))

    let getBufferContent() =
        getOpenDocuments() |> Seq.map (fun (doc:CodeAnalysis.Document) -> 
            async {
                let! text = doc.GetTextAsync() |> Async.AwaitTask
                return text
            }|> Async.RunSynchronously)

[vasily-kirichenko]

If so, it's easy to remove OpenDocumentTracker and make FileSystem use this code ^

[dsyme]

Renamed to show we're talking about Roslyn document snapshots

[vladima]

@cloudRoutine no, this is effectively the same a trying to read content of the buffer directly - there is no guarantee that workspace.CurrentSolution will be the same as document.Project.Solution which again means that request can be computed against the document that differs from the initially given snapshot.

What is good about Roslyn is that we can really forget about the origin of the source text - all we know is that we have a Project that can provide a set of documents and each document can be queried for its text or version of the text (and under the hood text will be read from the file or taken from the text buffer). In this mode OpenDocumentTracker becomes unnecessary as this work is already done by Roslyn.

Having this in mind I'd say that we can have an implementation of IFileSystem that be bound to a Project and will delegate all reading operations to the underlying project (or some precomputed results derived from the project since all members in IFileSystem are synchronous and Roslyn API is asynchronous). Some works also should be done in IncrementalBuild since currently it uses LastWriteTime to timestamp the input instead of opaque version.

// cc @CyrusNajmabadi in case if he wants to chime in.

[dsyme]

From the above I'd say we should also adjust FCS to pass an IFileSystem parameter explicitly, rather than using a global mutable.

[cartermp]

@vasily-kirichenko Any recent progress on this?

[vasily-kirichenko]

@cartermp no.

[vasily-kirichenko]

I don't think it'll ever be implemented. Closing.

[dsyme]

cc @cartermp @saul @vladima

@vasily-kirichenko Great that you took a first shot at this. It feels like we have to do this sometime. But maybe by a different approach?