[Proposal] Parameter Validation Source Generator Attributes

[MichaelPeter]

It is quite sad the !! operator did not make it into C# but also understand that it was quite specific and did not cover all cases.

But since the !! just generated code, how about we allow source generators for parameter validation
so everybody can build their own validation, with very readable attributes.

Usage Source Code:

// Throws an exception if citizen is null or number of years is 0 or smaller
public void IncreaseAge([NotNull] Citizen citizen, [GreaterZero] int numberOfYears)
{
     citizen.Age += numberOfYears;
}

public void Rename([NotNull] Citizen citizen, [NotNullOrWhitespace] string newName)
{
     citizen.Name = newName;
}

The Generators should still be very specific, so it is clear the code is always generated at the beginning of the method and in the order of the attributes.
This simular but differnt to #6063 since here the generated code is defined and no methods referenced

Attribute Implementation:

// It could be an attribute on the attribute if that improves compile time instead with the usual SourceGenerator syntax?
[ParameterValidationGenerator(typeof(NotNullValidationGenerator))] 
[AttributeUsage(AttributeTargets.Parameter)]
public class NotNullAttribute : Attribute
{
}

[ParameterValidationGenerator(typeof(NotNullOrWhitespaceValidationGenerator)), parameterType: typeof(System.String)]
[AttributeUsage(AttributeTargets.Parameter)]
public class NotNullOrWhitespaceAttribute : Attribute
{
}

[Generator]
public class NotNullValidationGenerator: IParameterValidationSourceGenerator
{
        public void Execute(ParameterValidationGeneratorExecutionContext context)
        {
                context.AddSourceAtMethodStart("ifnotnull", SourceText.From($"""if({context.Parameter.Name} == null) throw new System.ArgumentNullException("{context.Parameter.Name}");"""));
        }
}


[Generator]
public class NotNullOrWhitespaceValidationGenerator: IParameterValidationSourceGenerator
{
        public void Execute(ParameterValidationGeneratorExecutionContext context)
        {
                context.AddSourceAtMethodStart("ifnotnullorwhitespace", SourceText.From($"""if(string.IsNullOrWhitespace{context.Parameter.Name}) throw new System.ArgumentException("{context.Parameter.Name} cannot be null or whitespace.");"""));
        }
}

Geneated Code:

public void Rename([NotNull] Citizen citizen, [NotNullOrWhitespace] string newName)
{
     if(citizen == null) throw new System.ArgumentNullException("citizen");
     if(citizen > null) throw new System.ArgumentException("newName cannot be null or whitespace.");
     citizen.Name = newName;
}

Also standard attributes like [NotNull] [NotNullOrEmpty] [NotNullOrWhitespace] [GreaterZero] [NotNegative] [NotDefault] (for guids for example) could be included in the BCL.

Handing Attribute Properties & Shared code

Lets say we want attributes that take parameters and call our own method

