CLSCompliant(true) check for arrays as attribute arguments triggers on internal types

[jkoritzinsky]

Version Used: .NET 8.0.100 P4 SDK

Steps to Reproduce:

using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

[assembly:CLSCompliant(true)]

file class Foo
{
    [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvMemberFunction) })] // CS3016
    internal static void Bar(nint x){}
}

[CLSCompliant(false)] // CS3019
internal class Baz
{
    [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvMemberFunction) })] // CS3016 if CLSCompliant(false) is removed
    internal static void Bar(nint x){}
}

Warnings:

warning CS3019: CLS compliance checking will not be performed on 'Baz' because it is not visible from outside this assembly
warning CS3016: Arrays as attribute arguments is not CLS-compliant

If the CLSCompliant(false) attribute is removed, then a second CS3016 diagnostic is reported instead of CS3019.

SharpLab link

This is particularly rough for file-scoped types that can't be made partial.

Diagnostic Id: CS3016

Expected Behavior:

No CLS compliance warnings on array attribute parameters on attributes on non-exposed types.

Actual Behavior:

CLS compliance warnings on array attribute parameters on attributes on non-exposed types, and CS3019 if I put CLSCompliant(false) on the type with the attribute.

[RikkiGibson]

I don't see any problem with making the change, especially for the file type scenario, but I wondered if IVT needs to be accounted for in the normal internal type scenario? Maybe it's already accounted for, or it's such an unusual case that it doesn't matter?

[jkoritzinsky]

My PR that fixes this uses the same accessibilty check as the rest of the CLS Compliance checking (such as for method signatures), so it should behave identically with respect to IVT.

[AlekseyTs]

@jkoritzinsky Please include complete repro in the description. Links to external resources are good, but they are not a replacement. What would we do if that external resource disappears? Also, please include complete wording of the problematic diagnostic, I am pretty sure very small number of people remember what CS3016 is about.

[jkoritzinsky]

Updated the issue with the source and diagnostics inline in addition to the SharpLab link.

[AlekseyTs]

@jkoritzinsky

Updated the issue with the source and diagnostics inline

Could you please clarify location for each of the warnings?

[jkoritzinsky]

Updated

[AlekseyTs]

I think triage should decide what path we should take here. It appears, that originally Roslyn had the attribute check suppressed based on accessibility of the target. Then, we explicitly changed that behavior back to accessibility independent behavior. Do we want to flip back again after that many years? Apparently we had a good enough reason to end up where we are today with respect to the check.

[elachlan]

@AlekseyTs When did the behavior change? If it indeed was changed to the current behavior, it should probably be documented as by design and the suppression via the CLSCompliant(false) should then work (#68560)

[elachlan]

Related: #4293