BenMorris / NetArchTest

A fluent API for .Net that can enforce architectural rules in unit tests.
MIT License
1.36k stars 81 forks source link

IsImmutable has a surprising definition #100

Open simonthum opened 2 years ago

simonthum commented 2 years ago

Hi,

I just tested NetArchTest, mainly to document existing shortcomings in a codebase. So in one instance, I was suprised that no violation was found and tracked it down to these lines:

    /// <summary>Tests whether a field is readonly</summary>
    /// <param name="fieldDefinition">The field to test.</param>
    /// <returns>An indication of whether the field is readonly.</returns>
    public static bool IsReadonly(this FieldDefinition fieldDefinition) => !fieldDefinition.IsPublic || fieldDefinition.IsInitOnly || fieldDefinition.HasConstant || fieldDefinition.IsCompilerControlled;

The IsImmutable test check if all fields and all properties are read-only. That's fine, but a field or property which is non-public may well be mutated inside that class. The object may not be mutable from the outside, but I was interested in the inner structure.

That can be fixed or an "IsStrictlyImmutable" may be added. What do you think? Bug or feature? ;)

(I know I can probably whip up custom rules for that. Along those lines, I would be interested IsStateless.)

BenMorris commented 1 year ago

There's an argument for a stricter IsStateless function here to identify those classes that don't maintain any internal state at all.

We should keep IsImmutable to refer to the external mutability