Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace Microsoft.Extensions.Options.Generators
{
[Generator]
public class Generator : IIncrementalGenerator
public class OptionValidatorGenerator : IIncrementalGenerator
{
public void Initialize(IncrementalGeneratorInitializationContext context)
{
Expand Down
55 changes: 52 additions & 3 deletions src/libraries/Microsoft.Extensions.Options/gen/Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Globalization;
using System.Linq;
using System.Text;
Expand Down Expand Up @@ -469,14 +470,36 @@ private List<ValidatedMember> GetMembersToValidate(ITypeSymbol modelType, bool s
var validationAttr = new ValidationAttributeInfo(attributeType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat));
validationAttrs.Add(validationAttr);

foreach (var constructorArgument in attribute.ConstructorArguments)
ImmutableArray<IParameterSymbol> parameters = attribute.AttributeConstructor?.Parameters ?? ImmutableArray<IParameterSymbol>.Empty;
bool lastParameterDeclaredWithParamsKeyword = parameters.Length > 0 && parameters[parameters.Length - 1].IsParams;

ImmutableArray<TypedConstant> arguments = attribute.ConstructorArguments;

for (int i = 0; i < arguments.Length; i++)
{
validationAttr.ConstructorArguments.Add(GetArgumentExpression(constructorArgument.Type!, constructorArgument.Value));
TypedConstant argument = arguments[i];
if (argument.Kind == TypedConstantKind.Array)
Copy link
Member

@eiriktsarpalis eiriktsarpalis Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably too much of a niche scenario to consider here, but could this logic be tripped up by something like the following?

new MyValidationAttribute(new int[] { 1, 2, 3 });

public class MyValidationAttribute(params int[][] values) : ValidationAttribute
{
}

Copy link
Member Author

@tarekgh tarekgh Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can consider that in .NET 9.0 as I want to limit the changes, we are porting to .NET 8.0.

Copy link
Member

@ericstj ericstj Sep 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's actually not a valid attribute parameter type if you want to apply the attribute (could be interesting for testing, but I gather this generator only observes applied attributes).
https://sharplab.io/#v2:D4AQTAjAsAUCDMACciCyBPAagQwDYEsATbAF3wHsA7AQRJICd8AjAVxIFNEAuRWh5tpwDesRGORIMOAsTJU+jVhwAUAB2z1sAWwDOifJRIBtALqnEANzwt2OgJSIhiAL6xXMWEal4ipCjTpFQWVKdgB3fUNzJwgAGkQwePhnOxNYBGQwNHQAFXRVdlgRGHcgA===

error CS0181: Attribute constructor parameter 'values' has type 'int[][]', which is not a valid attribute parameter type

Attribute's parameters need to be represented in metadata and can only take a very limited set of types.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I did recall that it's possible to nest arrays in attribute declarations, but it looks like it's only possible for object[] parameters:

https://sharplab.io/#v2:D4AQTAjAsAUCDMACciCyBPAagQwDYEsATbAF3wHsA7AQRJICd8AjAVxIFNEAuRWh5tpwDesRGORIMOAsTJU+jVhwAUAB2z1sAWwDOickwBW7AMYkA2gF1EANzwt2OgJSIhiAL6xPMWOal4iUgpKZUp2AHd9I1MLazcwyINjMytXRAgPDydLWARkMDR0ABV0VXZYERhvIA===

I think that case would work fine with the logic as-is, but maybe a test is in order?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this was brought up here as well: #91934 (comment)

{
bool isParams = lastParameterDeclaredWithParamsKeyword && i == arguments.Length - 1;
validationAttr.ConstructorArguments.Add(GetArrayArgumentExpression(argument.Values, isParams));
}
else
{
validationAttr.ConstructorArguments.Add(GetArgumentExpression(argument.Type!, argument.Value));
}
}

foreach (var namedArgument in attribute.NamedArguments)
{
validationAttr.Properties.Add(namedArgument.Key, GetArgumentExpression(namedArgument.Value.Type!, namedArgument.Value.Value));
if (namedArgument.Value.Kind == TypedConstantKind.Array)
{
bool isParams = lastParameterDeclaredWithParamsKeyword && namedArgument.Key == parameters[parameters.Length - 1].Name;
validationAttr.Properties.Add(namedArgument.Key, GetArrayArgumentExpression(namedArgument.Value.Values, isParams));
}
else
{
validationAttr.Properties.Add(namedArgument.Key, GetArgumentExpression(namedArgument.Value.Type!, namedArgument.Value.Value));
}
}
}
}
Expand Down Expand Up @@ -637,6 +660,32 @@ private bool CanValidate(ITypeSymbol validatorType, ISymbol modelType)
return false;
}

