Skip to content

Commit

Permalink
Merge pull request #6427 from mavasani/CodeAnalysisTreatWarningsAsErr…
Browse files Browse the repository at this point in the history 
…ors_GlobalConfigs

Re-implement CodeAnalysisTreatWarningsAsErrors with globalconfig files
  • Loading branch information
@mavasani
mavasani authored Jan 13, 2023
2 parents b858999 + 1d7244a commit afee469
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 63 deletions.
8 changes: 4 additions & 4 deletions src/Tools/GenerateAnalyzerNuspec/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,15 +235,15 @@

if (globalAnalyzerConfigsDir.Length > 0 && Directory.Exists(globalAnalyzerConfigsDir))
{
foreach (string editorconfig in Directory.EnumerateFiles(globalAnalyzerConfigsDir))
foreach (string globalconfig in Directory.EnumerateFiles(globalAnalyzerConfigsDir))
{
if (Path.GetExtension(editorconfig) == ".editorconfig")
if (Path.GetExtension(globalconfig) == ".globalconfig")
{
result.AppendLine(FileElement(Path.Combine(globalAnalyzerConfigsDir, editorconfig), $"build\\config"));
result.AppendLine(FileElement(Path.Combine(globalAnalyzerConfigsDir, globalconfig), $"buildtransitive\\config"));
}
else
{
throw new InvalidDataException($"Encountered a file with unexpected extension: {editorconfig}");
throw new InvalidDataException($"Encountered a file with unexpected extension: {globalconfig}");
}
}
}
Expand Down
105 changes: 51 additions & 54 deletions src/Tools/GenerateDocumentationAndConfigFiles/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ void createPropsFiles()

var fileContents =
$@"<Project>
{disableNetAnalyzersImport}{getCodeAnalysisTreatWarningsAsErrors()}{getCompilerVisibleProperties()}
{disableNetAnalyzersImport}{getCompilerVisibleProperties()}
</Project>";
var directory = Directory.CreateDirectory(propsFileDir);
var fileWithPath = Path.Combine(directory.FullName, propsFileName);
Expand Down Expand Up @@ -310,20 +310,6 @@ We rely on the additional props file '{propsFileToDisableNetAnalyzersInNuGetPack
}
}

string getCodeAnalysisTreatWarningsAsErrors()
{
var allRuleIds = string.Join(';', allRulesById.Keys);
return $@"
<!--
This property group handles 'CodeAnalysisTreatWarningsAsErrors' for the CA rule ids implemented in this package.
-->
<PropertyGroup>
<CodeAnalysisRuleIds>{allRuleIds}</CodeAnalysisRuleIds>
<WarningsNotAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'false'"">$(WarningsNotAsErrors);$(CodeAnalysisRuleIds)</WarningsNotAsErrors>
<WarningsAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'true' and '$(TreatWarningsAsErrors)' != 'true'"">$(WarningsAsErrors);$(CodeAnalysisRuleIds)</WarningsAsErrors>
</PropertyGroup>";
}

