Merge pull request #1784 from OmniSharp/feature/warnings-not-as-errors

bjorkstromm · web-flow · commit 77c928e4948a · 2020-05-08T20:52:34.000+03:00
added support for WarningsNotAsErrors msbuild property
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@ All changes to the project will be documented in this file.
 
 ## [1.35.2] - not yet released
 * Added support for `WarningsAsErrors` in csproj files (PR: [#1779](https://github.com/OmniSharp/omnisharp-roslyn/pull/1779))
+* Added support for `WarningsNotAsErrors` in csproj files ([#1681](https://github.com/OmniSharp/omnisharp-roslyn/issues/1681), PR: [#1784](https://github.com/OmniSharp/omnisharp-roslyn/pull/1784))
 
 ## [1.35.1] - 2020-05-04
 * Fixed not supported exception when trying to decompile a BCL assembly on Mono. For now we do not try to resolve implementation assembly from a ref assembly (PR: [#1767](https://github.com/OmniSharp/omnisharp-roslyn/pull/1767))
diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.ProjectData.cs
@@ -51,6 +51,7 @@ private class ProjectData
             public ImmutableArray<string> AdditionalFiles { get; }
             public ImmutableArray<string> AnalyzerConfigFiles { get; }
             public ImmutableArray<string> WarningsAsErrors { get; }
+            public ImmutableArray<string> WarningsNotAsErrors { get; }
             public RuleSet RuleSet { get; }
             public ImmutableDictionary<string, string> ReferenceAliases { get; }
             public ImmutableDictionary<string, string> ProjectReferenceAliases { get; }
@@ -76,6 +77,7 @@ private ProjectData()
                 ReferenceAliases = ImmutableDictionary<string, string>.Empty;
                 ProjectReferenceAliases = ImmutableDictionary<string, string>.Empty;
                 WarningsAsErrors = ImmutableArray<string>.Empty;
+                WarningsNotAsErrors = ImmutableArray<string>.Empty;
             }
 
             private ProjectData(
@@ -94,6 +96,7 @@ private ProjectData(
                 ImmutableArray<string> preprocessorSymbolNames,
                 ImmutableArray<string> suppressedDiagnosticIds,
                 ImmutableArray<string> warningsAsErrors,
+                ImmutableArray<string> warningsNotAsErrors,
                 bool signAssembly,
                 string assemblyOriginatorKeyFile,
                 bool treatWarningsAsErrors,
@@ -126,6 +129,7 @@ private ProjectData(
                 PreprocessorSymbolNames = preprocessorSymbolNames.EmptyIfDefault();
                 SuppressedDiagnosticIds = suppressedDiagnosticIds.EmptyIfDefault();
                 WarningsAsErrors = warningsAsErrors.EmptyIfDefault();
+                WarningsNotAsErrors = warningsNotAsErrors.EmptyIfDefault();
 
                 SignAssembly = signAssembly;
                 AssemblyOriginatorKeyFile = assemblyOriginatorKeyFile;
@@ -153,6 +157,7 @@ private ProjectData(
                 ImmutableArray<string> preprocessorSymbolNames,
                 ImmutableArray<string> suppressedDiagnosticIds,
                 ImmutableArray<string> warningsAsErrors,
+                ImmutableArray<string> warningsNotAsErrors,
                 bool signAssembly,
                 string assemblyOriginatorKeyFile,
                 ImmutableArray<string> sourceFiles,
@@ -171,7 +176,7 @@ private ProjectData(
                 ImmutableDictionary<string, string> projectReferenceAliases)
                 : this(guid, name, assemblyName, targetPath, outputPath, intermediateOutputPath, projectAssetsFile,
                       configuration, platform, targetFramework, targetFrameworks, outputKind, languageVersion, nullableContextOptions, allowUnsafeCode, checkForOverflowUnderflow,
-                      documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors, signAssembly, assemblyOriginatorKeyFile, treatWarningsAsErrors, defaultNamespace, runAnalyzers, runAnalyzersDuringLiveAnalysis, ruleset)
+                      documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors, warningsNotAsErrors, signAssembly, assemblyOriginatorKeyFile, treatWarningsAsErrors, defaultNamespace, runAnalyzers, runAnalyzersDuringLiveAnalysis, ruleset)
             {
                 SourceFiles = sourceFiles.EmptyIfDefault();
                 ProjectReferences = projectReferences.EmptyIfDefault();
@@ -216,6 +221,7 @@ public static ProjectData Create(MSB.Evaluation.Project project)
                 var preprocessorSymbolNames = PropertyConverter.ToPreprocessorSymbolNames(project.GetPropertyValue(PropertyNames.DefineConstants));
                 var suppressedDiagnosticIds = PropertyConverter.ToSuppressedDiagnosticIds(project.GetPropertyValue(PropertyNames.NoWarn));
                 var warningsAsErrors = PropertyConverter.SplitList(project.GetPropertyValue(PropertyNames.WarningsAsErrors), ',');
+                var warningsNotAsErrors = PropertyConverter.SplitList(project.GetPropertyValue(PropertyNames.WarningsNotAsErrors), ',');
                 var signAssembly = PropertyConverter.ToBoolean(project.GetPropertyValue(PropertyNames.SignAssembly), defaultValue: false);
                 var assemblyOriginatorKeyFile = project.GetPropertyValue(PropertyNames.AssemblyOriginatorKeyFile);
                 var treatWarningsAsErrors = PropertyConverter.ToBoolean(project.GetPropertyValue(PropertyNames.TreatWarningsAsErrors), defaultValue: false);
@@ -225,7 +231,7 @@ public static ProjectData Create(MSB.Evaluation.Project project)
                 return new ProjectData(
                     guid, name, assemblyName, targetPath, outputPath, intermediateOutputPath, projectAssetsFile,
                     configuration, platform, targetFramework, targetFrameworks, outputKind, languageVersion, nullableContextOptions, allowUnsafeCode, checkForOverflowUnderflow,
-                    documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors, signAssembly, assemblyOriginatorKeyFile, treatWarningsAsErrors, defaultNamespace, runAnalyzers, runAnalyzersDuringLiveAnalysis, ruleset: null);
+                    documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors, warningsNotAsErrors, signAssembly, assemblyOriginatorKeyFile, treatWarningsAsErrors, defaultNamespace, runAnalyzers, runAnalyzersDuringLiveAnalysis, ruleset: null);
             }
 
             public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance)
@@ -260,6 +266,7 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance)
                 var preprocessorSymbolNames = PropertyConverter.ToPreprocessorSymbolNames(projectInstance.GetPropertyValue(PropertyNames.DefineConstants));
                 var suppressedDiagnosticIds = PropertyConverter.ToSuppressedDiagnosticIds(projectInstance.GetPropertyValue(PropertyNames.NoWarn));
                 var warningsAsErrors = PropertyConverter.SplitList(projectInstance.GetPropertyValue(PropertyNames.WarningsAsErrors), ',');
+                var warningsNotAsErrors = PropertyConverter.SplitList(projectInstance.GetPropertyValue(PropertyNames.WarningsNotAsErrors), ',');
                 var signAssembly = PropertyConverter.ToBoolean(projectInstance.GetPropertyValue(PropertyNames.SignAssembly), defaultValue: false);
                 var treatWarningsAsErrors = PropertyConverter.ToBoolean(projectInstance.GetPropertyValue(PropertyNames.TreatWarningsAsErrors), defaultValue: false);
                 var runAnalyzers = PropertyConverter.ToBoolean(projectInstance.GetPropertyValue(PropertyNames.RunAnalyzers), defaultValue: true);
@@ -333,7 +340,7 @@ public static ProjectData Create(MSB.Execution.ProjectInstance projectInstance)
                 return new ProjectData(guid, name,
                     assemblyName, targetPath, outputPath, intermediateOutputPath, projectAssetsFile,
                     configuration, platform, targetFramework, targetFrameworks,
-                    outputKind, languageVersion, nullableContextOptions, allowUnsafeCode, checkForOverflowUnderflow, documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors,
+                    outputKind, languageVersion, nullableContextOptions, allowUnsafeCode, checkForOverflowUnderflow, documentationFile, preprocessorSymbolNames, suppressedDiagnosticIds, warningsAsErrors, warningsNotAsErrors,
                     signAssembly, assemblyOriginatorKeyFile,
                     sourceFiles, projectReferences.ToImmutable(), references.ToImmutable(), packageReferences, analyzers, additionalFiles, editorConfigFiles, treatWarningsAsErrors, defaultNamespace, runAnalyzers, runAnalyzersDuringLiveAnalysis, ruleset,
                     referenceAliases.ToImmutableDictionary(), projectReferenceAliases.ToImmutable());
diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfo.cs
@@ -42,6 +42,7 @@ internal partial class ProjectFileInfo
         public ImmutableArray<string> PreprocessorSymbolNames => _data.PreprocessorSymbolNames;
         public ImmutableArray<string> SuppressedDiagnosticIds => _data.SuppressedDiagnosticIds;
         public ImmutableArray<string> WarningsAsErrors => _data.WarningsAsErrors;
+        public ImmutableArray<string> WarningsNotAsErrors => _data.WarningsNotAsErrors;
 
         public bool SignAssembly => _data.SignAssembly;
         public string AssemblyOriginatorKeyFile => _data.AssemblyOriginatorKeyFile;
diff --git a/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfoExtensions.cs b/src/OmniSharp.MSBuild/ProjectFile/ProjectFileInfoExtensions.cs
@@ -55,8 +55,10 @@ public static ImmutableDictionary<string, ReportDiagnostic> GetDiagnosticOptions
             var suppressions = CompilationOptionsHelper.GetDefaultSuppressedDiagnosticOptions(projectFileInfo.SuppressedDiagnosticIds);
             var specificRules = projectFileInfo.RuleSet?.SpecificDiagnosticOptions ?? ImmutableDictionary<string, ReportDiagnostic>.Empty;
 
+            // suppressions capture NoWarn and they have the highest priority
             var combinedRules = specificRules.Concat(suppressions.Where(x => !specificRules.Keys.Contains(x.Key))).ToDictionary(x => x.Key, x => x.Value);
 
+            // then handle WarningsAsErrors
             foreach (var warningAsError in projectFileInfo.WarningsAsErrors)
             {
                 if (!suppressions.ContainsKey(warningAsError))
@@ -65,6 +67,15 @@ public static ImmutableDictionary<string, ReportDiagnostic> GetDiagnosticOptions
                 }
             }
 
+            // WarningsNotAsErrors can overwrite WarningsAsErrors
+            foreach (var warningNotAsError in projectFileInfo.WarningsNotAsErrors)
+            {
+                if (!suppressions.ContainsKey(warningNotAsError))
+                {
+                    combinedRules[warningNotAsError] = ReportDiagnostic.Warn;
+                }
+            }
+
             return combinedRules.ToImmutableDictionary();
         }
 
diff --git a/src/OmniSharp.MSBuild/ProjectFile/PropertyNames.cs b/src/OmniSharp.MSBuild/ProjectFile/PropertyNames.cs
@@ -46,5 +46,6 @@ internal static class PropertyNames
         public const string VisualStudioVersion = nameof(VisualStudioVersion);
         public const string VsInstallRoot = nameof(VsInstallRoot);
         public const string WarningsAsErrors = nameof(WarningsAsErrors);
+        public const string WarningsNotAsErrors = nameof(WarningsNotAsErrors);
     }
 }
diff --git a/test-assets/test-projects/WarningsAsErrors/WarningsAsErrors.csproj b/test-assets/test-projects/WarningsAsErrors/WarningsAsErrors.csproj
@@ -3,8 +3,9 @@
   <PropertyGroup>
     <OutputType>Exe</OutputType>
     <TargetFramework>netcoreapp3.1</TargetFramework>
-    <WarningsAsErrors>CS1998,CS7080,CS7081</WarningsAsErrors>
     <NoWarn>CS7081</NoWarn>
+    <WarningsAsErrors>CS1998,CS7080,CS7081</WarningsAsErrors>
+    <WarningsNotAsErrors>CS7080,CS7082</WarningsNotAsErrors>
   </PropertyGroup>
 
 </Project>
diff --git a/tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs b/tests/OmniSharp.MSBuild.Tests/ProjectFileInfoTests.cs
@@ -210,13 +210,21 @@ public async Task WarningsAsErrors()
                 Assert.Contains("CS7080", projectFileInfo.WarningsAsErrors);
                 Assert.Contains("CS7081", projectFileInfo.WarningsAsErrors);
 
+                Assert.NotEmpty(projectFileInfo.WarningsNotAsErrors);
+                Assert.Contains("CS7080", projectFileInfo.WarningsNotAsErrors);
+                Assert.Contains("CS7082", projectFileInfo.WarningsNotAsErrors);
+
                 var compilationOptions = projectFileInfo.CreateCompilationOptions();
                 Assert.True(compilationOptions.SpecificDiagnosticOptions.ContainsKey("CS1998"), "Specific diagnostic option for CS1998 not found");
                 Assert.True(compilationOptions.SpecificDiagnosticOptions.ContainsKey("CS7080"), "Specific diagnostic option for CS7080 not found");
                 Assert.True(compilationOptions.SpecificDiagnosticOptions.ContainsKey("CS7081"), "Specific diagnostic option for CS7081 not found");
+                Assert.True(compilationOptions.SpecificDiagnosticOptions.ContainsKey("CS7082"), "Specific diagnostic option for CS7082 not found");
                 Assert.Equal(ReportDiagnostic.Error, compilationOptions.SpecificDiagnosticOptions["CS1998"]);
-                Assert.Equal(ReportDiagnostic.Error, compilationOptions.SpecificDiagnosticOptions["CS7080"]);
-                Assert.Equal(ReportDiagnostic.Suppress, compilationOptions.SpecificDiagnosticOptions["CS7081"]); // NoWarn should be given priority over WarningsAsErrors
+                // CS7080 is both in WarningsAsErrors and WarningsNotAsErrors, but WarningsNotAsErrors are higher priority
+                Assert.Equal(ReportDiagnostic.Warn, compilationOptions.SpecificDiagnosticOptions["CS7080"]); 
+                Assert.Equal(ReportDiagnostic.Warn, compilationOptions.SpecificDiagnosticOptions["CS7082"]);
+                // CS7081 is both WarningsAsErrors and NoWarn, but NoWarn are higher priority
+                Assert.Equal(ReportDiagnostic.Suppress, compilationOptions.SpecificDiagnosticOptions["CS7081"]); 
             }
         }
     }

Original file line number	Diff line number	Diff line change
`@@ -55,8 +55,10 @@ public static ImmutableDictionary<string, ReportDiagnostic> GetDiagnosticOptions`
`55`	`55`	`var suppressions = CompilationOptionsHelper.GetDefaultSuppressedDiagnosticOptions(projectFileInfo.SuppressedDiagnosticIds);`
`56`	`56`	`var specificRules = projectFileInfo.RuleSet?.SpecificDiagnosticOptions ?? ImmutableDictionary<string, ReportDiagnostic>.Empty;`
`57`	`57`
	`58`	`+ // suppressions capture NoWarn and they have the highest priority`
`58`	`59`	`var combinedRules = specificRules.Concat(suppressions.Where(x => !specificRules.Keys.Contains(x.Key))).ToDictionary(x => x.Key, x => x.Value);`
`59`	`60`
	`61`	`+ // then handle WarningsAsErrors`
`60`	`62`	`foreach (var warningAsError in projectFileInfo.WarningsAsErrors)`
`61`	`63`	`{`
`62`	`64`	`if (!suppressions.ContainsKey(warningAsError))`
`@@ -65,6 +67,15 @@ public static ImmutableDictionary<string, ReportDiagnostic> GetDiagnosticOptions`
`65`	`67`	`}`
`66`	`68`	`}`
`67`	`69`
	`70`	`+ // WarningsNotAsErrors can overwrite WarningsAsErrors`
	`71`	`+ foreach (var warningNotAsError in projectFileInfo.WarningsNotAsErrors)`
	`72`	`+ {`
	`73`	`+ if (!suppressions.ContainsKey(warningNotAsError))`
	`74`	`+ {`
	`75`	`+ combinedRules[warningNotAsError] = ReportDiagnostic.Warn;`
	`76`	`+ }`
	`77`	`+ }`
	`78`	`+`
`68`	`79`	`return combinedRules.ToImmutableDictionary();`
`69`	`80`	`}`
`70`	`81`
Original file line number	Diff line number	Diff line change
`@@ -46,5 +46,6 @@ internal static class PropertyNames`
`46`	`46`	`public const string VisualStudioVersion = nameof(VisualStudioVersion);`
`47`	`47`	`public const string VsInstallRoot = nameof(VsInstallRoot);`
`48`	`48`	`public const string WarningsAsErrors = nameof(WarningsAsErrors);`
	`49`	`+ public const string WarningsNotAsErrors = nameof(WarningsNotAsErrors);`
`49`	`50`	`}`
`50`	`51`	`}`