Some IDE Code Analysis rules are overzealous

[jackpercy-acl]

The below rules are quite opinionated and often want to make changes that reduce code readability. They should either be adjusted down to suggestion or none.

IDE0046 - Use conditional expression for return

This tries to convert a lot of if statements into ternary statements, often massively reducing code readability.

Example 1

Bad:

if (userIdClaim is null)
{
    return null;
}

return int.Parse(userIdClaim.Value, CultureInfo.InvariantCulture);

After auto-fix:

return userIdClaim is null ? null : int.Parse(userIdClaim.Value, CultureInfo.InvariantCulture);

Example 2

Bad:

if (endpointConfig?.Ui is null)
{
    throw new MissingConfigurationException("The 'Ui' property could be read from the 'EndpointConfig' configuration section.");
}

return services.AddCors(options =>
{
    options.AddPolicy(CorsPolicies.Default, builder =>
    {
        builder.WithOrigins(endpointConfig.Ui.ToString().TrimEnd('/'));
        builder.WithHeaders("authorization", "content-type", "connection", "host", "X-Requested-With", "Refer", "Origin");
        builder.WithMethods(HttpMethods.Get, HttpMethods.Post, HttpMethods.Put, HttpMethods.Delete, HttpMethods.Options);
        builder.AllowCredentials();
    });
});

After auto-fix:

return endpointConfig?.Ui is null
    ? throw new MissingConfigurationException("The 'Ui' property could be read from the 'EndpointConfig' configuration section.")
    : services.AddCors(options =>
{
    options.AddPolicy(CorsPolicies.Default, builder =>
    {
        builder.WithOrigins(endpointConfig.Ui.ToString().TrimEnd('/'));
        builder.WithHeaders("authorization", "content-type", "connection", "host", "X-Requested-With", "Refer", "Origin");
        builder.WithMethods(HttpMethods.Get, HttpMethods.Post, HttpMethods.Put, HttpMethods.Delete, HttpMethods.Options);
        builder.AllowCredentials();
    });
});

IDE0058 - Remove unnecessary expression value

Complains if you use a method that returns a value, but you do not need the return value. Auto-fixes with the discard pattern.

Bad:

if (!app.Environment.IsDevelopment())
{
    app.UseHsts();
}

After auto-fix:

if (!app.Environment.IsDevelopment())
{
    _ = app.UseHsts();
}

[OwenLacey28]

We've suppressed IDE0072 because we think it leads to less readable code.

Consider the below code:

        var value = model.PriceType switch
        {
            ContractPriceType.FullMinMax => model.Full_MaxPrice,
            ContractPriceType.PremiumMinMax => model.Premium_MaxPrice,
            ContractPriceType.PremiumMinMaxFutures => model.Premium_MaxPrice,
            ContractPriceType.FixedBaseMinimumPremium => model.Premium_MinPrice,
            ContractPriceType.FixedBaseMinMaxPremium => model.Premium_MinPrice,
            ContractPriceType.FixedBaseFixedPremium => model.Premium_Fixed,
            _ => 0
        };

This is how we should be writing switch expressions, so long as a default arm (_) is specified. This is because the potential fixes are less readable (below).

Populating the remaining legs looks like this:

        var value = model.PriceType switch
        {
            ContractPriceType.FullMinMax => model.Full_MaxPrice,
            ContractPriceType.PremiumMinMax => model.Premium_MaxPrice,
            ContractPriceType.PremiumMinMaxFutures => model.Premium_MaxPrice,
            ContractPriceType.FixedBaseMinimumPremium => model.Premium_MinPrice,
            ContractPriceType.FixedBaseMinMaxPremium => model.Premium_MinPrice,
            ContractPriceType.FixedBaseFixedPremium => model.Premium_Fixed,
            ContractPriceType.Flat or ContractPriceType.PremiumFixed or ContractPriceType.NFE
                or ContractPriceType.PremiumMinimum or ContractPriceType.NPE or ContractPriceType.FixedBase
                or ContractPriceType.FullFixedFutures or ContractPriceType.OptionalPremium
                or ContractPriceType.AnalysisOnly or _ => 0
        };

Switching to if/else looks like this:

        decimal? value;
        if (model.PriceType == ContractPriceType.FullMinMax)
        {
            value = model.Full_MaxPrice;
        }
        else if (model.PriceType is ContractPriceType.PremiumMinMax or ContractPriceType.PremiumMinMaxFutures)
        {
            value = model.Premium_MaxPrice;
        }
        else if (model.PriceType is ContractPriceType.FixedBaseMinimumPremium
                 or ContractPriceType.FixedBaseMinMaxPremium)
        {
            value = model.Premium_MinPrice;
        }
        else if (model.PriceType == ContractPriceType.FixedBaseFixedPremium)
        {
            value = model.Premium_Fixed;
        }
        else
        {
            value = 0;
        }

And changing to a ternary looks like this:

        var value = model.PriceType == ContractPriceType.FullMinMax ? model.Full_MaxPrice :
            model.PriceType == ContractPriceType.PremiumMinMax ? model.Premium_MaxPrice :
            model.PriceType == ContractPriceType.PremiumMinMaxFutures ? model.Premium_MaxPrice :
            model.PriceType == ContractPriceType.FixedBaseMinimumPremium ? model.Premium_MinPrice :
            model.PriceType == ContractPriceType.FixedBaseMinMaxPremium ? model.Premium_MinPrice :
            model.PriceType == ContractPriceType.FixedBaseFixedPremium ? model.Premium_Fixed : 0;