Roslyn-analyzers: CA1051 should not be applied to structs

[NN---]

Analyzer

Diagnostic ID: CA1051: `Do not declare visible instance fields

Describe the improvement

Declaring properties in struct is not good due to defensive copy problem.
Therefore it is common to declare public fields.

[StructLayout(LayoutKind.Sequential)]
struct SomeStruct
{
  public int a;
  public int b;
}

Describe suggestions on how to achieve the rule

There should be separate rule for class and for struct.
There are also several kind of structs:

• Optimization for managed code
• Usage for for P/Invoke or memory manipulation
  -- Managed struct
  -- Unmanaged struct

Additional context

[Evangelink]

I don't know if it makes sense to _ALWAYS_ suppress this rule for structs. IMO we should not report for class and struct if they are marked with StructLayoutAttribute as the description is pretty clear that you _HAVE TO_ use fields (not possible to use properties even the auto ones).

[NN---]

Good idea.

[mavasani]

@Evangelink I think we can do couple of things:

1. Change default behavior as per your suggestion https://github.com/dotnet/roslyn-analyzers/issues/4149#issuecomment-692716461
2. Add a new analyzer option to allow turning off this rule for all structs. I have seen large number of code bases that allow or prefer visible instance fields on structs, but not other named types. Roslyn repo is a good example where CA051 is hard to enforce due to lack of such an option.

[Joe4evr]

(not possible to use properties even the auto ones).

Untrue. You can use the field attribute specifier to set field offsets on auto-properties.

[Evangelink]

@Joe4evr You are right for the Explicit case but as far as I have seen that's not right for Sequential where there is no guarantee for the order of the auto-generated fields.