From 5a9db52622189179d5feb115791dee9713861c27 Mon Sep 17 00:00:00 2001 From: "Robert H. Engelhardt" <1584973+rheone@users.noreply.github.com> Date: Wed, 6 Nov 2019 15:31:39 -0700 Subject: [PATCH] Address CA1036 for IPAddressRange and Subnet CA1036 states that IPAddressRange and Subnet should override the comparison operators, specifically op_Equality, op_Inequality, op_LessThan and op_GreaterThan. Added less than or equal to and greater than or equal to so as not to disappoint. Issue #31 --- src/Arcus.Tests/IPAddressRangeTests.cs | 130 +++++++++++++++++++- src/Arcus.Tests/SubnetTests.cs | 129 ++++++++++++++++---- src/Arcus/IPAddressRange.cs | 140 +++++++++++++++++++--- src/Arcus/Subnet.cs | 158 ++++++++++++++++++++----- 4 files changed, 491 insertions(+), 66 deletions(-) diff --git a/src/Arcus.Tests/IPAddressRangeTests.cs b/src/Arcus.Tests/IPAddressRangeTests.cs index 075cdeb..2dd13ba 100644 --- a/src/Arcus.Tests/IPAddressRangeTests.cs +++ b/src/Arcus.Tests/IPAddressRangeTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -94,10 +94,138 @@ public void Implementation_Test() Assert.True(typeof(IIPAddressRange).IsAssignableFrom(ipAddressRangeType)); Assert.True(typeof(IComparable).IsAssignableFrom(ipAddressRangeType)); Assert.True(typeof(IEquatable).IsAssignableFrom(ipAddressRangeType)); + Assert.True(typeof(IComparable).IsAssignableFrom(ipAddressRangeType)); } #endregion //end: Class + #region CompareTo / Operators + + public static IEnumerable Comparison_Values() + { + var ipv4Slash16 = Subnet.Parse("192.168.0.0/16"); + var ipv6Slash64 = Subnet.Parse("ab:cd::/64"); + var ipv4Slash20 = Subnet.Parse("192.168.0.0/20"); + var ipv6Slash96 = Subnet.Parse("ab:cd::/96"); + var ipv4All = Subnet.Parse("0.0.0.0/0"); + var ipv6All = Subnet.Parse("::/0"); + var ipv4Single = Subnet.Parse("0.0.0.0/32"); + var ipv6Single = Subnet.Parse("::/128"); + + yield return new object[] {0, new IPAddressRange(ipv4Slash16.Head, ipv4Slash16.Tail), new IPAddressRange(ipv4Slash16.Head, ipv4Slash16.Tail)}; + yield return new object[] {0, new IPAddressRange(ipv6Slash64.Head, ipv6Slash64.Tail), new IPAddressRange(ipv6Slash64.Head, ipv6Slash64.Tail)}; + yield return new object[] {1, new IPAddressRange(ipv4Slash16.Head, ipv4Slash16.Tail), null}; + yield return new object[] {1, new IPAddressRange(ipv6Slash64.Head, ipv6Slash64.Tail), null}; + yield return new object[] {1, new IPAddressRange(ipv4Slash16.Head, ipv4Slash16.Tail), new IPAddressRange(ipv4Slash20.Head, ipv4Slash20.Tail)}; + yield return new object[] {-1, new IPAddressRange(ipv4Slash20.Head, ipv4Slash20.Tail), new IPAddressRange(ipv4Slash16.Head, ipv4Slash16.Tail)}; + yield return new object[] {1, new IPAddressRange(ipv6Slash64.Head, ipv6Slash64.Tail), new IPAddressRange(ipv6Slash96.Head, ipv6Slash96.Tail)}; + yield return new object[] {-1, new IPAddressRange(ipv6Slash96.Head, ipv6Slash96.Tail), new IPAddressRange(ipv6Slash64.Head, ipv6Slash64.Tail)}; + yield return new object[] {-1, new IPAddressRange(ipv4All.Head, ipv4All.Tail), new IPAddressRange(ipv6All.Head, ipv6All.Tail)}; + yield return new object[] {1, new IPAddressRange(ipv6All.Head, ipv6All.Tail), new IPAddressRange(ipv4All.Head, ipv4All.Tail)}; + yield return new object[] {-1, new IPAddressRange(ipv4Single.Head, ipv4Single.Tail), new IPAddressRange(ipv6Single.Head, ipv6Single.Tail)}; + yield return new object[] {1, new IPAddressRange(ipv6Single.Head, ipv6Single.Tail), new IPAddressRange(ipv4Single.Head, ipv4Single.Tail)}; + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void CompareTo_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left.CompareTo(right); + + // Assert + Assert.Equal(expected, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_Equals_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left == right; + + // Assert + Assert.Equal(expected == 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_NotEquals_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left != right; + + // Assert + Assert.Equal(expected != 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_GreaterThan_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left > right; + + // Assert + Assert.Equal(expected > 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_GreaterThanOrEqual_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left >= right; + + // Assert + Assert.Equal(expected >= 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_LessThan_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left < right; + + // Assert + Assert.Equal(expected < 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_LessThanOrEqual_Test(int expected, + IPAddressRange left, + IPAddressRange right) + { + // Arrange + // Act + var result = left <= right; + + // Assert + Assert.Equal(expected <= 0, result); + } + + #endregion end CompareTo / Operators + #region TailOverlappedBy [Theory] diff --git a/src/Arcus.Tests/SubnetTests.cs b/src/Arcus.Tests/SubnetTests.cs index 2924940..bb80866 100644 --- a/src/Arcus.Tests/SubnetTests.cs +++ b/src/Arcus.Tests/SubnetTests.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -59,42 +59,125 @@ public void Class_Implementation_Test() Assert.True(typeof(IIPAddressRange).IsAssignableFrom(subnetType)); Assert.True(typeof(IComparable).IsAssignableFrom(subnetType)); Assert.True(typeof(IEquatable).IsAssignableFrom(subnetType)); + Assert.True(typeof(IComparable).IsAssignableFrom(subnetType)); } #endregion // end: Class - #region CompareTo + #region CompareTo / Operators + + public static IEnumerable Comparison_Values() + { + yield return new object[] {0, Subnet.Parse("192.168.0.0/16"), Subnet.Parse("192.168.0.0/16")}; + yield return new object[] {0, Subnet.Parse("ab:cd::/64"), Subnet.Parse("ab:cd::/64")}; + yield return new object[] {1, Subnet.Parse("192.168.0.0/16"), null}; + yield return new object[] {1, Subnet.Parse("ab:cd::/64"), null}; + yield return new object[] {1, Subnet.Parse("192.168.0.0/16"), Subnet.Parse("192.168.0.0/20")}; + yield return new object[] {-1, Subnet.Parse("192.168.0.0/20"), Subnet.Parse("192.168.0.0/16")}; + yield return new object[] {1, Subnet.Parse("ab:cd::/64"), Subnet.Parse("ab:cd::/96")}; + yield return new object[] {-1, Subnet.Parse("ab:cd::/96"), Subnet.Parse("ab:cd::/64")}; + yield return new object[] {-1, Subnet.Parse("0.0.0.0/0"), Subnet.Parse("::/0")}; + yield return new object[] {1, Subnet.Parse("::/0"), Subnet.Parse("0.0.0.0/0")}; + yield return new object[] {-1, Subnet.Parse("0.0.0.0/32"), Subnet.Parse("::/128")}; + yield return new object[] {1, Subnet.Parse("::/128"), Subnet.Parse("0.0.0.0/32")}; + } [Theory] - [InlineData(0, "192.168.0.0/16", "192.168.0.0/16")] - [InlineData(0, "ab:cd::/64", "ab:cd::/64")] - [InlineData(1, "192.168.0.0/16", null)] - [InlineData(1, "ab:cd::/64", null)] - [InlineData(1, "192.168.0.0/16", "192.168.0.0/20")] - [InlineData(-1, "192.168.0.0/20", "192.168.0.0/16")] - [InlineData(1, "ab:cd::/64", "ab:cd::/96")] - [InlineData(-1, "ab:cd::/96", "ab:cd::/64")] - [InlineData(-1, "0.0.0.0/0", "::/0")] - [InlineData(1, "::/0", "0.0.0.0/0")] - [InlineData(-1, "0.0.0.0/32", "::/128")] - [InlineData(1, "::/128", "0.0.0.0/32")] + [MemberData(nameof(Comparison_Values))] public void CompareTo_Test(int expected, - string x, - string y) + Subnet left, + Subnet right) { // Arrange - var thisSubnet = Subnet.Parse(x); + // Act + var result = left.CompareTo(right); - if (!Subnet.TryParse(y, out var other)) - { - other = null; - } + // Assert + Assert.Equal(expected, result); + } + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_Equals_Test(int expected, + Subnet left, + Subnet right) + { + // Arrange // Act - var result = thisSubnet.CompareTo(other); + var result = left == right; // Assert - Assert.Equal(expected, result); + Assert.Equal(expected == 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_NotEquals_Test(int expected, + Subnet left, + Subnet right) + { + // Arrange + // Act + var result = left != right; + + // Assert + Assert.Equal(expected != 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_GreaterThan_Test(int expected, + Subnet left, + Subnet right) + { + // Arrange + // Act + var result = left > right; + + // Assert + Assert.Equal(expected > 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_GreaterThanOrEqual_Test(int expected, + Subnet left, + Subnet right) + { + // Arrange + // Act + var result = left >= right; + + // Assert + Assert.Equal(expected >= 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_LessThan_Test(int expected, + Subnet left, + Subnet right) + { + // Arrange + // Act + var result = left < right; + + // Assert + Assert.Equal(expected < 0, result); + } + + [Theory] + [MemberData(nameof(Comparison_Values))] + public void Operator_LessThanOrEqual_Test(int expected, + Subnet left, + Subnet right) + { + // Arrange + // Act + var result = left <= right; + + // Assert + Assert.Equal(expected <= 0, result); } #endregion diff --git a/src/Arcus/IPAddressRange.cs b/src/Arcus/IPAddressRange.cs index a615dbc..48e371f 100644 --- a/src/Arcus/IPAddressRange.cs +++ b/src/Arcus/IPAddressRange.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Net; @@ -15,8 +15,31 @@ namespace Arcus [PublicAPI] public class IPAddressRange : AbstractIPAddressRange, IEquatable, - IComparable + IComparable, + IComparable { + #region From Interface IComparable + + /// + public int CompareTo(object obj) + { + if (ReferenceEquals(null, obj)) + { + return 1; + } + + if (ReferenceEquals(this, obj)) + { + return 0; + } + + return obj is IPAddressRange other + ? this.CompareTo(other) + : throw new ArgumentException($"Object must be of type {nameof(IPAddressRange)}"); + } + + #endregion + #region From Interface IComparable /// @@ -34,8 +57,8 @@ public bool Equals(IPAddressRange other) { return !ReferenceEquals(null, other) && (ReferenceEquals(this, other) - || (Equals(Head, other.Head) - && Equals(Tail, other.Tail))); + || Equals(Head, other.Head) + && Equals(Tail, other.Tail)); } #endregion @@ -45,8 +68,8 @@ public override bool Equals(object obj) { return !ReferenceEquals(null, obj) && (ReferenceEquals(this, obj) - || (obj.GetType() == GetType() - && this.Equals((IPAddressRange) obj))); + || obj.GetType() == GetType() + && this.Equals((IPAddressRange) obj)); } /// @@ -58,12 +81,96 @@ public override int GetHashCode() } } + #region operators + + /// + /// Compares two objects for equality + /// + /// left hand operand + /// right hand operand + /// when both sides are equal + public static bool operator ==(IPAddressRange left, + IPAddressRange right) + { + return Equals(left, right); + } + + /// + /// Compares two objects for non-equality + /// + /// left hand operand + /// right hand operand + /// when both sides are not equal + public static bool operator !=(IPAddressRange left, + IPAddressRange right) + { + return !Equals(left, right); + } + + /// + /// Compares two objects for being less than + /// + /// + /// left hand operand + /// right hand operand + /// when is less than + public static bool operator <(IPAddressRange left, + IPAddressRange right) + { + return Comparer.Default.Compare(left, right) < 0; + } + + /// + /// Compares two objects for being greater than + /// + /// + /// left hand operand + /// right hand operand + /// when is greater than + public static bool operator >(IPAddressRange left, + IPAddressRange right) + { + return Comparer.Default.Compare(left, right) > 0; + } + + /// + /// Compares two objects for being less than or equal + /// + /// + /// left hand operand + /// right hand operand + /// + /// when is less than or equal to + /// + public static bool operator <=(IPAddressRange left, + IPAddressRange right) + { + return Comparer.Default.Compare(left, right) <= 0; + } + + /// + /// Compares two objects for being greater than or equal to + /// + /// + /// left hand operand + /// right hand operand + /// + /// when is greater than or equal to + /// + public static bool operator >=(IPAddressRange left, + IPAddressRange right) + { + return Comparer.Default.Compare(left, right) >= 0; + } + + #endregion end operators + #region Ctor /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - /// the + /// the public IPAddressRange([NotNull] IPAddress address) : base(address, address) { @@ -71,10 +178,10 @@ public IPAddressRange([NotNull] IPAddress address) } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// - /// head - /// tail + /// head + /// tail public IPAddressRange([NotNull] IPAddress head, [NotNull] IPAddress tail) : base(head, tail) @@ -156,9 +263,12 @@ public static bool TryCollapseAll(IEnumerable ranges, /// excluded ranges are expected to each be sub ranges of the initial range /// an attempt will be made to TryCollapseAll the excluded /// - /// the initial to exclude from - /// the various to exclude from the - /// the resulting + /// the initial to exclude from + /// + /// the various to exclude from the + /// + /// + /// the resulting /// unexpected invalid operation. public static bool TryExcludeAll(IPAddressRange initialRange, IEnumerable excludedRanges, @@ -271,7 +381,7 @@ public static bool TryExcludeAll(IPAddressRange initialRange, /// /// the left operand /// the right operand - /// the resulting + /// the resulting public static bool TryMerge(IPAddressRange left, IPAddressRange right, out IPAddressRange mergedRange) diff --git a/src/Arcus/Subnet.cs b/src/Arcus/Subnet.cs index 6aa6ac7..b1813a6 100644 --- a/src/Arcus/Subnet.cs +++ b/src/Arcus/Subnet.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.Globalization; using System.Linq; @@ -21,7 +21,8 @@ namespace Arcus [PublicAPI] public class Subnet : AbstractIPAddressRange, IEquatable, - IComparable + IComparable, + IComparable { /// /// Pattern that passes on valid IPv4 octet partials @@ -74,6 +75,28 @@ public class Subnet : AbstractIPAddressRange, /// public int RoutingPrefix { get; } + #region From Interface IComparable + + /// + public int CompareTo(object obj) + { + if (ReferenceEquals(null, obj)) + { + return 1; + } + + if (ReferenceEquals(this, obj)) + { + return 0; + } + + return obj is Subnet other + ? this.CompareTo(other) + : throw new ArgumentException($"Object must be of type {nameof(Subnet)}"); + } + + #endregion + #region From Interface IComparable /// @@ -162,7 +185,7 @@ public bool Overlaps(Subnet subnet) #region Ctor /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// Construct the smallest possible subnet that would contain both IP addresses /// typically the address specified are the Network and Broadcast addresses /// (lower and higher bounds) but this is not necessary. @@ -223,7 +246,7 @@ private static AddressTuple CtorFactory(IPAddress lowAddress, } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// /// the ip address /// the routing prefix @@ -279,7 +302,7 @@ private static AddressTuple CtorFactory([NotNull] IPAddress address, } /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// contains only a single ip address /// /// the ip address @@ -382,8 +405,8 @@ public static bool TryFromNetMask(IPAddress address, /// specified are the Network and Broadcast addresses (lower and higher bounds) but this is not necessary. Addresses /// *MUST* be the same address family (either Internetwork or InternetworkV6) /// - /// the lower address array - /// the high address array + /// the lower address array + /// the high address array public static Subnet FromBytes([NotNull] byte[] lowAddressBytes, [NotNull] byte[] highAddressBytes) { @@ -432,8 +455,8 @@ public static Subnet FromBytes([NotNull] byte[] lowAddressBytes, /// specified are the Network and Broadcast addresses (lower and higher bounds) but this is not necessary. Addresses /// *MUST* be the same address family (either Internetwork or InternetworkV6) /// - /// the lower address array - /// the high address array + /// the lower address array + /// the high address array /// the created subnet or on failure /// on success public static bool TryFromBytes(byte[] lowAddressBytes, @@ -567,10 +590,15 @@ public static bool TryParse(string subnetString, /// /// the address string /// the subnet routing prefix - /// has an invalid - /// is . - /// is less than 0 - /// is is out of range of the provided + /// + /// has an invalid + /// + /// is . + /// is less than 0 + /// + /// is is out of range of the provided + /// + /// public static Subnet Parse([NotNull] string addressString, int routingPrefix) { @@ -780,7 +808,7 @@ public static bool TryIPv6FromPartial(string input, || (hextetCount = input.Split(new[] {':'}, StringSplitOptions.RemoveEmptyEntries) .Length) > IPAddressUtilities.IPv6HextetCount - || (hextetCount >= IPAddressUtilities.IPv6HextetCount && input.EndsWith(":", StringComparison.OrdinalIgnoreCase)) + || hextetCount >= IPAddressUtilities.IPv6HextetCount && input.EndsWith(":", StringComparison.OrdinalIgnoreCase) ) // too many hextets { subnets = Enumerable.Empty(); @@ -801,7 +829,7 @@ public static bool TryIPv6FromPartial(string input, // a IPv6 cidr partial is provided if ((IPAddress.TryParse(input, out var address) // treat as a complete address, may contain a '::' or not - || (input.Contains("::") && IPAddress.TryParse(input.TrimEnd(':'), out address)) + || input.Contains("::") && IPAddress.TryParse(input.TrimEnd(':'), out address) || IPAddress.TryParse(input.TrimEnd(':') + "::", out address)) && address.IsIPv6()) // no collapse, but incomplete address { @@ -815,11 +843,7 @@ public static bool TryIPv6FromPartial(string input, // get the index of the collapse var collapse = hextets.Select((value, - index) => new - { - value, - index - }) + index) => new { value, index }) .FirstOrDefault(pair => string.IsNullOrWhiteSpace(pair.value)); int collapseIndex; @@ -892,15 +916,13 @@ bool DoubleColonsAppearsMultipleTimes(string @in) #endregion // end: Static Factory Methods - #region equality and hashcode - /// public bool Equals(Subnet other) { return !ReferenceEquals(null, other) && (ReferenceEquals(this, other) - || (Equals(this.NetworkPrefixAddress, other.NetworkPrefixAddress) - && this.RoutingPrefix == other.RoutingPrefix)); + || Equals(this.NetworkPrefixAddress, other.NetworkPrefixAddress) + && this.RoutingPrefix == other.RoutingPrefix); } /// @@ -908,8 +930,8 @@ public override bool Equals(object obj) { return !ReferenceEquals(null, obj) && (ReferenceEquals(this, obj) - || (obj.GetType() == GetType() - && this.Equals((Subnet) obj))); + || obj.GetType() == GetType() + && this.Equals((Subnet) obj)); } /// @@ -925,7 +947,89 @@ public override int GetHashCode() } } - #endregion + #region operators + + /// + /// Compares two objects for equality + /// + /// left hand operand + /// right hand operand + /// when both sides are equal + public static bool operator ==(Subnet left, + Subnet right) + { + return Equals(left, right); + } + + /// + /// Compares two objects for non-equality + /// + /// left hand operand + /// right hand operand + /// when both sides are not equal + public static bool operator !=(Subnet left, + Subnet right) + { + return !Equals(left, right); + } + + /// + /// Compares two objects for being less than + /// + /// + /// left hand operand + /// right hand operand + /// when is less than + public static bool operator <(Subnet left, + Subnet right) + { + return Comparer.Default.Compare(left, right) < 0; + } + + /// + /// Compares two objects for being greater than + /// + /// + /// left hand operand + /// right hand operand + /// when is greater than + public static bool operator >(Subnet left, + Subnet right) + { + return Comparer.Default.Compare(left, right) > 0; + } + + /// + /// Compares two objects for being less than or equal + /// + /// + /// left hand operand + /// right hand operand + /// + /// when is less than or equal to + /// + public static bool operator <=(Subnet left, + Subnet right) + { + return Comparer.Default.Compare(left, right) <= 0; + } + + /// + /// Compares two objects for being greater than or equal to + /// + /// + /// left hand operand + /// right hand operand + /// + /// when is greater than or equal to + /// + public static bool operator >=(Subnet left, + Subnet right) + { + return Comparer.Default.Compare(left, right) >= 0; + } + + #endregion end operators #region Static metods, may be appropriate for extracting