USP0024, USP0025: do not flag Input System messages as unused

doc/USP0024.md

@@ -0,0 +1,31 @@
+# USP0024 The Unity Input System invokes input messages
+
+Private methods with input message signatures are not unused.
+
+## Suppressed Diagnostic ID
+
+IDE0051 - Remove unused private members
+
+## Examples of code that produces a suppressed diagnostic
+```csharp
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+class Player : MonoBehaviour
+{
+	public Vector2 rawInput;
+
+	private void OnMove(InputValue value)
+	{
+		rawInput = value.Get<Vector2>();
+	}
+}
+```
+
+## Why is the diagnostic reported?
+
+The IDE cannot detect any calls to `OnMove` in the class.
+
+## Why do we suppress this diagnostic?
+
+With the `SendMessages` or `BroadcastMessages` notification behavior, `PlayerInput` invokes `On` + action name methods through `SendMessage`, which is undetectable by the IDE. The same applies to notification messages taking a `PlayerInput` argument (like `OnDeviceLost` or `OnPlayerJoined`) and to callbacks taking an `InputAction.CallbackContext` argument, invoked through serialized `UnityEvents`.

doc/USP0025.md

@@ -0,0 +1,29 @@
+# USP0025 The Unity Input System invokes input messages
+
+Unused parameters should not be removed from input messages.
+
+## Suppressed Diagnostic ID
+
+IDE0060 - Remove unused parameters
+
+## Examples of code that produces a suppressed diagnostic
+```csharp
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+class Player : MonoBehaviour
+{
+	private void OnFire(InputAction.CallbackContext context)
+	{
+		// do stuff, but context remains unused
+	}
+}
+```
+
+## Why is the diagnostic reported?
+
+The IDE does not detect that you've used `context`, and under normal circumstances, it would be reasonable to remove the unused parameter.
+
+## Why do we suppress this diagnostic?
+
+The IDE doesn't realize this method is invoked by the Input System, and therefore has no way of determining that it needs to have a specific signature to function correctly.

doc/index.md

@@ -73,4 +73,6 @@ ID | Suppressed ID | Justification
 [USP0021](USP0021.md) | IDE0041 | Prefer reference equality
 [USP0022](USP0022.md) | IDE0270 | Unity objects should not use if null coalescing
 [USP0023](USP0023.md) | IDE1006 | The Unity runtime invokes Unity messages
+[USP0024](USP0024.md) | IDE0051 | The Unity Input System invokes input messages
+[USP0025](USP0025.md) | IDE0060 | The Unity Input System invokes input messages
 

src/Microsoft.Unity.Analyzers.Tests/InputSystemMessageSuppressorTests.cs

