Qowaiv / qowaiv-analyzers

Roslyn Analyzers
MIT License
1 stars 1 forks source link

New Rule: Records should prefer required properties over constructors #44

Closed Corniel closed 1 month ago

Corniel commented 1 month ago

It has been suggested (offline) that the records should not use primary constructors, but required properties instead, a C# language feature, available since version 11.

I like the idea, but to make it work, there should not be False Positive's. So, what should be good reasons to to not go for this mechanism?

I'd like to investigate (and take those exceptions into account) before implementing this rule:

Should small structures, let say with 1, 2, or 3 properties, be excluded? (And should it be configurable?)

public sealed record TwoProperties(string Prop1, string Prop2);

When guarding (or other operations) are involved in setting a property, there is not other (practical) way to ensure that?!

public sealed record WithGuarding
{
    public WithGuarding(string value) { }

    public string Value { get; } = Guard.NotEmpty(value);
}

Properties with limited visibility should be excluded two, I suppose.

public sealed record WithLessVisibleProperty
{
    public WithGuarding(string value) { }

    protected string Value { get; } = value;
}

I'm also not sure if I would like to include records with a visibility other than public.

CptWesley commented 1 month ago

Constructor arguments that are nullable or that have a default value should not necessarily be marked as required.

I would personally not exclude small records, but since all of this is a matter of taste, I would make it configurable.

An auto-fix for this would also be nice.

Corniel commented 1 month ago

@CptWesley A code fix would indeed be added. Good point, for pointing out that some constructor arguments then should not become required. But in those cases, it should be still be desirable, not to use a constructor, do you agree?