private string GetArrayArgumentExpression(ImmutableArray<Microsoft.CodeAnalysis.TypedConstant> value, bool isParams)
{
var sb = new StringBuilder();
if (!isParams)
{
sb.Append("new[] { ");
}

for (int i = 0; i < value.Length; i++)
{
sb.Append(GetArgumentExpression(value[i].Type!, value[i].Value));

if (i < value.Length - 1)
{
sb.Append(", ");
}
}

if (!isParams)
{
sb.Append(" }");
}

return sb.ToString();
}

private string GetArgumentExpression(ITypeSymbol type, object? value)
{
if (value == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1610,15 +1610,15 @@ public partial class FirstModelValidator : IValidateOptions<FirstModel>

// Run the generator with C# 7.0 and verify that it fails.
var (diagnostics, generatedSources) = await RoslynTestUtils.RunGenerator(
new Generator(), refAssemblies.ToArray(), new[] { source }, includeBaseReferences: true, LanguageVersion.CSharp7).ConfigureAwait(false);
new OptionValidatorGenerator(), refAssemblies.ToArray(), new[] { source }, includeBaseReferences: true, LanguageVersion.CSharp7).ConfigureAwait(false);

Assert.NotEmpty(diagnostics);
Assert.Equal("SYSLIB1216", diagnostics[0].Id);
Assert.Empty(generatedSources);

// Run the generator with C# 8.0 and verify that it succeeds.
(diagnostics, generatedSources) = await RoslynTestUtils.RunGenerator(
new Generator(), refAssemblies.ToArray(), new[] { source }, includeBaseReferences: true, LanguageVersion.CSharp8).ConfigureAwait(false);
new OptionValidatorGenerator(), refAssemblies.ToArray(), new[] { source }, includeBaseReferences: true, LanguageVersion.CSharp8).ConfigureAwait(false);

