S2589 false alarm, possible null pointer instead

[jcurl]

Description

In the provided code snippet, the line highlighted shows S2589 which is wrong (it can be null). The problem is actually earlier where we use m_LocalConfig and it can be null.

Repro steps

public class Configuration
{
    private class LocalConfiguration
    {
        private string m_LocalConfig;
        private string m_AppName;

        public LocalConfiguration(string fileName, string appName)
        {
            m_LocalConfig = fileName;
            m_AppName = appName;
        }

        public string DefaultDir { get; set; }
    }

    private LocalConfiguration m_LocalConfig;

    public Configuration(string appName, string schema, string defConfig)
    {
        string config;

        if (!string.IsNullOrWhiteSpace(defConfig) && !Path.IsPathRooted(defConfig)) {
            defConfig = Path.Combine(AppDomain.CurrentDomain.BaseDirectory, defConfig);
        }

        if (string.IsNullOrWhiteSpace(defConfig) || !File.Exists(defConfig)) {
            string localConfig = Path.Combine(new string[] {
                System.Environment.GetFolderPath(System.Environment.SpecialFolder.LocalApplicationData),
                "HBAS", "Automation", "local.xml"});

            if (Path.IsPathRooted(localConfig)) {
                m_LocalConfig = new LocalConfiguration(localConfig, appName);
            }

            OpenFileDialog openDialog = new OpenFileDialog();
            // This condition is a possible null reference exception and is not shown
            openDialog.InitialDirectory = m_LocalConfig.DefaultDir;
            openDialog.Multiselect = false;
            openDialog.ShowReadOnly = true;
            openDialog.ReadOnlyChecked = true;
            openDialog.Title = "Open Configuration File";

            openDialog.ShowDialog();
            config = openDialog.FileName;
            if (string.IsNullOrWhiteSpace(config))
                throw new System.IO.FileNotFoundException("Configuration file not selected", config);

            // S2589 says 'Change this condition so that it does not always evaluate to 'true'
            if (m_LocalConfig != null) m_LocalConfig.DefaultDir = Path.GetDirectoryName(openDialog.FileName);
        }
    }
}

Expected behavior

Expect that the possible use of a null reference is caught instead of assuming it is non-null which results in the false alarm later.

Actual behavior

As described above

Known workarounds

If I fix the real problem, the warning goes away:

if (m_LocalConfig != null) openDialog.InitialDirectory = m_LocalConfig.DefaultDir;

Related information

• SonarC# Version 3.3.0
• Visual Studio Version 2015

[michalb-sonar]

Thanks for another good report @jcurl. There are two separate problems here, handled by two different rules.

1. Condition always true (S2589) The rule report is correct here. Either the exception is thrown when accessing m_LocalConfig.DefaultDir, or the condition is always true.
2. Possible NullPointerException (S2259) not reported. The rule only raises when it is certain about the problem. m_LocalConfig is a field, so could have been assigned a value in another method. We are going to improve it in the future, so more cases like that are also caught, especially for private fields, where we can make more assumptions.

[jcurl]

Thanks. It helped me uncover a real problem in any case.