@@ -0,0 +1,137 @@
+/*--------------------------------------------------------------------------------------------
+ *  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 Microsoft.CodeAnalysis;
+using Xunit;
+
+namespace Microsoft.Unity.Analyzers.Tests;
+
+public class InputSystemMessageSuppressorTests : BaseSuppressorVerifierTest<InputSystemMessageSuppressor>
+{
+	// The Input System package is not available in test infrastructure, so tests declare matching types.
+
+	[Fact]
+	public async Task InputValueMessageSuppressed()
+	{
+		const string test = @"
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+namespace UnityEngine.InputSystem
+{
+    public class InputValue
+    {
+        public T Get<T>() { return default(T); }
+    }
+}
+
+class Player : MonoBehaviour
+{
+    public Vector2 rawInput;
+
+    private void OnMove(InputValue value)
+    {
+        rawInput = value.Get<Vector2>();
+    }
+}";
+
+		var suppressor = ExpectSuppressor(InputSystemMessageSuppressor.MethodRule)
+			.WithLocation(17, 18);
+
+		await VerifyCSharpDiagnosticAsync(test, suppressor);
+	}
+
+	[Fact]
+	public async Task CallbackContextMessageAndParameterSuppressed()
+	{
+		const string test = @"
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+namespace UnityEngine.InputSystem
+{
+    public class InputAction
+    {
+        public struct CallbackContext { }
+    }
+}
+
+class Player : MonoBehaviour
+{
+    private void OnFire(InputAction.CallbackContext context)
+    {
+    }
+}";
+
+		var suppressors = new[] {
+			ExpectSuppressor(InputSystemMessageSuppressor.MethodRule).WithLocation(15, 18),
+			ExpectSuppressor(InputSystemMessageSuppressor.ParameterRule).WithLocation(15, 53),
+		};
+
+		await VerifyCSharpDiagnosticAsync(test, suppressors);
+	}
+
+	[Fact]
+	public async Task PlayerInputNotificationMessageAndParameterSuppressed()
+	{
+		const string test = @"
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+namespace UnityEngine.InputSystem
+{
+    public class PlayerInput { }
+}
+
+class Player : MonoBehaviour
+{
+    private void OnDeviceLost(PlayerInput input)
+    {
+    }
+}";
+
+		var suppressors = new[] {
+			ExpectSuppressor(InputSystemMessageSuppressor.MethodRule).WithLocation(12, 18),
+			ExpectSuppressor(InputSystemMessageSuppressor.ParameterRule).WithLocation(12, 43),
+		};
+
+		await VerifyCSharpDiagnosticAsync(test, suppressors);
+	}
+
+	[Fact]
+	public async Task MethodWithoutOnPrefixNotSuppressed()
+	{
+		const string test = @"
+using UnityEngine;
+using UnityEngine.InputSystem;
+
+namespace UnityEngine.InputSystem
+{
+    public class InputValue
+    {
+        public T Get<T>() { return default(T); }
+    }
+}
+
+class Player : MonoBehaviour
+{
+    public Vector2 rawInput;
+
+    private void HandleMove(InputValue value)
+    {
+        rawInput = value.Get<Vector2>();
+    }
+}";
+
+		var not = ExpectNotSuppressed(InputSystemMessageSuppressor.MethodRule)
+			.WithLocation(17, 18)
+			.WithSeverity(DiagnosticSeverity.Info)
+			.WithMessageFormat("Private member '{0}' is unused")
+			.WithArguments("Player.HandleMove");
+
+		await VerifyCSharpDiagnosticAsync(test, not);
+	}
+}

src/Microsoft.Unity.Analyzers/InputSystemMessageSuppressor.cs

@@ -0,0 +1,98 @@
+/*--------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See LICENSE in the project root for license information.
+ *-------------------------------------------------------------------------------------------*/
+
+using System;
+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 InputSystemMessageSuppressor : DiagnosticSuppressor
+{
+	internal static readonly SuppressionDescriptor MethodRule = new(
+		id: "USP0024",
+		suppressedDiagnosticId: "IDE0051",
+		justification: Strings.InputSystemMessageSuppressorJustification);
+
+	internal static readonly SuppressionDescriptor ParameterRule = new(
+		id: "USP0025",
+		suppressedDiagnosticId: "IDE0060",
+		justification: Strings.InputSystemMessageSuppressorJustification);
+
+	public override ImmutableArray<SuppressionDescriptor> SupportedSuppressions => ImmutableArray.Create(
+		MethodRule,
+		ParameterRule
+	);
+
+	public override void ReportSuppressions(SuppressionAnalysisContext context)
+	{
+		foreach (var diagnostic in context.ReportedDiagnostics)
+		{
+			AnalyzeDiagnostic(diagnostic, context);
+		}
+	}
+
+	private void AnalyzeDiagnostic(Diagnostic diagnostic, SuppressionAnalysisContext context)
+	{
+		var node = context.GetSuppressibleNode<SyntaxNode>(diagnostic, n => n is ParameterSyntax or MethodDeclarationSyntax);
+
+		if (node is ParameterSyntax)
+		{
+			node = node
+				.Ancestors()
+				.OfType<MethodDeclarationSyntax>()
+				.FirstOrDefault();
+		}
+
+		if (node == null)
+			return;
+
+		if (diagnostic.Location.SourceTree is not { } syntaxTree)
+			return;
+
+		var model = context.GetSemanticModel(syntaxTree);
+		if (model.GetDeclaredSymbol(node) is not IMethodSymbol methodSymbol)
+			return;
+
+		if (!IsInputSystemMessage(methodSymbol))
+			return;
+
+		foreach (var suppression in SupportedSuppressions)
+		{
+			if (suppression.SuppressedDiagnosticId == diagnostic.Id)
+				context.ReportSuppression(Suppression.Create(suppression, diagnostic));
+		}
+	}
+
+	private static bool IsInputSystemMessage(IMethodSymbol methodSymbol)
+	{
+		// With the SendMessages/BroadcastMessages notification behavior, PlayerInput invokes
+		// 'On' + action name methods taking an optional InputValue argument, and device/player
+		// notifications (OnDeviceLost, OnPlayerJoined, ...) taking a PlayerInput argument.
+		// With the UnityEvents notification behavior, callbacks take an InputAction.CallbackContext
+		// argument and are invoked through serialized events.
+		if (!methodSymbol.ReturnsVoid)
+			return false;
+
+		if (!methodSymbol.Name.StartsWith("On", StringComparison.Ordinal))
+			return false;
+
+		if (!methodSymbol.ContainingType.Extends(typeof(UnityEngine.MonoBehaviour)))
+			return false;
+
+		if (methodSymbol.Parameters.Length != 1)
+			return false;
+
+		var parameterType = methodSymbol.Parameters[0].Type;
+		return parameterType.Matches(typeof(UnityEngine.InputSystem.InputValue))
+			   || parameterType.Matches(typeof(UnityEngine.InputSystem.InputAction.CallbackContext))
+			   || parameterType.Matches(typeof(UnityEngine.InputSystem.PlayerInput));
+	}
+}

src/Microsoft.Unity.Analyzers/Resources/Strings.resx

@@ -466,6 +466,9 @@
   <data name="InputGetKeyDiagnosticTitle" xml:space="preserve">
     <value>Input.GetKey overloads with KeyCode argument</value>
   </data>
+  <data name="InputSystemMessageSuppressorJustification" xml:space="preserve">
+    <value>The Unity Input System invokes input messages.</value>
+  </data>
   <data name="TryGetComponentCodeFixTitle" xml:space="preserve">
     <value>Use TryGetComponent instead</value>
   </data>

src/Microsoft.Unity.Analyzers/UnityStubs.cs

@@ -628,6 +628,18 @@ protected virtual void OnCanvasHierarchyChanged() { }
 	}
 }
 
+namespace UnityEngine.InputSystem
+{
+	class InputValue { }
+
+	class InputAction
+	{
+		public struct CallbackContext { }
+	}
+
+	class PlayerInput : MonoBehaviour { }
+}
+
 namespace UnityEngine.Networking
 {
 	class NetworkReader { }