Merge pull request #38991 from dotnet/merges/master-to-features/local-function-attributes

Merge master to features/local-function-attributes

Author: RikkiGibson

docs/compilers/API Notes/9-30-19.md

@@ -0,0 +1,38 @@
+## API Review Notes for September 30th, 2019
+
+### Changes reviewed
+Starting commit: `12b125030c34e340086b7fa192fb9426aaddf054`
+
+Ending Commit: `38c90f8401f9e3ee5fb7c82aac36f6b85fdda979`
+
+### Notes
+
+For the AnalyzerConfig removals, we've said that we're fine with this change because the API was just published, and wasn't intended to be.
+We don't want to maintain the API shape in the future, and we're willing to accept the risk of breaks in order to remove it now, before many people can take a hard dependency on the API surface.
+
+ErrorLogPath removed from CommandLineArguments, moved to ErrorLogOptions.Path
+ - We should maintain this API.
+ - Just forward to the old implementation.
+ - @333fred will send a PR.
+
+TextDocument
+ - Should we not remove the constructor? This is a binary breaking change
+ - This empty constructor cannot be used.
+ - We'll keep the change.
+
+Workspace
+ - CanApplyParseOptionChange going from virtual protected to virtual public is a source and binary breaking change
+ - We should look at having a new non-virtual public method on Workspace and forward to this API.
+ - @jasonmalinowski will look at this fix.
+
+SyntaxFactory.AnonymousMethodExpression
+ - Added overload with multiple parameters.
+ - Part of https://github.com/dotnet/roslyn/pull/37674, which brings the API in line with MethodBodySyntax.
+   This change is fine.
+
+Formatter.OrganizeUsings
+ - Should make cancellationtoken optional?
+ - Need to followup with framework on what the current guidelines are
+ - Make this default for now
+ - @jasonmalinowski to follow up.
+

eng/Version.Details.xml

@@ -3,9 +3,9 @@
   <ProductDependencies>
   </ProductDependencies>
   <ToolsetDependencies>
-    <Dependency Name="Microsoft.DotNet.Arcade.Sdk" Version="5.0.0-beta.19477.2">
+    <Dependency Name="Microsoft.DotNet.Arcade.Sdk" Version="5.0.0-beta.19480.3">
       <Uri>https://github.com/dotnet/arcade</Uri>
-      <Sha>14abaee3dba41fbe608431fb6a4f4b2435dcac33</Sha>
+      <Sha>238f1bbb23ba67616818d0b242c5b55a18edec55</Sha>
     </Dependency>
   </ToolsetDependencies>
 </Dependencies>

eng/Versions.props

@@ -259,8 +259,6 @@
     <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.Language.NavigateTo.Interfaces" />
     <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.Language.StandardClassification" />
     <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.LanguageServer.Client" />
-    <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.LanguageServer.Protocol" />
-    <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.LanguageServer.Protocol.Extensions" />
     <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.LiveShare.LanguageServices.Guest" />
     <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.LiveShare.WebEditors" />
     <PrivateVisualStudioPackage Include="Microsoft.VisualStudio.OLE.Interop" />

global.json

@@ -7,6 +7,6 @@
     "xcopy-msbuild": "16.3.0-alpha"
   },
   "msbuild-sdks": {
-    "Microsoft.DotNet.Arcade.Sdk": "5.0.0-beta.19477.2"
+    "Microsoft.DotNet.Arcade.Sdk": "5.0.0-beta.19480.3"
   }
 }

src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs

@@ -11467,6 +11467,71 @@ public void AnalyzerWarnAsErrorDoesNotEmit(bool warnAsError)
             string binaryPath = Path.Combine(dir.Path, "temp.dll");
             Assert.True(File.Exists(binaryPath) == !warnAsError);
         }
