Skip to content

Public API for "Compiler needs to declaratively fail when loading newer analyzers" #62583

@RikkiGibson

Description

@RikkiGibson

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-AnalyzersConcept-APIThis issue involves adding, removing, clarification, or modification of an API.Feature Requestapi-approvedAPI was approved in API review, it can be implemented

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions