Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IsImmutable has a surprising definition #100

Open
simonthum opened this issue Feb 13, 2022 · 1 comment
Open

IsImmutable has a surprising definition #100

simonthum opened this issue Feb 13, 2022 · 1 comment

Comments

@simonthum
Copy link

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
Copy link
Owner

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

NeVeSpl added a commit to NeVeSpl/NetArchTest.eNhancedEdition that referenced this issue Nov 22, 2023
NeVeSpl added a commit to NeVeSpl/NetArchTest.eNhancedEdition that referenced this issue Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants