Limit UNT0007 and UNT0008 to `MonoBehaviour`?

[originalfoo]

Bug description

This is more of a query as to the scope of the rules, rather than a bug report.

• Version of analyzers assembly: Unofficial.Microsoft.Unity.Analyzers 1.0.0
• Analyzer rules: UNT0007 and UNT0008

These rules appear to be flagging anything that is derived from UnityEngine.Object, however it appears the issues tackled by UNT0007 and UNT0008 are primarily related to stuff derived from MonoBehaviour - as far as I can tell.

For example, in the game Cities: Skylines:

prop.m_finalProp.m_mesh.name

The prop and m_finalProp are both derived from MonoBehaviour - as expected, anything null-related is problematic with them. The analyzer correctly flags that issue.

However, the m_mesh is derived from Unity.Object but not MonoBehaviour. In my testing I've not encountered any problems using ?., ?? or == null - they have always behaved as expected.

Also, in the case of == null checking instances derived from MonoBehaviour:

// problematic - fails to spot null ref
if (someThing == null) ...

// reliable
if (someThing) ...

The analyzer does not catch that issue.

Note: Cities: Skylines runs on Mono "2.0" (in Dr. Evil air quotes, b/c #define VERSION 2.0 and b/c game dev applied some extra tweaks so it's sort of v2.ish).

Note 2: I've not done much testing with GameObject so not sure if that's problematic like MonoBehaviour.

[sailro]

Hi,

Operators are overridden in UnityEngine.Object, and nothing is done regarding this in GameObject nor in MonoBehaviour types. That's why those rules apply to all types derived from UnityEngine.Object.

// UnityEngine.Object
public static bool operator ==(Object x, Object y)
{
    return CompareBaseObjects(x, y);
}

// UnityEngine.Object
public static implicit operator bool(Object exists)
{
    return !CompareBaseObjects(exists, null);
}

// UnityEngine.Object
public static bool operator !=(Object x, Object y)
{
    return !CompareBaseObjects(x, y);
}

with

// UnityEngine.Object
private static bool CompareBaseObjects(Object lhs, Object rhs)
{
    bool flag = (object)lhs == null;
    bool flag2 = (object)rhs == null;
    if (flag2 && flag)
    {
        return true;
    }
    if (flag2)
    {
        return !IsNativeObjectAlive(lhs);
    }
    if (flag)
    {
        return !IsNativeObjectAlive(rhs);
    }
    return lhs.m_InstanceID == rhs.m_InstanceID;
}

[originalfoo]

Ouch. I will now cry over the loss of terse mod code.

In regards to the == null check, is there a way to make the analyzer flag that in similar way to UNT0007 and UNT0008?

[sailro]

To be discussed with @jbevain

This is something we can do, we can even offer to replace the check with a real-null-check-expression like:

• (object)obj == null
• or Object.ReferenceEquals(myGameObj, null)

But my fear is that Unity developers are now using this null check the 'Unity way' to effectively test if an object is marked as destroyed by the Unity Engine.

[sailro]

After discussing it, given it's the default from the very beginning and we don't want to clutter the error list with a ton of messages, we do not plan to add such an analyzer so far.

Closing this ticket.

Thanks!