diff --git a/doc/UNT0039.md b/doc/UNT0039.md index ca6de2a..ce12fa1 100644 --- a/doc/UNT0039.md +++ b/doc/UNT0039.md @@ -38,3 +38,21 @@ public class PlayerScript : MonoBehaviour ``` A code fix is offered for this diagnostic to automatically apply this change. + +## Examples of patterns that are not flagged by this analyzer + +`GetComponent` calls inside editor-only messages (`Reset`, `OnValidate`) are not flagged: those messages are typically used to populate serialized fields with default references that a user can override, so the component is not a hard requirement. + +```csharp +using UnityEngine; + +public class PlayerScript : MonoBehaviour +{ + public Rigidbody rb; + + void Reset() + { + rb = GetComponent(); + } +} +``` diff --git a/src/Microsoft.Unity.Analyzers.Tests/RequireComponentTests.cs b/src/Microsoft.Unity.Analyzers.Tests/RequireComponentTests.cs index 0a6c4ce..c1b0688 100644 --- a/src/Microsoft.Unity.Analyzers.Tests/RequireComponentTests.cs +++ b/src/Microsoft.Unity.Analyzers.Tests/RequireComponentTests.cs @@ -327,4 +327,64 @@ void Foo() await VerifyCSharpDiagnosticAsync(test); } + + [Fact] + public async Task GetComponentInResetMessage() + { + const string test = @" +using UnityEngine; + +public class PlayerScript : MonoBehaviour +{ + public Rigidbody rb; + + void Reset() + { + rb = GetComponent(); + } +}"; + + await VerifyCSharpDiagnosticAsync(test); + } + + [Fact] + public async Task GetComponentInOnValidateMessage() + { + const string test = @" +using UnityEngine; + +public class PlayerScript : MonoBehaviour +{ + public Rigidbody rb; + + void OnValidate() + { + rb = GetComponent(); + } +}"; + + await VerifyCSharpDiagnosticAsync(test); + } + + [Fact] + public async Task GetComponentInResetOverload() + { + const string test = @" +using UnityEngine; + +public class PlayerScript : MonoBehaviour +{ + public Rigidbody rb; + + void Reset(int value) + { + rb = GetComponent(); + } +}"; + + var diagnostic = ExpectDiagnostic() + .WithLocation(10, 14); + + await VerifyCSharpDiagnosticAsync(test, diagnostic); + } } diff --git a/src/Microsoft.Unity.Analyzers/GameObjectIsStatic.cs b/src/Microsoft.Unity.Analyzers/GameObjectIsStatic.cs index f230c5e..6d3b429 100644 --- a/src/Microsoft.Unity.Analyzers/GameObjectIsStatic.cs +++ b/src/Microsoft.Unity.Analyzers/GameObjectIsStatic.cs @@ -4,7 +4,6 @@ *-------------------------------------------------------------------------------------------*/ using System.Collections.Immutable; -using System.Linq; using System.Text.RegularExpressions; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; @@ -31,7 +30,6 @@ public class GameObjectIsStaticAnalyzer : DiagnosticAnalyzer public override ImmutableArray SupportedDiagnostics => ImmutableArray.Create(Rule); - private static readonly string[] EditorOnlyMessages = ["OnValidate", "Reset"]; private static readonly Regex EditorFolderRegex = new(@"[/\\]Assets[/\\].*[/\\]Editor[/\\]", RegexOptions.IgnoreCase | RegexOptions.Compiled); public override void Initialize(AnalysisContext context) @@ -59,7 +57,7 @@ private static void AnalyzeMemberAccess(SyntaxNodeAnalysisContext context) if (DirectiveHelper.IsInsideDirective(memberAccess, "UNITY_EDITOR")) return; - if (IsInsideEditorOnlyMessage(memberAccess, context.SemanticModel)) + if (memberAccess.IsInsideEditorOnlyMessage(context.SemanticModel)) return; if (IsInsideEditorFolder(context)) @@ -73,21 +71,4 @@ private static bool IsInsideEditorFolder(SyntaxNodeAnalysisContext context) var filePath = context.Node.SyntaxTree.FilePath; return !string.IsNullOrWhiteSpace(filePath) && EditorFolderRegex.IsMatch(filePath); } - - private static bool IsInsideEditorOnlyMessage(SyntaxNode node, SemanticModel semanticModel) - { - var methodSyntax = node.Ancestors().OfType().FirstOrDefault(); - if (methodSyntax == null) - return false; - - var methodSymbol = semanticModel.GetDeclaredSymbol(methodSyntax); - if (methodSymbol == null) - return false; - - var scriptInfo = new ScriptInfo(methodSymbol.ContainingType); - if (!scriptInfo.HasMessages) - return false; - - return EditorOnlyMessages.Contains(methodSymbol.Name) && scriptInfo.IsMessage(methodSymbol); - } } diff --git a/src/Microsoft.Unity.Analyzers/RequireComponent.cs b/src/Microsoft.Unity.Analyzers/RequireComponent.cs index b9686a0..7c8fb5a 100644 --- a/src/Microsoft.Unity.Analyzers/RequireComponent.cs +++ b/src/Microsoft.Unity.Analyzers/RequireComponent.cs @@ -65,9 +65,14 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context) if (!containerType.Extends(typeof(MonoBehaviour))) return; + // GetComponent inside editor-only messages is typically used to populate + // serialized fields with overridable defaults, so the component is not a hard requirement + if (invocation.IsInsideEditorOnlyMessage(model)) + return; + var componentType = method.TypeArguments.First(); // Checked by IsGenericGetComponent if (componentType.TypeKind == TypeKind.TypeParameter) - return; + return; if (IsTypeAlreadyRequired(containerType, componentType)) return; diff --git a/src/Microsoft.Unity.Analyzers/SyntaxNodeExtensions.cs b/src/Microsoft.Unity.Analyzers/SyntaxNodeExtensions.cs index 30ca913..8304ef5 100644 --- a/src/Microsoft.Unity.Analyzers/SyntaxNodeExtensions.cs +++ b/src/Microsoft.Unity.Analyzers/SyntaxNodeExtensions.cs @@ -3,12 +3,37 @@ * Licensed under the MIT License. See LICENSE in the project root for license information. *-------------------------------------------------------------------------------------------*/ +using System.Linq; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.CSharp.Syntax; namespace Microsoft.Unity.Analyzers; internal static class SyntaxNodeExtensions { + private static readonly string[] EditorOnlyMessages = ["OnValidate", "Reset"]; + + extension(SyntaxNode node) + { + public bool IsInsideEditorOnlyMessage(SemanticModel semanticModel) + { + var methodSyntax = node.Ancestors().OfType().FirstOrDefault(); + if (methodSyntax == null) + return false; + + var methodSymbol = semanticModel.GetDeclaredSymbol(methodSyntax); + if (methodSymbol == null) + return false; + + var scriptInfo = new ScriptInfo(methodSymbol.ContainingType); + if (!scriptInfo.HasMessages) + return false; + + return EditorOnlyMessages.Contains(methodSymbol.Name) && scriptInfo.IsMessage(methodSymbol); + } + } + extension(SyntaxNode first) { public SyntaxTriviaList MergeLeadingTriviaWith(SyntaxNode second)