Match Attributes for virtual methods and overrides

docs/error-codes.md

@@ -1636,4 +1636,22 @@ void RequirePublicMethodOnAType(
  // can break functionality when embedded in a single-file app. Use 'MethodFriendlyToSingleFile' instead. http://help/assemblyfiles
  MethodWithAssemblyFilesUsage();
  }
+ ```
+
+#### `IL3003`: Presence of 'RequiresAssemblyFilesAttribute' on method 'method' doesn't match overridden method 'base method'. All overridden methods must have 'RequiresAssemblyFilesAttribute'.
+
+- All overrides of a virtual method including the base method must either have or not have the `RequiresAssemblyFilesAttribute`.
+
+ ```C#
+ public class Base
+ {
+ [RequiresAssemblyFiles]
+ public virtual void TestMethod() {}
+ }
+
+ public class Derived : Base
+ {
+ // IL3003: Presence of 'RequiresAssemblyFilesAttribute' on method 'Derived.TestMethod()' doesn't match overridden method 'Base.TestMethod'. All overridden methods must have 'RequiresAssemblyFilesAttribute'.
+ public override void TestMethod() {}
+ }
  ```

src/ILLink.RoslynAnalyzer/RequiresAnalyzerBase.cs

@@ -21,6 +21,8 @@ public abstract class RequiresAnalyzerBase : DiagnosticAnalyzer
 
  private protected abstract DiagnosticDescriptor RequiresDiagnosticRule { get; }
 
+ private protected abstract DiagnosticDescriptor MatchOverridesRule { get; }
+
  public override void Initialize (AnalysisContext context)
  {
  context.EnableConcurrentExecution ();
@@ -31,6 +33,12 @@ public override void Initialize (AnalysisContext context)
  return;
  var incompatibleMembers = GetSpecialIncompatibleMembers (compilation);
 
+ context.RegisterSymbolAction (symbolAnalysisContext => {
+ var method = (IMethodSymbol) symbolAnalysisContext.Symbol;
+ if (method.IsVirtual || method.IsOverride)
+ CheckOverrides (symbolAnalysisContext, method);
+ }, SymbolKind.Method);
+
  context.RegisterOperationAction (operationContext => {
  var methodInvocation = (IInvocationOperation) operationContext.Operation;
  CheckCalledMember (operationContext, methodInvocation.TargetMethod, incompatibleMembers);
@@ -126,6 +134,28 @@ void CheckCalledMember (
  ReportRequiresDiagnostic (operationContext, member, requiresAttribute);
  }
  }
+
+ void CheckOverrides (
+ SymbolAnalysisContext symbolAnalysisContext,
+ IMethodSymbol method)
+ {
+ if (!method.HasAttribute (RequiresAttributeName)) {
+ while (method.OverriddenMethod != null) {
+ var overridden = method.OverriddenMethod;
+ if (overridden.HasAttribute (RequiresAttributeName))
+ ReportMatchOverridesDiagnostic (symbolAnalysisContext, method.Locations[0], overridden, method);
+ method = overridden;
+ }
+ } else {
+ while (method.OverriddenMethod != null) {
+ var overridden = method.OverriddenMethod;
+ if (!overridden.HasAttribute (RequiresAttributeName))
+ ReportMatchOverridesDiagnostic (symbolAnalysisContext, overridden.Locations[0], method, overridden);
+ method = overridden;
+ }
+ }
+ }
+
  });
  }
 
@@ -186,6 +216,15 @@ private void ReportRequiresDiagnostic (OperationAnalysisContext operationContext
  url));
  }
 
+ private void ReportMatchOverridesDiagnostic (SymbolAnalysisContext symbolAnalysisContext, Location location, IMethodSymbol methodWithAttribute, IMethodSymbol methodWithoutAttribute)
+ {
+ symbolAnalysisContext.ReportDiagnostic (Diagnostic.Create (
+ MatchOverridesRule,
+ location,
+ methodWithAttribute.ToString (),
+ methodWithoutAttribute.ToString ()));
+ }
+
  protected abstract string GetMessageFromAttribute (AttributeData? requiresAssemblyFilesAttribute);
 
  private string GetUrlFromAttribute (AttributeData? requiresAssemblyFilesAttribute)

src/ILLink.RoslynAnalyzer/RequiresAssemblyFilesAnalyzer.cs

@@ -16,6 +16,7 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase
  public const string IL3000 = nameof (IL3000);
  public const string IL3001 = nameof (IL3001);
  public const string IL3002 = nameof (IL3002);
+ public const string IL3003 = nameof (IL3003);
 
  private const string RequiresAssemblyFilesAttribute = nameof (RequiresAssemblyFilesAttribute);
  public const string RequiresAssemblyFilesAttributeFullyQualifiedName = "System.Diagnostics.CodeAnalysis." + RequiresAssemblyFilesAttribute;
@@ -53,7 +54,17 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase
  isEnabledByDefault: true,
  helpLinkUri: "https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/il3002");
 
- public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_locationRule, s_getFilesRule, s_requiresAssemblyFilesRule);
+ static readonly DiagnosticDescriptor s_matchOverridesRule = new DiagnosticDescriptor (
+ IL3003,
+ new LocalizableResourceString (nameof (Resources.MatchRequiresAssemblyFilesOverridesTitle),
+ Resources.ResourceManager, typeof (Resources)),
+ new LocalizableResourceString (nameof (Resources.MatchRequiresAssemblyFilesOverridesMessage),
+ Resources.ResourceManager, typeof (Resources)),
+ DiagnosticCategory.SingleFile,
+ DiagnosticSeverity.Warning,
+ isEnabledByDefault: true);
+
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_locationRule, s_getFilesRule, s_requiresAssemblyFilesRule, s_matchOverridesRule);
 
  private protected override string RequiresAttributeName => RequiresAssemblyFilesAttribute;
 
@@ -63,6 +74,8 @@ public sealed class RequiresAssemblyFilesAnalyzer : RequiresAnalyzerBase
 
  private protected override DiagnosticDescriptor RequiresDiagnosticRule => s_requiresAssemblyFilesRule;
 
+ private protected override DiagnosticDescriptor MatchOverridesRule => s_matchOverridesRule;
+
  protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation)
  {
  var isSingleFileAnalyzerEnabled = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.EnableSingleFileAnalyzer, compilation);

src/ILLink.RoslynAnalyzer/RequiresUnreferencedCodeAnalyzer.cs

@@ -13,6 +13,7 @@ namespace ILLink.RoslynAnalyzer
  public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
  {
  public const string IL2026 = nameof (IL2026);
+ public const string IL2046 = nameof (IL2046);
  const string RequiresUnreferencedCodeAttribute = nameof (RequiresUnreferencedCodeAttribute);
  public const string FullyQualifiedRequiresUnreferencedCodeAttribute = "System.Diagnostics.CodeAnalysis." + RequiresUnreferencedCodeAttribute;
 
@@ -26,7 +27,17 @@ public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
  DiagnosticSeverity.Warning,
  isEnabledByDefault: true);
 
- public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_requiresUnreferencedCodeRule);
+ static readonly DiagnosticDescriptor s_matchOverridesRule = new DiagnosticDescriptor (
+ IL2046,
+ new LocalizableResourceString (nameof (Resources.MatchRequiresUnreferencedCodeOverridesTitle),
+ Resources.ResourceManager, typeof (Resources)),
+ new LocalizableResourceString (nameof (Resources.MatchRequiresUnreferencedCodeOverridesMessage),
+ Resources.ResourceManager, typeof (Resources)),
+ DiagnosticCategory.Trimming,
+ DiagnosticSeverity.Warning,
+ isEnabledByDefault: true);
+
+ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create (s_requiresUnreferencedCodeRule, s_matchOverridesRule);
 
  private protected override string RequiresAttributeName => RequiresUnreferencedCodeAttribute;
 
@@ -36,6 +47,8 @@ public sealed class RequiresUnreferencedCodeAnalyzer : RequiresAnalyzerBase
 
  private protected override DiagnosticDescriptor RequiresDiagnosticRule => s_requiresUnreferencedCodeRule;
 
+ private protected override DiagnosticDescriptor MatchOverridesRule => s_matchOverridesRule;
+
  protected override bool IsAnalyzerEnabled (AnalyzerOptions options, Compilation compilation)
  {
  var isTrimAnalyzerEnabled = options.GetMSBuildPropertyValue (MSBuildPropertyOptionNames.EnableTrimAnalyzer, compilation);

src/ILLink.RoslynAnalyzer/Resources.resx

@@ -141,4 +141,16 @@
  <data name="RequiresAssemblyFilesMessage" xml:space="preserve">
  <value>Using member '{0}' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.{1}{2}</value>
  </data>
+ <data name="MatchRequiresUnreferencedCodeOverridesMessage" xml:space="preserve">
+ <value>Presence of 'RequiresUnreferencedCodeAttribute' on method '{0}' doesn't match overridden method '{1}'. All overridden methods must have 'RequiresUnreferencedCodeAttribute'.</value>
+ </data>
+ <data name="MatchRequiresUnreferencedCodeOverridesTitle" xml:space="preserve">
+ <value>All overrides of a virtual method including the base method must either have or not have the RequiresUnreferencedCodeAttribute</value>
+ </data>
+ <data name="MatchRequiresAssemblyFilesOverridesMessage" xml:space="preserve">
+ <value>Presence of 'RequiresAssemblyFilesAttribute' on method '{0}' doesn't match overridden method '{1}'. All overridden methods must have 'RequiresAssemblyFilesAttribute'.</value>
+ </data>
+ <data name="MatchRequiresAssemblyFilesOverridesTitle" xml:space="preserve">
+ <value>All overrides of a virtual method including the base method must either have or not have the RequiresAssemblyFilesAttribute</value>
+ </data>
 </root>

test/ILLink.RoslynAnalyzer.Tests/RequiresAssemblyFilesAnalyzerTests.cs

@@ -814,7 +814,7 @@ static void TestStaticCtorMarkingIsTriggeredByFieldAccess ()
  }
 }";
  return VerifyRequiresAssemblyFilesAnalyzer (src,
- // (18,11): warning IL2026: Using member 'StaticCtorTriggeredByFieldAccess.StaticCtorTriggeredByFieldAccess()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.. Message for --StaticCtorTriggeredByFieldAccess.Cctor--.
+ // (18,11): warning IL3002: Using member 'StaticCtorTriggeredByFieldAccess.StaticCtorTriggeredByFieldAccess()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.. Message for --StaticCtorTriggeredByFieldAccess.Cctor--.
  VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3002).WithSpan (18, 11, 18, 49).WithArguments ("StaticCtorTriggeredByFieldAccess.StaticCtorTriggeredByFieldAccess()", " Message for --StaticCtorTriggeredByFieldAccess.Cctor--.", "")
  );
  }
@@ -846,11 +846,61 @@ static void TestStaticCtorTriggeredByMethodCall ()
  }
 }";
  return VerifyRequiresAssemblyFilesAnalyzer (src,
- // (21,3): warning IL2026: Using member 'StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. Message for --StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--.
+ // (21,3): warning IL3002: Using member 'StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. Message for --StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--.
  VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3002).WithSpan (21, 3, 21, 69).WithArguments ("StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking()", " Message for --StaticCtorTriggeredByMethodCall.TriggerStaticCtorMarking--.", ""),
- // (21,3): warning IL2026: Using member 'StaticCtorTriggeredByMethodCall.StaticCtorTriggeredByMethodCall()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app.. Message for --StaticCtorTriggeredByMethodCall.Cctor--.
+ // (21,3): warning IL3002: Using member 'StaticCtorTriggeredByMethodCall.StaticCtorTriggeredByMethodCall()' which has 'RequiresAssemblyFilesAttribute' can break functionality when embedded in a single-file app. Message for --StaticCtorTriggeredByMethodCall.Cctor--.
  VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3002).WithSpan (21, 3, 21, 41).WithArguments ("StaticCtorTriggeredByMethodCall.StaticCtorTriggeredByMethodCall()", " Message for --StaticCtorTriggeredByMethodCall.Cctor--.", "")
  );
  }
+
+ [Fact]
+ public Task OverrideHasAttributeButBaseDoesnt ()
+ {
+ var src = @"
+using System.Diagnostics.CodeAnalysis;
+
+class DerivedClass : BaseClass
+{
+ [RequiresAssemblyFiles]
+ public override void VirtualMethod ()
+ {
+ }
+}
+
+class BaseClass
+{
+ public virtual void VirtualMethod ()
+ {
+ }
+}";
+ return VerifyRequiresAssemblyFilesAnalyzer (src,
+ // (14,22): warning IL3003: Presence of 'RequiresAssemblyFilesAttribute' on method 'DerivedClass.VirtualMethod()' doesn't match overridden method 'BaseClass.VirtualMethod()'. All overridden methods must have 'RequiresAssemblyFilesAttribute'.
+ VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3003).WithSpan (14, 22, 14, 35).WithArguments ("DerivedClass.VirtualMethod()", "BaseClass.VirtualMethod()"));
+ }
+
+ [Fact]
+ public Task VirtualHasAttributeButOverrideDoesnt ()
+ {
+ var src = @"
+using System.Diagnostics.CodeAnalysis;
+
+class DerivedClass : BaseClass
+{
+ public override void VirtualMethod ()
+ {
+ }
+}
+
+class BaseClass
+{
+ [RequiresAssemblyFiles]
+ public virtual void VirtualMethod ()
+ {
+ }
+}";
+ return VerifyRequiresAssemblyFilesAnalyzer (src,
+ // (6,23): warning IL3003: Presence of 'RequiresAssemblyFilesAttribute' on method 'BaseClass.VirtualMethod()' doesn't match overridden method 'DerivedClass.VirtualMethod()'. All overridden methods must have 'RequiresAssemblyFilesAttribute'.
+ VerifyCS.Diagnostic (RequiresAssemblyFilesAnalyzer.IL3003).WithSpan (6, 23, 6, 36).WithArguments ("BaseClass.VirtualMethod()", "DerivedClass.VirtualMethod()"));
+ }
  }
 }