IDISP023 Does not detect code flow correctly

[Pretasoc]

Consider the following snippet

public class Foo : IDisposable
{
    private Lazy<IDisposable> _bar; // Bar is Disposable

    protected virtual void Dispose(bool disposing)
    {
         if(disposing && _bar.IsValueCreated)
             _bar.Value?.Dispose();
    }
}

This snippet results in two IDISP023 warnings. One inside the if statements condition and one in the body. But in this example it can be proved, that _bar is never accessed inside the finalzier context, but the analyzer does not detect that correctly.

I also tried the following snippet and it resulted also in a wrong warning.

protected virtual void Dispose(bool disposing)
{
     if (!disposing)
         return;

    disposable.Dispose();
}

I didn't check, how the analyzer is implemented, but i think you could use the code flow model provided by roslyn. (Not sure how it is actually named, i just remember theres something like that used fur nullability)

[JohanLarsson]

Thanks for reporting, awesome issue with nice repro! Sorry about the bug.

[sungam3r]

Still actual for version 3.4.1:

 protected virtual void Dispose(bool disposing)
        {
            if (disposing && Interlocked.CompareExchange(ref _disposed, 1, 0) == 0) // <-- IDISP023
                someField.Dispose();
        }

[sungam3r]

OK for this:

   protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
                if (Interlocked.CompareExchange(ref _disposed, 1, 0) == 0)
                    _context.Dispose();
            }
        }

[JohanLarsson]

@sungam3r yes we can do much better figuring out flow here but question is where to stop. I agree handling if (a && b) is reasonable as it is a common and simple case.

[TheR00st3r]

@JohanLarsson The second case if(!disposing) return; would be really good. In my case all disposable implementations use this scheme.

[lonix1]

Another false positive: it's triggered when using a lock to ensure thread safety during disposal:

private readonly object _locker = new object();

protected virtual void Dispose(bool disposing)
{
  if (!_isDisposed)
  {
    lock (_locker)                      // <--------- IDISP023
    {

      // release managed resources
      if (disposing)
      {
        // ...
      }

      // release unmanaged resources
      //...

    }
  }
}

[Eli-Black-Work]

Another repo:

private bool _disposed;

protected virtual void Dispose(bool disposing)
{
    if (!_disposed || !disposing)
        return;

    // IDISP023 warning
    _myObject.Dispose();

    _disposed = true;
}