Check for immutability

[simonthum]

There is currently no way to check if a class is (truly) immutable. The model allows to check whether there are no (visible) setters, but I would prefer to check whether fields are readonly/init-only. This is not exposed, but I think it should be.

.net reflection equivalent is FieldInfo.IsInitOnly.

I am unsure if/how cecil covers readonly.

[fgather]

Hi, I think in Cecil it is called IsInitOnly as well and it looks like this is doable. Do you want to prepare a PR @simonthum? Or should it go onto the list? Should be a nice first task.

[grzegorzorwat]

Hi,

if no one is working on it, I'd like to work on this issue.

There is one thing to think about. Prior to C# 8, the only way to make a class immutable was to declare all of its fields as readonly and not add setters to its properties. After C# 8 there is also new init keyword to make properties immutable after object construction. C# 8 is supported by .net 5 and later, but it is not supported by .netstandard.

I see three possible solutions for this problem:

1. We can add .net 5 as a new target to the project and check init properties only in this target.
2. Because the init keyword is only syntactic sugar, it is possible to check it without targeting .net 5+. We can see if the FullName of ModifierType matches the name of the marker type used by .net to identify init setters.

bool isSetterInitOnly = propertyDefinition.SetMethod != null
    && propertyDefinition.SetMethod.ReturnType.IsRequiredModifier
    && ((RequiredModifierType)propertyDefinition.SetMethod.ReturnType).ModifierType.FullName
        == "System.Runtime.CompilerServices.IsExternalInit";

3. We can check if class is immutable without checking init setters. This is an option I dislike because it is a feature that has been available for almost three years and is commonly used in .net 5+ applications.

Let me know what you think about those solutions or if you have any better ideas.

[fgather]

Hi @grzegorzorwat, thanks for looking into it. I'm hesitant regarding option 1, we have to support older versions as well, so there would be some conditionals all over the place.

So I feel option two would be the way to go. As a matter of fact, there is some work done already in https://github.com/TNG/ArchUnitNET/pull/170, but I don't think it is complete for your use-case. I'd be happy, if you could have a look into that.

[simonthum]

@grzegorzorwat This looks exactly like what I had in mind :+1: