Add UNT0029, Pattern matching with null on Unity objects (#229)

* Add UNT0029, Pattern matching with null on Unity objects

* Use matching instead

* Fix all nullability warnings after Roslyn 3.7.0 bump (#230)

doc/UNT0029.md

@@ -0,0 +1,39 @@
+# UNT0029 Pattern matching with null on Unity objects
+
+Unity overrides the null comparison operator for Unity objects, which is incompatible with pattern matching with null.
+
+## Examples of patterns that are flagged by this analyzer
+
+```csharp
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public Transform a = null;
+
+    public void Update()
+    {
+        if (a is not null) { }
+    }
+}
+```
+
+## Solution
+
+Use null comparison:
+
+```csharp
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public Transform a = null;
+
+    public void Update()
+    {
+        if (a != null) { }
+    }
+}
+```
+
+A code fix is offered for this diagnostic to automatically apply this change.

doc/index.md

@@ -30,6 +30,7 @@ ID | Title | Category
 [UNT0026](UNT0026.md) | GetComponent always allocates | Performance
 [UNT0027](UNT0027.md) | Do not call PropertyDrawer.OnGUI() | Correctness
 [UNT0028](UNT0028.md) | Use non-allocating physics APIs | Performance
+[UNT0029](UNT0029.md) | Pattern matching with null on Unity objects | Correctness
 
 # Diagnostic Suppressors
 

src/Microsoft.Unity.Analyzers.Tests/UnityObjectNullHandlingTests.cs

@@ -344,7 +344,6 @@ public Transform NP()
 		await VerifyCSharpFixAsync(test, fixedTest);
 	}
 
-
 	[Fact]
 	public async Task CoalescingAssignmentForRegularObjects()
 	{
@@ -361,6 +360,102 @@ public System.Object NP()
         return a ??= b;
     }
 }
+";
+
+		await VerifyCSharpDiagnosticAsync(test);
+	}
+
+	[Fact]
+	public async Task IsNullPatternExpression()
+	{
+		const string test = @"
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public Transform a = null;
+
+    public void Update()
+    {
+        if (a is null) { }
+    }
+}
+";
+
+		var diagnostic = ExpectDiagnostic(UnityObjectNullHandlingAnalyzer.IsPatternRule)
+			.WithLocation(10, 13);
+
+		await VerifyCSharpDiagnosticAsync(test, diagnostic);
+
+		const string fixedTest = @"
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public Transform a = null;
+
+    public void Update()
+    {
+        if (a == null) { }
+    }
+}
+";
+		await VerifyCSharpFixAsync(test, fixedTest);
+	}
+
+	[Fact]
+	public async Task IsNotNullPatternExpression()
+	{
+		const string test = @"
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public Transform a = null;
+
+    public void Update()
+    {
+        if (a is not null) { }
+    }
+}
+";
+
+		var diagnostic = ExpectDiagnostic(UnityObjectNullHandlingAnalyzer.IsPatternRule)
+			.WithLocation(10, 13);
+
+		await VerifyCSharpDiagnosticAsync(test, diagnostic);
+
+		const string fixedTest = @"
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public Transform a = null;
+
+    public void Update()
+    {
+        if (a != null) { }
+    }
+}
+";
+		await VerifyCSharpFixAsync(test, fixedTest);
+	}
+
+	[Fact]
+	public async Task IsRegularPatternExpression()
+	{
+		const string test = @"
+using UnityEngine;
+
+class Camera : MonoBehaviour
+{
+    public object a = null;
+
+    public void Update()
+    {
+        if (a is not null) { }
+    }
+}
 ";
 
 		await VerifyCSharpDiagnosticAsync(test);

src/Microsoft.Unity.Analyzers/ContextMenuSuppressor.cs

@@ -43,8 +43,11 @@ public override void ReportSuppressions(SuppressionAnalysisContext context)
 
 	private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context)
 	{
-		var location = diagnostic.Location;
-		var model = context.GetSemanticModel(location.SourceTree);
+		var syntaxTree = diagnostic.Location.SourceTree;
+		if (syntaxTree == null)
+			return;
+
+		var model = context.GetSemanticModel(syntaxTree);
 
 		var node = context.GetSuppressibleNode<SyntaxNode>(diagnostic, n => n is MethodDeclarationSyntax or VariableDeclaratorSyntax);
 		switch (node)
@@ -69,13 +72,13 @@ private static bool IsReportable(ISymbol symbol)
 		switch (symbol)
 		{
 			case IMethodSymbol methodSymbol:
-				if (methodSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEngine.ContextMenu)) || a.AttributeClass.Matches(typeof(UnityEditor.MenuItem))))
+				if (methodSymbol.GetAttributes().Any(a => a.AttributeClass != null && (a.AttributeClass.Matches(typeof(UnityEngine.ContextMenu)) || a.AttributeClass.Matches(typeof(UnityEditor.MenuItem)))))
 					return true;
 				if (IsReferencedByContextMenuItem(methodSymbol, containingType))
 					return true;
 				break;
 			case IFieldSymbol fieldSymbol:
