Skip to content

Commit

Permalink
Simplify UNT0026
Browse files Browse the repository at this point in the history
  • Loading branch information
@sailro
sailro committed Jun 18, 2024
1 parent 01696b8 commit 9e61545
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 215 deletions.
173 changes: 0 additions & 173 deletions src/Microsoft.Unity.Analyzers.Tests/TryGetComponentTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,92 +265,6 @@ public void Update()
await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task VariableAssignmentNotNullConditionTest()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update()
{
Rigidbody rb;
rb = gameObject.GetComponent<Rigidbody>();
if (rb != null) {
Debug.Log(rb.name);
}
}
}
";

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

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update()
{
Rigidbody rb;
if (gameObject.TryGetComponent<Rigidbody>(out rb)) {
Debug.Log(rb.name);
}
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task FieldAssignmentNotNullConditionTest()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
private Rigidbody rb;
public void Update()
{
rb = gameObject.GetComponent<Rigidbody>();
if (rb != null) {
Debug.Log(rb.name);
}
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(10, 14);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
private Rigidbody rb;
public void Update()
{
if (gameObject.TryGetComponent<Rigidbody>(out rb)) {
Debug.Log(rb.name);
}
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task PropertyAssignmentNotNullConditionTest()
{
Expand Down Expand Up @@ -418,49 +332,6 @@ public void Update()
await VerifyCSharpDiagnosticAsync(test);
}

[Fact]
public async Task InlineIfAssignment()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update()
{
Component hit = null, platform = null;
if (hit != null)
platform = hit.GetComponent<Component>();
if (platform != null)
transform.parent = platform.transform;
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(11, 24);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update()
{
Component hit = null, platform = null;
if (hit != null && hit.TryGetComponent<Component>(out platform))
transform.parent = platform.transform;
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}

[Fact]
public async Task BlockBreaksDetection()
{
Expand Down Expand Up @@ -509,48 +380,4 @@ public void Update()

await VerifyCSharpDiagnosticAsync(test);
}

[Fact]
public async Task DoubleInlineIfAssignment()
{
const string test = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update()
{
Component hit = null, platform = null;
if (hit != null)
if (1 == 1)
platform = hit.GetComponent<Component>();
if (platform != null)
transform.parent = platform.transform;
}
}
";

var diagnostic = ExpectDiagnostic()
.WithLocation(12, 28);

await VerifyCSharpDiagnosticAsync(test, diagnostic);

const string fixedTest = @"
using UnityEngine;
class Camera : MonoBehaviour
{
public void Update()
{
Component hit = null, platform = null;
if (1 == 1 && hit != null && hit.TryGetComponent<Component>(out platform))
transform.parent = platform.transform;
}
}
";

await VerifyCSharpFixAsync(test, fixedTest);
}
}
63 changes: 21 additions & 42 deletions src/Microsoft.Unity.Analyzers/TryGetComponent.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,9 @@ private static bool IsTryGetComponentSupported(SyntaxNodeAnalysisContext context
}
}

internal class TryGetComponentContext(string targetIdentifier, bool isVariableDeclaration, IfStatementSyntax ifStatement, SyntaxKind conditionKind)
internal class TryGetComponentContext(string targetIdentifier, IfStatementSyntax ifStatement, SyntaxKind conditionKind)
{
public string TargetIdentifier { get; } = targetIdentifier;
public bool IsVariableDeclaration { get; } = isVariableDeclaration;
public IfStatementSyntax IfStatement { get; } = ifStatement;
public SyntaxKind ConditionKind { get; } = conditionKind;

Expand All @@ -78,12 +77,12 @@ internal class TryGetComponentContext(string targetIdentifier, bool isVariableDe
if (!IsCompatibleGetComponent(invocation, model))
return null;

// We want an assignment or variable declaration with invocation as initializer
// We want a variable declaration with invocation as initializer
var invocationParent = invocation.Parent;
if (invocationParent == null)
return null;

if (!TryGetTargetdentifier(model, invocationParent, out var targetIdentifier, out var isVariableDeclaration))
if (!TryGetTargetdentifier(invocationParent, out var targetIdentifier))
return null;

// We want the next line to be an if statement
Expand Down Expand Up @@ -111,7 +110,7 @@ internal class TryGetComponentContext(string targetIdentifier, bool isVariableDe
visitor = visitor.Parent;
}

return new TryGetComponentContext(targetIdentifier.Value.Text, isVariableDeclaration, ifStatement, binaryExpression.Kind());
return new TryGetComponentContext(targetIdentifier.Value.Text, ifStatement, binaryExpression.Kind());
}

private static bool IsCompatibleGetComponent(InvocationExpressionSyntax invocation, SemanticModel model)
Expand Down Expand Up @@ -164,31 +163,15 @@ private static bool TryGetConditionIdentifier(SyntaxNode ifNode, [NotNullWhen(tr
return conditionIdentifier.HasValue;
}

private static bool TryGetTargetdentifier(SemanticModel model, SyntaxNode invocationParent, [NotNullWhen(true)] out SyntaxToken? targetIdentifier, out bool isVariableDeclaration)
private static bool TryGetTargetdentifier(SyntaxNode invocationParent, [NotNullWhen(true)] out SyntaxToken? targetIdentifier)
{
targetIdentifier = null;
isVariableDeclaration = false;

switch (invocationParent)
{
case EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax variableDeclarator }:
isVariableDeclaration = true;
targetIdentifier = variableDeclarator.Identifier;
break;

case AssignmentExpressionSyntax { Left: IdentifierNameSyntax identifierName }:
{
// With an assignment, we want to work only with a field or local variable ('out' constraint)
var symbol = model.GetSymbolInfo(identifierName);
if (symbol.Symbol is not (ILocalSymbol or IFieldSymbol))
return false;

targetIdentifier = identifierName.Identifier;
break;
}
}
if (invocationParent is not EqualsValueClauseSyntax { Parent: VariableDeclaratorSyntax variableDeclarator })
return false;

return targetIdentifier.HasValue;
targetIdentifier = variableDeclarator.Identifier;
return true;
}

private static bool TryGetNextTopNode(SyntaxNode node, [NotNullWhen(true)] out SyntaxNode? nextNode)
Expand Down Expand Up @@ -274,22 +257,18 @@ private static async Task<Document> ReplaceWithTryGetComponentAsync(Document doc
}

var targetIdentifier = context.TargetIdentifier;
var newArgument = context.IsVariableDeclaration switch
{
// Creating var argument
true => Argument(
DeclarationExpression(
IdentifierName(
Identifier(TriviaList(),
SyntaxKind.VarKeyword,
"var",
"var",
TriviaList())),
SingleVariableDesignation(
Identifier(targetIdentifier)))),
// Reusing the target identifier
false => Argument(IdentifierName(targetIdentifier))
};

// Creating var argument
var newArgument = Argument(
DeclarationExpression(
IdentifierName(
Identifier(TriviaList(),
SyntaxKind.VarKeyword,
"var",
"var",
TriviaList())),
SingleVariableDesignation(
Identifier(targetIdentifier))));

// Add the 'out' component argument
newInvocation = newInvocation
Expand Down

0 comments on commit 9e61545

Please sign in to comment.