dotnet / roslyn-analyzers

MIT License
1.59k stars 466 forks source link

CA 1508 false positive when you check for null and then check for an empty string through the string.IsNullOrWhiteSpace method #5305

Closed DmitryKondratenko1986 closed 2 years ago

DmitryKondratenko1986 commented 3 years ago

Analyzer

Diagnostic ID: CA1508: Avoid dead conditional code

Analyzer source

SDK: Built-in CA analyzers in .NET 5 SDK or later

Version: SDK 5.0.302

AND

NuGet Package: Microsoft.CodeAnalysis.NetAnalyzers

Version: 1.1.118 (Latest)

Describe the bug

In the code example below, the analyzer assumes that the second if-condition int the set-sections in Model and Owner properties can never be met, considering this code to be "dead"

Steps To Reproduce

Provide the steps to reproduce the behavior:

  1. Create class library project with .net 5.0
  2. Add project property AnalysisMode: AllEnabledByDefault
  3. Create a simple class (see the code example below)
  4. See CA1508 warning in the second if-branch in the Owner and Model properties

Sample

public class Vehicle
{
    private string model;
    private string owner;

    public Vehicle(string model, string owner)
    {
        this.Model = model;
        this.Owner = owner;
    }

    public string Model
    {
        get => this.model;
        private set
        {
            if (value is null)
            {
                throw new ArgumentNullException(nameof(value));
            }

            if (string.IsNullOrWhiteSpace(value))
            {
                throw new ArgumentException($"Model cannot be empty or whitespace", nameof(value));
            }

            this.model = value;
        }
    }

    public string Owner
    {
        get => this.owner;
        set
        {
            if (value is null)
            {
                throw new ArgumentNullException(nameof(value));
            }

            if (string.IsNullOrWhiteSpace(value))
            {
                throw new ArgumentException($"Owner cannot be empty or whitespace", nameof(value));
            }

            this.owner = value;
        }
    }
}

Expected behavior

No CA1508.

Actual behavior

CA1508 false positive.

Additional context

изображение

I went into this branch of code using a debugger изображение

mavasani commented 3 years ago

This is likely fixed in the latest NuGet package/.NET 6 SDK

mavasani commented 2 years ago

Yep, confirmed this is fixed in .NET6 SDK