IsInitOnly boolean property for PropertyDefinition in SDK

[dbeylkhanov]

API Proposal

namespace System.Reflection;

public class PropertyInfo
{
+       public bool IsInitOnly { get; } 
}

Note "IsInitOnly" is used since that is similar to FieldInfo's. However, "IsExternalInit" is an alternative based on #34978 and #43088 (comment).

Original description

I am trying to check that my property has init-only setter, e.g.
public int Foo {get; init;}

it would be nice to have the IsInitOnly property in PropertyDefinition similar to which in FieldDefinition class from Mono.Cecil

Below I posted the IsInitOnly property from FieldDefinition

public bool IsInitOnly
    {
      get => this.attributes.GetAttributes((ushort) 32);
      set => this.attributes = this.attributes.SetAttributes((ushort) 32, value);
    }

By using the IsInitOnly prop I can validate in my arch tests that all properties in Domain models are init-only

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[KalleOlaviNiemitalo]

See #34978 and especially #43088 (comment) for sample code that detects init aka IsExternalInit by using the existing System.Reflection API.

Alternatively, your arch tests could perhaps use IMethodSymbol.IsInitOnly in the Roslyn API.

[dbeylkhanov]

#43088 (comment)

yeah, I saw this comment before the opening this issue. I think that the suggested solution is a crutch. I don't want use that in my tests

[nagilson]

May you elaborate as to why it's a crutch (there are 3 suggested solutions) and why you want that property in particular? Probably we will move this elsewhere, maybe the Runtime, doesn't seem like it's part of the SDK.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I am trying to check that my property has init-only setter, e.g.
public int Foo {get; init;}

it would be nice to have the IsInitOnly property in PropertyDefinition similar to which in FieldDefinition class from Mono.Cecil

Below I posted the IsInitOnly property from FieldDefinition

public bool IsInitOnly
    {
      get => this.attributes.GetAttributes((ushort) 32);
      set => this.attributes = this.attributes.SetAttributes((ushort) 32, value);
    }

By using the IsInitOnly prop I can validate in my arch tests that all properties in Domain models are init-only

Author:	dbeylkhanov
Assignees:	nagilson
Labels:	api-suggestion, area-System.ComponentModel
Milestone:	-

[steveharter]

it would be nice to have the IsInitOnly property in PropertyDefinition similar to which in FieldDefinition class from Mono.Cecil

Do you mean PropertyInfo (not PropertyDefinition)? FieldInfo already has an IsInitOnly...

Also the auto-labeler moved this to the System.ComponentModel namespace; I assume it was meant for System.Reflection.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

---

I am trying to check that my property has init-only setter, e.g.
public int Foo {get; init;}

it would be nice to have the IsInitOnly property in PropertyDefinition similar to which in FieldDefinition class from Mono.Cecil

Below I posted the IsInitOnly property from FieldDefinition

public bool IsInitOnly
    {
      get => this.attributes.GetAttributes((ushort) 32);
      set => this.attributes = this.attributes.SetAttributes((ushort) 32, value);
    }

By using the IsInitOnly prop I can validate in my arch tests that all properties in Domain models are init-only

Author:	dbeylkhanov
Assignees:	-
Labels:	api-suggestion, area-System.Reflection, untriaged
Milestone:	-

[dbeylkhanov]

it would be nice to have the IsInitOnly property in PropertyDefinition similar to which in FieldDefinition class from Mono.Cecil

Do you mean PropertyInfo (not PropertyDefinition)? FieldInfo already has an IsInitOnly...

Also the auto-labeler moved this to the System.ComponentModel namespace; I assume it was meant for System.Reflection.

I have a property, not field

[dbeylkhanov]

May you elaborate as to why it's a crutch (there are 3 suggested solutions)

@nagilson well, do you suggest me use enumerator for every property in my type by checking existing of init like below way

foreach (var m in typeof(Foo).GetMethod("set_Boo").ReturnParameter.GetRequiredCustomModifiers())

correct? if yes, you really think that it's not crutch?)

[steveharter]

it would be nice to have the IsInitOnly property in PropertyDefinition similar to which in FieldDefinition class from Mono.Cecil

Do you mean PropertyInfo (not PropertyDefinition)? FieldInfo already has an IsInitOnly...

Also the auto-labeler moved this to the System.ComponentModel namespace; I assume it was meant for System.Reflection.

I have a property, not field

My point is that "PropertyDefinition" is not a type in .NET reflection. -- it's called "PropertyInfo". Just making sure we're talking about the same thing (or not).

Assuming we're talking about "PropertyInfo" then the ask here is to add:

namespace System.Reflection
{
    public class PropertyInfo
    {
+       public bool IsExternalInit { get; } // not "IsInitOnly" based on discussion in issue #34978
}

[steveharter]

May you elaborate as to why it's a crutch (there are 3 suggested solutions)

@nagilson well, do you suggest me use enumerator for every property in my type by checking existing of init like below way
foreach (var m in typeof(Foo).GetMethod("set_Boo").ReturnParameter.GetRequiredCustomModifiers())
correct? if yes, you really think that it's not crutch?)

The workaround is to add your own extensions method; I do however think that reflection should be updated to expose this directly. Sample workaround:

  internal class Program
  {
      static void Main(string[] args)
      {
          PropertyInfo propertyInfo = typeof(Person_InitExample).GetProperty("YearOfBirth")!;
          Console.WriteLine($"IsInit: {propertyInfo.IsExternalEmit()}");
      }
  }

  internal static class ReflectionExtensions
  {
      public static bool IsExternalEmit(this PropertyInfo propertyInfo)
      {
          MethodInfo? setter = propertyInfo.GetSetMethod();
          if (setter is not null)
          {
              foreach (Type modreq in setter.ReturnParameter.GetRequiredCustomModifiers())
              {
                  if (modreq == typeof(System.Runtime.CompilerServices.IsExternalInit))
                  {
                      return true;
                  }
              }
          }

          return false;
      }
  }

  class Person_InitExample
  {
      private int _yearOfBirth;

      public int YearOfBirth
      {
          get { return _yearOfBirth; }
          init { _yearOfBirth = value; }
      }
  }

[dbeylkhanov]

@steveharter

My point is that "PropertyDefinition" is not a type in .NET reflection. -- it's called "PropertyInfo". Just making sure we're talking about the same thing (or not).

oops, I meant Mono.Cecil.TypeDefinition

[dbeylkhanov]

all right, chatGPT answered my question about accessing to IsInitOnly.
And yes, your advice with the checking by using PropertyInfo works, thanks!

[steveharter]

Moving to future; the ask for the "IsInitOnly" is valid and useful.

[terrajobst]

Video

• How much overhead does this add?
• How about other concepts we recently added, such as required?
• We recently added nullability information but we put them on a secondary type
• We should probably think about how we want to expose those concepts. It seems worthwhile to batch multiple C# features together, rather than doing this piecemeal.

namespace System.Reflection;

public partial class PropertyInfo
{
    public bool IsInitOnly { get; }
}

[Neme12]

IMO This is the wrong approach. Whether a setter is init-only isn't a property of the property itself, but rather of the setter (and as far as the language semantics go, it's actually a different kind of accessor altogether). For example, you can have a property that is init-only, the same way you can have a property that is set-only, without any getter whatsoever. The API as proposed here would give the impression that it's checking whether there is an init accessor and no get accessor.

This should instead eiher be something like:

 namespace System.Reflection;

 public abstract class PropertyInfo : MemberInfo
 {
     public virtual MethodInfo? GetMethod { get; }
     public virtual MethodInfo? SetMethod { get; }
+    public virtual bool SetMethodIsInitOnly { get; } // or IsSetAccessorInitOnly, or IsSetterInitOnly, etc...
 }

or

 namespace System.Reflection;

 public abstract class PropertyInfo : MemberInfo
 {
     public abstract bool CanRead { get; }
     public abstract bool CanWrite { get; }

     // if true, CanWrite can also return true to maintain backwards compatibility, or they can be separate
     // and CanWrite could now return false and only CanInit would return true
+    public virtual bool CanInit { get; }
 }

or

 namespace System.Reflection;

 public abstract class MethodInfo : MethodBase
 {
+    public virtual bool IsGetter { get; } // or IsGet or IsGetMethod
+    public virtual bool IsSetter { get; } // or IsSet or IsSetMethod
+    public virtual bool IsAdder { get; } // or IsAdd or IsAddMethod
+    public virtual bool IsRemover { get; } // or IsRemove or IsRemoveMethod

     // same compatibility concern as above
+    public virtual bool IsInitter { get; } // or IsInit or IsInitMethod
 }

or

 namespace System.Reflection;

+public enum AccessorKind
+{
+    Get,
+    Set,
+    Init,
+    Add,
+    Remove,
+}

 public abstract class MethodInfo : MethodBase
 {
+    public virtual AccessorKind? AccessorKind { get; }
 }

or maybe even this, although the same compatibility applies here as well (whether CanWrite would also return true and SetMethod would return the method like today to maintain backwards compatibility, or they can be separate and only CanInit would return true and InitMethod would return the method)

 namespace System.Reflection;

 public abstract class PropertyInfo : MemberInfo
 {
     public abstract bool CanRead { get; }
     public abstract bool CanWrite { get; }
+    public virtual bool CanInit { get; }
     public virtual MethodInfo? GetMethod { get; }
     public virtual MethodInfo? SetMethod { get; }
+    public virtual MethodInfo? InitMethod { get; }
 }

[Neme12]

• How about other concepts we recently added, such as required?
• We recently added nullability information but we put them on a secondary type
• We should probably think about how we want to expose those concepts. It seems worthwhile to batch multiple C# features together, rather than doing this piecemeal.

There is no problem with making them piece-meal. Even C# added all these features piece by piece with every release. Whether a property or field is required actually has nothing to do with whether it is nullable or not, it's a completely orthogonal feature. Yes, the common pattern will probably be to make non-nullable properties required and nullable ones optional but:

1. You can have a required property that is nullable. This simply means that you have to explicitly initialize it in an object initializer - even if you initialize it to null.
2. You can of course have a property that is non-nullable and isn't required. This (hopefully) means that the constructor sets it to a non-null value.

[Neme12]

Notably, with required: Unlike init, required is actually a property of the whole property/field (you can't even put it on an accessor), so there would be no problem with simply adding:

 public abstract class FieldInfo : MemberInfo
 {
+    public virtual bool IsRequired { get; }
 }

 public abstract class PropertyInfo : MemberInfo
 {
+    public virtual bool IsRequired { get; }
 }

This seems much more straightforward (although it gets a bit complicated with constructors that set all required fields... but I think checking whether the property was declared as required or not should still be possible).

[Neme12]

cc @jaredpar I think the C# folk should also have a say or add some input here since this is about C# features.

[Neme12]

Regarding whether the property should support externally-defined polyfilled modreqs rather than just the builtin corelib ones - YES it should, because the compiler does as well. Many libraries declare their own IsExternalInit (there are nuget packages for that) and use init to their heart's content (e.g. IsExternalInit and PolySharp - heck, even Roslyn defines their own IsExternalInit to be able to use init on netstandard). Just check whether the fully qualified name matches System.Runtime.CompilerServices.IsExternalInit, like the compiler does.

[jaredpar]

Whether a setter is init-only isn't a property of the property itself, but rather of the setter

This is true but at the same time PropertyInfo already has a few properties that effectively dig into the underlying get / set method. For example CanRead / CanWrite is talking more about the underlying members not the property itself. A property like CanInit would seem to fit into the existing pattern.

if (modreq == typeof(System.Runtime.CompilerServices.IsExternalInit))

I would recommend against this pattern. Yes that is the canonical IsExternalInit but due to down targetting and projects like PolySharp there are other definitions of IsExternalInit in play. I would instead check the fully qualified type name for equivalence to "System.Runtime.CompilerServices.IsExternalInit".

[Neme12]

This is true but at the same time PropertyInfo already has a few properties that effectively dig into the underlying get / set method. For example CanRead / CanWrite is talking more about the underlying members not the property itself. A property like CanInit would seem to fit into the existing pattern.

Right. I would be OK with a property like CanInit. My pushback was that IsInitOnly suggests that it means there is no getter - setter-only properties exist and they have no getter. Similarly, init-only properties also exist and they also have no getter. Unless IsInitOnly was a property of the setter itself. Or something like CanInit doesn't have this problem, nor does something like IsSetterInitOnly.

[Neme12]

I would recommend against this pattern. Yes that is the canonical IsExternalInit but due to down targetting and projects like PolySharp there are other definitions of IsExternalInit in play.

Exactly. That's a really, really bad pattern that would break half the world. The language feature isn't defined by whether the modreq comes from mscorlib. Just this fact is an argument in favor of the API existing (and doing it correctly), since many people who will implement it manually will likely do it the wrong way.

[Neme12]

Right. I would be OK with a property like CanInit. My pushback was that IsInitOnly suggests that it means there is no getter - setter-only properties exist and they have no getter. Similarly, init-only properties also exist and they also have no getter. Something like CanInit doesn't have this problem, nor does something like IsSetterInitOnly.

That said, a feature like this:

 namespace System.Reflection;

+public enum AccessorKind
+{
+    Get,
+    Set,
+    Init,
+    Add,
+    Remove,
+}

 public abstract class MethodInfo : MethodBase
 {
+    public virtual AccessorKind? AccessorKind { get; }
+    public virtual MemberInfo? AccessorMember { get; }

     // Or maybe:
+    public virtual MemberInfo? Parent { get; } // or DeclaringMember
     // where Parent could be a property of an property accessor, an event of an event accessor,
     // a parent method of a local function, or regular parent type of a regular method.

     // Or even better, a MethodKind like Roslyn has that would allow checking not only if it's an accessor,
     // but also a local function, operator, lambda, etc. Roslyn has this as `IMethodSymbol.MethodKind`.
 }

would be really useful anyway as it would allow checking whether a method is an accessor when all you have is MethodInfo. This is really cumbersome today as you have to rely on implementation details. The same is true when you want to check whether a method is a local function and get the parent method.

[KalleOlaviNiemitalo]

This enum AccessorKind looks incomplete from the ECMA-335 MethodSemantics viewpoint. Both properties and events can have Other accessors and it would be good to distinguish those from methods that are not accessors. Events can also have Fire accessors.

Moreover, ECMA-335 allows the same method to be an accessor of multiple properties and/or events, if you don't care about CLS compliance. So MethodInfo.AccessorKind and MethodInfo.Parent would have to pick one, ugh.

[Neme12]

@KalleOlaviNiemitalo That was just an example. I'm not too familiar with the runtime itself - it could of course be changed to be compliant. Although if we want this to have semantics of the runtime and not C#, then an init accessor would always have to be considered to be set accessor - and whether it's actually init would be a property of that set accessor.

[Neme12]

FWIW, in Roslyn, whether a property has an init accessor is also a property of the accessor (IMethodSymbol.IsInitOnly), not of the property (IPropertySymbol). I really think with a name like IsInitOnly, it should be a property on the accessor itself and not on the property. If it's on the property, then it needs a different name (such as IsSetterInitOnly, because a property with { get; init; } is not init-only, it's init-and-get. Only the setter is "init-only".

[Neme12]

As an alternative in case the BCL doesn't want to expose C# specific concepts in reflection... Maybe it should be made easy to get Roslyn's IMethodSymbol from a MethodInfo, ITypeSymbol from a Type, etc? E.g. via extension methods. Probably in a separate NuGet package like Microsoft.CodeAnalysis.Reflection. @jaredpar Would this be possible, to wrap Roslyn's symbol types around runtime types?

This would have the advantage that it could work for VB-specific concepts as well. (And IMO, ISymbol should even be extended to be able to represent F# specific concepts and gather them from reflection types as well).