Support accessible protected and internal CancellationTokens

Author: menees

src/Directory.Build.props

@@ -21,7 +21,7 @@
 		<AssemblyOriginatorKeyFile>../Analyzers.snk</AssemblyOriginatorKeyFile>
 
 		<!-- NOTE: Change the version in Vsix\source.extension.vsixmanifest to match this! -->
-		<Version>3.3.2</Version>
+		<Version>3.3.3</Version>
 
 		<RoslynVersion>3.11.0</RoslynVersion>
 	</PropertyGroup>

src/Menees.Analyzers.Vsix/source.extension.vsixmanifest

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="utf-8"?>
 <PackageManifest Version="2.0.0" xmlns="http://schemas.microsoft.com/developer/vsx-schema/2011" xmlns:d="http://schemas.microsoft.com/developer/vsx-schema-design/2011">
     <Metadata>
-        <Identity Id="Menees.Analyzers.Vsix" Version="3.3.2" Language="en-US" Publisher="Bill Menees"/>
+        <Identity Id="Menees.Analyzers.Vsix" Version="3.3.3" Language="en-US" Publisher="Bill Menees"/>
         <DisplayName>Menees.Analyzers.Vsix</DisplayName>
         <Description xml:space="preserve">Provides analyzers for validating that tabs are used for indentation, that the lengths of lines, methods, properties, and files are acceptable, and that #regions are used within long files and files that contain multiple types.</Description>
         <License>License.txt</License>

src/Menees.Analyzers/Men019SupportAsyncCancellationToken.cs

@@ -73,11 +73,8 @@ private sealed class NestedAnalyzer(
 		private readonly INamedTypeSymbol[] fixedTaskTypes = fixedTaskTypes;
 		private readonly INamedTypeSymbol[] genericTaskTypes = genericTaskTypes;
 		private readonly INamedTypeSymbol asyncMethodBuilderAttributeType = asyncMethodBuilderAttributeType;
-#pragma warning disable IDE0079 // Remove unnecessary suppression. False positive.
-#pragma warning disable RS1024 // Compare symbols correctly. False positive; this is using a SymbolEqualityComparer.
-		private readonly ConcurrentDictionary<ITypeSymbol, bool> typeHasCancellationTokenProperty = new(SymbolComparer);
-#pragma warning restore RS1024 // Compare symbols correctly
-#pragma warning restore IDE0079 // Remove unnecessary suppression
+		private readonly ConcurrentDictionary<(ITypeSymbol Source, ITypeSymbol Target), bool> canAccessCancellationTokenProperty
+			= new(TypeAccessComparer.Instance);
 
 		#endregion
 
@@ -101,16 +98,20 @@ private sealed class NestedAnalyzer(
 
 		public void HandleNamedTypeSymbol(SymbolAnalysisContext context)
 		{
-			// Look at each overload method group (including inherited methods).
-			// If a method group has any eligible async/awaitable methods and none
-			// of those are cancellable, then we'll report each eligible method.
 			INamedTypeSymbol namedType = (INamedTypeSymbol)context.Symbol;
 
+			// If the namedType has an instance-level CancellationToken property, then instance methods don't need a parameter.
+			bool checkInstanceMethods = !CanAccessCancellationTokenProperty(source: namedType, target: namedType);
+
+			// Look at each overload method group (including inherited methods).
+			// If a method group has any eligible async/awaitable methods and none
+			// of those are cancellable, then we'll report the "most eligible" method.
 #pragma warning disable IDE0079 // Remove unnecessary suppression. False positive!
 #pragma warning disable RS1024 // Compare symbols correctly. False positive! We're grouping by the Name not IMethodSymbol.
 			IEnumerable<IGrouping<string, IMethodSymbol>> methodGroups = GetTypeAndBaseTypes(namedType)
 				.SelectMany(type => type.GetMembers())
 				.OfType<IMethodSymbol>()
+				.Where(method => checkInstanceMethods || method.IsStatic)
 				.GroupBy(m => m.Name, m => m, StringComparer.Ordinal);
 #pragma warning restore RS1024 // Compare symbols correctly
 #pragma warning restore IDE0079 // Remove unnecessary suppression
@@ -138,7 +139,7 @@ public void HandleNamedTypeSymbol(SymbolAnalysisContext context)
 
 						List<IParameterSymbol> parameters = GetEligibleParameters(method);
 						if (HasCancellationTokenParameter(parameters)
-							|| parameters.Any(ParameterHasCancellationTokenProperty))
+							|| parameters.Any(p => CanAccessCancellationTokenProperty(source: namedType, target: p.Type)))
 						{
 							isAtLeastOneOverloadCancellable = true;
 						}
@@ -286,17 +287,22 @@ private bool HasCancellationTokenParameter(List<IParameterSymbol> parameters)
 			return result;
 		}
 
-		private bool ParameterHasCancellationTokenProperty(IParameterSymbol parameterSymbol)
+		private bool CanAccessCancellationTokenProperty(ITypeSymbol source, ITypeSymbol target)
 		{
 			ISet<string> propertyNamesToCheck = this.callingAnalyzer.Settings.PropertyNamesForCancellation;
 			bool result = propertyNamesToCheck.Count > 0
-				&& typeHasCancellationTokenProperty.GetOrAdd(parameterSymbol.Type, type =>
+				&& canAccessCancellationTokenProperty.GetOrAdd((source, target), tuple =>
 				{
 					bool hasCancellationTokenProperty = false;
-					ITypeSymbol[] typeHierarchy = [.. GetTypeAndBaseTypes(type)];
+					ITypeSymbol[] targetTypeHierarchy = [.. GetTypeAndBaseTypes(tuple.Target)];
 					IEnumerable<IPropertySymbol> publicCancellationProperties = propertyNamesToCheck
-						.SelectMany(propertyName => typeHierarchy.SelectMany(t => t.GetMembers(propertyName)))
-						.Where(m => m.Kind == SymbolKind.Property && m.DeclaredAccessibility == Accessibility.Public)
+						.SelectMany(propertyName => targetTypeHierarchy.SelectMany(t => t.GetMembers(propertyName)))
+						.Where(m => m.Kind == SymbolKind.Property
+							// Static CancellationToken properties are rare and don't go well with their
+							// per-operation intent. https://devblogs.microsoft.com/dotnet/net-4-cancellation-framework/
+							&& !m.IsStatic
+							// If m is an inherited property, we have to use its containing type not target.
+							&& m.DeclaredAccessibility >= GetMinimumAccessibility(tuple.Source, m.ContainingType))
 						.Cast<IPropertySymbol>();
 					foreach (IPropertySymbol propertySymbol in publicCancellationProperties)
 					{
@@ -313,8 +319,59 @@ private bool ParameterHasCancellationTokenProperty(IParameterSymbol parameterSym
 			return result;
 		}
 
+		private Accessibility GetMinimumAccessibility(ITypeSymbol source, ITypeSymbol target)
+		{
+			Accessibility result = Accessibility.Public;
+
+			if (SymbolComparer.Equals(source, target))
+			{
+				result = Accessibility.NotApplicable;
+			}
+			else
+			{
+				bool useProtected = GetTypeAndBaseTypes(source).Contains(target, SymbolComparer);
+
+				// This doesn't handle InternalsVisibleTo because Roslyn doesn't expose public members for those checks.
+				// https://github.com/dotnet/roslyn/blob/main/src/Compilers/CSharp/Portable/Binder/Semantics/AccessCheck.cs
+				bool useInternal = SymbolComparer.Equals(source.ContainingAssembly, target.ContainingAssembly);
+
+				if (useProtected && useInternal)
+				{
+					result = Accessibility.ProtectedAndInternal;
+				}
+				else if (useProtected)
+				{
+					result = Accessibility.Protected;
+				}
+				else if (useInternal)
+				{
+					result = Accessibility.Internal;
+				}
+			}
+
+			return result;
+		}
+
 		#endregion
 	}
 
+	private sealed class TypeAccessComparer : IEqualityComparer<(ITypeSymbol Source, ITypeSymbol Target)>
+	{
+		private TypeAccessComparer()
+		{
+		}
+
+		public static TypeAccessComparer Instance { get; } = new();
+
+		public bool Equals(
+			(ITypeSymbol Source, ITypeSymbol Target) x,
+			(ITypeSymbol Source, ITypeSymbol Target) y)
+			=> SymbolComparer.Equals(x.Source, y.Source)
+			&& SymbolComparer.Equals(x.Target, y.Target);
+
+		public int GetHashCode((ITypeSymbol Source, ITypeSymbol Target) obj)
+			=> HashCode.Combine(SymbolComparer.GetHashCode(obj.Source), SymbolComparer.GetHashCode(obj.Target));
+	}
+
 	#endregion
 }

tests/Menees.Analyzers.Test/Men019UnitTests.cs

@@ -143,7 +143,16 @@ public void SyncCancel(CancellationToken c = default) { }
 
 	public Task SplitMethodGroup() => Task.CompletedTask; // Base class's overload is cancellable.
 	public int Ineligible(int i, int multiplier) => i * multiplier;
-}"
+}
+
+public abstract class Job {	protected CancellationToken Cancellation {get;} }
+public sealed class DerivedJob : Job
+{
+	public Task InheritsCancellationProperty() => Task.CompletedTask;
+	public static Task TakesCancellableParameter(Job job) => Task.CompletedTask;
+	public static Task TakesCancellableParameter2(Job job) => Task.CompletedTask; // Cache should already have (DerivedJob,Job) accessibility
+}
+"
 + Environment.NewLine + SharedCode;
 
 		this.VerifyCSharpDiagnostic(test);