Add ISourceText to language service and use Roslyn's SourceText for FSharp.Editor

[TIHan]

We were doing .ToString() on Roslyn's SourceText type in FSharp.Editor. This is very bad as we are making full copies of the source text and for larges files making large allocations on the LOH. To solve this, we added an ISourceText interface that looks similar to Roslyn's SourceText type. It is then implemented in FSharp.Editor with Roslyn's SourceText as the implementation detail.

This also eradicates adding a new line at the end of every source file in FCS.

This should resolve these issues:
#5935
#5936
#5937

[dsyme]

Looking great!

[dsyme]

(@TIHan just found a couple of review comments I hadn't submitted)

[TIHan]

getXmlDocablesImpl requires that we have all lines as text strings from source text. After looking at it, I think it is possible that we do not have to.

[TIHan]

We are getting closer. I am a little worried that we have two different implementations of ISourceText, a Roslyn one and one that handles just a string. Perhaps I need to write some tests against both implementations to verify correct behavior.

[cartermp]

Awesome!

[TIHan]

I'm ready to merge this. Any objections?

[cartermp]

🚢 🇮🇹

[auduchinok]

This PR doesn't seem to have been merged to master. I wonder if there any others too.

[cartermp]

Lots. It will be in master once dev16 ships and we merge into master.

[auduchinok]

I think it's bad for OSS FCS development in this repo.
We now have diverged code for FCS and VS integration solutions and FCS clients cannot update their code until dev16.0 is merged back to master.

[baronfel]

@auduchinok for FCS we could integrate to the FCS from from a branch other than master, I just chose master because it seemed convenient. If we want features faster we could just as easily pick a non-master branch. 🤷‍♂️

[dsyme]

I think it's bad for OSS FCS development in this repo. We now have diverged code for FCS and VS integration solutions and FCS clients cannot update their code until dev16.0 is merged back to master.

I think it might be ok. What's the divergence between FCS master and visualfsharp master?

[dsyme]

The fact there are pending unshipped changes in dev16.0 is a slight divergence but it is on a path back to convergence when dev16.0 ships. That's normal?

[auduchinok]

What's the divergence between FCS master and visualfsharp master?

My point is about not having some PRs merged to master, assuming one builds FCS from source in this repo. Due to all the development is currently happening here I doubt there's a reason to build it from the other repo unless all the FCS development and stuff is moved there as well.

[auduchinok]

@auduchinok for FCS we could integrate to the FCS from from a branch other than master, I just chose master because it seemed convenient. If we want features faster we could just as easily pick a non-master branch. 🤷‍♂️

And other branches may not contain some of PRs merged that are already in master in the same way. I'd expect master to have all changes that FCS clients will have to deal with.
Taking a branch that is tied to some future VS release doesn't feel right for FCS development that is not a part of VS integration plugin.

[auduchinok]

The fact there are pending unshipped changes in dev16.0 is a slight divergence but it is on a path back to convergence when dev16.0 ships.

From a glance there're PRs that are really important like #6191 and this one or are general improvements that'd be great to see in action like #6048 and #6058.

That's normal?

It'd probably be better to try to have it diverged for less time, e.g. until the first public VS preview is released and not the actual release. Other tools have to be updated for changes like this PR as well and it's better to have the changes in previews.
And changes like #6191 don't depend on VS-specific changes at all so they would seem better going to master first.

[dsyme]

@auduchinok Yes, things should really go into master unless they specifically need to go into dev16.0

However we have to cut some slack here: @TIHan and others at Microsoft are doing us a great favor by always sitting on the latest VS preview and checking that things are good and performant there.

If you're doing regular work there it's kind of hard to switch back and do serious dev work on VS2017 (which is master). So it's better to just live with a bit of divergence and reconcile later.

[dsyme]

The most important thing is that the automatic flows are set up to integrate and flow correctly, which they are.

[auduchinok]

I'm building Fantomas against FCS built from dev16.1 and getting the following errors. They're expected for me and, despite they are easy to fix by either using SourceText.ofString or changing Fantomas API, it can be quite unexpected for other FCS clients to have to change their public APIs this way now.
I wonder if preserving existing APIs (with an obsolete attribute and maybe a future removal notice?) and adding new overloads could be a better approach?