S2589 in a while loop with post-if counter

[samyonr (Samyon Ristov)]

• What language is this for? C#
• Which rule? S2589
  Warning S2589 Change this condition so that it does not always evaluate to ‘True’.
• Why do you believe it’s a false-positive/false-negative?
  The counter is incremented after the if-statement, and eventually is higher than the pre-defined value.
• Are you using
  • SonarCloud? No
  • SonarQube - which version? No
  • SonarLint - which IDE/version? Visual Studio 2022 Version 17.6.6, with SonarLint Version 7.3.0.77872
    • in connected mode with SonarQube or SonarCloud? No
• How can we reproduce the problem? Give us a self-contained snippet of code (formatted text, no screenshots)

    public class SamplesMonitor
    {
        private readonly ChannelReader<GenericSample> _reader;

        private const int WaitForMoreDelay = 1000; // ms
        private const int WaitForMoreAttempts = 3;
        private const int WaitForMoreSamplesCount = 100;

        public SamplesMonitor(ChannelReader<GenericSample> reader)
        {
            _reader = reader;
        }

        public async Task<(IList<GenericSample> samples, bool noMoreData)> LoadSamplesAsync(CancellationToken cancellationToken = default)
        {
            try
            {
                var samples = new List<GenericSample>();
                var noMoreData = false;
                var tryRead = true;
                var waitForMoreAttempt = 1;

                if (await _reader.WaitToReadAsync(cancellationToken))
                {
                    noMoreData = false;

                    while (tryRead)
                    {
                        // Fast loop around available items
                        while (_reader.TryRead(out var item))
                        {
                            samples.Add(item);
                        }

                        if (samples.Count < WaitForMoreSamplesCount && waitForMoreAttempt <= WaitForMoreAttempts) // waitForMoreAttempt <= WaitForMoreAttempts causes S2589 with < and with <=
                        {
                            waitForMoreAttempt++;
                            await Task.Delay(WaitForMoreDelay, cancellationToken);
                        }
                        else
                        {
                            tryRead = false;
                        }
                    }
                }
                else
                {
                    noMoreData = true;
                }
                return (samples, noMoreData);
            }
            catch (OperationCanceledException)
            {
                throw;
            }
            catch (Exception e)
            {
                throw;
            }
        }
    }

In the code above, waitForMoreAttempt <= WaitForMoreAttempt triggers the warning. Also, changing <= to <, i.e. waitForMoreAttempt < WaitForMoreAttempt, triggers the warning.
Deleting samples.Count < WaitForMoreSamplesCount && so the if-statements gets to be:

if (waitForMoreAttempt <= WaitForMoreAttempts)

Doesn’t help either.

Thanks in advance,
Samyon.

[tim.pohlmann]

Hello @samyonr! Welcome to our community, and thanks a lot for your report.
Unfortunately, this issue is caused by a limitation of our Symbolic Execution engine. For performance reasons, it validates only two iterations of a given loop. This means the engine never sees waitForMoreAttempt being bigger than 2 when analyzing the condition.
We have some tricks in place to circumvent these kinds of issues for counter-variables, but they don’t work for your specific example. We want to improve this rule in the future but have no detailed plans yet.

What you can try is to change your loop to something like this and see if it resolves the issue:

var waitForMoreAttempt = 0;
do
{
    if (waitForMoreAttempt > 0)
    {
        await Task.Delay(WaitForMoreDelay, cancellationToken);
    }
    // Fast loop around available items
    while (_reader.TryRead(out var item))
    {
        samples.Add(item);
    }
    waitForMoreAttempt++;
}
while (samples.Count < WaitForMoreSamplesCount && waitForMoreAttempt <= WaitForMoreAttempts);

[samyonr (Samyon Ristov)]

Hi Tim, thanks for checking it.
It’s indeed unfortunate that there are such limitations on this type of logic. However, the solution you suggested seems to work well - no warnings appear. It’ll do the trick for me. Thanks!