csharp-styleguide icon indicating copy to clipboard operation
csharp-styleguide copied to clipboard

See if .NET 8 analyzers have fixed issues with CA1508

Open pakdev opened this issue 1 year ago • 3 comments

@jonathanou has seen false positives of the .NET 7 implementation of the CA1508 analyzer in internal codebases. The .NET 8 implementation needs to be tested to see if those issues are resolved.

pakdev avatar Nov 08 '23 16:11 pakdev

Issues still present using the latest preview .NET 8 analyzers. Particularly two issues:

This code got a false positive saying fileStream is never null in the finally block:

FileStream fileStream = null;
try
{
    fileStream = LongPathFile.OpenRead(file);
    using (var reader = new BinaryReader(fileStream))
    {
        fileStream = null;
        byte[] bytes = reader.ReadBytes(_pngHeader.Length);
        return _bmpHeader.SequenceEqual(bytes.Take(_bmpHeader.Length)) || _pngHeader.SequenceEqual(bytes.Take(_pngHeader.Length));
    }
}
catch (IOException)
{
    return false;
}
finally
{
    if (fileStream != null)
    {
        fileStream.Dispose();
    }
}

The second one was the double-lock pattern was not being recognized, saying _systemAssessment == null in the second check is always true:

if (_systemAssessment == null)
{
    lock (_lockObject)
    {
        if (_systemAssessment == null)
        {
            // ...
        }
    }
}

I'll look to create github issues for these issues, but we'll have to keep CA 1508 disabled for now.

jonathanou avatar Nov 08 '23 17:11 jonathanou

For the double-locked pattern false-positive, there is already a github issue: https://github.com/dotnet/roslyn-analyzers/issues/1649

For for-loop false-positive (I didn't list this in the reply above, but I did also see this in ASW): https://github.com/dotnet/roslyn-analyzers/issues/3874

I filed a new issue https://github.com/dotnet/roslyn-analyzers/issues/7028 for the fileStream false positive example.

jonathanou avatar Nov 14 '23 03:11 jonathanou

Additional info: looking at the open issues related to CA 1508, they are quite numerous and many of them unresolved for multiple years: https://github.com/dotnet/roslyn-analyzers/issues?q=is%3Aissue+is%3Aopen+CA1508

jonathanou avatar Nov 14 '23 03:11 jonathanou