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
772 stars 226 forks source link

Fix S2589 FP: List count with AddRange() #8570

Closed rodchenkov closed 1 week ago

rodchenkov commented 7 months ago

Description

I have warning S2589 for list.Count == 0 but I'm sure it is false positive.

Repro steps

Please find code below:

        public void Apply(OpenApiOperation operation, OperationFilterContext context)
        {
            ArgumentNullException.ThrowIfNull(operation);
            ArgumentNullException.ThrowIfNull(context);

            if (Array.Exists(context.MethodInfo.GetCustomAttributes(true), x => x is AllowAnonymousAttribute)) return;

            var authorizeAttributes = new List<AuthorizeAttribute>();
            var swaggerSecurityAttributes = new List<SwaggerSecurityAttribute>();

            // Get method attributes
            authorizeAttributes.AddRange(context.MethodInfo.GetCustomAttributes(true).OfType<AuthorizeAttribute>());
            swaggerSecurityAttributes.AddRange(context.MethodInfo.GetCustomAttributes(true).OfType<SwaggerSecurityAttribute>());

            // Get class attributes
            if (context.MethodInfo.DeclaringType != null)
            {
                authorizeAttributes.AddRange(context.MethodInfo.DeclaringType.GetCustomAttributes(true).OfType<AuthorizeAttribute>());
                swaggerSecurityAttributes.AddRange(context.MethodInfo.DeclaringType.GetCustomAttributes(true).OfType<SwaggerSecurityAttribute>());
            }

#pragma warning disable S2589
            if (authorizeAttributes.Count == 0) return;
#pragma warning restore S2589

            ...

         }

Expected behavior

No warning S2589 for line if (authorizeAttributes.Count == 0) return;

Actual behavior

Warning S2589 for line if (authorizeAttributes.Count == 0) return;

Known workarounds

Only suppress warning

Related information

gregory-paidis-sonarsource commented 7 months ago

Hey @rodchenkov , I confirm that this is a false positive. I added it to our backlog, where it will be taken care of along other S2589 FPs soon. Thanks for raising this.

gregory-paidis-sonarsource commented 6 months ago

Something to add, implementation-wise: In the expression first.AddRange(second), second is cleared of CollectionConstraint because it's used as Argument. This might be problematic when trying to establish when AddRange should influence the CollectionConstraint of first.

devedse commented 1 month ago

I also ran into this issue, wouldn't it be better to disable this rule for now until it's fixed?

Here's another simple scenario to reproduce it:

namespace SonarQubeRulesTest;

internal class Program
{
    static void Main(string[] args)
    {
        var theList = new List<int>();

        theList.AddRange(GetNumbers());

        if (theList.Any())
        {
            Console.WriteLine("Numbers found");
        }
        else
        {
            Console.WriteLine("No numbers found");
        }

    }

    public static IEnumerable<int> GetNumbers()
    {
        return new List<int> { };
    }
}
Tim-Pohlmann commented 1 week ago

Another user report