+
+        // Currently, configuring no location diagnostics through editorconfig is not supported.
+        [Theory(Skip = "https://github.com/dotnet/roslyn/issues/38042")]
+        [CombinatorialData]
+        public void AnalyzerConfigRespectedForNoLocationDiagnostic(ReportDiagnostic reportDiagnostic, bool isEnabledByDefault, bool noWarn)
+        {
+            var analyzer = new AnalyzerWithNoLocationDiagnostics(isEnabledByDefault);
+            TestAnalyzerConfigRespectedCore(analyzer, analyzer.Descriptor, reportDiagnostic, noWarn);
+        }
+
+        [WorkItem(37876, "https://github.com/dotnet/roslyn/issues/37876")]
+        [Theory]
+        [CombinatorialData]
+        public void AnalyzerConfigRespectedForDisabledByDefaultDiagnostic(ReportDiagnostic analyzerConfigSeverity, bool isEnabledByDefault, bool noWarn)
+        {
+            var analyzer = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault);
+            TestAnalyzerConfigRespectedCore(analyzer, analyzer.Descriptor, analyzerConfigSeverity, noWarn);
+        }
+
+        private void TestAnalyzerConfigRespectedCore(DiagnosticAnalyzer analyzer, DiagnosticDescriptor descriptor, ReportDiagnostic analyzerConfigSeverity, bool noWarn)
+        {
+            if (analyzerConfigSeverity == ReportDiagnostic.Default)
+            {
+                // "dotnet_diagnostic.ID.severity = default" is not supported.
+                return;
+            }
+
+            var dir = Temp.CreateDirectory();
+            var src = dir.CreateFile("test.cs").WriteAllText(@"class C { }");
+            var analyzerConfig = dir.CreateFile(".editorconfig").WriteAllText($@"
+[*.cs]
+dotnet_diagnostic.{descriptor.Id}.severity = {analyzerConfigSeverity.ToAnalyzerConfigString()}");
+
+            var arguments = new[] {
+                "/nologo",
+                "/t:library",
+                "/preferreduilang:en",
+                "/analyzerconfig:" + analyzerConfig.Path,
+                src.Path };
+            if (noWarn)
+            {
+                arguments = arguments.Append($"/nowarn:{descriptor.Id}");
+            }
+
+            var cmd = CreateCSharpCompiler(null, dir.Path, arguments,
+                analyzers: ImmutableArray.Create<DiagnosticAnalyzer>(analyzer));
+
+            Assert.Equal(analyzerConfig.Path, Assert.Single(cmd.Arguments.AnalyzerConfigPaths));
+
+            var outWriter = new StringWriter(CultureInfo.InvariantCulture);
+            var exitCode = cmd.Run(outWriter);
+
+            var expectedErrorCode = analyzerConfigSeverity == ReportDiagnostic.Error ? 1 : 0;
+            Assert.Equal(expectedErrorCode, exitCode);
+
+            if (analyzerConfigSeverity == ReportDiagnostic.Error || analyzerConfigSeverity == ReportDiagnostic.Warn || analyzerConfigSeverity == ReportDiagnostic.Info)
+            {
+                var prefix = analyzerConfigSeverity == ReportDiagnostic.Error ? "error" : analyzerConfigSeverity == ReportDiagnostic.Warn ? "warning" : "info";
+                Assert.Contains($"{prefix} {descriptor.Id}: {descriptor.MessageFormat}", outWriter.ToString());
+            }
+            else
+            {
+                Assert.DoesNotContain(descriptor.Id.ToString(), outWriter.ToString());
+            }
+        }
     }
 
     [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]

src/Compilers/CSharp/Test/CommandLine/Microsoft.CodeAnalysis.CSharp.CommandLine.UnitTests.csproj

@@ -23,6 +23,9 @@
   <ItemGroup>
     <EmbeddedResource Include="..\..\csc\csc.rsp" LogicalName="csc.rsp" />
   </ItemGroup>
+  <ItemGroup>
+    <PackageReference Include="Xunit.Combinatorial" Version="$(XunitCombinatorialVersion)" />
+  </ItemGroup>
   <ItemGroup>
     <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
   </ItemGroup>

src/Compilers/Core/MSBuildTask/Microsoft.CSharp.Core.targets

@@ -3,8 +3,8 @@
 <Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <Import Project="Microsoft.Managed.Core.targets"/>
 
-  <PropertyGroup Condition="('$(TargetFrameworkIdentifier)' != '.NETCoreApp' OR '$(TargetFrameworkVersion)' != 'v3.0') AND
-                            ('$(TargetFrameworkIdentifier)' != '.NETStandard' OR '$(TargetFrameworkVersion)' != 'v2.1')">
+  <PropertyGroup Condition="('$(TargetFrameworkIdentifier)' != '.NETCoreApp' OR '$(_TargetFrameworkVersionWithoutV)' &lt; '3.0') AND
+                            ('$(TargetFrameworkIdentifier)' != '.NETStandard' OR '$(_TargetFrameworkVersionWithoutV)' &lt; '2.1')">
     <MaxSupportedLangVersion Condition="'$(MaxSupportedLangVersion)' == ''">7.3</MaxSupportedLangVersion>
     <LangVersion Condition="'$(LangVersion)' == ''">$(MaxSupportedLangVersion)</LangVersion>
   </PropertyGroup>

src/Compilers/Core/Portable/CommandLine/CommandLineArguments.cs

@@ -127,6 +127,12 @@ public abstract class CommandLineArguments
         /// </summary>
 #nullable enable
         public ErrorLogOptions? ErrorLogOptions { get; internal set; }
+
+        /// <summary>
+        /// Options controlling the generation of a SARIF log file containing compilation or
+        /// analysis diagnostics, or null if no log file is desired.
+        /// </summary>
+        public string? ErrorLogPath => ErrorLogOptions?.Path;
 #nullable restore
 
         /// <summary>

src/Compilers/Core/Portable/DiagnosticAnalyzer/AnalyzerManager.cs

@@ -308,12 +308,19 @@ internal bool IsDiagnosticAnalyzerSuppressed(
                 // Is this diagnostic suppressed by default (as written by the rule author)
                 var isSuppressed = !diag.IsEnabledByDefault;
 
-                // If the user said something about it, that overrides the author.
+                // Compilation wide user settings from ruleset/nowarn/warnaserror overrides the analyzer author.
                 if (diagnosticOptions.TryGetValue(diag.Id, out var severity))
                 {
                     isSuppressed = severity == ReportDiagnostic.Suppress;
                 }
 
+                // Editorconfig user settings override compilation wide settings.
+                if (isSuppressed &&
+                    isEnabledWithAnalyzerConfigOptions(diag.Id, analyzerExecutor.Compilation))
+                {
+                    isSuppressed = false;
+                }
+
                 if (!isSuppressed)
                 {
                     return false;
@@ -332,6 +339,23 @@ internal bool IsDiagnosticAnalyzerSuppressed(
             }
 
             return true;
+
+            static bool isEnabledWithAnalyzerConfigOptions(string diagnosticId, Compilation compilation)
+            {
+                if (compilation != null)
+                {
+                    foreach (var tree in compilation.SyntaxTrees)
+                    {
+                        if (tree.DiagnosticOptions.TryGetValue(diagnosticId, out var configuredValue) &&
+                            configuredValue != ReportDiagnostic.Suppress)
+                        {
+                            return true;
+                        }
+                    }
+                }
+
+                return false;
+            }
         }
 
         internal static bool HasNotConfigurableTag(IEnumerable<string> customTags)

src/EditorFeatures/CSharpTest/DisposeAnalysis/DisposableFieldsShouldBeDisposedTests.cs

@@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.DisposeAnalysis
     public sealed class DisposableFieldsShouldBeDisposedTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest
     {
         internal override (DiagnosticAnalyzer, CodeFixProvider) CreateDiagnosticProviderAndFixer(Workspace workspace)
-            => (new DisposableFieldsShouldBeDisposedDiagnosticAnalyzer(), null);
+            => (new DisposableFieldsShouldBeDisposedDiagnosticAnalyzer(isEnabledByDefault: true), null);
 
         private Task TestDiagnosticsAsync(string initialMarkup, params DiagnosticDescription[] expectedDiagnostics)
             => TestDiagnosticsAsync(initialMarkup, parseOptions: null, expectedDiagnostics);