bchavez / Bogus

:card_index: A simple fake data generator for C#, F#, and VB.NET. Based on and ported from the famed faker.js.
Other
8.66k stars 495 forks source link

Properties with private setters are not assigned if the property is declared on the base class #389

Closed thomaslevesque closed 3 years ago

thomaslevesque commented 3 years ago

Version Information

Software Version(s)
Bogus NuGet Package 32.0.2
.NET Core? .NET 5.0
.NET Full Framework? N/A
Windows OS? 10
Linux OS? N/A
Visual Studio? irrelevant (I use Rider)

What locale are you using with Bogus?

en-US (probably irrelevant)

What is the expected behavior?

public class FooBase
{
    public int Baz { get; private set; }
}

public class Foo : FooBase
{
    public int Bar { get; private set; }
}

var foo = new Faker<Foo>()
        .RuleFor(f => f.Bar, 42)
        .RuleFor(f => f.Baz, 123)
        .Generate();

foo.Bar should be 42, foo.Baz should be 123

What is the actual behavior?

foo.Bar is 42, but foo.Baz is 0

The only difference I can see between the two is that Baz is declared in the base class. It works fine if I make Baz's setter public.

Please provide a stack trace.

N/A (no exception)

Any possible solutions?

Make the setter public (not acceptable in my case)

How do you reproduce the issue?

See code above

Do you have a unit test that can demonstrate the bug?

No, but easy to write using the code above

Can you identify the location in Bogus' source code where the problem exists?

Not yet

If the bug is confirmed, would you be willing to submit a PR?

Yes

thomaslevesque commented 3 years ago

Looks like a weird reflection issue...

In Binder, CanWrite returns true for Bar, but false for Baz

Testing in LinqPad, I get this:

    typeof(Foo).GetProperty("Baz").CanWrite.Dump(); // false
    typeof(Foo).GetProperty("Baz").GetSetMethod(true).Dump(); // null

    typeof(FooBase).GetProperty("Baz").CanWrite.Dump(); // true
    typeof(FooBase).GetProperty("Baz").GetSetMethod(true).Dump(); // FooBase.set_Baz

So apparently it doesn't say the same thing depending on which type is being reflected... This kinda makes sense: from the point of view of the reflected type, Bar is writable, and Baz is not. But we don't care about the point of view of Foo here, we just want to know if the property has a setter...

thomaslevesque commented 3 years ago

I have a fix, but it's slightly ugly... Basically, when we're looking at a property, return the property found on the base type, rather than the inherited one. I'll submit a PR.

bchavez commented 3 years ago

Thank you very much @thomaslevesque, I'll try to take a look at this today.

bchavez commented 3 years ago

Sorry for the delay on this issue @thomaslevesque; it's been a few busy weeks at work.

Bogus v33.1.1 was released with your PR #390 fix.

https://www.nuget.org/packages/Bogus/33.1.1

Thanks, Brian

thomaslevesque commented 3 years ago

Hey @bchavez, no worries, I know what it's like! Thanks for the merge and the release!