From 01f95475ffc593540d1eb7a26874ac8d4f2d3735 Mon Sep 17 00:00:00 2001
From: Owen Smith <owen@omsmith.ca>
Date: Mon, 5 Apr 2021 12:06:08 -0400
Subject: [PATCH] raise a diagnostic if rpc context is marked [Dependency]
 (#737)

Closes: https://github.com/Brightspace/D2L.CodeStyle/issues/716
---
 .../ApiUsage/RpcAnalyzer.cs                   | 33 ++++++++++++++-----
 src/D2L.CodeStyle.Analyzers/Diagnostics.cs    |  9 +++++
 .../Specs/RpcAnalyzer.cs                      | 15 +++++++++
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/D2L.CodeStyle.Analyzers/ApiUsage/RpcAnalyzer.cs b/src/D2L.CodeStyle.Analyzers/ApiUsage/RpcAnalyzer.cs
index a224cf780..025503501 100644
--- a/src/D2L.CodeStyle.Analyzers/ApiUsage/RpcAnalyzer.cs
+++ b/src/D2L.CodeStyle.Analyzers/ApiUsage/RpcAnalyzer.cs
@@ -8,8 +8,11 @@
 namespace D2L.CodeStyle.Analyzers.ApiUsage {
 	[DiagnosticAnalyzer( LanguageNames.CSharp )]
 	internal sealed class RpcAnalyzer : DiagnosticAnalyzer {
-		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
-			=> ImmutableArray.Create( Diagnostics.RpcContextFirstArgument, Diagnostics.RpcArgumentSortOrder );
+		public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
+			Diagnostics.RpcContextFirstArgument,
+			Diagnostics.RpcArgumentSortOrder,
+			Diagnostics.RpcContextMarkedDependency
+		);
 
 		public override void Initialize( AnalysisContext context ) {
 			context.EnableConcurrentExecution();
@@ -83,7 +86,8 @@ INamedTypeSymbol dependencyAttributeType
 				method,
 				rpcContextType: rpcContextType,
 				rpcPostContextType: rpcPostContextType,
-				rpcPostContextBaseType: rpcPostContextBaseType
+				rpcPostContextBaseType: rpcPostContextBaseType,
+				dependencyAttributeType: dependencyAttributeType
 			);
 
 			// dependencyAttributeType may be null if that DLL isn't
@@ -118,7 +122,8 @@ private static void CheckThatFirstArgumentIsIRpcContext(
 			MethodDeclarationSyntax method,
 			INamedTypeSymbol rpcContextType,
 			INamedTypeSymbol rpcPostContextType,
-			INamedTypeSymbol rpcPostContextBaseType
+			INamedTypeSymbol rpcPostContextBaseType,
+			INamedTypeSymbol dependencyAttributeType
 		) {
 			var ps = method.ParameterList.Parameters;
 
@@ -141,6 +146,12 @@ INamedTypeSymbol rpcPostContextBaseType
 				context.ReportDiagnostic(
 					Diagnostic.Create( Diagnostics.RpcContextFirstArgument, firstParam.GetLocation() )
 				);
+			} else if( dependencyAttributeType != null
+				&& IsMarkedDependency( dependencyAttributeType, firstParam, context.SemanticModel )
+			) {
+				context.ReportDiagnostic(
+					Diagnostic.Create( Diagnostics.RpcContextMarkedDependency, firstParam.GetLocation() )
+				);
 			}
 		}
 
@@ -151,10 +162,7 @@ INamedTypeSymbol dependencyAttributeType
 		) {
 			bool doneDependencies = false;
 			foreach( var param in ps.Skip( 1 ) ) {
-				var isDep = param
-					.AttributeLists
-					.SelectMany( al => al.Attributes )
-					.Any( attr => IsAttribute( dependencyAttributeType, attr, context.SemanticModel ) );
+				var isDep = IsMarkedDependency( dependencyAttributeType, param, context.SemanticModel );
 
 				if( !isDep && !doneDependencies ) {
 					doneDependencies = true;
@@ -164,6 +172,15 @@ INamedTypeSymbol dependencyAttributeType
 			}
 		}
 
+		private static bool IsMarkedDependency(
+			INamedTypeSymbol dependencyAttributeType,
+			ParameterSyntax parameter,
+			SemanticModel model
+		) => parameter
+			.AttributeLists
+			.SelectMany( al => al.Attributes )
+			.Any( attr => IsAttribute( dependencyAttributeType, attr, model ) );
+
 		private static bool IsAttribute(
 			INamedTypeSymbol expectedType,
 			AttributeSyntax attr,
diff --git a/src/D2L.CodeStyle.Analyzers/Diagnostics.cs b/src/D2L.CodeStyle.Analyzers/Diagnostics.cs
index 3f126f85f..1d516e5a2 100644
--- a/src/D2L.CodeStyle.Analyzers/Diagnostics.cs
+++ b/src/D2L.CodeStyle.Analyzers/Diagnostics.cs
@@ -580,5 +580,14 @@ public static class Diagnostics {
 			defaultSeverity: DiagnosticSeverity.Error,
 			isEnabledByDefault: true
 		);
+
+		public static readonly DiagnosticDescriptor RpcContextMarkedDependency = new DiagnosticDescriptor(
+			id: "D2L0078",
+			title: "RPC context argument should not be marked [Dependency]",
+			messageFormat: "RPC context argument should not be marked [Dependency]",
+			category: "Correctness",
+			defaultSeverity: DiagnosticSeverity.Error,
+			isEnabledByDefault: true
+		);
 	}
 }
diff --git a/tests/D2L.CodeStyle.Analyzers.Test/Specs/RpcAnalyzer.cs b/tests/D2L.CodeStyle.Analyzers.Test/Specs/RpcAnalyzer.cs
index 54fe9c7c7..09c80b71b 100644
--- a/tests/D2L.CodeStyle.Analyzers.Test/Specs/RpcAnalyzer.cs
+++ b/tests/D2L.CodeStyle.Analyzers.Test/Specs/RpcAnalyzer.cs
@@ -123,5 +123,20 @@ public static void IncorrectDependencySortOrderEvenWithRandomBadAttributeInTheMi
 			[UndefinedAttribute] int x,
 			/* RpcArgumentSortOrder */ [Dependency] FooDependency foo /**/
 		) { }
+
+		[Rpc]
+		public static void RpcContextMarkedDependency(
+			/* RpcContextMarkedDependency */ [Dependency] IRpcContext context /**/
+		) { }
+
+		[Rpc]
+		public static void RpcPostContextMarkedDependency(
+			/* RpcContextMarkedDependency */ [Dependency] IRpcPostContext context /**/
+		) { }
+
+		[Rpc]
+		public static void RpcPostContextBaseMarkedDependency(
+			/* RpcContextMarkedDependency */ [Dependency] IRpcPostContextBase context /**/
+		) { }
 	}
 }