Merge remote-tracking branch 'upstream/master' into removeCastParams

src/Analyzers/CSharp/Tests/UseSystemHashCode/UseSystemHashCodeTests.cs

@@ -1260,6 +1260,57 @@ public override int GetHashCode()
         hash.Add(i);
         return hash.ToHashCode();
     }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSystemHashCode)]
+        public async Task TestNotOnSingleReturnedMember()
+        {
+            await TestMissingAsync(
+@"namespace System { public struct HashCode { } }
+
+class C
+{
+    int j;
+
+    public override int $$GetHashCode()
+    {
+        return j;
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSystemHashCode)]
+        public async Task TestNotOnSingleMemberWithInvokedGetHashCode()
+        {
+            await TestMissingAsync(
+@"namespace System { public struct HashCode { } }
+
+class C
+{
+    int j;
+
+    public override int $$GetHashCode()
+    {
+        return j.GetHashCode();
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseSystemHashCode)]
+        public async Task TestNotOnSimpleBaseReturn()
+        {
+            await TestMissingAsync(
+@"namespace System { public struct HashCode { } }
+
+class C
+{
+    int j;
+
+    public override int $$GetHashCode()
+    {
+        return base.GetHashCode();
+    }
 }");
         }
     }

src/Analyzers/Core/Analyzers/UseSystemHashCode/Analyzer.cs

@@ -58,34 +58,23 @@ public static bool TryGetAnalyzer(Compilation compilation, [NotNullWhen(true)] o
         /// Analyzes the containing <c>GetHashCode</c> method to determine which fields and
         /// properties were combined to form a hash code for this type.
         /// </summary>
-        public (bool accessesBase, ImmutableArray<ISymbol> members) GetHashedMembers(ISymbol owningSymbol, IOperation? operation)
+        public (bool accessesBase, ImmutableArray<ISymbol> members, ImmutableArray<IOperation> statements) GetHashedMembers(ISymbol owningSymbol, IOperation? operation)
         {
             if (!(operation is IBlockOperation blockOperation))
-            {
                 return default;
-            }
 
             // Owning symbol has to be an override of Object.GetHashCode.
             if (!(owningSymbol is IMethodSymbol { Name: nameof(GetHashCode) } method))
-            {
                 return default;
-            }
 
-            if (method.Locations.Length != 1 ||
-                method.DeclaringSyntaxReferences.Length != 1)
-            {
+            if (method.Locations.Length != 1 || method.DeclaringSyntaxReferences.Length != 1)
                 return default;
-            }
 
             if (!method.Locations[0].IsInSource)
-            {
                 return default;
-            }
 
             if (!OverridesSystemObject(method))
-            {
                 return default;
-            }
 
             // Unwind through nested blocks. This also handles if we're in an 'unchecked' block in C#
             while (blockOperation.Operations.Length == 1 &&
@@ -95,9 +84,12 @@ public static bool TryGetAnalyzer(Compilation compilation, [NotNullWhen(true)] o
             }
 
             var statements = blockOperation.Operations.WhereAsArray(o => !o.IsImplicit);
-            return MatchAccumulatorPattern(method, statements) ??
-                   MatchTuplePattern(method, statements) ??
-                   default;
+            var (accessesBase, members) =
+                MatchAccumulatorPattern(method, statements) ??
+                MatchTuplePattern(method, statements) ??
+                default;
+
+            return (accessesBase, members, statements);
         }
 
         private (bool accessesBase, ImmutableArray<ISymbol> members)? MatchTuplePattern(

src/Analyzers/Core/Analyzers/UseSystemHashCode/UseSystemHashCodeDiagnosticAnalyzer.cs

@@ -5,6 +5,7 @@
 #nullable enable
 
 using System.Collections.Immutable;
+using System.Diagnostics;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Diagnostics;
 
@@ -39,26 +40,37 @@ protected override void InitializeWorker(AnalysisContext context)
         private void AnalyzeOperationBlock(Analyzer analyzer, OperationBlockAnalysisContext context)
         {
             if (context.OperationBlocks.Length != 1)
-            {
                 return;
-            }
 
             var owningSymbol = context.OwningSymbol;
             var operation = context.OperationBlocks[0];
-            var (accessesBase, hashedMembers) = analyzer.GetHashedMembers(owningSymbol, operation);
-            if (!accessesBase && hashedMembers.IsDefaultOrEmpty)
-            {
+            var (accessesBase, hashedMembers, statements) = analyzer.GetHashedMembers(owningSymbol, operation);
+            var elementCount = (accessesBase ? 1 : 0) + (hashedMembers.IsDefaultOrEmpty ? 0 : hashedMembers.Length);
+
+            // No members to call into HashCode.Combine with.  Don't offer anything here.
+            if (elementCount == 0)
                 return;
-            }
+
+            // Just one member to call into HashCode.Combine. Only offer this if we have multiple statements that we can
+            // reduce to a single statement.  It's not worth it to offer to replace:
+            //
+            //      `return x.GetHashCode();` with `return HashCode.Combine(x);`
+            //
+            // But it is work it to offer to replace:
+            //
+            //      `return (a, b).GetHashCode();` with `return HashCode.Combine(a, b);`
+            if (elementCount == 1 && statements.Length < 2)
+                return;
+
+            // We've got multiple members to hash, or multiple statements that can be reduced at this point.
+            Debug.Assert(elementCount >= 2 || statements.Length >= 2);
 
             var syntaxTree = operation.Syntax.SyntaxTree;
             var cancellationToken = context.CancellationToken;
 
             var option = context.Options.GetOption(CodeStyleOptions2.PreferSystemHashCode, operation.Language, syntaxTree, cancellationToken);
             if (option?.Value != true)
-            {
                 return;
-            }
 
             var operationLocation = operation.Syntax.GetLocation();
             var declarationLocation = context.OwningSymbol.DeclaringSyntaxReferences[0].GetSyntax(cancellationToken).GetLocation();

src/Analyzers/Core/CodeFixes/UseSystemHashCode/UseSystemHashCodeCodeFixProvider.cs

@@ -74,7 +74,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
                 var method = semanticModel.GetDeclaredSymbol(methodDecl, cancellationToken);
                 var methodBlock = declarationService.GetDeclarations(method)[0].GetSyntax(cancellationToken);
 
-                var (accessesBase, members) = analyzer.GetHashedMembers(method, operation);
+                var (accessesBase, members, _) = analyzer.GetHashedMembers(method, operation);
                 if (accessesBase || !members.IsDefaultOrEmpty)
                 {
                     // Produce the new statements for the GetHashCode method and replace the

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Utilities/SpeculationAnalyzer.cs

@@ -426,6 +426,11 @@ protected override bool ReplacementChangesSemanticsForNodeLanguageSpecific(Synta
                 // expression type are not broken.
 
                 var newSwitchStatement = (SwitchStatementSyntax)currentReplacedNode;
+                var previousReplacedExpression = (ExpressionSyntax)previousReplacedNode;
+
+                // it is never legal to use `default/null` in a switch statement's expression.
+                if (previousReplacedExpression.WalkDownParentheses().IsKind(SyntaxKind.NullLiteralExpression, SyntaxKind.DefaultLiteralExpression))
+                    return true;
 
                 var originalSwitchLabels = originalSwitchStatement.Sections.SelectMany(section => section.Labels).ToArray();
                 var newSwitchLabels = newSwitchStatement.Sections.SelectMany(section => section.Labels).ToArray();