string getCompilerVisibleProperties()
{
return analyzerPackageName switch
Expand Down Expand Up @@ -794,12 +780,15 @@ void CreateGlobalConfigsForVersion(
{
var analysisLevelVersionString = GetNormalizedVersionStringForEditorconfigFileNameSuffix(version);

foreach (var analysisMode in Enum.GetValues(typeof(AnalysisMode)))
foreach (var warnAsError in new[] { true, false })
{
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, releaseTrackingData, category: null);
foreach (var category in categories!)
foreach (var analysisMode in Enum.GetValues(typeof(AnalysisMode)))
{
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, releaseTrackingData, category);
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, warnAsError, releaseTrackingData, category: null);
foreach (var category in categories!)
{
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, warnAsError, releaseTrackingData, category);
}
}
}
}
Expand All @@ -809,26 +798,38 @@ void CreateGlobalConfig(
bool isShippedVersion,
string analysisLevelVersionString,
AnalysisMode analysisMode,
bool warnAsError,
ImmutableArray<ReleaseTrackingData> releaseTrackingData,
string? category)
{
var analysisLevelPropName = "AnalysisLevel";
var title = $"Rules from '{version}' release with '{analysisMode}' analysis mode";
var description = $"Rules with enabled-by-default state from '{version}' release with '{analysisMode}' analysis mode. Rules that are first released in a version later than '{version}' are disabled.";

if (category != null)
{
analysisLevelPropName += category;
title = $"'{category}' {title}";
description = $"'{category}' {description}";
}

CreateGlobalconfig(
analyzerGlobalconfigsDir,
#pragma warning disable CA1308 // Normalize strings to uppercase
$"{analysisLevelPropName}_{analysisLevelVersionString}_{analysisMode!.ToString()!.ToLowerInvariant()}.editorconfig",
var globalconfigFileName = $"{analysisLevelPropName}_{analysisLevelVersionString}_{analysisMode!.ToString()!.ToLowerInvariant()}";
#pragma warning restore CA1308 // Normalize strings to uppercase
title,

if (warnAsError)
{
globalconfigFileName += "_warnaserror";
title += " escalated to 'error' severity";
description += " Enabled rules with 'warning' severity are escalated to 'error' severity to respect 'CodeAnalysisTreatWarningsAsErrors' MSBuild property.";
}

CreateGlobalconfig(
analyzerGlobalconfigsDir,
$"{globalconfigFileName}.globalconfig",
title,
description,
warnAsError,
analysisMode,
category,
allRulesById,
Expand Down Expand Up @@ -1158,34 +1159,37 @@ private static void Validate(string fileWithPath, string fileContents, List<stri

private static void CreateGlobalconfig(
string folder,
string editorconfigFileName,
string editorconfigTitle,
string editorconfigDescription,
string fileName,
string title,
string description,
bool warnAsError,
AnalysisMode analysisMode,
string? category,
SortedList<string, DiagnosticDescriptor> sortedRulesById,
(ImmutableArray<ReleaseTrackingData> releaseTrackingData, Version version, bool isShippedVersion) releaseTrackingDataAndVersion)
{
Debug.Assert(editorconfigFileName.EndsWith(".editorconfig", StringComparison.Ordinal));
Debug.Assert(fileName.EndsWith(".globalconfig", StringComparison.Ordinal));

var text = GetGlobalconfigText(
editorconfigTitle,
editorconfigDescription,
title,
description,
warnAsError,
analysisMode,
category,
sortedRulesById,
releaseTrackingDataAndVersion);
var directory = Directory.CreateDirectory(folder);
#pragma warning disable CA1308 // Normalize strings to uppercase - Need to use 'ToLowerInvariant' for file names in non-Windows platforms
var editorconfigFilePath = Path.Combine(directory.FullName, editorconfigFileName.ToLowerInvariant());
var configFilePath = Path.Combine(directory.FullName, fileName.ToLowerInvariant());
#pragma warning restore CA1308 // Normalize strings to uppercase
File.WriteAllText(editorconfigFilePath, text);
File.WriteAllText(configFilePath, text);
return;

// Local functions
static string GetGlobalconfigText(
string editorconfigTitle,
string editorconfigDescription,
string title,
string description,
bool warnAsError,
AnalysisMode analysisMode,
string? category,
SortedList<string, DiagnosticDescriptor> sortedRulesById,
Expand All @@ -1200,8 +1204,8 @@ void StartGlobalconfig()
{
result.AppendLine(@"# NOTE: Requires **VS2019 16.7** or later");
result.AppendLine();
result.AppendLine($@"# {editorconfigTitle}");
result.AppendLine($@"# Description: {editorconfigDescription}");
result.AppendLine($@"# {title}");
result.AppendLine($@"# Description: {description}");
result.AppendLine();
result.AppendLine($@"is_global = true");
result.AppendLine();
Expand Down Expand Up @@ -1240,6 +1244,11 @@ bool AddRule(DiagnosticDescriptor rule, string? category)
}

var (isEnabledByDefault, severity) = GetEnabledByDefaultAndSeverity(rule, analysisMode);
if (warnAsError && severity == DiagnosticSeverity.Warning && isEnabledByDefault)
{
severity = DiagnosticSeverity.Error;
}

if (rule.IsEnabledByDefault == isEnabledByDefault &&
severity == rule.DefaultSeverity)
{
Expand Down Expand Up @@ -1395,7 +1404,6 @@ static string GetCommonContents(string packageName, IOrderedEnumerable<string> c
}

stringBuilder.Append(GetMSBuildContentForPropertyAndItemOptions());
stringBuilder.Append(GetCodeAnalysisTreatWarningsAsErrorsTargetContents());
return stringBuilder.ToString();
}

Expand Down Expand Up @@ -1443,8 +1451,14 @@ static string GetGlobalAnalyzerConfigTargetContents(string packageName, string?
<_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName} Condition=""'$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName})' == 'AllDisabledByDefault'"">{nameof(AnalysisMode.None)}</_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName}>
<_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName} Condition=""'$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName})' == ''"">{nameof(AnalysisMode.Default)}</_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName}>
<!-- Default 'EffectiveCodeAnalysisTreatWarningsAsErrors' to 'CodeAnalysisTreatWarningsAsErrors' for escalating relevant code analysis warnings to errors. -->
<!-- We use a separate property to allow users to override 'CodeAnalysisTreatWarningsAsErrors' implementation from .NET7 or older SDK, which had a known issue: https://github.com/dotnet/roslyn-analyzers/issues/6281 -->
<EffectiveCodeAnalysisTreatWarningsAsErrors Condition=""'$(EffectiveCodeAnalysisTreatWarningsAsErrors)' == ''"">$(CodeAnalysisTreatWarningsAsErrors)</EffectiveCodeAnalysisTreatWarningsAsErrors>
<!-- Choose GlobalAnalyzerConfig file with '_warnaserror' suffix if 'EffectiveCodeAnalysisTreatWarningsAsErrors' is 'true'. -->
<_GlobalAnalyzerConfigFileName_{trimmedPackageName}_WarnAsErrorSuffix Condition=""'$(EffectiveCodeAnalysisTreatWarningsAsErrors)' == 'true'"">_warnaserror</_GlobalAnalyzerConfigFileName_{trimmedPackageName}_WarnAsErrorSuffix>
<!-- GlobalAnalyzerConfig file name based on user specified package version '{packageVersionPropName}', if any. We replace '.' with '_' to map the version string to file name suffix. -->
<_GlobalAnalyzerConfigFileName_{trimmedPackageName} Condition=""'$({packageVersionPropName})' != ''"">{analysisLevelPropName}_$({packageVersionPropName}.Replace(""."",""_""))_$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName}).editorconfig</_GlobalAnalyzerConfigFileName_{trimmedPackageName}>
<_GlobalAnalyzerConfigFileName_{trimmedPackageName} Condition=""'$({packageVersionPropName})' != ''"">{analysisLevelPropName}_$({packageVersionPropName}.Replace(""."",""_""))_$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName})$(_GlobalAnalyzerConfigFileName_{trimmedPackageName}_WarnAsErrorSuffix).globalconfig</_GlobalAnalyzerConfigFileName_{trimmedPackageName}>
<_GlobalAnalyzerConfigFileName_{trimmedPackageName}>$(_GlobalAnalyzerConfigFileName_{trimmedPackageName}.ToLowerInvariant())</_GlobalAnalyzerConfigFileName_{trimmedPackageName}>
<_GlobalAnalyzerConfigDir_{trimmedPackageName} Condition=""'$(_GlobalAnalyzerConfigDir_{trimmedPackageName})' == ''"">$(MSBuildThisFileDirectory)config</_GlobalAnalyzerConfigDir_{trimmedPackageName}>
Expand Down Expand Up @@ -1580,23 +1594,6 @@ static void AddMSBuildContentForItemOptions(StringBuilder builder)
}
}

static string GetCodeAnalysisTreatWarningsAsErrorsTargetContents()
{
return $@"
<!--
Design-time target to handle 'CodeAnalysisTreatWarningsAsErrors' for the CA rule ids implemented in this package.
Note that similar 'WarningsNotAsErrors' and 'WarningsAsErrors'
property groups are present in the generated props file to ensure this functionality on command line builds.
-->
<Target Name=""_CodeAnalysisTreatWarningsAsErrors"" BeforeTargets=""CoreCompile"" Condition=""'$(DesignTimeBuild)' == 'true' OR '$(BuildingProject)' != 'true'"">
<PropertyGroup>
<WarningsNotAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'false'"">$(WarningsNotAsErrors);$(CodeAnalysisRuleIds)</WarningsNotAsErrors>
<WarningsAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'true' and '$(TreatWarningsAsErrors)' != 'true'"">$(WarningsAsErrors);$(CodeAnalysisRuleIds)</WarningsAsErrors>
</PropertyGroup>
</Target>
";
}

static string GetPackageSpecificContents(string packageName)
=> packageName switch
{
Expand Down
7 changes: 2 additions & 5 deletions src/Tools/GenerateDocumentationAndConfigFiles/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,5 @@ Following are the precedence rules as per the values of these properties:

2. For CAxxxx rules:

1. If `CodeAnalysisTreatWarningsAsErrors` and `TreatWarningsAsErrors` both are not set, no bulk settings to escalate or de-escalate warnings to errors is done.
2. If `CodeAnalysisTreatWarningsAsErrors` is set, it overrides `TreatWarningsAsErrors` to determine if CA warnings are bulk escalated to errors or not.
3. If `CodeAnalysisTreatWarningsAsErrors` is not set, it defaults to `TreatWarningsAsErrors`.
4. If final value of `CodeAnalysisTreatWarningsAsErrors = false`, we append all CA rule IDs to `WarningsNotAsErrors` to ensure they are not escalated to errors. Users can still bump individual rule IDs to errors by editorconfig/ruleset entry, etc.
5. If final value of `CodeAnalysisTreatWarningsAsErrors = true`, we append all CA rule IDs to `WarningsAsErrors` to ensure they are escalated to errors. We optimize it a bit more by avoiding this append if `TreatWarningsAsErrors` is also true, because then the compiler itself will take care of bumping all warnings to errors, and we don't need to pollute the command line with large number of CA rules IDs in a `/warnaserror+` switch. We expect this to be the most common case as well (`TreatWarningsAsErrors` is set by user to true, `CodeAnalysisTreatWarningsAsErrors` is never set and hence defaults to `true`), and we want to ensure we don't end up polluting the entire command line with CA rules IDs unless `TreatWarningsAsErrors` and `CodeAnalysisTreatWarningsAsErrors` have different settings.
1. If `CodeAnalysisTreatWarningsAsErrors` is set to true, enabled CA warnings are bulk escalated to errors by choosing the appropriate globalconfig file with the error severity settings.
2. Otherwise, if `TreatWarningsAsErrors` is set to true, this property translates to `/warnaserror` command line switch and the compiler bumps all warnings, including enabled CA warnings, to errors.

0 comments on commit afee469

Please sign in to comment.