// The order of attributes is importaint, since MinAgeValidation would throw a null exception, if citizen was null
public void GiveDrivingLicence([NotNull] [MinimumAgeValidation(18)] Citizen citizen)
{
      citizen.Documents.Add(new DrivingLicence(type: DrvingLicenceType.Car);
}

[ParameterValidationGenerator(typeof(MinimumAgeValidationGenerator)), parameterType: typeof(MyNamespace.Citizen)]
[AttributeUsage(AttributeTargets.Parameter)]
public class MinimumAgeValidationAttribute : Attribute
{
     public MinimumAgeValidationAttribute(int minimumAge) 
     {
           MinimumAge = minimumAge;
     }

     public int MinimumAge {get; set; }
}

[Generator]
public class MinimumAgeValidationGenerator: IParameterValidationSourceGenerator
{
        public void Execute(ParameterValidationGeneratorExecutionContext context)
        {
               var minAgeAttribute = (MinimumAgeValidationAttribute)context.Attribute;

                context.AddSourceAtMethodStart("ifminage", SourceText.From($"""if(MyNamespace.AgeValidation.HasMinimumAge{context.Parameter.Name}, minAgeAttribute.MinAge) throw new System.ArgumentException("The citizen in the parameter {context.Parameter.Name} needs to be at least {minAgeAttribute.MinimumAge} years old.");"""));
        }
}

The referenced method could then be generated conditionally in a normal source generator or defined in a referenced assembly.

namespace MyNamespace;

public class AgeValidation 
{
     public static bool HasMinimumAge([NotNull] Citizen citizen, [NotNegative] int minAge) => citizen.Age >= minAge;
}

Generated Code

public void GiveDrivingLicence([NotNull] [MinimumAgeValidation(18)] Citizen citizen)
{
     if(citizen == null) throw new System.ArgumentNullException("citizen");
     if(MyNamespace.AgeValidation.HasMinimumAge(citizen, 18)  throw new System.ArgumentException("The citizen in the parameter citizen needs to be at least 18 years old.");

      citizen.Documents.Add(new DrivingLicence(type: DrvingLicenceType. Car)));
}

[HaloFour]

The use of attributes to drive this validation had been discussed during design meetings and wasn't preferred for a couple of reasons. One is that it's quite verbose, at least compared to the alternatives. Another is that it causes attributes to change the behavior of the code as it is written, which is something attributes don't (or rarely) do today. Lastly is that these attributes already exist in the BCL and do not imply this behavior, so it'd be impossible to use the already existing attributes without causing breaking changes, so a duplicate set of attributes would have to be introduced, which isn't exactly an ideal circumstance.

The team has also expressed that they are not interested in using source generators for AOP scenarios, like validation. The issue seemed to be mostly that there is behavior silently inserted into the code of an existing method which would make it a challenge to understand what the method does. I personally do not agree with this sentiment as annotation-driven AOP is somewhat common in Java and is not a source of confusion. Perhaps parameter validation is a simple enough case to not cause this concern, but I'm of the opinion that a narrow implementation of AOP via source generators is not all that worthwhile.

I'm kind of just hoping that !! will be reconsidered. It wouldn't be the first time that a proposed feature drew a lot of criticism over the syntax but with no clear better options presented, so the feature was eventually just added to the language a few versions later with no fanfare and without completely wrecking the language as it was claimed to do.

[MichaelPeter]

@HaloFour Thank you for your detailed reply. Yes I also hope that !! is reconsidered... I mean it works in swift..

Regarding SourceGeneration I am happy we have the same opinion - It works in Java. To make it distinguishable maybe the Ide could highlight these source modifying attributes somehow. (Also maybe there could be SourceGeneratorAttribute base class to clearly Identify?) I would not want a new a new language concept for that like attributes with () to indicate source generation MyMethod((NotNull) Citizen citizen)

[AartBluestoke]

if the concern is "undefined extra behavior in the class" from attributes that trigger source generation
a) how is 'foo([CheckMe] string theVal){}' different in understandability and 'hidden' functionality from foo(string theVal){checkMe(theVal);}
b) if visibility of code changes is a concern, source gen from two bracket attributes?, 'foo([[CheckMe]] string theVal){}' says 'something extra was done at runtime here'
c) Microsoft in general doesn't follow this policy as ASP.Net, azure functions, and other core libraries MS written libraries rely in many places on attributes affecting runtime binding behavior.
d) anyone dealing with polymorphic code calls (ie, everyone in c#) is already well used to "whelp, this call to IDBContext.Open() might do almost anything, including either throwing any type of database or network exception, or even nothing because that is just a mock context"

[HaloFour]

c) Microsoft in general doesn't follow this policy as ASP.Net, azure functions, and other core libraries MS written libraries rely in many places on attributes affecting runtime binding behavior.

The binding between the components, yes, but not the behavior of the code as it is written. Although as some will point out there are a couple of very narrow cases where attributes do have an impact on the compiler and the code as it is written.

I don't necessarily agree with the reasoning, I am just repeating it. If team members want to contradict (or even better, reconsider), I'll let them do so. 😄

[Rekkonnect]

What you're suggesting is basically attributes that specify constraints for method contracts, as was majorly mentioned against the introduction of the !! operator.

[MichaelPeter]

Yes and by using attributes it is also clear to the outside what kind of "contract" a method has and what parameter values are expected.
It could go so far that the compiler checks certain BCL "contract" Attributes.

[HaloFour]

It could go so far that the compiler checks certain BCL "contract" Attributes.

That's also kind of a problem, those aren't enforced today and for the compiler to start enforcing them would be a breaking change. So this would necessitate either a compiler switch, or a completely new set of attributes. Given how loathe the compiler team is to add compiler switches I'd suggest that this is very unlikely to get traction. I don't think automatic parameter validation falls into the same category as something like NRTs. Splitting the ecosystem may be more palatable but it still leaves a very confusing situation where you need to know which flavor or combination of attributes actually result in enforcement.

All of this would require that the compiler team accept the notion that attributes affect runtime behavior of the code within the method, whether directly or through AOP, and that is already a tall order given their stance on code generators.

[MichaelPeter]

Thanks for your active responses :)

Ok sorry I am not getting why this is a breaking change.
Right now there are no Attributes which would cause compiler errors, right? These would need to be defined and would only affect now code, decorated with these attributes.

NRT require the Compiler switch, but they are changing how the language works.

Hmm yea you mentioned that, only idea I had to convince them would be to make a poll amoung the developers
if they think Attributes should be able to affect runtime behavior.