Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 ReadonlyCollectionPropertiesBehavior does not select collection Add method correctly #1339

Open
ArturDorochowicz opened this issue Apr 8, 2022 · 1 comment
Labels
bug triage Something that's being investigated

Comments

@ArturDorochowicz
Copy link

Describe the Bug

ReadonlyCollectionPropertiesBehavior does not select collection Add method correctly.

The ReadonlyCollectionPropertiesCommand class tries to select any method called Add:

foreach (var pi in this.PropertyQuery.SelectProperties(specimenType))
{
var addMethod = new InstanceMethodQuery(pi.GetValue(specimen), nameof(ICollection<object>.Add))
.SelectMethods()
.SingleOrDefault();
if (addMethod == null) continue;

This is incorrect in two ways:

  1. There may be more than one Add method which then causes an exception from SingleOrDefault
  2. It finds any method called Add. It should likely specifically select ICollection<>.Add (though that's probably still not fully specific as the type may implement multiple ICollection<>)

Scenario

In this scenario the readonly collection property is a Dictionary<,> which has multiple Add methods.

using System.Collections.Generic;
using System.Linq;
using AutoFixture;
using Xunit;

public class Bugs
{
    class MultipleAddMethodsModel
    {
        public Dictionary<int, int> Items { get; } = new();
    }

    [Fact]
    public void MultipleAddMethods()
    {
        var fixture = new Fixture();
        fixture.Behaviors.Add(new ReadonlyCollectionPropertiesBehavior());

        // Throws
        fixture.Create<MultipleAddMethodsModel>();
    }
}
/*
System.InvalidOperationException
Sequence contains more than one element
   at System.Linq.ThrowHelper.ThrowMoreThanOneElementException()
   at System.Linq.Enumerable.TryGetSingle[TSource](IEnumerable`1 source, Boolean& found)
   at AutoFixture.Kernel.ReadonlyCollectionPropertiesCommand.Execute(Object specimen, ISpecimenContext context)
   at AutoFixture.Kernel.Postprocessor`1.Create(Object request, ISpecimenContext context)
   at AutoFixture.Fixture.Create(Object request, ISpecimenContext context)
   at AutoFixture.Kernel.SpecimenContext.Resolve(Object request)
   at AutoFixture.Kernel.SeedIgnoringRelay.Create(Object request, ISpecimenContext context)
   at AutoFixture.Kernel.CompositeSpecimenBuilder.Create(Object request, ISpecimenContext context)
   at AutoFixture.Kernel.CompositeSpecimenBuilder.Create(Object request, ISpecimenContext context)
   at AutoFixture.Kernel.Postprocessor`1.Create(Object request, ISpecimenContext context)
   at AutoFixture.AutoPropertiesTarget.Create(Object request, ISpecimenContext context)
   at AutoFixture.Kernel.CompositeSpecimenBuilder.Create(Object request, ISpecimenContext context)
   at AutoFixture.Kernel.TerminatingWithPathSpecimenBuilder.Create(Object request, ISpecimenContext context)
*/

Expected Behavior

Code above should not throw. Items should be added to the dictionary.

Tasks

Using AutoFixture 4.17

@Ergamon
Copy link

Ergamon commented Apr 14, 2022

You already figured out the problem.

The current source code expects the following:
The ReadOnlyCollection implements ICollection and has public Add(T item) method.

As you are using a Dictionary<TKey, TValue> that is not the case. As this class implements ICollection<KeyValuePair<TKey, TValue>> and explicitly implements the interface.

Simplified AutoFixture uses the following code:

MethodInfo GetAddMethod(object obj)
{
  var type = obj.GetType();

  return type.GetMethod(nameof(ICollection<object>.Add));
}

while it should use this

static MethodInfo GetAddMethod(object obj)
{
  var type = obj.GetType();

  var collectionType = type.GetInterfaces()
    .Where(i => i.IsGenericType)
    .Single(i => i.GetGenericTypeDefinition() == typeof(ICollection<>));

  return collectionType.GetMethod(nameof(ICollection<object>.Add));
}

@aivascu aivascu added the triage Something that's being investigated label May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug triage Something that's being investigated
Projects
None yet
Development

No branches or pull requests

3 participants