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

OptionsValidator source-gen shouldn't cover static and const members #88150

Closed
xakep139 opened this issue Jun 28, 2023 · 3 comments · Fixed by #88254
Closed

OptionsValidator source-gen shouldn't cover static and const members #88150

xakep139 opened this issue Jun 28, 2023 · 3 comments · Fixed by #88254
Assignees
Milestone

Comments

@xakep139
Copy link
Member

xakep139 commented Jun 28, 2023

Description

Recently added OptionsValidator (issue #85475 and PR #87587) considers all members of a TOption for validation, however we shouldn't validate static ones. Additionally, const fields should be ignored or a warning should be emitted if such members are annotated with data validation attributes (inherited from ValidationAttribute).

Reproduction Steps

[OptionsValidator]
public partial class MyValidator : IValidateOptions<MyOptions>
{
}

public class MyOptions
{
    [Required]
    public static string? StaticStringField;

    [Required]
    public static string? StaticStringProperty => "static";

    [Required]
    public const string? ConstString = null;
}

Expected behavior

A warning is emitted for all three members, no static or const members are validated in the generated code.

Actual behavior

The generator produces the next code which won't compile:
image

Regression?

No response

Known Workarounds

No response

Configuration

.NET SDK 8.0.100-preview.7.23327.3

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 28, 2023
@ghost
Copy link

ghost commented Jun 28, 2023

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

Issue Details

Description

Recently added OptionsValidator (issue #85475 and PR #87587) considers all members of a TOption for validation, however we shouldn't validate static ones. Additionally, const fields should be ignored or a warning should be emitted if such members are annotated with data validation attributes.

Reproduction Steps

[OptionsValidator]
public partial class MyValidator : IValidateOptions<MyOptions>
{
}

public class MyOptions
{
    [Required]
    public static string? StaticStringField;

    [Required]
    public static string? StaticStringProperty => "static";

    [Required]
    public const string? ConstString = null;
}

Expected behavior

A warning is emitted for all three members, no static or const members are validated in the generated code.

Actual behavior

The generator produces the next code which won't compile:
image

Regression?

No response

Known Workarounds

No response

Configuration

.NET SDK 8.0.100-preview.7.23327.3

Other information

No response

Author: xakep139
Assignees: -
Labels:

area-System.ComponentModel.DataAnnotations

Milestone: -

@xakep139
Copy link
Member Author

cc @tarekgh

@tarekgh tarekgh added this to the 8.0.0 milestone Jun 29, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 29, 2023
@tarekgh tarekgh self-assigned this Jun 29, 2023
@ghost
Copy link

ghost commented Jun 29, 2023

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

Issue Details

Description

Recently added OptionsValidator (issue #85475 and PR #87587) considers all members of a TOption for validation, however we shouldn't validate static ones. Additionally, const fields should be ignored or a warning should be emitted if such members are annotated with data validation attributes (inherited from ValidationAttribute).

Reproduction Steps

[OptionsValidator]
public partial class MyValidator : IValidateOptions<MyOptions>
{
}

public class MyOptions
{
    [Required]
    public static string? StaticStringField;

    [Required]
    public static string? StaticStringProperty => "static";

    [Required]
    public const string? ConstString = null;
}

Expected behavior

A warning is emitted for all three members, no static or const members are validated in the generated code.

Actual behavior

The generator produces the next code which won't compile:
image

Regression?

No response

Known Workarounds

No response

Configuration

.NET SDK 8.0.100-preview.7.23327.3

Other information

No response

Author: xakep139
Assignees: tarekgh
Labels:

area-Extensions-Options

Milestone: 8.0.0

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 30, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 2, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants