Fix IDE remote analyzer loader on .Net Core#57407
Fix IDE remote analyzer loader on .Net Core#57407genlu merged 1 commit intodotnet:release/dev17.1from
Conversation
src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.Core.cs
Outdated
Show resolved
Hide resolved
|
@jaredpar I think we had discussed eliminating the |
src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoaderService.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Remote/ServiceHub/Host/RemoteAnalyzerAssemblyLoaderService.cs
Outdated
Show resolved
Hide resolved
jasonmalinowski
left a comment
There was a problem hiding this comment.
Fix seems reasonable, as long as we still feel the listing of assemblies is still the right mechanism. But this isn't doing anything worse than it already is.
src/Compilers/Core/Portable/DiagnosticAnalyzer/DefaultAnalyzerAssemblyLoader.Core.cs
Outdated
Show resolved
Hide resolved
| // The following are special assemblies since they contain IDE analyzers and/or their dependencies, | ||
| // but in the meantime, they also contain the host of compiler in remote process. Therefore on coreclr, | ||
| // we must ensure they are only loaded once and in the same ALC compiler asemblies are loaded into. | ||
| // Otherwise these analyzers will fail to interoperate with the host due to mismatch in assembly identity. |
There was a problem hiding this comment.
Do we have a bug tracking the removal of the WorkspaceAnalyzerOptions that's the cause of all of this?
There was a problem hiding this comment.
Not sure about this. If this is the intention, could you please file a bug for it? Thanks!
There was a problem hiding this comment.
#8605 tracks removal of WorkspaceAnalyzerOptions and potentially seal AnalyzerOptions
Unsure if this is right after all.
|
I see some shiproom related labels. Please be sure this is targeted to the appropriate branch. |
|
@RikkiGibson I have rebased against 17.1, thanks |
| @@ -0,0 +1,16 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <!-- Licensed to the .NET Foundation under one or more agreements. The .NET Foundation licenses this file to you under the MIT license. See the LICENSE file in the project root for more information. --> | |||
| <Project Sdk="Microsoft.NET.Sdk"> | |||
There was a problem hiding this comment.
I have to add a new test project because we currently don't have any IDE tests runs on .Net Core above workspaces layer
| @@ -1,4 +1,5 @@ | |||
| Microsoft Visual Studio Solution File, Format Version 12.00 | |||
| | |||
We ran into an issue that "unnecessary usings" are not fading when IDE analyzers are hosted on .Net Core host. Further investigation shows that the issue might lie in here
The way analyzer loader work on .Net core, MS.CA.Features is loaded into a separate DirectoryLoadContext when used as an IDE analyzer (# 2 in the screenshot), but it's also loaded into Roslyn's ALC when our service is created in ServiceHub host (# 0) therefore
analyzerOptions is not WorkspaceAnalyzerOptions workspaceAnalyzerOptions)is evaluated to true, even though analyzerOptions is indeed of type WorkspaceAnalyzerOptions, but because the type is from another ALC.Fix #57395