Static analysis: protect against mutating fields in hash calculations

Part of #26805.
dotnet · Dec 12, 2021 · ed5ae5c · ed5ae5c
1 parent 1fa736f
commit ed5ae5c
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 52 deletions.
diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.Helper.cs
@@ -340,6 +340,7 @@ public AliasUniquifier(HashSet<string> usedAliases)
 
     private sealed class TableReferenceExpression : Expression
     {
+        private int? _hashCode;
         private SelectExpression _selectExpression;
 
         public TableReferenceExpression(SelectExpression selectExpression, string alias)
@@ -392,7 +393,15 @@ private bool Equals(TableReferenceExpression tableReferenceExpression)
 
         /// <inheritdoc />
         public override int GetHashCode()
-            => Alias.GetHashCode();
+        {
+            // ReSharper disable NonReadonlyMemberInGetHashCode
+            // Ensure hashcode does not change after it has been created for this object.
+            Check.DebugAssert(
+                _hashCode == null || _hashCode == HashCode.Combine(Alias),
+                "Hashed value has mutated.");
+
+            return _hashCode ??= HashCode.Combine(Alias);
+        }
     }
 
     private sealed class ConcreteColumnExpression : ColumnExpression

diff --git a/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs b/src/EFCore.Relational/Query/SqlExpressions/SelectExpression.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Runtime.CompilerServices;
 using Microsoft.EntityFrameworkCore.Metadata.Internal;
 using Microsoft.EntityFrameworkCore.Query.Internal;
 
@@ -3500,54 +3501,6 @@ private bool Equals(SelectExpression selectExpression)
 
     /// <inheritdoc />
     public override int GetHashCode()
-    {
-        var hash = new HashCode();
-        hash.Add(base.GetHashCode());
-
-        foreach (var projection in _projection)
-        {
-            hash.Add(projection);
-        }
-
-        foreach (var projection in _clientProjections)
-        {
-            hash.Add(projection);
-        }
-
-        foreach (var projectionMapping in _projectionMapping)
-        {
-            hash.Add(projectionMapping.Key);
-            hash.Add(projectionMapping.Value);
-        }
-
-        foreach (var tag in Tags)
-        {
-            hash.Add(tag);
-        }
-
-        foreach (var table in _tables)
-        {
-            hash.Add(table);
-        }
-
-        hash.Add(Predicate);
-
-        foreach (var groupingKey in _groupBy)
-        {
-            hash.Add(groupingKey);
-        }
-
-        hash.Add(Having);
-
-        foreach (var ordering in _orderings)
-        {
-            hash.Add(ordering);
-        }
-
-        hash.Add(Offset);
-        hash.Add(Limit);
-        hash.Add(IsDistinct);
-
-        return hash.ToHashCode();
-    }
+        // Since equality above is reference equality, hash code can also be based on reference.
+        => RuntimeHelpers.GetHashCode(this);
 }
diff --git a/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs b/src/EFCore.Relational/Query/SqlExpressions/TableExpressionBase.cs
@@ -14,6 +14,8 @@ namespace Microsoft.EntityFrameworkCore.Query.SqlExpressions;
 /// </summary>
 public abstract class TableExpressionBase : Expression, IPrintableExpression
 {
+    private int? _hashCode;
+
     /// <summary>
     ///     Creates a new instance of the <see cref="TableExpressionBase" /> class.
     /// </summary>
@@ -62,5 +64,13 @@ private bool Equals(TableExpressionBase tableExpressionBase)
 
     /// <inheritdoc />
     public override int GetHashCode()
-        => HashCode.Combine(Alias);
+    {
+        // ReSharper disable NonReadonlyMemberInGetHashCode
+        // Ensure hashcode does not change after it has been created for this object.
+        Check.DebugAssert(
+            _hashCode == null || _hashCode == HashCode.Combine(Alias),
+            "Hashed value has mutated.");
+
+        return _hashCode ??= HashCode.Combine(Alias);
+    }
 }