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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strict mode problem with obsolete property #450

Open
Gakk opened this issue Nov 17, 2022 · 1 comment
Open

Strict mode problem with obsolete property #450

Gakk opened this issue Nov 17, 2022 · 1 comment
Labels

Comments

@Gakk
Copy link

Gakk commented Nov 17, 2022

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

@bchavez
Copy link
Owner

bchavez commented Nov 18, 2022

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.
    • {
      if( m.GetCustomAttributes(typeof(CompilerGeneratedAttribute), true).Any() )
      {
      //no compiler generated stuff
      return false;
      }
      if( m is PropertyInfo pi )
      {
      return pi.CanWrite;
      }
      if( m is FieldInfo fi )
      {
      //No private fields.
      //GitHub Issue #13
      return !fi.IsPrivate;
      }

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 bchavez added the bug label Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants