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

Fix options source gen with non-accessible validation attributes #88613

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Jul 10, 2023

The Options source generator creates code for validating an options class. This options class may be inherited from a base class located in a different assembly. It is also possible for the base class to have properties marked with the internal validation attribute, which means they should not be accessible outside the base class assembly. In such situations, the source generator should exclude the validation of these properties since it cannot generate code to access internal properties. Here's an example code illustrating this problem:

Assembly 1

        using System;
        using System.ComponentModel.DataAnnotations;

        #nullable enable

        namespace ValidationTest;

        public class BaseOptions
        {
            [Timeout] // internal attribute not visible outside the assembly
            public int Prop1 { get; set; }

            [Required]
            public string Prop2 { get; set; }
        }

        [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Parameter, AllowMultiple = false)]
        internal sealed class TimeoutAttribute : ValidationAttribute
        {
            protected override ValidationResult IsValid(object? value, ValidationContext? validationContext)
            {
                return ValidationResult.Success!;
            }
        }

Assembly 2 reference Assembly 1 and uses the source gen

        using Microsoft.Extensions.Options;
        using System.ComponentModel.DataAnnotations;

        #nullable enable
        #pragma warning disable CS1591

        namespace ValidationTest;
   
        public class ExtOptions : BaseOptions
        {
            [Range(0, 10)]
            public int Prop3 { get; set; }
         }

        [OptionsValidator]
        internal sealed partial class ExtOptionsValidator : IValidateOptions<ExtOptions>
        {
        }
    }

@ghost
Copy link

ghost commented Jul 10, 2023

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

Issue Details

The Options source generator creates code for validating an options class. This options class may be inherited from a base class located in a different assembly. It is also possible for the base class to have properties marked with the internal validation attribute, which means they should not be accessible outside the base class assembly. In such situations, the source generator should exclude the validation of these properties since it cannot generate code to access internal properties. Here's an example code illustrating this problem:

Assembly 1

            using System;
            using System.ComponentModel.DataAnnotations;

            #nullable enable

            namespace ValidationTest;

            public class BaseOptions
            {
                [Timeout] // internal attribute not visible outside the assembly
                public int Prop1 { get; set; }

                [Required]
                public string Prop2 { get; set; }
            }

            [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Parameter, AllowMultiple = false)]
            internal sealed class TimeoutAttribute : ValidationAttribute
            {
                protected override ValidationResult IsValid(object? value, ValidationContext? validationContext)
                {
                    return ValidationResult.Success!;
                }
            }

Assembly 2 reference Assembly 1 and uses the source gen

            using Microsoft.Extensions.Options;
            using System.ComponentModel.DataAnnotations;

            namespace ValidationTest;

            #nullable enable
            #pragma warning disable CS1591

            public class ExtOptions : BaseOptions
            {
                [Range(0, 10)]
                public int Prop3 { get; set; }
             }

            [OptionsValidator]
            internal sealed partial class ExtOptionsValidator : IValidateOptions<ExtOptions>
            {
            }
        }
Author: tarekgh
Assignees: tarekgh
Labels:

area-Extensions-Options

Milestone: -

@tarekgh tarekgh added this to the 8.0.0 milestone Jul 10, 2023
@tarekgh
Copy link
Member Author

tarekgh commented Jul 10, 2023

@xakep139 @geeknoid could you please have a look? Thanks!

@tarekgh tarekgh removed the request for review from jeffhandley July 11, 2023 18:39
@tarekgh tarekgh closed this Jul 11, 2023
@tarekgh tarekgh reopened this Jul 11, 2023
@tarekgh tarekgh merged commit 24150b3 into dotnet:main Jul 11, 2023
99 of 105 checks passed
@tarekgh tarekgh deleted the FixOptionsSourceGenWithNonAccessibleValidationAttributes branch July 11, 2023 23:55
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants