dotnet / roslyn-analyzers

MIT License
1.59k stars 466 forks source link

CA1815, CA1066, and CA2231 on public ref struct #2324

Open Zenexer opened 5 years ago

Zenexer commented 5 years ago

Analyzer package

Microsoft.CodeQuality.Analyzers from Microsoft.CodeAnalysis.FxCopAnalyzers

Package Version

2.9.1

Diagnostic ID

Repro steps

CA1815 and CA2231:

namespace Example
{
    public ref struct Test
    {
        byte _data;
    }
}

CA1066:

namespace Example
{
    public ref struct Test
    {
        byte _data;

        public override bool Equals(object obj) => false;
    }
}

The problems

  1. CA1815 doesn't make sense because Equals must always return false, since ref structs can't be cast to object.
  2. CA1066 doesn't make sense because ref structs can't implement interfaces.
  3. CA2231 doesn't make sense without also exposing an Equals method. It's possible to implement operator == and operator !=, but it's probably not a great idea to promote that as the only equality checking method (which is what would happen if CA1815 and CA1066 were removed).

Proposed solution

  1. Exclude ref structs from CA1815.
  2. Exclude ref structs from CA1066 and anything else that recommends implementing an interface.
  3. Exclude ref structs from CA2231, as the wording and proposed fix don't make sense for ref structs.
  4. Add an analyzer that recommends adding either public bool Equals(RefType other) or public static bool Equals(RefType left, RefType right)
  5. Add an analyzer that recommends overriding == and != if either public bool Equals(RefType other) or public static bool Equals(RefType left, RefType right) are present.
Evangelink commented 4 years ago

@mavasani For points 4 and 5, shall we create 2 separate rules or add those checks to some existing rules?