Roslyn: IDE0044 triggers on private arrays and classes marked [Serializable]

[vsfeedback]

VS2017 15.7.2
IDE0044 triggers on private arrays, and Microsoft explicitly states in its coding best practice that readonly on internal arrays should NOT be used (even private ones). https://msdn.microsoft.com/en-us/library/ms182299.aspx ​CA2105: Array fields should not be read only.

Also, I happily went and added all my readonly tags to a [Serializable] class only to discover (of course) that a binary deserialize will fail in this instance due to the readonly property. But NO warning of this until the runtime error.

So: Should readonly be advocated for internal arrays or not, and should it be advocated for [Serializable] classes (I get that you could deserialize to a non readonly class version but that would be a bit unusual)

_This issue has been moved from https://developercommunity.visualstudio.com/content/problem/258822/ide0044-triggers-on-private-arrays-and-classes-mar.html
VSTS ticketId: 620873_
_These are the original issue comments:_
(no comments)
_These are the original issue solutions:_
(no solutions)

[sharwell]

There are two separate issues here.

1. A request to not report IDE0044 on array fields due to a conflict with CA2105: Array fields should not be read only
2. A request to not report IDE0044 on serializable types

I'm going to mark this for a design review before making it up-for-grabs.

[Neme12]

With respect to "CA2105: Array fields should not be read only", I'm actually really surprised by this guidance. The argument essentially seems to be that "readonly" is confusing and could make someone think that it actually means "(deeply) immutable", which really troubles me because this is a very basic fact about "readonly". The same argument could be made for not making fields of any mutable object (which is likely the majority of objects) readonly because it could be mistaken for an immutable object.

In fact, I want most of my array fields to be readonly even if they happen to be mutable. It still prevents a few things such as assigning null or being able to change the length of the array.

[sharwell]

@Neme12 I too disagree with that rule. It looks like this rule is getting eliminated per dotnet/roslyn-analyzers#484.

[CyrusNajmabadi]

I agree. '1' is not a sensible rule. '2' is definitely interesting and something that could potentially be looked for by the feature.