WIP update to FCS 10

[smoothdeveloper]

changes to get solution to compile with FCS 10

notes:

• I've fixed framework >= net40 in paket, this pulls less things and keep project files tidier
• new FCS made IsResultObsolete internal, it was used once but oddly we don't seem to use the callback we are giving there anywhere else
• call to ParseAndCheckFileInProject changed, I haven't checked what was the null parameter we were passing, using explicit parameter names for this call now

Caution I haven't run those bits yet, just pushing the changes to get it compiling.

[smoothdeveloper]

Looking into failing test, there is SymbolClassifierTests with seemingly moved ranges.

My VS SDK stuff is apparently broken, trying to fix it to test debugging the branch.

[vasily-kirichenko]

If the tests check Printf specifiers, then yes, the ranges were wrong and now they fixed - start column is shifted to one position to left, so you should remove something like (printfRange.StartColumn + 1) in the classifier.

[smoothdeveloper]

Thanks, I'll fix those tests accordingly.

[smoothdeveloper]

I'm getting warning on build:

Warning MSB3243: No way to resolve conflict between "Microsoft.VisualStudio.Threading, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" and "Microsoft.VisualStudio.Threading, Version=12.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a". Choosing "Microsoft.VisualStudio.Threading, Version=14.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" arbitrarily. (1, 1)

Not sure if it is something to be concerned about.

Running the extension, I'm seeing exception (leading to crash) in TaskListCommentExtractor:

I vaguely remember something about lexer cache being broken during VS2017 development, any clue about that?

[vasily-kirichenko]

Just remove that List Comment extractor feature altogether. I've know nobody who used it.

[smoothdeveloper]

I fixed the tests regarding printf formatting, now searching for the correct place in the code as it highlights 1 column more:

I'm looking at https://github.com/fsprojects/VisualFSharpPowerTools/blob/master/src/FSharp.Editing.VisualStudio/Symbol/PrintfSpecifiersUsageTagger.fs but don't see obvious location

@latkin by chance do you know the place I'd need to adjust the tagging?

[smoothdeveloper]

I'm getting issues related to breaking changes in FCS:

System.MissingMethodException : Method not found: 'Void Microsoft.FSharp.Compiler.SourceCodeServices.FSharpProjectOptions..ctor(System.String, System.String[], System.String[], System.Tuple2<System.String,Microsoft.FSharp.Compiler.SourceCodeServices.FSharpProjectOptions>[], Boolean, Boolean, System.DateTime, Microsoft.FSharp.Core.FSharpOption1<Microsoft.FSharp.Compiler.SourceCodeServices.UnresolvedReferencesSet>)'.

I think we need to have FSharpLint & Fantomas updates to FCS 10.

[vasily-kirichenko]

Yes, FSharpLint and Fantomas must be update as well, it's a known issue.

[cloudRoutine]

@smoothdeveloper fsharp/fsharp-compiler-docs#696 this is going to hold you up

[vasily-kirichenko]

About wrong specifier ranges, remove + 1 here https://github.com/fsprojects/VisualFSharpPowerTools/blob/03c613acd3aa67cb5e3a339d3dcf1c7c783f30c2/src/FSharp.Editing/Coloring/PrintfSpecifiersUsageGetter.fs#L25

[smoothdeveloper]

@vasily-kirichenko thanks, somehow I tried that already but I'm still having the same behaviour.

If that's the last bug remaining after I complete the fixes, I think we could just keep that as a known defect for that release until we find the culprit.

I've made a local build of Fantomas with latest FCS and going to give that new version a try for a short while.

[vasily-kirichenko]

thanks, somehow I tried that already but I'm still having the same behaviour.

Have you tried to debug it? We should not publish a new release having such a defect unfixed,

[smoothdeveloper]

@vasily-kirichenko I'm trying now and it seems to solve the issue, I don't know what was up with CTRL-F5 but it seems it wouldn't deploy new code for some reason.

[latkin]

The printf feature has definitely not been broken for the past ~year or whenever it was debuted.

So why does it now have an off-by-1 bug? Maybe there is a regression in the range reporting?

[dsyme]

THis should be textSnapshotInfo = null?

[dsyme]

So why does it now have an off-by-1 bug? Maybe there is a regression in the range reporting?

There was some +1/-1 change to the code when it got integrated into the Visual F# Tools. I don't fully understand what it was. I've just assumed it's now working in the Visual F# Tools and in FCS we fixed up a bunch of tests to match

[smoothdeveloper]

@latkin thanks, actually this is fixed in both FCS 10 & VFPT in this PR now, the - 1 have been tamed.

In my experience I find it better to add a bit of verbose comments about those things, but my issue was mainly about changing the code and seeing the impact in VS instance, sometimes a ctrl+F5 doesn't do the job etc.

@dsyme thanks for the review, yes I'll push an update soon with this textSnapshotInfo being removed, the PR has been on hold until new release of Fantomas is published on nuget (@dungpa, would you review the fsprojects/fantomas#206 PR?).

I'm using succesfully the PR in VS2015 with my day job solution, it seems to help a bit staying at 2.3gb instead of reaching the 2.6/2.8gb borderline, this is with resharper and mid size solution of about 40 mixed languages projects, if we could trim memory used by assembly references this would help to stay even longer in safe/stable zone.

[smoothdeveloper]

I'm looking at failing tests related to gotodefinition, they seem to timeout, when I debug I reach https://github.com/fsprojects/VisualFSharpPowerTools/blob/64b407050a2e56fa109766f2b7464ee94b4717e0/tests/FSharp.Editing.VisualStudio.Tests/Mocks.fs#L291 where projectItem is null.

Any idea what is causing this in our tests? When I try similar go to definition, for example on System.IO.File, I'm getting properly redirected (to referencesource.microsoft.com).