-				if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute))))
+				if (fieldSymbol.GetAttributes().Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute))))
 					return true;
 				break;
 		}
@@ -92,7 +95,7 @@ private static bool IsReferencedByContextMenuItem(IMethodSymbol symbol, INamedTy
 
 			var attributes = fieldSymbol
 				.GetAttributes()
-				.Where(a => a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute)))
+				.Where(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEngine.ContextMenuItemAttribute)))
 				.ToArray();
 
 			if (!attributes.Any())

src/Microsoft.Unity.Analyzers/CreateInstance.cs

@@ -152,7 +152,7 @@ private static async Task<Document> ReplaceWithInvocationAsync(Document document
 							SingletonSeparatedList<TypeSyntax>(
 								IdentifierName(typeInfo.Type.Name))))));
 
-		var newRoot = root.ReplaceNode(creation, invocation);
+		var newRoot = root?.ReplaceNode(creation, invocation);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/EmptyUnityMessage.cs

@@ -54,11 +54,17 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context)
 			return;
 
 		var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration);
+		if (typeSymbol == null)
+			return;
+
 		var scriptInfo = new ScriptInfo(typeSymbol);
 		if (!scriptInfo.HasMessages)
 			return;
 
 		var symbol = context.SemanticModel.GetDeclaredSymbol(method);
+		if (symbol == null)
+			return;
+
 		if (!scriptInfo.IsMessage(symbol))
 			return;
 
@@ -106,7 +112,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
 	private static async Task<Document> DeleteEmptyMessageAsync(Document document, MethodDeclarationSyntax declaration, CancellationToken cancellationToken)
 	{
 		var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
-		var newRoot = root.RemoveNode(declaration, SyntaxRemoveOptions.KeepNoTrivia);
+		var newRoot = root?.RemoveNode(declaration, SyntaxRemoveOptions.KeepNoTrivia);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs

@@ -36,7 +36,11 @@ private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext
 		if (methodDeclarationSyntax == null)
 			return;
 
-		var model = context.GetSemanticModel(diagnostic.Location.SourceTree);
+		var syntaxTree = diagnostic.Location.SourceTree;
+		if (syntaxTree == null)
+			return;
+
+		var model = context.GetSemanticModel(syntaxTree);
 		if (model.GetDeclaredSymbol(methodDeclarationSyntax) is not IMethodSymbol methodSymbol)
 			return;
 
@@ -49,6 +53,6 @@ private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext
 	private bool IsSuppressable(IMethodSymbol methodSymbol)
 	{
 		// The Unity code stripper will consider any attribute with the exact name "PreserveAttribute", regardless of the namespace or assembly
-		return methodSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(JetBrains.Annotations.UsedImplicitlyAttribute)) || a.AttributeClass.Name == nameof(UnityEngine.Scripting.PreserveAttribute));
+		return methodSymbol.GetAttributes().Any(a => a.AttributeClass != null && (a.AttributeClass.Matches(typeof(JetBrains.Annotations.UsedImplicitlyAttribute)) || a.AttributeClass.Name == nameof(UnityEngine.Scripting.PreserveAttribute)));
 	}
 }

src/Microsoft.Unity.Analyzers/ImproperMenuItemMethod.cs

@@ -46,7 +46,7 @@ private static void AnalyzeMethodDeclaration(SyntaxNodeAnalysisContext context)
 		if (context.SemanticModel.GetDeclaredSymbol(method) is not { } methodSymbol)
 			return;
 
-		if (!methodSymbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEditor.MenuItem))))
+		if (!methodSymbol.GetAttributes().Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEditor.MenuItem))))
 			return;
 
 		if (methodSymbol.IsStatic)
@@ -89,7 +89,7 @@ private static async Task<Document> AddStaticDeclarationAsync(Document document,
 			newMethodDeclaration = newMethodDeclaration.AddModifiers(SyntaxFactory.Token(SyntaxKind.StaticKeyword));
 		}
 
-		var newRoot = root.ReplaceNode(methodDeclaration, newMethodDeclaration);
+		var newRoot = root?.ReplaceNode(methodDeclaration, newMethodDeclaration);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/ImproperSerializeField.cs

@@ -38,7 +38,7 @@ public override void Initialize(AnalysisContext context)
 	private static void AnalyzeMemberDeclaration(SyntaxNodeAnalysisContext context)
 	{
 		var model = context.SemanticModel;
-		ISymbol symbol;
+		ISymbol? symbol;
 
 		switch (context.Node)
 		{
@@ -58,6 +58,9 @@ private static void AnalyzeMemberDeclaration(SyntaxNodeAnalysisContext context)
 				return;
 		}
 
+		if (symbol == null)
+			return;
+
 		if (!IsReportable(symbol))
 			return;
 
@@ -69,7 +72,7 @@ private static bool IsReportable(ISymbol symbol)
 		if (!symbol.ContainingType.Extends(typeof(UnityEngine.Object)))
 			return false;
 
-		if (!symbol.GetAttributes().Any(a => a.AttributeClass.Matches(typeof(UnityEngine.SerializeField))))
+		if (!symbol.GetAttributes().Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEngine.SerializeField))))
 			return false;
 
 		return symbol switch
