[C# Style]: don't need to use `required` keyword on parameters that have to be set through the CTOR

[Aaronontheweb]

Rule Name

DontUseRequiredProperties

Rule Description

Cursor was emitting a bunch of record types that looked like this:

using System;
using System.Diagnostics.Contracts;
using Microsoft.Extensions.Logging;

namespace Incrementalist
{
    /// <summary>
    ///     The settings used for this execution of incremental build analysis.
    /// </summary>
    public class BuildSettings
    {
        public static readonly TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);

        public BuildSettings(string targetBranch, string solutionFile, string workingDirectory,
            TimeSpan? timeoutDuration = null)
        {
            ArgumentNullException.ThrowIfNull(targetBranch);
            ArgumentNullException.ThrowIfNull(solutionFile);
            ArgumentNullException.ThrowIfNull(workingDirectory);
            
            TargetBranch = targetBranch;
            SolutionFile = solutionFile;
            WorkingDirectory = workingDirectory;
            TimeoutDuration = timeoutDuration ?? DefaultTimeout;
        }

        /// <summary>
        ///     The target branch to compare the current Git HEAD against,
        ///     i.e. the `dev` or `master` branch of the repository.
        /// </summary>
        public required string TargetBranch { get; init; }

        /// <summary>
        ///     The current solution file for us to analyze inside this repository.
        /// </summary>
        public required string SolutionFile { get; init; }

        /// <summary>
        ///     The folder Incrementalist will be working from.
        /// </summary>
        public required string WorkingDirectory { get; init; }

        /// <summary>
        ///     The length of time we're going to allow this Incrementalist operation to run
        ///     prior to cancelling it.
        /// </summary>
        public TimeSpan TimeoutDuration { get; init; }
    }
}

There's no need to make a parameter that already gets set through the only CTOR as required - we should probably include some guidance on this. Seems to come up most often with record types.

Good Example

// Good:
/// <summary>
///     The settings used for this execution of incremental build analysis.
/// </summary>
public class BuildSettings
{
    public static readonly TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);

    public BuildSettings(string targetBranch, string solutionFile, string workingDirectory,
        TimeSpan? timeoutDuration = null)
    {            
        TargetBranch = targetBranch;
        SolutionFile = solutionFile;
        WorkingDirectory = workingDirectory;
        TimeoutDuration = timeoutDuration ?? DefaultTimeout;
    }

    /// <summary>
    ///     The target branch to compare the current Git HEAD against,
    ///     i.e. the `dev` or `master` branch of the repository.
    /// </summary>
    public string TargetBranch { get; init; }

    /// <summary>
    ///     The current solution file for us to analyze inside this repository.
    /// </summary>
    public string SolutionFile { get; init; }

    /// <summary>
    ///     The folder Incrementalist will be working from.
    /// </summary>
    public string WorkingDirectory { get; init; }

    /// <summary>
    ///     The length of time we're going to allow this Incrementalist operation to run
    ///     prior to cancelling it.
    /// </summary>
    public TimeSpan TimeoutDuration { get; init; }
}

Bad Example

// Bad: adds `required` parameters where not needed 
  public class BuildSettings
    {
        public static readonly TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);

        public BuildSettings(string targetBranch, string solutionFile, string workingDirectory,
            TimeSpan? timeoutDuration = null)
        {            
            TargetBranch = targetBranch;
            SolutionFile = solutionFile;
            WorkingDirectory = workingDirectory;
            TimeoutDuration = timeoutDuration ?? DefaultTimeout;
        }

        /// <summary>
        ///     The target branch to compare the current Git HEAD against,
        ///     i.e. the `dev` or `master` branch of the repository.
        /// </summary>
        public required string TargetBranch { get; init; }

        /// <summary>
        ///     The current solution file for us to analyze inside this repository.
        /// </summary>
        public required string SolutionFile { get; init; }

        /// <summary>
        ///     The folder Incrementalist will be working from.
        /// </summary>
        public required string WorkingDirectory { get; init; }

        /// <summary>
        ///     The length of time we're going to allow this Incrementalist operation to run
        ///     prior to cancelling it.
        /// </summary>
        public TimeSpan TimeoutDuration { get; init; }
    }
}

Rationale

This is designed to discourage the LLM from introducing additional syntax that only serves to complicate things for the developer

Suggested Severity

Warning (should be followed)

Additional Context

This seems to only come up in cases, in my code so far, where we have record types that don't use primary constructors in a pre-existing code base.