Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the code that computes effective severity for EditorConfig UX similar to #58461 #58462

Closed
mavasani opened this issue Dec 22, 2021 · 2 comments
Assignees
Labels
Area-IDE Bug Need Design Review The end user experience design needs to be reviewed and approved.

Comments

@mavasani
Copy link
Contributor

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:

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;
}

Originally posted by @mavasani in #58461 (comment)

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 22, 2021
@mavasani mavasani added Area-IDE Bug and removed Area-Analyzers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 22, 2021
@mavasani mavasani added this to the 17.1 milestone Dec 22, 2021
@sharwell
Copy link
Member

sharwell commented Dec 22, 2021

⚠️ This feature is likely already working as designed.

#58461 (review)

@mavasani mavasani added the Need Design Review The end user experience design needs to be reviewed and approved. label Dec 22, 2021
@jmarolf jmarolf modified the milestones: 17.1, 17.2 Jan 10, 2022
@mavasani
Copy link
Contributor Author

I believe this issue can be closed as per Sam's last comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Need Design Review The end user experience design needs to be reviewed and approved.
Projects
Archived in project
Development

No branches or pull requests

3 participants