Skip to content

Commit

Permalink
Add UNT0030, Calling Destroy or DestroyImmediate on a Transform (
Browse files Browse the repository at this point in the history
…#231)

* Add UNT0030, Calling Destroy or DestroyImmediate on a Transform

* Remove extra spaces
  • Loading branch information
sailro authored Jul 15, 2022
1 parent db21109 commit 2faed96
Show file tree
Hide file tree
Showing 6 changed files with 383 additions and 0 deletions.
33 changes: 33 additions & 0 deletions doc/UNT0030.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# UNT0030 Calling Destroy or DestroyImmediate on a Transform

Calling `Object.Destroy` or `Object.DestroyImmediate` using a `Transform` argument is not allowed and will produce an error message at runtime.

## Examples of patterns that are flagged by this analyzer

```csharp
using UnityEngine;

class Camera : MonoBehaviour
{
public void Update() {
Destroy(transform);
}
}
```

## Solution

Destroy the related `GameObject` instead:

```csharp
using UnityEngine;

class Camera : MonoBehaviour
{
public void Update() {
Destroy(transform.gameObject);
}
}
```

A code fix is offered for this diagnostic to automatically apply this change.
1 change: 1 addition & 0 deletions doc/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ ID | Title | Category
[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
[UNT0030](UNT0030.md) | Calling Destroy or DestroyImmediate on a Transform | Correctness

# Diagnostic Suppressors

Expand Down
162 changes: 162 additions & 0 deletions src/Microsoft.Unity.Analyzers.Tests/DestroyTransformTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/*--------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE in the project root for license information.
*-------------------------------------------------------------------------------------------*/

using System.Threading.Tasks;
using Xunit;

namespace Microsoft.Unity.Analyzers.Tests;

public class DestroyTransformTests : BaseCodeFixVerifierTest<DestroyTransformAnalyzer, DestroyTransformCodeFix>
{
[Fact]
public async Task TestValidDestroy()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Destroy(gameObject);
}
}
";

await VerifyCSharpDiagnosticAsync(test);
}

[Fact]
public async Task TestDestroy()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Destroy(transform);
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(7, 9);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Destroy(transform.gameObject);
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task TestObjectDestroy()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Object.Destroy(transform, 5);
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(7, 9);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Object.Destroy(transform.gameObject, 5);
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task TestDestroyImmediate()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
DestroyImmediate(transform);
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(7, 9);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
DestroyImmediate(transform.gameObject);
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task TestObjectDestroyImmediate()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Object.DestroyImmediate(transform, true);
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(7, 9);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update() {
Object.DestroyImmediate(transform.gameObject, true);
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

}
139 changes: 139 additions & 0 deletions src/Microsoft.Unity.Analyzers/DestroyTransform.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*--------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See LICENSE in the project root for license information.
*-------------------------------------------------------------------------------------------*/

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.Unity.Analyzers.Resources;

namespace Microsoft.Unity.Analyzers;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class DestroyTransformAnalyzer : DiagnosticAnalyzer
{
internal static readonly DiagnosticDescriptor Rule = new(
id: "UNT0030",
title: Strings.DestroyTransformDiagnosticTitle,
messageFormat: Strings.DestroyTransformDiagnosticMessageFormat,
category: DiagnosticCategory.Correctness,
defaultSeverity: DiagnosticSeverity.Info,
isEnabledByDefault: true,
description: Strings.DestroyTransformDiagnosticDescription);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterSyntaxNodeAction(AnalyzeInvocation, SyntaxKind.InvocationExpression);
}

internal static readonly HashSet<string> DestroyMethodNames = new(new[] {"Destroy", "DestroyImmediate"});

private static bool InvocationMatches(SyntaxNode node)
{
switch (node)
{
case InvocationExpressionSyntax ies:
return InvocationMatches(ies.Expression);
case MemberAccessExpressionSyntax maes:
return InvocationMatches(maes.Name);
case IdentifierNameSyntax ins:
var text = ins.Identifier.Text;
return DestroyMethodNames.Contains(text);
default:
return false;
}
}

internal static bool InvocationMatches(InvocationExpressionSyntax invocation, [NotNullWhen(true)] out ExpressionSyntax? argument)
{
argument = null;

if (!InvocationMatches(invocation))
return false;

argument = invocation
.ArgumentList
.Arguments
.FirstOrDefault()?
.Expression;

return argument != null;
}

private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
{
if (context.Node is not InvocationExpressionSyntax invocation)
return;

if (!InvocationMatches(invocation, out var argument))
return;

var model = context.SemanticModel;
if (model.GetSymbolInfo(invocation.Expression).Symbol is not IMethodSymbol methodSymbol)
return;

var typeSymbol = methodSymbol.ContainingType;
if (!typeSymbol.Matches(typeof(UnityEngine.Object)))
return;

var transformTypeSymbol = model
.GetTypeInfo(argument)
.Type;

if (!transformTypeSymbol.Extends(typeof(UnityEngine.Transform)))
return;

context.ReportDiagnostic(Diagnostic.Create(Rule, invocation.GetLocation()));
}
}

[ExportCodeFixProvider(LanguageNames.CSharp)]
public class DestroyTransformCodeFix : CodeFixProvider
{
public sealed override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(DestroyTransformAnalyzer.Rule.Id);

public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
var invocation = await context.GetFixableNodeAsync<InvocationExpressionSyntax>();
if (invocation == null)
return;

if (!DestroyTransformAnalyzer.InvocationMatches(invocation, out var argument))
return;

context.RegisterCodeFix(
CodeAction.Create(
Strings.DestroyTransformCodeFixTitle,
ct => UseGameObjectAsync(context.Document, argument, ct),
invocation.ToFullString()),
context.Diagnostics);
}

private async Task<Document> UseGameObjectAsync(Document document, ExpressionSyntax argument, CancellationToken cancellationToken)
{
var gameObject = SyntaxFactory.IdentifierName("gameObject");
var memberAccess = SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, argument, gameObject);

var root = await document
.GetSyntaxRootAsync(cancellationToken)
.ConfigureAwait(false);

var newRoot = root?.ReplaceNode(argument, memberAccess);
return newRoot == null ? document : document.WithSyntaxRoot(newRoot);
}
}
36 changes: 36 additions & 0 deletions src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2faed96

Please sign in to comment.