[C#] false positive : change this condition so it does not always evaluate to 'false'

[Keith Roberts]

I've come across a case where sonarqube C# is reporting an issue that isn't an issue.

In the code below, on the highlighted section (paramCode != null) it reports "Change this condition so that it does not always evaluate to 'false'; some subsequent code is never executed"

This isn't the real logic (obviously), but it represents it.  paramCode can clearly either be null or "abc" depending on conditions, routeCode as well.  Yet the C# plugin believes that paramCode cannot ever be non-null.

        public void Function()

        {

            string routeCode = null, paramCode = null;

            if (5 == new Random().Next(10))

            {

                if (3 == new Random().Next(10))

                {

                    paramCode = "abc";

                }

                else if (4 == new Random().Next(10))

                {

                    routeCode = "def";

                }

            }

            if (routeCode != null && paramCode != null)

                throw new LogicalBadRequestException("provider code specified in both route and param");

        }

Even if I change it to this

               if (3 == 3) //new Random().Next(10))

               {

                    paramCode = "abc2";

               }

I still get the spurious warning.

[valeri....@sonarsource.com]

Hi Keith,

I could not reproduce the problem with the provided sample, to be sure what's going on I would need a stable reproducer.

I will try to guess what is happening behind the warning for the provided code:

1) when routeCode is null, paramCode is not evaluated (because && is short-circuit-ing)

2) when routeCode is not null, paramCode is null

3) when the evaluation of the paths completes, paramCode was found to be only null, hence we raise an issue

Basically the engine follows all possible control flow paths using the variable values for each branch and if a condition is evaluated only with true or false we raise.

Kind regards,

Valeri

[Keith Roberts]

Strange that you cannot reproduce it.  I used sonarlint in visual studio hooked up to sonarqube 6.4 and c# plugin 6.4.1 (build 3596)

In the actual code, the outer [if (5 == new Random().Next(10))] part is actually a foreach loop on a collection, so the inner section can run multiple times.  Here is a screenshot from the web view, and below is the actual code triggering this.

Thanks,

Keith

        public override void OnActionExecuting(HttpActionContext actionContext)

        {

            // allow controller methods to turn provider mapping off

            if (actionContext.ActionDescriptor.GetCustomAttributes<DisableProviderCodeMappingAttribute>(false).Any())

            {

                actionContext.Request.Properties.Add(ProviderCodeMediaTypeFormatter.ProviderCodeProperty, ProviderCodeMediaTypeFormatter.ProviderCodeNotNeeded);

                return;

            }

            string routeCode = null, paramCode = null;

            string sourceRoute = null, sourceParam = null;

            string bodyCode = GetBodyCode(actionContext.Request);

            foreach (var p in actionContext.ActionDescriptor.GetParameters())

            {

                if (p.GetCustomAttributes<EpsIdAttribute>().Any())

                {

                    string curValue = actionContext.ActionArguments[p.ParameterName] as string;

                    if (curValue == null)

                        continue;

                    var newVal = ProcessValue(curValue);

                    actionContext.ActionArguments[p.ParameterName] = newVal.Item1;

                    if (routeCode != null && newVal.Item2 != routeCode)

                        throw new LogicalBadRequestException($"conflicting provider codes in route params: src '{sourceRoute}' = '{routeCode}', got '{p.ParameterName}' = '{newVal.Item2}");

                    if (routeCode == null)

                    {

                        sourceRoute = p.ParameterName;

                        routeCode = newVal.Item2;

                    }

                }

                else if (p.ParameterType == typeof(LendingProviderCode) || Nullable.GetUnderlyingType(p.ParameterType) == typeof(LendingProviderCode))

                {

                    var pvalue = actionContext.ActionArguments[p.ParameterName];

                    if (pvalue == null)

                        continue;

                    var curValue = ((int)(LendingProviderCode)pvalue).ToString();

                    if (paramCode != null && paramCode != curValue)

                        throw new LogicalBadRequestException($"conflicting provider codes in query params: src '{sourceParam}' = '{paramCode}', got '{p.ParameterName}' = '{curValue}");

                    if (paramCode == null)

                    {

                        sourceParam = p.ParameterName;

                        paramCode = curValue;

                    }

                }

            }

            if (routeCode == null && paramCode == null && bodyCode == null)

                throw new LogicalBadRequestException("no provider code in route or param");

            if (routeCode != null && paramCode != null)

                1throw new LogicalBadRequestException("provider code specified in both route and param");

            if ((routeCode != null || paramCode != null) && !AllAreEqual(routeCode, bodyCode, paramCode))

                throw new LogicalBadRequestException($"provider code mismatch: route:'{routeCode}', body:'{bodyCode}', 'param':{paramCode}");

            SetProperty(actionContext.Request, routeCode ?? paramCode ?? bodyCode);