Should IRuleContext methods take nullable parameters in CSLA 10?

[mtavares628]

Now that nullable types are part of CSLA 10, and that BusinessRuleBase.PrimaryProperty is now nullable, does it make sense to make the method signatures on IRuleContext also take nullable parameters? Now that I've updated to 10, I'm getting hundreds of warnings within my rules that PrimaryProperty could be null. Here's an example of what I mean:

protected override async Task ExecuteAsync(IRuleContext context)
{
    var srName = context.GetInputValue<string>(NameProperty);
    var ID = context.GetInputValue<int?>(PrimaryProperty);
}

I'm getting warnings for all of my calls to Context.GetInputValue when I'm using PrimaryProperty. I would expect that GetInputValue would now take an IPropertyInfo? parameter now instead of just IPropertyInfo to avoid this warning, since it is checking for null anyway. I know that I can just pass in PrimaryProperty! or switch it up to use:

var ID = context.InputPropertyValues[PrimaryProperty];

But I would think it would make less work for people if the method signatures now take those nullable types as well.

[StefanOssendorf]

Hi.

I would expect that GetInputValue would now take an IPropertyInfo? parameter now instead of just IPropertyInfo to avoid this warning, since it is checking for null anyway.

It's not checking for null but it's protecting against null. Which means IPropertyInfo? in the interface would be lying because you get an exception when passing in null.

The actual problem is that ObjectRule is a BusinessRule with having a PrimaryProperty. For me that looks like a design issue in these cases.

If we change the parameter from IPropertyInfo to IPrropertyInfo? the only difference is that in an actual case where null is being used (ObjectRule, nullable field, etc.) the NRT wouldn't give you a warning where it's appropriate.

In your case (and my in the future at work) is currently to use ! to suppress the warning.
In csla I think we should get somehow rid of the PrimaryProperty for ObjectRule so PrimaryProperty can be properly annotated. I didn't think of that during adding NRT - sorry.

[mtavares628]

@StefanOssendorf Ok, I follow what you are saying. It's unfortunate, because I don't have a single ObjectRule. All of my rules are on properties so it's just tedious to use the warning suppressor, but I understand the reasoning now.

I have a related follow-up question regarding business properties. So with all the nullable changes, does that mean I can no longer use a regular string type without getting a possible null reference return warning?

public static readonly PropertyInfo<string> NamesProperty = RegisterProperty<string>(nameof(Names), "Names", string.Empty);

public string Names
{
    get => ReadProperty(NamesProperty);
    private set => LoadProperty(NamesProperty, value);
}

ReadProperty now generates a "Possible null reference return" warning for all of my string properties that are written like above. How would I suppress this? Because if I use the suppressor, I then get the warning that Properties that use managed backing fields should only use Get/Set/Read/Load methods and nothing else. What is the new recommended approach if I don't want to make my property a nullable string but don't want warnings either?