Do not declare visible instance fields warning in sequential struct

Question

I'm using some DllImports in a wpf application to capture the screen. I'm calling GetWindowRect in user32.dll. It requires a rect struct passed to it. The layout of the struct matters, since it's a native call.

I'm trying out VS 2019 preview 2 which gives me warnings I hadn't seen before. All the fields in rect generate the same warning:

CA1051 Do not declare visible instance fields

In the rest of the code, I fixed this by turning the field into a property by appending {get;set;} to it. I don't know if I can safely do this in a struct where layout matters.

Rect is also giving me a warning that I should override Equals.

CA1815 Rect should override Equals.

CA1815 Rect should override the equality (==) and inequality (!=) operators.

I never compare it though and definitely don't need to, I just want to fix the warning.

public static class NativeMethods
{
    [DllImport("user32.dll")]
    private static extern IntPtr GetForegroundWindow();

    public static IntPtr _GetForegroundWindow()
    {
        return GetForegroundWindow();
    }

    [DllImport("user32.dll", CharSet = CharSet.Auto, ExactSpelling = true)]
    private static extern IntPtr GetDesktopWindow();

    public static IntPtr _GetDesktopWindow()
    {
        return GetDesktopWindow();
    }

    //Am unable to get code analysis to shut up about this.
    [DllImport("user32.dll")]
    private static extern int GetWindowRect(IntPtr hWnd, ref Rect rect);

    public static IntPtr _GetWindowRect(IntPtr hWnd, ref Rect rect)
    {
        return (IntPtr)GetWindowRect(hWnd, ref rect);
    }        
}

[StructLayout(LayoutKind.Sequential)]
public struct Rect
{
    public int Left;
    public int Top;
    public int Right;
    public int Bottom;
}    

How can I fix these warnings?

Tilly

As I stated in the question (In the rest of the code, I fixed this by turning the field into a property by appending {get;set;} to it. I don't know if I can safely do this in a struct where layout matters. ) that's how I usually fix it. This is a special case since it's a struct used in a native call. {get;set;} will generate a getter and setter, potentially changing the struct's layout. Are you sure that {get;set;} is safe? – Tilly

Just ignore the warnings in this particular case. – Sheedy

@Sheedy I know that you could do that and that it's probably fine but I'm hoping that there's a better solution. – Tilly

These are interop types with shapes dictated by external (non-C#) forces. You cannot guarantee that any interop types will conform to good C# practices and so I'd definitely be looking at suppression here. – Piccaninny

Don't turn fields to properties. Ignore the warning (first, it's a warning, not error, and second, it's from code analysis, not C# complier). Or make the NativeMethods class and Rect struct (and similar) internal. – Soble

@IvanStoev marking it internal fixes it! Please add your answer so that I can mark it. – Tilly

Answer 1

The documentation of CA1051: Do not declare visible instance fields says:

Cause

An externally visible type has an externally visible instance field.

The key point for both type and field is external. Hence the fix (since this is supposed to be used only inside your application) is to make the struct (and the class that exposes it) internal:

[StructLayout(LayoutKind.Sequential)]
internal struct Rect
{
    public int Left;
    public int Top;
    public int Right;
    public int Bottom;
}    

internal static class NativeMethods
{
    // ...
}

Please note that CA1051 warning is not generated by the C# compiler, but Code Analysis, hence can be excluded or ignored from the CA rule set (although the documentation suggests to not suppress it).

Soble

Answer 2

You can suppress warnings in a file like this:

#pragma warning disable CA1051, CA1815

or disable it in csproj file for the whole project

<NoWarn>CA1051, CA1815</NoWarn>

EDIT If you want to fix the warning instead of suppress it, you should follow the warning message.

I never compare it though and definitely don't need to, I just want to fix the warning.

The warning will appear unless you add operators like that suggested by the message. The warning means that "it probably works for you now, but not the best practice". Overriding equal operators for structs improves the readability and performance, also structs are supposed to be immutable, public fields break the immutability and hide potential bugs.

Benzoin

I know how to suppress warnings, I'm looking to fix the warning, not suppress it. – Tilly