OleDbConnectionInternal.BuildInfoLiterals should return non-null

[krwq]

This method itself is internal but it affects annotations of couple of public methods.

OleDbConnectionInternal.BuildInfoLiterals code suggests that:

                if (null == dbInfo)
                {
                    return null;
                }

is likely a dead code since dbInfo is annotated as non-nullable which suggests this method will never return null.

Found when applying nullable annotations

[ghost]

Tagging subscribers to this area: @roji, @ajcvickers
See info in area-owners.md if you want to be subscribed.

[ajcvickers]

@roji Can you take a look at this along with the other nullability tweaks?

[roji]

Yep, already on my list.

[roji]

The pattern of doing:

UnsafeNativeMethods.IDBInfo dbInfo = wrapper.Value;
if (null == dbInfo)
{
    return false;
}

... seems common in the codebase, e.g. AddInfoKeywordsToTable does this as well. dbInfo is assigned from IDbInfoWrapper._value, which comes from native COM components; I suspect null would only be observed in some sort of error condition, but I simply don't know enough about OleDB/COM to be sure... So it seems a bit risky to assume that dbInfo is never null.

Note also that OleDbConnection.GetOleDbSchemaTable - the public method wrapping BuildInfoLiterals - also needs to return null because of GetSchemaRowset, which is also annotated to return null. There as well, the rowset variable is assigned by native code and compared to null; this could again be dead code, but we'd have to dive into the native code to be sure.

So given that this is effectively legacy code, and to be on the safe side, it seems better to leave this annotated as nullable... @krwq thoughts?

[ajcvickers]

Closing this as by-design. Can re-open if @krwq has any input.