@@ -126,7 +129,7 @@ private static async Task<Document> RemoveSerializeFieldAttributeAsync(Document
 			if (nodes.Any())
 			{
 				var newAttributes = attributeList.RemoveNodes(nodes, SyntaxRemoveOptions.KeepNoTrivia);
-				if (newAttributes.Attributes.Any())
+				if (newAttributes != null && newAttributes.Attributes.Any())
 					attributes = attributes.Add(newAttributes);
 			}
 			else
@@ -137,7 +140,7 @@ private static async Task<Document> RemoveSerializeFieldAttributeAsync(Document
 			.WithAttributeLists(attributes)
 			.WithLeadingTrivia(declaration.GetLeadingTrivia());
 
-		var newRoot = root.ReplaceNode(declaration, newDeclaration);
+		var newRoot = root?.ReplaceNode(declaration, newDeclaration);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/IndirectionMessage.cs

@@ -87,7 +87,7 @@ private static async Task<Document> DeleteIndirectionAsync(Document document, Me
 	{
 		var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 		var newExpression = access.Expression;
-		var newRoot = root.ReplaceNode(access, newExpression);
+		var newRoot = root?.ReplaceNode(access, newExpression);
 
 		if (newRoot == null)
 			return document;

src/Microsoft.Unity.Analyzers/InitializeOnLoadStaticCtor.cs

@@ -44,10 +44,12 @@ private static void AnalyzeClassDeclaration(SyntaxNodeAnalysisContext context)
 			return;
 
 		var typeSymbol = context.SemanticModel.GetDeclaredSymbol(classDeclaration);
+		if (typeSymbol == null)
+			return;
 
 		var isInitOnLoad = typeSymbol
 			.GetAttributes()
-			.Any(a => a.AttributeClass.Matches(typeof(UnityEditor.InitializeOnLoadAttribute)));
+			.Any(a => a.AttributeClass != null && a.AttributeClass.Matches(typeof(UnityEditor.InitializeOnLoadAttribute)));
 
 		if (!isInitOnLoad)
 			return;
@@ -94,7 +96,7 @@ private static async Task<Document> CreateStaticCtorAsync(Document document, Cla
 		var newClassDeclaration = classDeclaration
 			.WithMembers(classDeclaration.Members.Insert(0, emptyStaticConstructor));
 
-		var newRoot = root.ReplaceNode(classDeclaration, newClassDeclaration);
+		var newRoot = root?.ReplaceNode(classDeclaration, newClassDeclaration);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/InputGetKey.cs

@@ -162,7 +162,7 @@ private async Task<Document> UseKeyCodeMemberAsArgumentAsync(Document document,
 				.ReplaceNode(argument, GetKeyCodeArgument(les)
 					.WithTrailingTrivia(argument.GetTrailingTrivia())));
 
-		var newRoot = root.ReplaceNode(invocation, newInvocation);
+		var newRoot = root?.ReplaceNode(invocation, newInvocation);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/LoadAttributeMethod.cs

@@ -63,7 +63,7 @@ private static bool IsDecorated(ISymbol symbol)
 	{
 		return symbol
 			.GetAttributes()
-			.Any(a => IsLoadAttributeType(a.AttributeClass));
+			.Any(a => a.AttributeClass != null && IsLoadAttributeType(a.AttributeClass));
 	}
 
 	private static bool IsLoadAttributeType(ITypeSymbol type)
@@ -124,7 +124,7 @@ private static async Task<Document> FixMethodAsync(Document document, MethodDecl
 				.AddModifiers(SyntaxFactory.Token(SyntaxKind.StaticKeyword));
 		}
 
-		var newRoot = root.ReplaceNode(methodDeclaration, newMethodDeclaration);
+		var newRoot = root?.ReplaceNode(methodDeclaration, newMethodDeclaration);
 		if (newRoot == null)
 			return document;
 

src/Microsoft.Unity.Analyzers/LoadAttributeMethodSuppressor.cs

@@ -29,7 +29,11 @@ public override void ReportSuppressions(SuppressionAnalysisContext context)
 
 		private static void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context)
 		{
-			var model = context.GetSemanticModel(diagnostic.Location.SourceTree);
+			var syntaxTree = diagnostic.Location.SourceTree;
+			if (syntaxTree == null)
+				return;
+
+			var model = context.GetSemanticModel(syntaxTree);
 			var methodDeclarationSyntax = context.GetSuppressibleNode<MethodDeclarationSyntax>(diagnostic);
 
 			// Reuse the same detection logic regarding decorated methods with *InitializeOnLoadMethodAttribute or DidReloadScripts