Assert.Empty(diagnostics);
Assert.Single(generatedSources);
Expand All @@ -1638,6 +1638,129 @@ public partial class FirstModelValidator : IValidateOptions<FirstModel>
Assert.Equal(0, diags.Length);
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser), nameof(PlatformDetection.IsNetCore))]
public async Task DataAnnotationAttributesWithParams()
{
var (diagnostics, generatedSources) = await RunGenerator(@"""
public class MyOptions
{
[Required]
public string P1 { get; set; }

[Length(10, 20)]
public string P2 { get; set; }

[AllowedValues(10, 20, 30)]
public int P3 { get; set; }

[DeniedValues(""One"", ""Ten"", ""Hundred"")]
public string P4 { get; set; }
}

[OptionsValidator]
public partial class MyOptionsValidator : IValidateOptions<MyOptions>
{
}
""");

Assert.Empty(diagnostics);
Assert.Single(generatedSources);

var generatedSource = """

// <auto-generated/>
#nullable enable
#pragma warning disable CS1591 // Compensate for https://github.com/dotnet/roslyn/issues/54103
namespace Test
{
partial class MyOptionsValidator
{
/// <summary>
/// Validates a specific named options instance (or all when <paramref name="name"/> is <see langword="null" />).
/// </summary>
/// <param name="name">The name of the options instance being validated.</param>
/// <param name="options">The options instance.</param>
/// <returns>Validation result.</returns>
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")]
public global::Microsoft.Extensions.Options.ValidateOptionsResult Validate(string? name, global::Test.MyOptions options)
{
global::Microsoft.Extensions.Options.ValidateOptionsResultBuilder? builder = null;
var context = new global::System.ComponentModel.DataAnnotations.ValidationContext(options);
var validationResults = new global::System.Collections.Generic.List<global::System.ComponentModel.DataAnnotations.ValidationResult>();
var validationAttributes = new global::System.Collections.Generic.List<global::System.ComponentModel.DataAnnotations.ValidationAttribute>(1);

context.MemberName = "P1";
context.DisplayName = string.IsNullOrEmpty(name) ? "MyOptions.P1" : $"{name}.P1";
validationAttributes.Add(global::__OptionValidationStaticInstances.__Attributes.A1);
if (!global::System.ComponentModel.DataAnnotations.Validator.TryValidateValue(options.P1, context, validationResults, validationAttributes))
{
(builder ??= new()).AddResults(validationResults);
}

context.MemberName = "P2";
context.DisplayName = string.IsNullOrEmpty(name) ? "MyOptions.P2" : $"{name}.P2";
validationResults.Clear();
validationAttributes.Clear();
validationAttributes.Add(global::__OptionValidationStaticInstances.__Attributes.A2);
if (!global::System.ComponentModel.DataAnnotations.Validator.TryValidateValue(options.P2, context, validationResults, validationAttributes))
{
(builder ??= new()).AddResults(validationResults);
}

context.MemberName = "P3";
context.DisplayName = string.IsNullOrEmpty(name) ? "MyOptions.P3" : $"{name}.P3";
validationResults.Clear();
validationAttributes.Clear();
validationAttributes.Add(global::__OptionValidationStaticInstances.__Attributes.A3);
if (!global::System.ComponentModel.DataAnnotations.Validator.TryValidateValue(options.P3, context, validationResults, validationAttributes))
{
(builder ??= new()).AddResults(validationResults);
}

context.MemberName = "P4";
context.DisplayName = string.IsNullOrEmpty(name) ? "MyOptions.P4" : $"{name}.P4";
validationResults.Clear();
validationAttributes.Clear();
validationAttributes.Add(global::__OptionValidationStaticInstances.__Attributes.A4);
if (!global::System.ComponentModel.DataAnnotations.Validator.TryValidateValue(options.P4, context, validationResults, validationAttributes))
{
(builder ??= new()).AddResults(validationResults);
}

return builder is null ? global::Microsoft.Extensions.Options.ValidateOptionsResult.Success : builder.Build();
}
}
}
namespace __OptionValidationStaticInstances
{
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")]
file static class __Attributes
{
internal static readonly global::System.ComponentModel.DataAnnotations.RequiredAttribute A1 = new global::System.ComponentModel.DataAnnotations.RequiredAttribute();

internal static readonly global::System.ComponentModel.DataAnnotations.LengthAttribute A2 = new global::System.ComponentModel.DataAnnotations.LengthAttribute(
(int)10,
(int)20);

internal static readonly global::System.ComponentModel.DataAnnotations.AllowedValuesAttribute A3 = new global::System.ComponentModel.DataAnnotations.AllowedValuesAttribute(
(int)10, (int)20, (int)30);

internal static readonly global::System.ComponentModel.DataAnnotations.DeniedValuesAttribute A4 = new global::System.ComponentModel.DataAnnotations.DeniedValuesAttribute(
"One", "Ten", "Hundred");
}
}
namespace __OptionValidationStaticInstances
{
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")]
file static class __Validators
{
}
}

""";
Assert.Equal(generatedSource.Replace("\r\n", "\n"), generatedSources[0].SourceText.ToString().Replace("\r\n", "\n"));
}

