Drop unused IEquatable from range wrappers and fix FoldingReference (#2302)

* Drop unused `IEquatable` from range wrappers and fix `FoldingReference`

We had silenced CA1067 (types implementing `IEquatable<T>` without overriding
`object.Equals`/`GetHashCode`) instead of fixing it. Promote it to `error` and
resolve the offenders at the root.

The four wrapper structs in `EditorFileRanges.cs` — `OmnisharpLspPosition`,
`OmnisharpLspRange`, `BufferFilePosition`, and `BufferFileRange` — are
`internal`, only ever surfaced through their interfaces
(`ILspFilePosition`/`IFileRange`/etc.), and never compared anywhere. Their
`IEquatable<T>` (added reflexively in #1183) was dead code, and the parallel
public classes (`FilePosition`/`LspFilePosition`/...) never implemented it. So
rather than completing the contract with `Equals(object)`/`GetHashCode`/
operators, I dropped `IEquatable<T>` and the typed `Equals` entirely — less
code, and CA1067 no longer applies to them. OmniSharp can't reach them either:
they're `internal` (unnameable for any generic equality), `InternalsVisibleTo`
lists only first-party assemblies, and the `ToOmnisharp*` conversions hand back
OmniSharp's own types, so an instance never flows back into the library.

`FoldingReference` is the one type that genuinely participates in comparison
(`Array.Sort` via `SafeAdd`, and xUnit `Assert.Equal` in the folding tests), so
it keeps `IEquatable`/`IComparable` and gets the real fix:

- `Equals(FoldingReference other)` was `=> CompareTo(other) == 0`, and
  `CompareTo` dereferences its argument, so `Equals(null)` and `CompareTo(null)`
  threw a `NullReferenceException` instead of returning `false`/`1`. Both are
  now null-safe.
- Added `Equals(object)` and a netstandard2.0-compatible `GetHashCode` (manual
  XOR, since `System.HashCode` is unavailable on that target).

Added focused tests covering the `FoldingReference` null-safety, value equality,
and hash-code consistency.

Drafted by Copilot (Claude Opus 4.8).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix `FoldingReference.CompareTo` antisymmetry for differing kinds

The Copilot PR reviewer noticed that when two references share the same
range but have different non-null `Kind`s, `CompareTo` fell through to
`return -1` for both `a.CompareTo(b)` and `b.CompareTo(a)`. That violates
the `IComparable` antisymmetry contract and can yield unstable ordering in
`Array.Sort`.

We now order differing non-null kinds deterministically with
`string.CompareOrdinal` over their string representations (stable LSP
values like `comment` and `region`), so the two directions always have
opposite signs. The null-vs-kind cases are unchanged: a reference without
a kind still sorts after one that has a kind.

Added a test asserting the comparison is antisymmetric for differing kinds.

Drafted by Copilot (Claude Opus 4.8).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: andyleejordan

.editorconfig

@@ -48,7 +48,7 @@ dotnet_diagnostic.CS1998.severity = suggestion
 dotnet_diagnostic.CS4014.severity = suggestion
 
 # CA1067: Should override Equals because it implements IEquatable<T>
-dotnet_diagnostic.CA1067.severity = silent
+dotnet_diagnostic.CA1067.severity = error
 # CA1068: CancellationToken parameters must come last
 dotnet_diagnostic.CA1068.severity = error
 # CA1501: Avoid excessive inheritance

src/PowerShellEditorServices/Extensions/EditorFileRanges.cs

@@ -274,7 +274,7 @@ public interface ILspCurrentFileContext : IFileContext
         ILspFileRange SelectionRange { get; }
     }
 
-    internal readonly struct OmnisharpLspPosition : ILspFilePosition, IEquatable<OmnisharpLspPosition>
+    internal readonly struct OmnisharpLspPosition : ILspFilePosition
     {
         private readonly Position _position;
 
@@ -283,11 +283,9 @@ public interface ILspCurrentFileContext : IFileContext
         public int Line => _position.Line;
 
         public int Character => _position.Character;
-
-        public bool Equals(OmnisharpLspPosition other) => _position == other._position;
     }
 
-    internal readonly struct OmnisharpLspRange : ILspFileRange, IEquatable<OmnisharpLspRange>
+    internal readonly struct OmnisharpLspRange : ILspFileRange
     {
         private readonly Range _range;
 
@@ -296,11 +294,9 @@ public interface ILspCurrentFileContext : IFileContext
         public ILspFilePosition Start => new OmnisharpLspPosition(_range.Start);
 
         public ILspFilePosition End => new OmnisharpLspPosition(_range.End);
-
-        public bool Equals(OmnisharpLspRange other) => _range == other._range;
     }
 
-    internal readonly struct BufferFilePosition : IFilePosition, IEquatable<BufferFilePosition>
+    internal readonly struct BufferFilePosition : IFilePosition
     {
         private readonly BufferPosition _position;
 
@@ -309,15 +305,9 @@ public interface ILspCurrentFileContext : IFileContext
         public int Line => _position.Line;
 
         public int Column => _position.Column;
-
-        public bool Equals(BufferFilePosition other)
-        {
-            return _position == other._position
-                || _position.Equals(other._position);
-        }
     }
 
-    internal readonly struct BufferFileRange : IFileRange, IEquatable<BufferFileRange>
+    internal readonly struct BufferFileRange : IFileRange
     {
         private readonly BufferRange _range;
 
@@ -326,12 +316,6 @@ public bool Equals(BufferFilePosition other)
         public IFilePosition Start => new BufferFilePosition(_range.Start);
 
         public IFilePosition End => new BufferFilePosition(_range.End);
-
-        public bool Equals(BufferFileRange other)
-        {
-            return _range == other._range
-                || _range.Equals(other._range);
-        }
     }
 
     /// <summary>

src/PowerShellEditorServices/Services/TextDocument/FoldingReference.cs

@@ -42,6 +42,9 @@ internal class FoldingReference : IComparable<FoldingReference>, IEquatable<Fold
         /// </summary>
         public int CompareTo(FoldingReference that)
         {
+            // A null instance sorts before (is less than) any actual reference.
+            if (that is null) { return 1; }
+
             // Initially look at the start line
             if (StartLine < that.StartLine) { return -1; }
             if (StartLine > that.StartLine) { return 1; }
@@ -57,23 +60,30 @@ public int CompareTo(FoldingReference that)
             if (EndCharacter < that.EndCharacter) { return -1; }
             if (EndCharacter > that.EndCharacter) { return 1; }
 
-            // They're the same range, but what about kind
-            if (Kind == that.Kind)
-            {
-                return 0;
-            }
+            // They're the same range, so now consider the kind. Equal kinds
+            // (including both null) compare as equal.
+            if (Kind == that.Kind) { return 0; }
 
-            // That has a kind but this doesn't.
-            if (Kind is null && that.Kind is not null)
-            {
-                return 1;
-            }
+            // A reference without a kind sorts after one that has a kind.
+            if (Kind is null) { return 1; }
+            if (that.Kind is null) { return -1; }
 
-            // This has a kind but that doesn't.
-            return -1;
+            // Both have a kind but they differ: order deterministically by their
+            // string representation so the comparison stays antisymmetric (i.e.
+            // a.CompareTo(b) and b.CompareTo(a) have opposite signs).
+            return string.CompareOrdinal(Kind.Value.ToString(), that.Kind.Value.ToString());
         }
 
-        public bool Equals(FoldingReference other) => CompareTo(other) == 0;
+        public bool Equals(FoldingReference other) => other is not null && CompareTo(other) == 0;
+
+        public override bool Equals(object obj) => Equals(obj as FoldingReference);
+
+        public override int GetHashCode() =>
+            StartLine.GetHashCode()
+            ^ StartCharacter.GetHashCode()
+            ^ EndLine.GetHashCode()
+            ^ EndCharacter.GetHashCode()
+            ^ (Kind?.GetHashCode() ?? 0);
     }
 
     /// <summary>

