SonarSource / sonar-dotnet

Code analyzer for C# and VB.NET projects
https://redirect.sonarsource.com/plugins/csharp.html
GNU Lesser General Public License v3.0
788 stars 227 forks source link

S1172 FP: Parameter used as extension delegate target #5338

Open Luburic opened 2 years ago

Luburic commented 2 years ago

Description

False positive for rule ID S1172 (Unused method parameters should be removed) when the parameter is used as a method group.

Repro steps

The following code snippet raises the issue on SonarCloud for the smallerOrEqualArray:

    private static double GetDegreeOfOverlap(string[] largerArray, string[] smallerOrEqualArray)
    {
        return (double) largerArray.Count(smallerOrEqualArray.Contains) / largerArray.Length;
    }

Expected behavior

The rule should not be triggered.

Actual behavior

The rule is triggered for the smallerOrEqualArray parameter.

Related information

pavel-mikula-sonarsource commented 2 years ago

Hi @Luburic,

Thank you for reporting this case. I can confirm it as False Positive.

There must be something very tricky going on under the hood, as we've seen a similar case working under .NET compilation but not working under .NET Framework. This one reproduces under .NET as well.

pavel-mikula-sonarsource commented 2 years ago

This has the same root cause as the .NET Framework variant. It's reported to Roslyn here: https://github.com/dotnet/roslyn/issues/56644

DJAxel commented 2 years ago

I have similar issues with the following methods:

private T GetConfig<T>(string key)
{
    return _config.GetValue<T>($"{_configKey}:{key}");
}

private static string GetFilenameWithoutExtension(string filename)
{
    return filename.Substring(0, filename.IndexOf('.'));
}

Not sure if it has the same root cause, but it looks like there is a lit of cases where SonarLint throws in a false positive on S1172. Hopefully my two examples contribute to finding the problem.

pavel-mikula-sonarsource commented 2 years ago

Hi @DJAxel, your case is more likely related to the #5491 that was fixed last week and will be part of the next release.

mary-georgiou-sonarsource commented 1 year ago

Another post reporting this issue here.

m-gallesio commented 1 year ago

To reiterate it here, the problem seems to be only with private methods and not public ones:

using System.Collections.Generic;
using System.Linq;

namespace Test;

public class Test
{
    private void BuildSectionPrivate(
        IReadOnlyDictionary<int, string> items, // <- triggers the rule
        IEnumerable<int> values
    )
    {
        var groups = values.Select(items.GetValueOrDefault);
    }

    public void BuildSectionPublic(
        IReadOnlyDictionary<int, string> items, // <- does not trigger the rule
        IEnumerable<int> values
    )
    {
        var groups = values.Select(items.GetValueOrDefault);
    }
}
mary-georgiou-sonarsource commented 1 year ago

Another similar issue here: https://github.com/SonarSource/sonar-dotnet/issues/7717

static void ConfigureServices()
{
    IServiceCollection services = null!;
    IConfiguration configuration = null!;

    void ConfigureOptions<TOptions>(string sectionKey) // <- S1172 sectionKey is not used
        where TOptions : class
    {
        services.Configure<TOptions>(configuration
            .GetSection(sectionKey)
            .Bind);
    }
}

Once the delegate Bind is added to be passed as a parameter, the ParameterReferenceOperation for sectionKey disappears from the control flow graph.

cristian-ambrosini-sonarsource commented 1 year ago

Another similar issue was reported by the community:

private static IEnumerable<DbContext> GetAllDbContextInstances(IServiceProvider serviceProvider) // Remove this parameter 'serviceProvider', whose value is ignored in the method.
{
    var types = GetAllDbContextTypes();
    var instances = types.Select(serviceProvider.GetRequiredService);
    foreach (var instance in instances)
    {
        yield return (DbContext)instance;
    }
}
m-gallesio commented 7 months ago

Bump, this still applies as of SonarQube 10.4.