private static CSharpCompilation CreateCompilationForOptionsSource(string assemblyName, string source, string? refAssemblyPath = null)
{
// Ensure the generated source compiles
Expand Down Expand Up @@ -1676,7 +1799,7 @@ private static CSharpCompilation CreateCompilationForOptionsSource(string assemb
refAssemblies.Add(refAssembly);
}

return await RoslynTestUtils.RunGenerator(new Generator(), refAssemblies.ToArray(), new List<string> { source }, includeBaseReferences: true, languageVersion).ConfigureAwait(false);
return await RoslynTestUtils.RunGenerator(new OptionValidatorGenerator(), refAssemblies.ToArray(), new List<string> { source }, includeBaseReferences: true, languageVersion).ConfigureAwait(false);
}

private static async Task<(IReadOnlyList<Diagnostic> diagnostics, ImmutableArray<GeneratedSourceResult> generatedSources)> RunGenerator(
Expand Down Expand Up @@ -1733,7 +1856,7 @@ private static CSharpCompilation CreateCompilationForOptionsSource(string assemb
assemblies.Add(Assembly.GetAssembly(typeof(Microsoft.Extensions.Options.ValidateObjectMembersAttribute))!);
}

var result = await RoslynTestUtils.RunGenerator(new Generator(), assemblies.ToArray(), new[] { text })
var result = await RoslynTestUtils.RunGenerator(new OptionValidatorGenerator(), assemblies.ToArray(), new[] { text })
.ConfigureAwait(false);

return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,37 @@ public void TestValidationWithCyclicReferences()
ValidateOptionsResult result2 = dataAnnotationValidateOptions.Validate("MyOptions", options);
Assert.True(result1.Succeeded);
}

#if NET8_0_OR_GREATER
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
public void TestNewDataAnnotationFailures()
{
NewAttributesValidator sourceGenValidator = new();

OptionsUsingNewAttributes validOptions = new()
{
P1 = "123456", P2 = 2, P3 = 4, P4 = "c", P5 = "d"
};

ValidateOptionsResult result = sourceGenValidator.Validate("OptionsUsingNewAttributes", validOptions);
Assert.True(result.Succeeded);

OptionsUsingNewAttributes invalidOptions = new()
{
P1 = "123", P2 = 4, P3 = 1, P4 = "e", P5 = "c"
};

result = sourceGenValidator.Validate("OptionsUsingNewAttributes", invalidOptions);

Assert.Equal(new []{
"P1: The field OptionsUsingNewAttributes.P1 must be a string or collection type with a minimum length of '5' and maximum length of '10'.",
"P2: The OptionsUsingNewAttributes.P2 field does not equal any of the values specified in AllowedValuesAttribute.",
"P3: The OptionsUsingNewAttributes.P3 field equals one of the values specified in DeniedValuesAttribute.",
"P4: The OptionsUsingNewAttributes.P4 field does not equal any of the values specified in AllowedValuesAttribute.",
"P5: The OptionsUsingNewAttributes.P5 field equals one of the values specified in DeniedValuesAttribute."
}, result.Failures);
}
#endif // NET8_0_OR_GREATER
}

public class MyOptions
Expand Down Expand Up @@ -270,4 +301,29 @@ public struct MyOptionsStruct
public partial class MySourceGenOptionsValidator : IValidateOptions<MyOptions>
{
}

#if NET8_0_OR_GREATER
public class OptionsUsingNewAttributes
{
[Length(5, 10)]
public string P1 { get; set; }

[AllowedValues(1, 2, 3)]
public int P2 { get; set; }

[DeniedValues(1, 2, 3)]
public int P3 { get; set; }

[AllowedValues(new object?[] { "a", "b", "c" })]
public string P4 { get; set; }

[DeniedValues(new object?[] { "a", "b", "c" })]
public string P5 { get; set; }
}

[OptionsValidator]
public partial class NewAttributesValidator : IValidateOptions<OptionsUsingNewAttributes>
{
}
#endif // NET8_0_OR_GREATER
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public async Task TestEmitter()
}

var (d, r) = await RoslynTestUtils.RunGenerator(
new Generator(),
new OptionValidatorGenerator(),
new[]
{
Assembly.GetAssembly(typeof(RequiredAttribute))!,
Expand Down