From 9e61545c61bc22d001aa23bad6146933e58fd3c2 Mon Sep 17 00:00:00 2001 From: Sebastien Lebreton Date: Tue, 18 Jun 2024 08:57:56 +0200 Subject: [PATCH] Simplify UNT0026 --- .../TryGetComponentTests.cs | 173 ------------------ .../TryGetComponent.cs | 63 +++---- 2 files changed, 21 insertions(+), 215 deletions(-) diff --git a/src/Microsoft.Unity.Analyzers.Tests/TryGetComponentTests.cs b/src/Microsoft.Unity.Analyzers.Tests/TryGetComponentTests.cs index 9c8a3f7..a8af3f8 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/TryGetComponentTests.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/TryGetComponentTests.cs @@ -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(); - 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(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(); - 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(out rb)) { - Debug.Log(rb.name); - } - } -} -"; - - await VerifyCSharpFixAsync(test, fixedTest); - } - [Fact] public async Task PropertyAssignmentNotNullConditionTest() { @@ -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(); - 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(out platform)) - transform.parent = platform.transform; - } -} -"; - - await VerifyCSharpFixAsync(test, fixedTest); - } - [Fact] public async Task BlockBreaksDetection() { @@ -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(); - 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(out platform)) - transform.parent = platform.transform; - } -} -"; - - await VerifyCSharpFixAsync(test, fixedTest); - } } diff --git a/src/Microsoft.Unity.Analyzers/TryGetComponent.cs b/src/Microsoft.Unity.Analyzers/TryGetComponent.cs index 845d0c3..58ad790 100644 --- a/src/Microsoft.Unity.Analyzers/TryGetComponent.cs +++ b/src/Microsoft.Unity.Analyzers/TryGetComponent.cs @@ -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; @@ -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 @@ -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) @@ -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) @@ -274,22 +257,18 @@ private static async Task 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