Read a BigInteger's bytes as bytes, so converting one to a Number stops throwing (#585)

BREAKING-CHANGES.md

@@ -15,6 +15,40 @@ read first.
 
 ---
 
+## Unreleased — since 2.2.0
+
+### At a glance
+
+| Silent? | What | Was | Is |
+|---|---|---|---|
+| | `(Entity.Number)someBigInteger` | `FormatException: Illegal character found`, for almost every value | the number |
+
+### A `BigInteger` converts to a `Number` instead of throwing
+
+`Entity` and `Entity.Number` both offer an implicit conversion from
+`System.Numerics.BigInteger`, and they did not agree. `Entity`'s read the two's-complement bytes;
+`Number`'s passed the same bytes to `EInteger.FromString`, whose `byte[]` overload reads them as
+**ASCII digits**:
+
+```csharp
+Entity        fine  = new BigInteger(123456789);   // 123456789
+Entity.Number threw = new BigInteger(123456789);   // FormatException: Illegal character found
+```
+
+So the conversion failed for every value whose bytes are not digit characters — which is nearly all
+of them, `1` included. The few that worked did so by accident: `12594` is the two bytes `'2'` and
+`'1'`, and came out as `21`.
+
+Nothing in the library reached it, which is why no test caught it; it is only on the public surface.
+Both conversions now read the bytes, and a test requires the two to agree rather than merely to
+work.
+
+Found while documenting the members a `#pragma warning disable CS1591` was covering — the two
+conversions sit in different files and had to be read side by side to look wrong.
+[#585](https://github.com/asc-community/AngouriMath/issues/585).
+
+---
+
 ## 2.2.0 — since 2.1.0
 
 ### At a glance

Sources/AngouriMath/Core/Entity/Continuous/Entity.Continuous.Number.Definition.cs

@@ -42,26 +42,84 @@ public abstract partial record Number : ContinuousNode
             /// For example, integers and rationals are such
             /// </summary>
             public abstract bool IsExact { get; }
-#pragma warning disable CS1591 // Missing XML comment for publicly visible type or member
+            // Each of these lands on the narrowest kind that holds the value exactly: the
+            // integral types on Integer, a ratio on Rational, the rest on Real. Number is
+            // abstract, so what comes back is always one of its concrete kinds.
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(sbyte value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(byte value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(short value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(ushort value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(int value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(uint value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(long value) => Integer.Create(value);
+
+            /// <summary>The number as an <see cref="Integer"/>.</summary>
             public static implicit operator Number(ulong value) => Integer.Create(value);
+
+            /// <summary>
+            /// The integer as an <see cref="Integer"/>, of any size — this is the conversion to
+            /// reach for where a value will not fit in a <see langword="long"/>.
+            /// </summary>
             public static implicit operator Number(EInteger value) => Integer.Create(value);
+
+            /// <summary>
+            /// The ratio as a <see cref="Rational"/>, in lowest terms and with a positive
+            /// denominator — or an <see cref="Integer"/> where the denominator reduces to one.
+            /// </summary>
             public static implicit operator Number(ERational value) => Rational.Create(value);
+
+            /// <summary>The decimal as a <see cref="Real"/>.</summary>
             public static implicit operator Number(EDecimal value) => Real.Create(value);
+
+            /// <summary>
+            /// The value as a <see cref="Real"/>, read as the binary <see langword="float"/> it
+            /// is — so <c>0.1f</c> becomes the value that literal holds, not one tenth.
+            /// </summary>
             public static implicit operator Number(float value) => Real.Create(EDecimal.FromSingle(value));
+
+            /// <summary>
+            /// The value as a <see cref="Real"/>, read as the binary <see langword="double"/> it
+            /// is rather than as the decimal it was written as.
+            /// </summary>
             public static implicit operator Number(double value) => Real.Create(EDecimal.FromDouble(value));
+
+            /// <summary>
+            /// The value as a <see cref="Real"/>. A <see langword="decimal"/> is decimal
+            /// already, so this one keeps the digits that were written.
+            /// </summary>
             public static implicit operator Number(decimal value) => Real.Create(EDecimal.FromDecimal(value));
+
+            /// <summary>
+            /// The .NET complex number as a <see cref="Complex"/>, through its two
+            /// <see langword="double"/> parts and their precision.
+            /// </summary>
             public static implicit operator Number(System.Numerics.Complex value)
                 => Complex.Create(EDecimal.FromDouble(value.Real), EDecimal.FromDouble(value.Imaginary));
+            /// <summary>The big integer as an <see cref="Integer"/>, exactly and at any size.</summary>
+            /// <remarks>
+            /// Read as the two's-complement bytes it is, which is what
+            /// <c>BigInteger.ToByteArray</c> returns and what the same conversion on
+            /// <see cref="Entity"/> has always done. The overload reached before was
+            /// <c>EInteger.FromString(byte[])</c>, which reads a byte array as ASCII digits, so
+            /// every value whose bytes are not digit characters threw a <c>FormatException</c> —
+            /// which is nearly all of them, 1 included.
+            /// </remarks>
             public static implicit operator Number(System.Numerics.BigInteger bigInt)
-                => Integer.Create(EInteger.FromString(bigInt.ToByteArray()));
-#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member
+                => Integer.Create(EInteger.FromBytes(bigInt.ToByteArray(), littleEndian: true));
         }
     }
 }

Sources/Tests/UnitTests/Core/BigIntegerConversionTest.cs

@@ -0,0 +1,75 @@
+//
+// Copyright (c) 2019-2026 Angouri.
+// AngouriMath is licensed under MIT.
+// Details: https://github.com/asc-community/AngouriMath/blob/master/LICENSE.md.
+// Website: https://am.angouri.org.
+//
+
+using System.Numerics;
+using AngouriMath;
+using Xunit;
+
+namespace AngouriMath.Tests.Core
+{
+    /// <summary>
+    /// The two conversions from <see cref="BigInteger"/>, which used to disagree: the one to
+    /// <see cref="Entity"/> read the bytes and the one to <see cref="Entity.Number"/> parsed
+    /// them as text, so the second threw on nearly every input.
+    /// </summary>
+    [Trait("Area", "Core")]
+    public sealed class BigIntegerConversionTest
+    {
+        public static readonly TheoryData<string> Values = new()
+        {
+            "0", "1", "-1", "2", "-2", "127", "128", "255", "256", "-128", "-129",
+            "123456789", "-123456789", "9223372036854775807", "9223372036854775808",
+            "-9223372036854775808", "-9223372036854775809",
+            "170141183460469231731687303715884105728",
+            "-170141183460469231731687303715884105728",
+        };
+
+        /// <summary>To <see cref="Entity.Number"/>, which is the one that was broken.</summary>
+        [Theory]
+        [MemberData(nameof(Values))]
+        public void ToNumber(string text)
+        {
+            Entity.Number converted = BigInteger.Parse(text);
+            Assert.Equal(text, converted.Stringize());
+        }
+
+        /// <summary>And to <see cref="Entity"/>, which was not, pinned so it stays that way.</summary>
+        [Theory]
+        [MemberData(nameof(Values))]
+        public void ToEntity(string text)
+        {
+            Entity converted = BigInteger.Parse(text);
+            Assert.Equal(text, converted.Stringize());
+        }
+
+        /// <summary>
+        /// And the two agree, which is the property whose absence was the defect: one conversion
+        /// of one value should not depend on which of them the caller reaches.
+        /// </summary>
+        [Theory]
+        [MemberData(nameof(Values))]
+        public void AndTheTwoAgree(string text)
+        {
+            var big = BigInteger.Parse(text);
+            Entity asEntity = big;
+            Entity.Number asNumber = big;
+            Assert.Equal(asEntity, (Entity)asNumber);
+        }
+
+        /// <summary>
+        /// A value whose bytes happen to be ASCII digits was the only kind that used to work,
+        /// and it worked by accident: 12594 is the two bytes '2' and '1', which the old code
+        /// read as the number 21.
+        /// </summary>
+        [Fact]
+        public void TheValuesThatUsedToWorkWorkedByAccident()
+        {
+            Entity.Number converted = new BigInteger(12594);
+            Assert.Equal("12594", converted.Stringize());
+        }
+    }
+}