stride icon indicating copy to clipboard operation
stride copied to clipboard

[Editor] Upgrade RolsynPad.Roslyn to v4.8.0

Open a-tsymbal opened this issue 1 year ago • 6 comments

PR Details

Upgraded RoslynPad.Roslyn* packages to version 4.8.0

Description

Upgraded required packages, added ability for MEF to access internal implementations of contracts using IgnoresAccessChecksToGenerator.

To get access to some required unpublished as Nuget packages contracts from Microsoft.CodeAnalysis.LanguageServer.Protocol namespace added Stride.RestoreHelper. Copied GetLibReferences to other places where Stride.props and Stride.Core.props were not referenced to fix build.

The approach was the same as was used by RoslynPad in relation to Microsoft.CodeAnalysis.* internal dependencies. On Stride side same actions were required in relation to RoslynPad.Roslyn internal dependencies.

! Commented out RoslynWorkspace.EnableDiagnostics() (same happens with DiagnosticProvider.Enable()) call because of error:

  InvalidCastException: Unable to cast object of type 'Stride.Assets.Presentation.AssetEditors.ScriptEditor.StrideRoslynWorkspace' to type 'RoslynPad.Roslyn.RoslynWorkspace'.
     at RoslynPad.Roslyn.DocumentTrackingServiceFactory.DocumentTrackingService..ctor(Workspace workspace)
     at RoslynPad.Roslyn.DocumentTrackingServiceFactory.CreateService(HostWorkspaceServices workspaceServices)
     at Microsoft.CodeAnalysis.Host.Mef.MefWorkspaceServices.<>c__DisplayClass5_0.<.ctor>b__1()
     at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
     at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
     at System.Lazy`1.CreateValue()
     at Microsoft.CodeAnalysis.Host.Mef.MefWorkspaceServices.GetService[TWorkspaceService]()
     at Microsoft.CodeAnalysis.Host.HostWorkspaceServices.GetRequiredService[TWorkspaceService]()
     at Microsoft.CodeAnalysis.SolutionCrawler.SolutionCrawlerRegistrationService.WorkCoordinator..ctor(IAsynchronousOperationListener listener, IEnumerable`1 analyzerProviders, Boolean initializeLazily, Registration registration)
     at Microsoft.CodeAnalysis.SolutionCrawler.SolutionCrawlerRegistrationService.EnsureRegistration(Workspace workspace, Boolean initializeLazily)
     at Microsoft.CodeAnalysis.SolutionCrawler.SolutionCrawlerRegistrationService.Register(Workspace workspace)
     at Microsoft.CodeAnalysis.SolutionCrawler.SolutionCrawlerRegistrationService.Microsoft.CodeAnalysis.SolutionCrawler.ISolutionCrawlerRegistrationService.Register(Workspace workspace)
     at Microsoft.CodeAnalysis.DiagnosticProvider.Enable(Workspace workspace)
     at Stride.Assets.Presentation.AssetEditors.ScriptEditor.StrideRoslynWorkspace..ctor(RoslynHost host) in E:\github\stride\sources\editor\Stride.Assets.Presentation\AssetEditors\ScriptEditor\StrideRoslynWorkspace.cs:line 37
     at Stride.Assets.Presentation.AssetEditors.ScriptEditor.RoslynHost..ctor() in E:\github\stride\sources\editor\Stride.Assets.Presentation\AssetEditors\ScriptEditor\RoslynHost.cs:line 50

Probably MEF is resolving RoslynPad's workspace type instead of Stride's.

! So maybe some things related to diagnostics might stop working.

Related Issue

2043 Fixed issue mentioned in this comment

Motivation and Context

Fixes some functionality related to code analysis in script editor.

Types of changes

  • [ ] Docs change / refactoring / dependency upgrade
  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [ ] My change requires a change to the documentation.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.
  • [x] I have built and run the editor to try this change out.

a-tsymbal avatar Jan 25 '24 08:01 a-tsymbal

Instead, we should either properly reimplement the custom workspace/host or reuse the one provided by RoslynPad library.

That won't do. The RoslynPad workspace is not a MSBuild workspace operating on a solution with many projects like Stride does. RoslynPad creates a simple Roslyn workspace per document (as they are independent from one another), and is way simpler than what we have in Stride. Also it has no need to detect external changes, as RoslynPad has all of this "project" structure in memory.

At least, that's the way it was implemented the last time I checked (last year).

I have a prototype I made some time ago where I integrated RoslynPad inside Stride (i.e. not as an assembly reference but as code, to ease customization and get only the needed parts). I had to reimplement some parts of the Stride workspace mixing it with RoslynPad's.

Ethereal77 avatar Jan 25 '24 11:01 Ethereal77

Instead, we should either properly reimplement the custom workspace/host or reuse the one provided by RoslynPad library.

When I was working on this PR I wondered why some code looks almost the same as in RoslynPad repository and why it wasn't used. But from this https://github.com/stride3d/stride/pull/2124#issuecomment-1909960326 explained it.

In this PR I just fixed issue by issue that was happening after upgrading RoslynPad.Roslyn* packages.

a-tsymbal avatar Jan 25 '24 21:01 a-tsymbal

i cant run this PR , ive ran git clean -xfd dotnet restore

but i end up with these errors it says that language server protocols 4.8 isnt found and that stride presentation dll wasnt found grafik https://gist.github.com/IXLLEGACYIXL/54ce8f77f4e0c8b52c29bc49e5f39a08

IXLLEGACYIXL avatar Jan 25 '24 22:01 IXLLEGACYIXL

Probably you don't have this package in the .nuget folder. It is located here: C:\Documents and Settings%user%.nuget\packages\microsoft.codeanalysis.languageserver.protocol\4.8.0-7.23558.1\lib\net7.0\Microsoft.CodeAnalysis.LanguageServer.Protocol.dll

I think it was added there by some of the .NET SDKs as it is not available on nuget website, here is my SDKs:

dotnet --info

.NET SDKs installed:
  7.0.115 [C:\Program Files\dotnet\sdk]
  8.0.101 [C:\Program Files\dotnet\sdk]

a-tsymbal avatar Jan 25 '24 22:01 a-tsymbal

Probably you don't have this package in the .nuget folder. It is located here: C:\Documents and Settings%user%.nuget\packages\microsoft.codeanalysis.languageserver.protocol\4.8.0-7.23558.1\lib\net7.0\Microsoft.CodeAnalysis.LanguageServer.Protocol.dll

I think it was added there by some of the .NET SDKs as it is not available on nuget website, here is my SDKs:

dotnet --info

.NET SDKs installed:
  7.0.115 [C:\Program Files\dotnet\sdk]
  8.0.101 [C:\Program Files\dotnet\sdk]

grafik i have these sdks too

IXLLEGACYIXL avatar Jan 25 '24 22:01 IXLLEGACYIXL

Try to remove api.nuget.org source from the Stride.RestoreHelper.csproj image

I see this package in another source: https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/index.json

All versions of this package: https://pkgs.dev.azure.com/dnceng/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/_packaging/d1622942-d16f-48e5-bc83-96f4539e7601/nuget/v3/flat2/Microsoft.CodeAnalysis.LanguageServer.Protocol/index.json`

Version used by RolsynPad: https://pkgs.dev.azure.com/dnceng/9ee6d478-d288-47f7-aacc-f6e6d082ae6d/_packaging/d1622942-d16f-48e5-bc83-96f4539e7601/nuget/v3/registrations2-semver2/Microsoft.CodeAnalysis.LanguageServer.Protocol/4.8.0-7.23558.1.json

In the response from the second link there is a direct link to nupkg file.

a-tsymbal avatar Jan 28 '24 16:01 a-tsymbal