diff --git a/doc/USP0019.md b/doc/USP0019.md new file mode 100644 index 00000000..dba65131 --- /dev/null +++ b/doc/USP0019.md @@ -0,0 +1,34 @@ +# USP0019 Don't flag private methods decorated with PreserveAttribute or UsedImplicitlyAttribute as unused + +Methods decorated with `PreserveAttribute` or `UsedImplicitlyAttribute` attributes are not unused. + +## Suppressed Diagnostic ID + +IDE0051 - Remove unused private members + +## Examples of code that produces a suppressed diagnostic +```csharp +using UnityEngine; +using UnityEgine.Scripting; + +class Loader +{ + [PreserveAttribute] + private void InvokeMe() + { + } + + public string Name; // "InvokeMe" serialized + private void Update() { + Invoke(Name, 0); + } +} +``` + +## Why is the diagnostic reported? + +The IDE cannot find any references to the method `InvokeMe` and believes it to be unused. + +## Why do we suppress this diagnostic? + +Even though the IDE cannot find any references to `InvokeMe` , it will be called by Unity, and should not be removed. \ No newline at end of file diff --git a/doc/index.md b/doc/index.md index a74f66c4..c33ff081 100644 --- a/doc/index.md +++ b/doc/index.md @@ -51,3 +51,4 @@ ID | Suppressed ID | Justification [USP0016](USP0016.md) | CS8618 | Initialization detection with nullable reference types [USP0017](USP0017.md) | IDE0074 | Unity objects should not use coalescing assignment [USP0018](USP0018.md) | IDE0016 | Unity objects should not be used with throw expressions +[USP0019](USP0012.md) | IDE0051 | Don't flag private methods with PreserveAttribute or UsedImplicitlyAttribute as unused diff --git a/src/Microsoft.Unity.Analyzers.Tests/ImplicitUsageAttributeSuppressorTests.cs b/src/Microsoft.Unity.Analyzers.Tests/ImplicitUsageAttributeSuppressorTests.cs new file mode 100644 index 00000000..a0937357 --- /dev/null +++ b/src/Microsoft.Unity.Analyzers.Tests/ImplicitUsageAttributeSuppressorTests.cs @@ -0,0 +1,78 @@ +/*-------------------------------------------------------------------------------------------- + * 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 ImplicitUsageAttributeSuppressorTests : BaseSuppressorVerifierTest +{ + [Fact] + public async Task UnityPreserveTest() + { + const string test = @" +using UnityEngine; +using UnityEngine.Scripting; + +class Camera : MonoBehaviour +{ + [Preserve] + private void Foo() { + } +} +"; + + var suppressor = ExpectSuppressor(ImplicitUsageAttributeSuppressor.Rule) + .WithLocation(8, 18); + + await VerifyCSharpDiagnosticAsync(test, suppressor); + } + + [Fact] + public async Task OwnPreserveTest() + { + const string test = @" +using UnityEngine; + +class Camera : MonoBehaviour +{ + [My.Own.Stuff.Preserve] + private void Foo() { + } +} + +namespace My.Own.Stuff { + public class PreserveAttribute : System.Attribute { } +} +"; + + var suppressor = ExpectSuppressor(ImplicitUsageAttributeSuppressor.Rule) + .WithLocation(7, 18); + + await VerifyCSharpDiagnosticAsync(test, suppressor); + } + + [Fact] + public async Task UsedImplicitlyTest() + { + const string test = @" +using UnityEngine; +using JetBrains.Annotations; + +class Camera : MonoBehaviour +{ + [UsedImplicitly] + private void Foo() { + } +} +"; + + var suppressor = ExpectSuppressor(ImplicitUsageAttributeSuppressor.Rule) + .WithLocation(8, 18); + + await VerifyCSharpDiagnosticAsync(test, suppressor); + } +} diff --git a/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs b/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs new file mode 100644 index 00000000..934da7df --- /dev/null +++ b/src/Microsoft.Unity.Analyzers/ImplicitUsageAttributeSuppressor.cs @@ -0,0 +1,54 @@ +/*-------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *-------------------------------------------------------------------------------------------*/ + +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.Unity.Analyzers.Resources; + +namespace Microsoft.Unity.Analyzers; + +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public class ImplicitUsageAttributeSuppressor : DiagnosticSuppressor +{ + internal static readonly SuppressionDescriptor Rule = new( + id: "USP0019", + suppressedDiagnosticId: "IDE0051", + justification: Strings.ImplicitUsageAttributeSuppressorJustification); + + public override void ReportSuppressions(SuppressionAnalysisContext context) + { + foreach (var diagnostic in context.ReportedDiagnostics) + { + AnalyzeDiagnostic(diagnostic, context); + } + } + + public override ImmutableArray SupportedSuppressions => ImmutableArray.Create(Rule); + + private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context) + { + var methodDeclarationSyntax = context.GetSuppressibleNode(diagnostic); + if (methodDeclarationSyntax == null) + return; + + var model = context.GetSemanticModel(diagnostic.Location.SourceTree); + if (model.GetDeclaredSymbol(methodDeclarationSyntax) is not IMethodSymbol methodSymbol) + return; + + if (!IsSuppressable(methodSymbol)) + return; + + context.ReportSuppression(Suppression.Create(Rule, diagnostic)); + } + + 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)); + } +} diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs index ccce7652..7e1f8182 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.Designer.cs @@ -258,6 +258,15 @@ internal static string GetComponentIncorrectTypeDiagnosticTitle { } } + /// + /// Looks up a localized string similar to Don't flag private methods decorated with PreserveAttribute or UsedImplicitlyAttribute as unused.. + /// + internal static string ImplicitUsageAttributeSuppressorJustification { + get { + return ResourceManager.GetString("ImplicitUsageAttributeSuppressorJustification", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use static method. /// diff --git a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx index e92dcdda..a0236d77 100644 --- a/src/Microsoft.Unity.Analyzers/Resources/Strings.resx +++ b/src/Microsoft.Unity.Analyzers/Resources/Strings.resx @@ -486,4 +486,7 @@ Do not use Throw expressions with Unity objects. + + Don't flag private methods decorated with PreserveAttribute or UsedImplicitlyAttribute as unused. + \ No newline at end of file diff --git a/src/Microsoft.Unity.Analyzers/UnityStubs.cs b/src/Microsoft.Unity.Analyzers/UnityStubs.cs index 225ddf8e..ea623c62 100644 --- a/src/Microsoft.Unity.Analyzers/UnityStubs.cs +++ b/src/Microsoft.Unity.Analyzers/UnityStubs.cs @@ -618,6 +618,11 @@ namespace UnityEngine.UIElements class VisualElement { } } +namespace UnityEngine.Scripting +{ + class PreserveAttribute : Attribute { } +} + namespace UnityEditor.AssetImporters { class MaterialDescription { } @@ -728,4 +733,9 @@ static void OnPostprocessAllAssets(string[] importedAssets, string[] deletedAsse } +namespace JetBrains.Annotations +{ + class UsedImplicitlyAttribute : Attribute { } +} + #pragma warning enable