Background and Motivation
Follow-up to #61252
Similarly to ReferencesFramework #47100, we want to produce a specific error for a specific kind of analyzer misconfiguration. In this case, we want to give a warning when the compiler loads an analyzer which references a newer compiler than the one currently running.
One common place where this might occur is if you are running VS 17.0 and editing a project which uses dotnet sdk version 6.0.300. This is a supported configuration, but when we build in VS, we use the compiler included with VS, and we end up loading analyzers which need the newer compiler from the newer SDK, and the build gives a fairly opaque diagnostic:
Warning CS8032: An instance of analyzer Microsoft.CodeAnalysis.MakeFieldReadonly.MakeFieldReadonlyDiagnosticAnalyzer cannot be created from C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CodeStyle.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified..
This change lets us give a more specific error like the following:
warning CS9057: The analyzer assembly 'C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CodeStyle.dll' references version '4.2.0.0' of the compiler, which is newer than the currently running version '4.0.0.0'.
Presumably this condition is one that we wish to avoid in the future across our supported configurations. But in the event that something slips through or the user manually references an analyzer that needs a newer compiler, it's still valuable to use a higher quality diagnostic message here.
Proposed API
namespace Microsoft.CodeAnalysis.Diagnostics
{
public sealed class AnalyzerLoadFailureEventArgs : EventArgs
{
public enum FailureErrorCode
{
None = 0,
UnableToLoadAnalyzer = 1,
UnableToCreateAnalyzer = 2,
NoAnalyzers = 3,
ReferencesFramework = 4,
+ ReferencesNewerCompiler = 5
}
public string? TypeName { get; }
public string Message { get; }
public FailureErrorCode ErrorCode { get; }
public Exception? Exception { get; }
+ public Version? ReferencedCompilerVersion { get; init; }
public AnalyzerLoadFailureEventArgs(FailureErrorCode errorCode, string message, Exception? exceptionOpt = null, string? typeNameOpt = null)
Usage Examples
The IDE needs to check for this specific error condition in order to add the right diagnostic to the error list and update the analyzer node in Solution Explorer appropriately.
|
case AnalyzerLoadFailureEventArgs.FailureErrorCode.ReferencesFramework: |
|
id = GetLanguageSpecificId(language, WRN_AnalyzerReferencesNetFrameworkId, WRN_AnalyzerReferencesNetFrameworkIdCS, WRN_AnalyzerReferencesNetFrameworkIdVB); |
|
message = string.Format(FeaturesResources.The_assembly_0_containing_type_1_references_NET_Framework, fullPath, e.TypeName); |
|
break; |
|
|
|
case AnalyzerLoadFailureEventArgs.FailureErrorCode.ReferencesNewerCompiler: |
|
id = GetLanguageSpecificId(language, WRN_AnalyzerReferencesNewerCompilerId, WRN_AnalyzerReferencesNewerCompilerIdCS, WRN_AnalyzerReferencesNewerCompilerIdVB); |
|
message = string.Format(FeaturesResources.The_assembly_0_references_compiler_version_1_newer_than_2, fullPath, e.ReferencedCompilerVersion, typeof(AnalyzerLoadFailureEventArgs).Assembly.GetName().Version); |
|
break; |
Alternative Designs
We could reduce the accessibility of ReferencedCompilerVersion.init to internal. There isn't really a reason for users of the compiler to produce these load failure events--just to consume them. However, it is already possible to create these EventArgs and pass everything they need to them from the public context, so it would be inconsistent to disallow just this property.
We could also delete the init accessor here and instead introduce another constructor with an additional parameter. We might want to eliminate the default arguments from the existing constructor at that time, so that binary compat is preserved and we also match our guideline to avoid multiple overloads with optional parameters.
Risks
The rest of the type uses get-only properties. I'm not sure if we've exposed init-only properties in Roslyn yet, so that could also be risky, since init properties are not technically supported in some of the settings we ship in.
Tagging @mavasani and @333fred as area owners.
Background and Motivation
Follow-up to #61252
Similarly to
ReferencesFramework#47100, we want to produce a specific error for a specific kind of analyzer misconfiguration. In this case, we want to give a warning when the compiler loads an analyzer which references a newer compiler than the one currently running.One common place where this might occur is if you are running VS 17.0 and editing a project which uses dotnet sdk version 6.0.300. This is a supported configuration, but when we build in VS, we use the compiler included with VS, and we end up loading analyzers which need the newer compiler from the newer SDK, and the build gives a fairly opaque diagnostic:
Warning CS8032: An instance of analyzer Microsoft.CodeAnalysis.MakeFieldReadonly.MakeFieldReadonlyDiagnosticAnalyzer cannot be created from C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CodeStyle.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified..This change lets us give a more specific error like the following:
warning CS9057: The analyzer assembly 'C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Sdks\Microsoft.NET.Sdk\codestyle\cs\Microsoft.CodeAnalysis.CodeStyle.dll' references version '4.2.0.0' of the compiler, which is newer than the currently running version '4.0.0.0'.Presumably this condition is one that we wish to avoid in the future across our supported configurations. But in the event that something slips through or the user manually references an analyzer that needs a newer compiler, it's still valuable to use a higher quality diagnostic message here.
Proposed API
namespace Microsoft.CodeAnalysis.Diagnostics { public sealed class AnalyzerLoadFailureEventArgs : EventArgs { public enum FailureErrorCode { None = 0, UnableToLoadAnalyzer = 1, UnableToCreateAnalyzer = 2, NoAnalyzers = 3, ReferencesFramework = 4, + ReferencesNewerCompiler = 5 } public string? TypeName { get; } public string Message { get; } public FailureErrorCode ErrorCode { get; } public Exception? Exception { get; } + public Version? ReferencedCompilerVersion { get; init; } public AnalyzerLoadFailureEventArgs(FailureErrorCode errorCode, string message, Exception? exceptionOpt = null, string? typeNameOpt = null)Usage Examples
The IDE needs to check for this specific error condition in order to add the right diagnostic to the error list and update the analyzer node in Solution Explorer appropriately.
roslyn/src/Features/LanguageServer/Protocol/Features/Diagnostics/DocumentAnalysisExecutor_Helpers.cs
Lines 101 to 109 in be8eed3
Alternative Designs
We could reduce the accessibility of
ReferencedCompilerVersion.inittointernal. There isn't really a reason for users of the compiler to produce these load failure events--just to consume them. However, it is already possible to create these EventArgs and pass everything they need to them from the public context, so it would be inconsistent to disallow just this property.We could also delete the
initaccessor here and instead introduce another constructor with an additional parameter. We might want to eliminate the default arguments from the existing constructor at that time, so that binary compat is preserved and we also match our guideline to avoid multiple overloads with optional parameters.Risks
The rest of the type uses get-only properties. I'm not sure if we've exposed init-only properties in Roslyn yet, so that could also be risky, since
initproperties are not technically supported in some of the settings we ship in.Tagging @mavasani and @333fred as area owners.