Bogus icon indicating copy to clipboard operation
Bogus copied to clipboard

Strict mode problem with obsolete property

Open Gakk opened this issue 3 years ago • 1 comments

Please describe why you are requesting a feature

When the object you are trying to fake has an obsolete property marked to generate compile error, the validation to ensure all properties have rules fails:

Bogus.ValidationException : Validation was called to ensure all properties / fields have rules.
There are missing rules for Faker<T> 'MyObject'.
=========== Missing Rules ===========
SomeOldProperty

Since this property is marked to generate a compile time error, I belive it should have been ignored by default, as has already been implemented for readonly properties in v3.0.6 with #13 Strict mode problem with read only property.

There is now way to generate or get data for this obsolete property, so I believe it should be safe to silently ignore it.

Please provide a code example of what you are trying to achieve

My class has this properties:

public string TheNewProperty { get; set; }

[Obsolete("Use property 'TheNewProperty' instead", true)]
public string SomeOldProperty { get; set; }

The key here is the argument true on the obsolete property.

When creating a new faker this will fail with the mentioned ValidationException for the obsolete property:

var myObjectFaker = new Faker<MyObject>().StrictMode(true)
    .RuleFor(p => p.TheNewProperty, f => f.Address.StreetName());

One way to work around this is using the Ignore statement

var myObjectFaker = new Faker<MyObject>().StrictMode(true)
    .RuleFor(p => p.TheNewProperty, f => f.Address.StreetName())
    .Ignore("SomeOldProperty"); // Obsolete property, was replaced by 'TheNewProperty'

This ignore has to be done by property name, as referencing the property with an expression would have the compiler throwing an error.

Please answer any or all of the questions below

  • Is the feature something that currently cannot be done?

It can be done with a manual ignore-rule, but this will reference the name as a string and not be future proof for any changes to the object.

  • What alternatives have you considered?

I have not found any other alternatives than using the ignore-rule with the property name as string.

  • Is this feature request any issues or current problems?

It is a problem as the concept of using StrictMode is to ensure rules will be updated if the object is changed in the future. With ignore a future removal of the old obsolete property would not fail and cleaning up the ignore rule would probably not be detected.

  • Has the feature been requested in the past?

No.

If the feature request is approved, would you be willing to submit a PR?

Yes

Gakk avatar Nov 17 '22 10:11 Gakk

Great find. Thank you for bringing this to my attention. I agree this should be fixed.

The compiler already does the enforcement of the [Obsolete("", true)] object property by failing compilation. There's no need for Bogus to complain about the property and there's no reason for Bogus to ask the user to do something that would result in a failed compilation anyway.

You can create a PR for this, but if you take on this task; please:

  • Write, at a minimum, two unit tests for [Obsolete("", true)] and [Obsolete("", false)]
  • The crux of the changes would be targeted in the default Binder object which is responsible for getting a projected view of an object's properties that Bogus uses for validation.
    • https://github.com/bchavez/Bogus/blob/e29f0205f6af6d736570646f4b08231eda6dba19/Source/Bogus/Binder.cs#L69-L86

Currently, workarounds for v34.0.2 and below are:

  • Use the .Ignore("SomeOldProperty"); as originally reported
  • Use a CustomBinder : Binder and overriding CustomBinder.GetMembers() with the necessary fix to filter out properties that would contain [Obsolete("", true)].

bchavez avatar Nov 18 '22 00:11 bchavez