test/PowerShellEditorServices.Test/Language/FoldingReferenceEqualityTests.cs

@@ -0,0 +1,88 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using Microsoft.PowerShell.EditorServices.Services.TextDocument;
+using OmniSharp.Extensions.LanguageServer.Protocol.Models;
+using Xunit;
+
+namespace PowerShellEditorServices.Test.Language
+{
+    public class FoldingReferenceEqualityTests
+    {
+        private static FoldingReference CreateFoldingReference() => new()
+        {
+            StartLine = 1,
+            StartCharacter = 2,
+            EndLine = 3,
+            EndCharacter = 4,
+            Kind = FoldingRangeKind.Region
+        };
+
+        [Fact]
+        public void EqualsNullReferenceReturnsFalse()
+        {
+            FoldingReference reference = CreateFoldingReference();
+            FoldingReference other = null;
+            // Previously this threw a NullReferenceException via CompareTo.
+            Assert.False(reference.Equals(other));
+        }
+
+        [Fact]
+        public void EqualsNullObjectReturnsFalse()
+        {
+            FoldingReference reference = CreateFoldingReference();
+            Assert.False(reference.Equals((object)null));
+        }
+
+        [Fact]
+        public void CompareToNullReturnsOne()
+        {
+            FoldingReference reference = CreateFoldingReference();
+            // A null instance sorts before any actual reference.
+            Assert.Equal(1, reference.CompareTo(null));
+        }
+
+        [Fact]
+        public void EqualReferencesAreEqualWithSameHashCode()
+        {
+            FoldingReference first = CreateFoldingReference();
+            FoldingReference second = CreateFoldingReference();
+            Assert.True(first.Equals(second));
+            Assert.True(first.Equals((object)second));
+            Assert.Equal(first.GetHashCode(), second.GetHashCode());
+        }
+
+        [Fact]
+        public void DifferentReferencesAreNotEqual()
+        {
+            FoldingReference first = CreateFoldingReference();
+            FoldingReference second = CreateFoldingReference();
+            second.EndLine = 42;
+            Assert.False(first.Equals(second));
+        }
+
+        [Fact]
+        public void EqualsDifferentTypeReturnsFalse()
+        {
+            FoldingReference reference = CreateFoldingReference();
+            Assert.False(reference.Equals("not a folding reference"));
+        }
+
+        [Fact]
+        public void CompareToWithDifferingKindsIsAntisymmetric()
+        {
+            FoldingReference comment = CreateFoldingReference();
+            comment.Kind = FoldingRangeKind.Comment;
+            FoldingReference region = CreateFoldingReference();
+            region.Kind = FoldingRangeKind.Region;
+
+            // Same range but different (non-null) kinds must order
+            // deterministically: the comparisons must have opposite signs so
+            // Array.Sort stays stable. Previously both returned -1.
+            int forward = comment.CompareTo(region);
+            int backward = region.CompareTo(comment);
+            Assert.NotEqual(0, forward);
+            Assert.Equal(-System.Math.Sign(forward), System.Math.Sign(backward));
+        }
+    }
+}