Merge pull request #39014 from JoeRobich/enable-warnaserror

Enable warnAsError for correctness builds

Author: RikkiGibson

azure-pipelines.yml

@@ -37,7 +37,7 @@ jobs:
   timeoutInMinutes: 90
 
   steps:
-    - script: eng/cibuild.cmd -configuration $(_configuration) -prepareMachine -testDesktop -$(_testKind) -procdump -warnAsError
+    - script: eng/cibuild.cmd -configuration $(_configuration) -prepareMachine -testDesktop -$(_testKind) -procdump -skipAnalyzers
       displayName: Build and Test
 
     - task: PublishTestResults@2

eng/build.ps1

@@ -223,12 +223,10 @@ function BuildSolution() {
   $suppressExtensionDeployment = if (!$deployExtensions) { "/p:DeployExtension=false" } else { "" } 
 
   # The warnAsError flag for MSBuild will promote all warnings to errors. This is true for warnings
-  # that MSBuild output as well as ones that custom tasks output. This causes problems for us as 
-  # portions of our build will issue warnings: style analyzers being the most prominent example. Hence
-  # rather than a blanket include of warnings we include a fixed set.
+  # that MSBuild output as well as ones that custom tasks output.
   #
   # In all cases we pass /p:TreatWarningsAsErrors=true to promote compiler warnings to errors
-  $msbuildWarnAsError = if ($warnAsError) { "/warnAsError:MSB3270,MSB3277" } else { "" }
+  $msbuildWarnAsError = if ($warnAsError) { "/warnAsError" } else { "" }
 
   # Workaround for some machines in the AzDO pool not allowing long paths (%5c is msbuild escaped backslash)
   $ibcDir = Join-Path $RepoRoot ".o%5c"

eng/targets/Settings.props

@@ -18,7 +18,7 @@
     <ToolsetPackagesDir>$(RepoRoot)build\ToolsetPackages\</ToolsetPackagesDir>
 
     <RoslynPortableTargetFrameworks>net472;netcoreapp2.1</RoslynPortableTargetFrameworks>
-    <RoslynEnforceCodeStyle Condition="'$(ContinuousIntegrationBuild)' != 'true'">true</RoslynEnforceCodeStyle>
+    <RoslynCheckCodeStyle Condition="'$(ContinuousIntegrationBuild)' != 'true' or '$(RoslynEnforceCodeStyle)' == 'true'">true</RoslynCheckCodeStyle>
     <UseSharedCompilation>true</UseSharedCompilation>
 
     <Features>strict, IOperation</Features>
@@ -126,20 +126,20 @@
 
   <!-- Language-specific analyzer packages -->
   <Choose>
-    <When Condition="'$(Language)' == 'VB' and '$(RoslynEnforceCodeStyle)' == 'true'">
+    <When Condition="'$(Language)' == 'VB' and '$(RoslynCheckCodeStyle)' == 'true'">
       <ItemGroup>
         <PackageReference Include="Microsoft.CodeAnalysis.VisualBasic.CodeStyle" Version="$(MicrosoftCodeAnalysisVisualBasicCodeStyleVersion)" PrivateAssets="all" />
       </ItemGroup>
     </When>
 
-    <When Condition="'$(Language)' == 'C#' and '$(RoslynEnforceCodeStyle)' == 'true'">
+    <When Condition="'$(Language)' == 'C#' and '$(RoslynCheckCodeStyle)' == 'true'">
       <ItemGroup>
         <PackageReference Include="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="$(MicrosoftCodeAnalysisCSharpCodeStyleVersion)" PrivateAssets="all" />
       </ItemGroup>
     </When>
   </Choose>
 
-  <PropertyGroup>
+  <PropertyGroup Condition="'$(RoslynEnforceCodeStyle)' != 'true'">
     <!-- Don't treat FormattingAnalyzer as an error, even when TreatWarningsAsErrors is specified. -->
     <WarningsNotAsErrors>$(WarningsNotAsErrors);IDE0055</WarningsNotAsErrors>
   </PropertyGroup>

eng/test-build-correctness.ps1

@@ -34,7 +34,7 @@ try {
   Push-Location $RepoRoot
 
   Write-Host "Building Roslyn"
-  Exec-Block { & (Join-Path $PSScriptRoot "build.ps1") -restore -build -ci:$ci -configuration:$configuration -pack -binaryLog -useGlobalNuGetCache:$false -warnAsError:$false -properties "/p:RoslynEnforceCodeStyle=true"}
+  Exec-Block { & (Join-Path $PSScriptRoot "build.ps1") -restore -build -ci:$ci -configuration:$configuration -pack -binaryLog -useGlobalNuGetCache:$false -warnAsError:$true -properties "/p:RoslynEnforceCodeStyle=true"}
 
   # Verify the state of our various build artifacts
   Write-Host "Running BuildBoss"

src/Features/CSharp/Portable/SignatureHelp/InitializerExpressionSignatureHelpProvider.cs

@@ -22,7 +22,7 @@ public override bool IsRetriggerCharacter(char ch)
             => ch == '}';
 
         private bool TryGetInitializerExpression(SyntaxNode root, int position, ISyntaxFactsService syntaxFacts, SignatureHelpTriggerReason triggerReason, CancellationToken cancellationToken, out InitializerExpressionSyntax expression)
-            => CommonSignatureHelpUtilities.TryGetSyntax(root, position, syntaxFacts, triggerReason, IsTriggerToken, IsInitializerExpressionToken, cancellationToken, out expression) && 
+            => CommonSignatureHelpUtilities.TryGetSyntax(root, position, syntaxFacts, triggerReason, IsTriggerToken, IsInitializerExpressionToken, cancellationToken, out expression) &&
                expression != null;
 
         private bool IsTriggerToken(SyntaxToken token)

src/Workspaces/CSharpTest/CodeGeneration/AddImportsTests.cs

@@ -44,7 +44,7 @@ private async Task<Document> GetDocument(string code, bool withAnnotations)
                     {
                         var symbol = model.GetSymbolInfo(o).Symbol;
                         return symbol != null
-                            ? c.WithAdditionalAnnotations(SymbolAnnotation.Create(symbol), Simplifier.Annotation) 
+                            ? c.WithAdditionalAnnotations(SymbolAnnotation.Create(symbol), Simplifier.Annotation)
                             : c;
                     });
                 doc = doc.WithSyntaxRoot(root);
@@ -179,8 +179,8 @@ class C
 class C
 {
     public List<int> F;
-}", 
-                safe, 
+}",
+                safe,
                 useSymbolAnnotations,
                 options => options.WithChangedOption(GenerationOptions.PlaceSystemNamespaceFirst, LanguageNames.CSharp, false)
 );

