Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Unofficial.Microsoft.Unity.Analyzers nuget package #922

Merged
merged 1 commit into from
Jun 4, 2020

Conversation

DaEgi01
Copy link
Contributor

@DaEgi01 DaEgi01 commented Jun 3, 2020

edited by @aubergine10

Unity breaks how null works...

Specifically:

// 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);
}

// 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;
}

This means that != null, ?. and ?? no longer mean what we think they mean when used on anything based on UnityEngine.Object.

The Microsoft.Unity.Analyzers package (specifically UNT0007 and UNT0008) warns when ?. and ?? are used on something derived from UnityEngine.Object.

Note, however, it does not currently warn about != null issues.

When testing a UE.Object replace:

if (something != null) ...

With:

if (!something) ...

More reliable/comprehensible alternatives might be:

(object)obj == null

// or...

Object.ReferenceEquals(myGameObj, null)

@originalfoo originalfoo added code cleanup Refactor code, remove old code, improve maintainability meta Build environment, github environment, etc. EXTERNAL Mod conflict or other external factor labels Jun 3, 2020
@originalfoo originalfoo added this to the 11.6.0 milestone Jun 3, 2020
@originalfoo
Copy link
Member

^ Updated OP with detailed info of the specific things we are hoping to tackle with the analyzer, and also clarify one case the analyzer doesn't currently detect (but is being considered - see: microsoft/Microsoft.Unity.Analyzers#79 )

Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
JB Rider has built-in similar feature (just an example, ModUI is MonoBehaviour)
image

@DaEgi01
Copy link
Contributor Author

DaEgi01 commented Jun 3, 2020

what about
x is null, I would go for that option if possible since its "normal" c#.

@originalfoo
Copy link
Member

if x is null check works, then yes that would be my preferred option.

Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@DaEgi01 DaEgi01 merged commit 483b883 into master Jun 4, 2020
@DaEgi01 DaEgi01 deleted the Unofficial_Microsoft_Unity_Analyzers branch June 4, 2020 21:34
@originalfoo originalfoo changed the title added Unofficial.Microsoft.Unity.Analyzers nuget package. Add Unofficial.Microsoft.Unity.Analyzers nuget package Jun 4, 2020
@originalfoo originalfoo linked an issue Jun 9, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup Refactor code, remove old code, improve maintainability EXTERNAL Mod conflict or other external factor meta Build environment, github environment, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Unofficial.Microsoft.Unity.Analyzers
3 participants