Re-implement CodeAnalysisTreatWarningsAsErrors with globalconfig files

src/Tools/GenerateAnalyzerNuspec/Program.cs

@@ -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}");
         }
     }
 }

src/Tools/GenerateDocumentationAndConfigFiles/Program.cs

@@ -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);
@@ -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
@@ -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);
+                            }
                         }
                     }
                 }
@@ -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,
@@ -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,
@@ -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();
@@ -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)
                         {
@@ -1395,7 +1404,6 @@ static string GetCommonContents(string packageName, IOrderedEnumerable<string> c
                 }
 
                 stringBuilder.Append(GetMSBuildContentForPropertyAndItemOptions());
-                stringBuilder.Append(GetCodeAnalysisTreatWarningsAsErrorsTargetContents());
                 return stringBuilder.ToString();
             }
 
@@ -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}>
@@ -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
                 {

src/Tools/GenerateDocumentationAndConfigFiles/README.md

@@ -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.