Count() return wrong value when using ZLinq with ICollection<T> based collection.

[filzrev]

I'm trying to merge following System.Linq tests to ZLinq compatibility tests.
dotnet/runtime@087c690

But following edge case tests are failed on ZLinq.
(Currently it also failed with SystemLinq. Because these fix is not released yet)

• AppendOverflowCountWithICollection()
• AppendPrependNOverflowCountWithICollection()
• PrependOverflowCountWithICollection()

It seems following method needed to be modified. (with checked?)

• ZLinq/src/ZLinq/Linq/Prepend.cs

  Lines 36 to 43 in c6d15be

  	if (source.TryGetNonEnumeratedCount(out count) && count < int.MaxValue)
  	{
  	count++;
  	return true;
  	}
  	count = 0;
  	return false;
  	}

• ZLinq/src/ZLinq/Linq/Append.cs

  Lines 35 to 39 in c6d15be

  	if (source.TryGetNonEnumeratedCount(out count) && count < int.MaxValue)
  	{
  	count++;
  	return true;
  	}

TestCode

public class UnitTests
{
    [Fact]
    public void EdgeCaseTest()
    {
        // Arrange
        var source = new MockCollection<int>(int.MaxValue); // Use custom ICollection based collection.

        // Act
        var results = source.AsValueEnumerable().Append(1).Prepend(1);

        // Assert
        var count = results.Count(); // It should be failed by OverflowException.

        Assert.Equal(2, count); // 2 returned instead. it seems source.Count is not used.
    }
}

public sealed class MockCollection<T> : ICollection<T>
{
    private readonly int _count;

    public MockCollection(int count) => _count = count;

    public int Count => _count;
    public bool IsReadOnly => true;

    public void Add(T item) => throw new NotSupportedException();
    public void Clear() => throw new NotSupportedException();
    public bool Contains(T item) => false;
    public void CopyTo(T[] array, int arrayIndex) { }
    public bool Remove(T item) => throw new NotSupportedException();
    public IEnumerator<T> GetEnumerator() => Enumerable.Empty<T>().GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

[neuecc]

Thank you!
Adding checked to TryGetNonEnumeratedCount is too problematic due to the large side effects.
There's a possibility it could fail even in cases where it should be working correctly.

In the case of ZLinq, it determines that iteration is needed and goes through the GetEnumerator path, but that's returning Empty.
The mock itself is also poorly constructed.
Of course, iteration isn't necessary (if it were iterating, the test would never finish), so it's true that it would be better to return the result immediately as an Optimized Path, but...

I can't think of a viable implementation off the top of my head.

[prozolic]

As an indirect factor, I think the upper limit guard using int.MaxValue that I adjusted in #128 is also related.
For example, Removing this check and adding checked will cause it to throw OverflowException just like System.Linq does.

        public bool TryGetNonEnumeratedCount(out int count)
        {
            if (source.TryGetNonEnumeratedCount(out count))
            {
                checked { count++; }
                return true;
            }
            count = 0;
            return false;
        }

However, with code like #128, an OverflowException will be thrown from LongCount.

var intMaxSequence = Enumerable.Range(0, int.MaxValue); 
var countForZLinq = intMaxSequence.AsValueEnumerable().Append(1).LongCount();  //  OverflowException

If we remove the optimization path using TryGetNonEnumeratedCount, it should work fine, but it's difficult because it would degrade performance.

[github-actions]

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days.

[filzrev]

This issue is not stale.
It need to be fixed if possible for LINQ compatibility.