Skip to content

Commit 6c028d3

Browse files
mavasanimavasani
committed
Re-implement CodeAnalysisTreatWarningsAsErrors with globalconfig files
Fixes #6281 Prior implementation for `CodeAnalysisTreatWarningsAsErrors` passed `/warnaserror+:CAxxxx` to the command line for each CA rule ID. This led to couple of issues: 1. All disabled by default CA warnings seem to get enabled as errors 2. Editorconfig/globalconfig based configuration of the enabled CA rules stops working. The new implementation updates our globalconfig tooling to generate parallel globalconfigs with `_warnaserror` suffix, such that rules which would have had effective warning severity are set to error severity in the new globalconfig files. The targets/props that picks the appropriate globalconfig based on the `AnalysisLevel` property now chooses the file with `_warnaserror` suffix if user has enabled code analysis warnings to be treated as errors. Another couple of notes: 1. Given that the buggy implementation of `CodeAnalysisTreatWarningsAsErrors` has already shipped in .NET7 and prior SDKs, we use a separate property `EffectiveCodeAnalysisTreatWarningsAsErrors`, so users using the newer NuGet package, but older .NET SDK can still get this functionality. Users that move to the newer .NET8 preview SDK with this fix can use `CodeAnalysisTreatWarningsAsErrors` itself. 2. I have adjusted the tooling to generate the config files into buildtransitive folder instead of build folder. This is needed as a follow-up to #6325 for globalconfig files to be correctly mapped. 3. I have also renamed all the tooling generated globalconfig files to have .globalconfig extension instead of .editorconfig extension, as that was misleading. I have verified that `CodeAnalysisTreatWarningsAsErrors` works fine with the locally built package when `EffectiveCodeAnalysisTreatWarningsAsErrors` is set to true. If I manually edit the targets/props shipped inside .NET7 SDK to delete all the prior shipped `CodeAnalysisTreatWarningsAsErrors` logic, then setting `CodeAnalysisTreatWarningsAsErrors` also works fine.
1 parent 3168c39 commit 6c028d3

File tree

2 files changed

+55
-58
lines changed

2 files changed

+55
-58
lines changed

‎src/Tools/GenerateAnalyzerNuspec/Program.cs

+4-4
Original file line numberDiff line numberDiff line change
@@ -235,15 +235,15 @@
235235

236236
if (globalAnalyzerConfigsDir.Length > 0 && Directory.Exists(globalAnalyzerConfigsDir))
237237
{
238-
foreach (string editorconfig in Directory.EnumerateFiles(globalAnalyzerConfigsDir))
238+
foreach (string globalconfig in Directory.EnumerateFiles(globalAnalyzerConfigsDir))
239239
{
240-
if (Path.GetExtension(editorconfig) == ".editorconfig")
240+
if (Path.GetExtension(globalconfig) == ".globalconfig")
241241
{
242-
result.AppendLine(FileElement(Path.Combine(globalAnalyzerConfigsDir, editorconfig), $"build\\config"));
242+
result.AppendLine(FileElement(Path.Combine(globalAnalyzerConfigsDir, globalconfig), $"buildtransitive\\config"));
243243
}
244244
else
245245
{
246-
throw new InvalidDataException($"Encountered a file with unexpected extension: {editorconfig}");
246+
throw new InvalidDataException($"Encountered a file with unexpected extension: {globalconfig}");
247247
}
248248
}
249249
}

‎src/Tools/GenerateDocumentationAndConfigFiles/Program.cs

+51-54
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ void createPropsFiles()
252252

253253
var fileContents =
254254
$@"<Project>
255-
{disableNetAnalyzersImport}{getCodeAnalysisTreatWarningsAsErrors()}{getCompilerVisibleProperties()}
255+
{disableNetAnalyzersImport}{getCompilerVisibleProperties()}
256256
</Project>";
257257
var directory = Directory.CreateDirectory(propsFileDir);
258258
var fileWithPath = Path.Combine(directory.FullName, propsFileName);
@@ -310,20 +310,6 @@ We rely on the additional props file '{propsFileToDisableNetAnalyzersInNuGetPack
310310
}
311311
}
312312

313-
string getCodeAnalysisTreatWarningsAsErrors()
314-
{
315-
var allRuleIds = string.Join(';', allRulesById.Keys);
316-
return $@"
317-
<!--
318-
This property group handles 'CodeAnalysisTreatWarningsAsErrors' for the CA rule ids implemented in this package.
319-
-->
320-
<PropertyGroup>
321-
<CodeAnalysisRuleIds>{allRuleIds}</CodeAnalysisRuleIds>
322-
<WarningsNotAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'false'"">$(WarningsNotAsErrors);$(CodeAnalysisRuleIds)</WarningsNotAsErrors>
323-
<WarningsAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'true' and '$(TreatWarningsAsErrors)' != 'true'"">$(WarningsAsErrors);$(CodeAnalysisRuleIds)</WarningsAsErrors>
324-
</PropertyGroup>";
325-
}
326-
327313
string getCompilerVisibleProperties()
328314
{
329315
return analyzerPackageName switch
@@ -794,12 +780,15 @@ void CreateGlobalConfigsForVersion(
794780
{
795781
var analysisLevelVersionString = GetNormalizedVersionStringForEditorconfigFileNameSuffix(version);
796782

797-
foreach (var analysisMode in Enum.GetValues(typeof(AnalysisMode)))
783+
foreach (var warnAsError in new[] { true, false })
798784
{
799-
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, releaseTrackingData, category: null);
800-
foreach (var category in categories!)
785+
foreach (var analysisMode in Enum.GetValues(typeof(AnalysisMode)))
801786
{
802-
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, releaseTrackingData, category);
787+
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, warnAsError, releaseTrackingData, category: null);
788+
foreach (var category in categories!)
789+
{
790+
CreateGlobalConfig(version, isShippedVersion, analysisLevelVersionString, (AnalysisMode)analysisMode!, warnAsError, releaseTrackingData, category);
791+
}
803792
}
804793
}
805794
}
@@ -809,26 +798,38 @@ void CreateGlobalConfig(
809798
bool isShippedVersion,
810799
string analysisLevelVersionString,
811800
AnalysisMode analysisMode,
801+
bool warnAsError,
812802
ImmutableArray<ReleaseTrackingData> releaseTrackingData,
813803
string? category)
814804
{
815805
var analysisLevelPropName = "AnalysisLevel";
816806
var title = $"Rules from '{version}' release with '{analysisMode}' analysis mode";
817807
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.";
808+
818809
if (category != null)
819810
{
820811
analysisLevelPropName += category;
821812
title = $"'{category}' {title}";
822813
description = $"'{category}' {description}";
823814
}
824815

825-
CreateGlobalconfig(
826-
analyzerGlobalconfigsDir,
827816
#pragma warning disable CA1308 // Normalize strings to uppercase
828-
$"{analysisLevelPropName}_{analysisLevelVersionString}_{analysisMode!.ToString()!.ToLowerInvariant()}.editorconfig",
817+
var globalconfigFileName = $"{analysisLevelPropName}_{analysisLevelVersionString}_{analysisMode!.ToString()!.ToLowerInvariant()}";
829818
#pragma warning restore CA1308 // Normalize strings to uppercase
830-
title,
819+
820+
if (warnAsError)
821+
{
822+
globalconfigFileName += "_warnaserror";
823+
title += " escalated to 'error' severity";
824+
description += " Enabled rules with 'warning' severity are escalated to 'error' severity to respect 'CodeAnalysisTreatWarningsAsErrors' MSBuild property.";
825+
}
826+
827+
CreateGlobalconfig(
828+
analyzerGlobalconfigsDir,
829+
$"{globalconfigFileName}.globalconfig",
830+
title,
831831
description,
832+
warnAsError,
832833
analysisMode,
833834
category,
834835
allRulesById,
@@ -1158,34 +1159,37 @@ private static void Validate(string fileWithPath, string fileContents, List<stri
11581159

11591160
private static void CreateGlobalconfig(
11601161
string folder,
1161-
string editorconfigFileName,
1162-
string editorconfigTitle,
1163-
string editorconfigDescription,
1162+
string fileName,
1163+
string title,
1164+
string description,
1165+
bool warnAsError,
11641166
AnalysisMode analysisMode,
11651167
string? category,
11661168
SortedList<string, DiagnosticDescriptor> sortedRulesById,
11671169
(ImmutableArray<ReleaseTrackingData> releaseTrackingData, Version version, bool isShippedVersion) releaseTrackingDataAndVersion)
11681170
{
1169-
Debug.Assert(editorconfigFileName.EndsWith(".editorconfig", StringComparison.Ordinal));
1171+
Debug.Assert(fileName.EndsWith(".globalconfig", StringComparison.Ordinal));
11701172

11711173
var text = GetGlobalconfigText(
1172-
editorconfigTitle,
1173-
editorconfigDescription,
1174+
title,
1175+
description,
1176+
warnAsError,
11741177
analysisMode,
11751178
category,
11761179
sortedRulesById,
11771180
releaseTrackingDataAndVersion);
11781181
var directory = Directory.CreateDirectory(folder);
11791182
#pragma warning disable CA1308 // Normalize strings to uppercase - Need to use 'ToLowerInvariant' for file names in non-Windows platforms
1180-
var editorconfigFilePath = Path.Combine(directory.FullName, editorconfigFileName.ToLowerInvariant());
1183+
var configFilePath = Path.Combine(directory.FullName, fileName.ToLowerInvariant());
11811184
#pragma warning restore CA1308 // Normalize strings to uppercase
1182-
File.WriteAllText(editorconfigFilePath, text);
1185+
File.WriteAllText(configFilePath, text);
11831186
return;
11841187

11851188
// Local functions
11861189
static string GetGlobalconfigText(
1187-
string editorconfigTitle,
1188-
string editorconfigDescription,
1190+
string title,
1191+
string description,
1192+
bool warnAsError,
11891193
AnalysisMode analysisMode,
11901194
string? category,
11911195
SortedList<string, DiagnosticDescriptor> sortedRulesById,
@@ -1200,8 +1204,8 @@ void StartGlobalconfig()
12001204
{
12011205
result.AppendLine(@"# NOTE: Requires **VS2019 16.7** or later");
12021206
result.AppendLine();
1203-
result.AppendLine($@"# {editorconfigTitle}");
1204-
result.AppendLine($@"# Description: {editorconfigDescription}");
1207+
result.AppendLine($@"# {title}");
1208+
result.AppendLine($@"# Description: {description}");
12051209
result.AppendLine();
12061210
result.AppendLine($@"is_global = true");
12071211
result.AppendLine();
@@ -1240,6 +1244,11 @@ bool AddRule(DiagnosticDescriptor rule, string? category)
12401244
}
12411245

12421246
var (isEnabledByDefault, severity) = GetEnabledByDefaultAndSeverity(rule, analysisMode);
1247+
if (warnAsError && severity == DiagnosticSeverity.Warning && isEnabledByDefault)
1248+
{
1249+
severity = DiagnosticSeverity.Error;
1250+
}
1251+
12431252
if (rule.IsEnabledByDefault == isEnabledByDefault &&
12441253
severity == rule.DefaultSeverity)
12451254
{
@@ -1395,7 +1404,6 @@ static string GetCommonContents(string packageName, IOrderedEnumerable<string> c
13951404
}
13961405

13971406
stringBuilder.Append(GetMSBuildContentForPropertyAndItemOptions());
1398-
stringBuilder.Append(GetCodeAnalysisTreatWarningsAsErrorsTargetContents());
13991407
return stringBuilder.ToString();
14001408
}
14011409

@@ -1443,8 +1451,14 @@ static string GetGlobalAnalyzerConfigTargetContents(string packageName, string?
14431451
<_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName} Condition=""'$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName})' == 'AllDisabledByDefault'"">{nameof(AnalysisMode.None)}</_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName}>
14441452
<_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName} Condition=""'$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName})' == ''"">{nameof(AnalysisMode.Default)}</_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName}>
14451453
1454+
<!-- Default 'EffectiveCodeAnalysisTreatWarningsAsErrors' to 'CodeAnalysisTreatWarningsAsErrors' for escalating relevant code analysis warnings to errors. -->
1455+
<!-- 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 -->
1456+
<EffectiveCodeAnalysisTreatWarningsAsErrors Condition=""'$(EffectiveCodeAnalysisTreatWarningsAsErrors)' == ''"">$(CodeAnalysisTreatWarningsAsErrors)</EffectiveCodeAnalysisTreatWarningsAsErrors>
1457+
<!-- Choose GlobalAnalyzerConfig file with '_warnaserror' suffix if 'EffectiveCodeAnalysisTreatWarningsAsErrors' is 'true'. -->
1458+
<_GlobalAnalyzerConfigFileName_{trimmedPackageName}_WarnAsErrorSuffix Condition=""'$(EffectiveCodeAnalysisTreatWarningsAsErrors)' == 'true'"">_warnaserror</_GlobalAnalyzerConfigFileName_{trimmedPackageName}_WarnAsErrorSuffix>
1459+
14461460
<!-- GlobalAnalyzerConfig file name based on user specified package version '{packageVersionPropName}', if any. We replace '.' with '_' to map the version string to file name suffix. -->
1447-
<_GlobalAnalyzerConfigFileName_{trimmedPackageName} Condition=""'$({packageVersionPropName})' != ''"">{analysisLevelPropName}_$({packageVersionPropName}.Replace(""."",""_""))_$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName}).editorconfig</_GlobalAnalyzerConfigFileName_{trimmedPackageName}>
1461+
<_GlobalAnalyzerConfigFileName_{trimmedPackageName} Condition=""'$({packageVersionPropName})' != ''"">{analysisLevelPropName}_$({packageVersionPropName}.Replace(""."",""_""))_$(_GlobalAnalyzerConfigAnalysisMode_{trimmedPackageName})$(_GlobalAnalyzerConfigFileName_{trimmedPackageName}_WarnAsErrorSuffix).globalconfig</_GlobalAnalyzerConfigFileName_{trimmedPackageName}>
14481462
<_GlobalAnalyzerConfigFileName_{trimmedPackageName}>$(_GlobalAnalyzerConfigFileName_{trimmedPackageName}.ToLowerInvariant())</_GlobalAnalyzerConfigFileName_{trimmedPackageName}>
14491463
14501464
<_GlobalAnalyzerConfigDir_{trimmedPackageName} Condition=""'$(_GlobalAnalyzerConfigDir_{trimmedPackageName})' == ''"">$(MSBuildThisFileDirectory)config</_GlobalAnalyzerConfigDir_{trimmedPackageName}>
@@ -1580,23 +1594,6 @@ static void AddMSBuildContentForItemOptions(StringBuilder builder)
15801594
}
15811595
}
15821596

1583-
static string GetCodeAnalysisTreatWarningsAsErrorsTargetContents()
1584-
{
1585-
return $@"
1586-
<!--
1587-
Design-time target to handle 'CodeAnalysisTreatWarningsAsErrors' for the CA rule ids implemented in this package.
1588-
Note that similar 'WarningsNotAsErrors' and 'WarningsAsErrors'
1589-
property groups are present in the generated props file to ensure this functionality on command line builds.
1590-
-->
1591-
<Target Name=""_CodeAnalysisTreatWarningsAsErrors"" BeforeTargets=""CoreCompile"" Condition=""'$(DesignTimeBuild)' == 'true' OR '$(BuildingProject)' != 'true'"">
1592-
<PropertyGroup>
1593-
<WarningsNotAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'false'"">$(WarningsNotAsErrors);$(CodeAnalysisRuleIds)</WarningsNotAsErrors>
1594-
<WarningsAsErrors Condition=""'$(CodeAnalysisTreatWarningsAsErrors)' == 'true' and '$(TreatWarningsAsErrors)' != 'true'"">$(WarningsAsErrors);$(CodeAnalysisRuleIds)</WarningsAsErrors>
1595-
</PropertyGroup>
1596-
</Target>
1597-
";
1598-
}
1599-
16001597
static string GetPackageSpecificContents(string packageName)
16011598
=> packageName switch
16021599
{

0 commit comments

Comments
 (0)