Do not report redundant suppressions on trimmed members (#2999)

* Test repro + fix

* fix marked filter for events and properties

* formatting fix

* simplified Suppression constructor

* Add more tests for events and properties

4 files changed, 163 insertions, 14 deletions

diff --git a/src/linker/Linker.Steps/CheckSuppressionsDispatcher.cs b/src/linker/Linker.Steps/CheckSuppressionsDispatcher.cs
index 4bb15635c..0250dfeca 100644
--- a/src/linker/Linker.Steps/CheckSuppressionsDispatcher.cs
+++ b/src/linker/Linker.Steps/CheckSuppressionsDispatcher.cs
@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using ILLink.Shared;
+using Mono.Cecil;
 
 namespace Mono.Linker.Steps
 {
@@ -23,13 +24,30 @@ namespace Mono.Linker.Steps
 			// Suppressions targeting RedundantSuppression warning should not be reported.
 			redundantSuppressions = redundantSuppressions
 				.Where (suppression => ((DiagnosticId) suppression.SuppressMessageInfo.Id).GetDiagnosticCategory () == DiagnosticCategory.Trimming)
-				.Where (suppression => ((DiagnosticId) suppression.SuppressMessageInfo.Id) != DiagnosticId.RedundantSuppression);
+				.Where (suppression => ((DiagnosticId) suppression.SuppressMessageInfo.Id) != DiagnosticId.RedundantSuppression)
+				.Where (suppression => ProviderIsMarked (suppression.Provider));
 
 			foreach (var suppression in redundantSuppressions) {
 				var source = context.Suppressions.GetSuppressionOrigin (suppression);
 
 				context.LogWarning (new MessageOrigin (source), DiagnosticId.RedundantSuppression, $"IL{suppression.SuppressMessageInfo.Id:0000}");
 			}
+
+			bool ProviderIsMarked (ICustomAttributeProvider provider)
+			{
+				if (provider is PropertyDefinition property) {
+					return (property.GetMethod != null && context.Annotations.IsMarked (property.GetMethod))
+						|| (property.SetMethod != null && context.Annotations.IsMarked (property.SetMethod));
+				}
+
+				if (provider is EventDefinition @event) {
+					return (@event.AddMethod != null && context.Annotations.IsMarked (@event.AddMethod))
+						|| (@event.InvokeMethod != null && context.Annotations.IsMarked (@event.InvokeMethod))
+						|| (@event.RemoveMethod != null && context.Annotations.IsMarked (@event.RemoveMethod));
+				}
+
+				return context.Annotations.IsMarked (provider);
+			}
 		}
 	}
 }
diff --git a/src/linker/Linker/UnconditionalSuppressMessageAttributeState.cs b/src/linker/Linker/UnconditionalSuppressMessageAttributeState.cs
index 168b1ef72..2aff747d8 100644
--- a/src/linker/Linker/UnconditionalSuppressMessageAttributeState.cs
+++ b/src/linker/Linker/UnconditionalSuppressMessageAttributeState.cs
@@ -19,14 +19,13 @@ namespace Mono.Linker
 		public class Suppression
 		{
 			public SuppressMessageInfo SuppressMessageInfo { get; }
-			public bool Used { get; set; }
+			public bool Used { get; set; } = false;
 			public CustomAttribute OriginAttribute { get; }
 			public ICustomAttributeProvider Provider { get; }
 
-			public Suppression (SuppressMessageInfo suppressMessageInfo, bool used, CustomAttribute originAttribute, ICustomAttributeProvider provider)
+			public Suppression (SuppressMessageInfo suppressMessageInfo, CustomAttribute originAttribute, ICustomAttributeProvider provider)
 			{
 				SuppressMessageInfo = suppressMessageInfo;
-				Used = used;
 				OriginAttribute = originAttribute;
 				Provider = provider;
 			}
@@ -270,7 +269,7 @@ namespace Mono.Linker
 				if (!TryDecodeSuppressMessageAttributeData (ca, out var info))
 					continue;
 
-				yield return new Suppression (info, used: false, originAttribute: ca, provider);
+				yield return new Suppression (info, originAttribute: ca, provider);
 			}
 		}
 
@@ -297,13 +296,13 @@ namespace Mono.Linker
 
 				var scope = info.Scope?.ToLower ();
 				if (info.Target == null && (scope == "module" || scope == null)) {
-					yield return new Suppression (info, used: false, originAttribute: instance, provider);
+					yield return new Suppression (info, originAttribute: instance, provider);
 					continue;
 				}
 
 				switch (scope) {
 				case "module":
-					yield return new Suppression (info, used: false, originAttribute: instance, provider);
+					yield return new Suppression (info, originAttribute: instance, provider);
 					break;
 
 				case "type":
@@ -312,7 +311,7 @@ namespace Mono.Linker
 						break;
 
 					foreach (var result in DocumentationSignatureParser.GetMembersForDocumentationSignature (info.Target, module, _context))
-						yield return new Suppression (info, used: false, originAttribute: instance, result);
+						yield return new Suppression (info, originAttribute: instance, result);
 
 					break;
 				default:
diff --git a/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsInMembersAndTypes.cs b/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsInMembersAndTypes.cs
index 6488cdacb..336a12814 100644
--- a/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsInMembersAndTypes.cs
+++ b/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsInMembersAndTypes.cs
@@ -19,17 +19,27 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 			RedundantSuppressionOnType.Test ();
 			RedundantSuppressionOnMethod.Test ();
 			RedundantSuppressionOnNestedType.Test ();
+
 			RedundantSuppressionOnPropertyGet.Test ();
 			RedundantSuppressionOnProperty.Test ();
+			RedundantSuppressionOnPropertyWithOnlyGet.Test ();
+			RedundantSuppressionOnPropertyWithOnlySet.Test ();
+			RedundantSuppressionOnPropertyAccessedByReflection.Test ();
+
 			RedundantSuppressionOnEventAdd.Test ();
 			RedundantSuppressionOnEvent.Test ();
+			RedundantSuppressionOnEventAccessedByReflection.Test ();
+
 			MultipleRedundantSuppressions.Test ();
 			RedundantAndUsedSuppressions.Test ();
+
 			DoNotReportNonLinkerSuppressions.Test ();
 			DoNotReportSuppressionsOnMethodsConvertedToThrow.Test ();
+
 			SuppressRedundantSuppressionWarning.Test ();
-			RedundantSuppressionWithRUC.Test ();
 			DoNotReportUnnecessaryRedundantWarningSuppressions.Test ();
+
+			RedundantSuppressionWithRUC.Test ();
 		}
 
 		public static Type TriggerUnrecognizedPattern ()
@@ -101,6 +111,58 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 			public static void Test ()
 			{
 				var property = TrimmerCompatibleProperty;
+				TrimmerCompatibleProperty = "test";
+			}
+
+			[ExpectedWarning ("IL2121", "IL2071", ProducedBy = ProducedBy.Trimmer)]
+			[UnconditionalSuppressMessage ("Test", "IL2071")]
+			public static string TrimmerCompatibleProperty {
+				get {
+					return TrimmerCompatibleMethod ();
+				}
+				set {
+					value = TrimmerCompatibleMethod ();
+				}
+			}
+		}
+
+		public class RedundantSuppressionOnPropertyWithOnlyGet
+		{
+			public static void Test ()
+			{
+				var property = TrimmerCompatibleProperty;
+			}
+
+			[ExpectedWarning ("IL2121", "IL2071", ProducedBy = ProducedBy.Trimmer)]
+			[UnconditionalSuppressMessage ("Test", "IL2071")]
+			public static string TrimmerCompatibleProperty {
+				get {
+					return TrimmerCompatibleMethod ();
+				}
+			}
+		}
+
+		public class RedundantSuppressionOnPropertyWithOnlySet
+		{
+			public static void Test ()
+			{
+				TrimmerCompatibleProperty = "test";
+			}
+
+			[ExpectedWarning ("IL2121", "IL2071", ProducedBy = ProducedBy.Trimmer)]
+			[UnconditionalSuppressMessage ("Test", "IL2071")]
+			public static string TrimmerCompatibleProperty {
+				set {
+					value = TrimmerCompatibleMethod ();
+				}
+			}
+		}
+
+		public class RedundantSuppressionOnPropertyAccessedByReflection
+		{
+			public static void Test ()
+			{
+				typeof (RedundantSuppressionOnPropertyAccessedByReflection).GetProperty ("TrimmerCompatibleProperty");
 			}
 
 			[ExpectedWarning ("IL2121", "IL2071", ProducedBy = ProducedBy.Trimmer)]
@@ -116,7 +178,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 		{
 			public static void Test ()
 			{
-				Event += EventSubscriber;
+				TrimmerCompatibleEvent += EventSubscriber;
 			}
 
 			static void EventSubscriber (object sender, EventArgs e)
@@ -124,7 +186,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 
 			}
 
-			static event EventHandler<EventArgs> Event {
+			public static event EventHandler<EventArgs> TrimmerCompatibleEvent {
 				[ExpectedWarning ("IL2121", "IL2072", ProducedBy = ProducedBy.Trimmer)]
 				[UnconditionalSuppressMessage ("Test", "IL2072")]
 				add { TrimmerCompatibleMethod (); }
@@ -136,7 +198,7 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 		{
 			public static void Test ()
 			{
-				Event += EventSubscriber;
+				TrimmerCompatibleEvent += EventSubscriber;
 			}
 
 			static void EventSubscriber (object sender, EventArgs e)
@@ -146,7 +208,22 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 
 			[ExpectedWarning ("IL2121", "IL2072", ProducedBy = ProducedBy.Trimmer)]
 			[UnconditionalSuppressMessage ("Test", "IL2072")]
-			static event EventHandler<EventArgs> Event {
+			public static event EventHandler<EventArgs> TrimmerCompatibleEvent {
+				add { TrimmerCompatibleMethod (); }
+				remove { }
+			}
+		}
+
+		public class RedundantSuppressionOnEventAccessedByReflection
+		{
+			public static void Test ()
+			{
+				typeof (RedundantSuppressionOnEventAccessedByReflection).GetEvent ("TrimmerCompatibleEvent");
+			}
+
+			[ExpectedWarning ("IL2121", "IL2072", ProducedBy = ProducedBy.Trimmer)]
+			[UnconditionalSuppressMessage ("Test", "IL2072")]
+			public static event EventHandler<EventArgs> TrimmerCompatibleEvent {
 				add { TrimmerCompatibleMethod (); }
 				remove { }
 			}
@@ -240,7 +317,6 @@ namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
 			public static void Test ()
 			{
 				MethodMarkedRUC ();
-
 			}
 
 			[ExpectedWarning ("IL2121", "IL2072", ProducedBy = ProducedBy.Trimmer)]
diff --git a/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsTrimmedMembersTarget.cs b/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsTrimmedMembersTarget.cs
new file mode 100644
index 000000000..f23e2312b
--- /dev/null
+++ b/test/Mono.Linker.Tests.Cases/Warnings/WarningSuppression/DetectRedundantSuppressionsTrimmedMembersTarget.cs
@@ -0,0 +1,56 @@
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System;
+using System.Diagnostics.CodeAnalysis;
+using System.Linq.Expressions;
+using Mono.Linker.Tests.Cases.Expectations.Assertions;
+using Mono.Linker.Tests.Cases.Expectations.Metadata;
+
+[assembly: UnconditionalSuppressMessage ("Test", "IL2071",
+	Scope = "type",
+	Target = "T:Mono.Linker.Tests.Cases.Warnings.WarningSuppression.UnusedTypeWithRedundantSuppression")]
+[assembly: UnconditionalSuppressMessage ("Test", "IL2071",
+	Scope = "member",
+	Target = "P:Mono.Linker.Tests.Cases.Warnings.WarningSuppression.UnusedTypeWithMembers.UnusedPropertyWithSuppression")]
+[assembly: UnconditionalSuppressMessage ("Test", "IL2071",
+	Scope = "member",
+	Target = "E:Mono.Linker.Tests.Cases.Warnings.WarningSuppression.UnusedTypeWithMembers.UnusedEventWithSuppression")]
+[assembly: UnconditionalSuppressMessage ("Test", "IL2071",
+	Scope = "member",
+	Target = "M:Mono.Linker.Tests.Cases.Warnings.WarningSuppression.UnusedTypeWithMembers.UnusedMethodWithSuppression")]
+
+namespace Mono.Linker.Tests.Cases.Warnings.WarningSuppression
+{
+	[ExpectedNoWarnings]
+	[SkipKeptItemsValidation]
+	class DetectRedundantSuppressionsTrimmedMembersTarget
+	{
+		[ExpectedWarning ("IL2072")]
+		static void Main ()
+		{
+			Expression.Call (TriggerUnrecognizedPattern (), "", Type.EmptyTypes);
+		}
+
+		public static Type TriggerUnrecognizedPattern ()
+		{
+			return typeof (DetectRedundantSuppressionsTrimmedMembersTarget);
+		}
+	}
+
+	class UnusedTypeWithRedundantSuppression
+	{
+	}
+
+	class UnusedTypeWithMembers
+	{
+		int UnusedPropertyWithSuppression { get; set; }
+
+		event EventHandler<EventArgs> UnusedEventWithSuppression {
+			add { }
+			remove { }
+		}
+
+		void UnusedMethodWithSuppression () { }
+	}
+}