ILLink analyzer handling of IL3003 on properties and events

[vitek-karas]

This is currently only about RequiresAssemblyFiles as it's allowed on properties and events. RequiresUnreferencedCode and RequiresDynamicCode are not allowed on properties and events (yet - #83194).

Take this as an example:

class Base
{
 [RequiresAssemblyFiles("property")]
 public int MyProperty
 {
 get;
 [RequiresAssemblyFiles("setter")]
 set;
 }
}

class Derived
{
 [RequiresAssemblyFiles("property")]
 public int MyProperty
 {
 get;
 set; // IL3003 - the Derived.MyProperty.set doesn't have the attribute while Base.MyProperty.set does
 }
}

Basically the analyzer treats the property and the methods as separate entities and validates the mismatch of the attribute on each separately. But the semantics of the attribute are that if it's on the property it's as if it applies to both accessors.
So from a single-file compatibility, the above example is perfectly compatible, as on 4 accessors will produce IL3002 when called as all 4 fall under the property-level attribute.

There are other similar variations of this, where setting on property on Base and then setting it on both accessors in derived still causes warnings (and vice versa).

NativeAOT will implement this by looking at methods only, so the above will not produce a warning. As in the end the behavior on method level is what actually causes single-file incompatibility.

We should change the analyzer to this behavior as well - it should basically warn less. There's a small chance of breaks if somebody suppresses the warning using pragmas (which nobody should do on this warning really) and only scope it to the property declaration without including the accessors. I would be surprised if that actually happens even once.

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This is currently only about RequiresAssemblyFiles as it's allowed on properties and events. RequiresUnreferencedCode and RequiresDynamicCode are not allowed on properties and events (yet - #83194).

Take this as an example:

class Base
{
 [RequiresAssemblyFiles("property")]
 public int MyProperty
 {
 get;
 [RequiresAssemblyFiles("setter")]
 set;
 }
}

class Derived
{
 [RequiresAssemblyFiles("property")]
 public int MyProperty
 {
 get;
 set; // IL3003 - the Derived.MyProperty.set doesn't have the attribute while Base.MyProperty.set does
 }
}

Basically the analyzer treats the property and the methods as separate entities and validates the mismatch of the attribute on each separately. But the semantics of the attribute are that if it's on the property it's as if it applies to both accessors.
So from a single-file compatibility, the above example is perfectly compatible, as on 4 accessors will produce IL3002 when called as all 4 fall under the property-level attribute.

There are other similar variations of this, where setting on property on Base and then setting it on both accessors in derived still causes warnings (and vice versa).

NativeAOT will implement this by looking at methods only, so the above will not produce a warning. As in the end the behavior on method level is what actually causes single-file incompatibility.

We should change the analyzer to this behavior as well - it should basically warn less. There's a small chance of breaks if somebody suppresses the warning using pragmas (which nobody should do on this warning really) and only scope it to the property declaration without including the accessors. I would be surprised if that actually happens even once.

Author:	vitek-karas
Assignees:	-
Labels:	area-Tools-ILLink
Milestone:	-

[agocke]

Agreed with your specification, I think that's the right behavior.