IDE change to handle /warnaserror with Warning analyzer bulk configur…

[mavasani]

…ation in editorconfig

Underlying issue: #55541
Corresponding batch compiler fix: #58460
This change fixes the code that computes effective severity for Analyzers node. We also need to update the code that computes effective severity for EditorConfig UX, I will file a separate issue for the same.

NOTE: We have #52720 to track avoiding code duplication for computing effective severity between EditorConfig UX and other parts of the IDE

[mavasani]

We also need to update the code that computes effective severity for EditorConfig UX, I will file a separate issue for the same.

FYI @jmarolf - I'll file an issue for this and assign it to you. I couldn't figure out a trivial fix to thread in CompilationOptions into the below methods used for EditorConfig UX:

roslyn/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/DiagnosticDescriptorExtensions.cs

Lines 53 to 135 in bf84ccf

	public static bool IsDefinedInEditorConfig(this DiagnosticDescriptor descriptor, AnalyzerConfigOptions analyzerConfigOptions)
	{
	// Check if the option is defined explicitly in the editorconfig
	var diagnosticKey = $"{DotnetDiagnosticPrefix}.{descriptor.Id}.{SeveritySuffix}";
	if (analyzerConfigOptions.TryGetValue(diagnosticKey, out var value) &&
	EditorConfigSeverityStrings.TryParse(value, out var severity))
	{
	return true;
	}
	// Check if the option is defined as part of a bulk configuration
	// Analyzer bulk configuration does not apply to:
	// 1. Disabled by default diagnostics
	// 2. Compiler diagnostics
	// 3. Non-configurable diagnostics
	if (!descriptor.IsEnabledByDefault ||
	descriptor.ImmutableCustomTags().Any(tag => tag is WellKnownDiagnosticTags.Compiler or WellKnownDiagnosticTags.NotConfigurable))
	{
	return false;
	}
	// If user has explicitly configured default severity for the diagnostic category, that should be respected.
	// For example, 'dotnet_analyzer_diagnostic.category-security.severity = error'
	var categoryBasedKey = $"{DotnetAnalyzerDiagnosticPrefix}.{CategoryPrefix}-{descriptor.Category}.{SeveritySuffix}";
	if (analyzerConfigOptions.TryGetValue(categoryBasedKey, out value) &&
	EditorConfigSeverityStrings.TryParse(value, out severity))
	{
	return true;
	}
	// Otherwise, if user has explicitly configured default severity for all analyzer diagnostics, that should be respected.
	// For example, 'dotnet_analyzer_diagnostic.severity = error'
	if (analyzerConfigOptions.TryGetValue(DotnetAnalyzerDiagnosticSeverityKey, out value) &&
	EditorConfigSeverityStrings.TryParse(value, out severity))
	{
	return true;
	}
	// option not defined in editorconfig, assumed to be the default
	return false;
	}
	public static ReportDiagnostic GetEffectiveSeverity(this DiagnosticDescriptor descriptor, AnalyzerConfigOptions analyzerConfigOptions)
	{
	// Check if the option is defined explicitly in the editorconfig
	var diagnosticKey = $"{DotnetDiagnosticPrefix}.{descriptor.Id}.{SeveritySuffix}";
	if (analyzerConfigOptions.TryGetValue(diagnosticKey, out var value) &&
	EditorConfigSeverityStrings.TryParse(value, out var severity))
	{
	return severity;
	}
	// Check if the option is defined as part of a bulk configuration
	// Analyzer bulk configuration does not apply to:
	// 1. Disabled by default diagnostics
	// 2. Compiler diagnostics
	// 3. Non-configurable diagnostics
	if (!descriptor.IsEnabledByDefault ||
	descriptor.ImmutableCustomTags().Any(tag => tag is WellKnownDiagnosticTags.Compiler or WellKnownDiagnosticTags.NotConfigurable))
	{
	return ReportDiagnostic.Default;
	}
	// If user has explicitly configured default severity for the diagnostic category, that should be respected.
	// For example, 'dotnet_analyzer_diagnostic.category-security.severity = error'
	var categoryBasedKey = $"{DotnetAnalyzerDiagnosticPrefix}.{CategoryPrefix}-{descriptor.Category}.{SeveritySuffix}";
	if (analyzerConfigOptions.TryGetValue(categoryBasedKey, out value) &&
	EditorConfigSeverityStrings.TryParse(value, out severity))
	{
	return severity;
	}
	// Otherwise, if user has explicitly configured default severity for all analyzer diagnostics, that should be respected.
	// For example, 'dotnet_analyzer_diagnostic.severity = error'
	if (analyzerConfigOptions.TryGetValue(DotnetAnalyzerDiagnosticSeverityKey, out value) &&
	EditorConfigSeverityStrings.TryParse(value, out severity))
	{
	return severity;
	}
	// option not defined in editorconfig, assumed to be the default
	return ReportDiagnostic.Default;
	}

[sharwell]

For the scenario described by #55541, the Analyzers node and the EditorConfig UX should both show Warning as the effective severity. During build or live analysis, the diagnostics reported for these analyzers should appear in the build output and error list as Error severity. The different values shown in these locations is intentional and desired.

[mavasani]

@sharwell Can we take this to design meeting and confirm with the team?

[jmarolf]

Thanks @mavasani. I have a plan for unifying this so I’ll take care of the plumbing there. My general intuition is that the editorconfig ui should be a representation of the file itself. So of the file shows it as a warning I would expect it to display as one. We should do something here though. We could at least hint to the developer that warn-as-error is active in some build configurations

[mavasani]

I agree with both your stance. I’ll close out this PR for now.