src/Workspaces/Core/Portable/AddImports/AbstractAddImportsService.cs

@@ -81,7 +81,7 @@ private bool HasExistingImport(
         }
 
         protected abstract bool IsEquivalentImport(SyntaxNode a, SyntaxNode b);
-        
+
         public SyntaxNode GetImportContainer(SyntaxNode root, SyntaxNode? contextLocation, SyntaxNode import)
         {
             contextLocation ??= root;
@@ -142,7 +142,7 @@ public SyntaxNode AddImports(
         protected abstract SyntaxNode Rewrite(
             TExternSyntax[] externAliases, TUsingOrAliasSyntax[] usingDirectives, TUsingOrAliasSyntax[] staticUsingDirectives,
             TUsingOrAliasSyntax[] aliasDirectives, SyntaxNode externContainer, SyntaxNode usingContainer,
-            SyntaxNode staticUsingContainer, SyntaxNode aliasContainer, bool placeSystemNamespaceFirst, SyntaxNode root, 
+            SyntaxNode staticUsingContainer, SyntaxNode aliasContainer, bool placeSystemNamespaceFirst, SyntaxNode root,
             CancellationToken cancellationToken);
 
         private void GetContainers(SyntaxNode root, SyntaxNode contextLocation, out SyntaxNode externContainer, out SyntaxNode usingContainer, out SyntaxNode staticUsingContainer, out SyntaxNode aliasContainer)