Consider CLS-compliancy

[Abrynos]

Overview

Currently we have (among others) following properties set in our Directory.Build.props:

    <AnalysisMode>AllEnabledByDefault</AnalysisMode>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <WarningsAsErrors/>

Due to CA1014, we've marked our assemblies with

[assembly: CLSCompliant(true)]

This we do not want to change.

Previously we've made use of code generation and marked our viewmodels with the ObservableObjectAttribute. However, with the newest version, this triggers the analyzer MVVMTK0033 ("The type XXX is using the [ObservableObject] attribute while having no base type, and it should instead inherit from ObservableObject").

If we follow this advice and inherit from ObservableObject, the analyzer CS3009 triggers because even tho the class is technically compliant with the common language specification, it is not marked as such.

API breakdown

I would like to propose marking either the whole assembly as CLSCompliant or (at least) marking ObservableObject as such.

Usage example

public sealed partial class MyViewModel : ObservableObject {
    ...
}

Breaking change?

No

Alternatives

[ObservableObject]
#pragma warning disable MVVMTK0033
public sealed partial class MyViewModel {
#pragma warning restore MVVMTK0033
    ...
}

Additional context

Thank you in advance for considering the inclusion of this.

Help us help you

Yes, but only if others can assist

[Sergio0694]

As a workaround in the meantime, instead of annotating every single case with #pragma, you can also add this to your .csproj:

<NoWarn>$(NoWarn);MVVMTK0033</NoWarn>

And that'll just disable the warnings from that analyzer. I would recommend using the base type if possible though to get the binary size improvements, instead of generating the code every single time in each different viewmodel type 🙂

[Abrynos]

Disabling it in the Directory.Build.props is how we actually disabled it 😜

I've just had a quick poke at modifying the source code (just CommunityToolkit.Mvvm and not the other projects) and it seems like there is not that much work involved:

• Add [assembly: CLSCompliant(true)] (e.g. in AssemblyInfo.cs).
• Split up [MemberNotNull(nameof(entries), nameof(buckets))] into two separate attributes because array attribute parameters are not compliant.
• Do something about CommunityToolkit.Mvvm.ComponentModel.__Internals namespace and the two contained classes; It's possible to add [CLSCompliant(false)] directly to the two classes, but that doesn't solve the issue of the namespace containing underscores as well.

[Sergio0694]

Yeah I had a look and that namespace is indeed the one issue that's not really clear to me how to solve 🤔

[Abrynos]

The easiest solution would probably be to rename it and adapt the code-generation accordingly :( No idea how much work that would be tho.

[Sergio0694]

The problem is that you might have an assembly that was compiled using an older version of the generator being referenced by one using the new version. The new version would be loaded, which doesn't have the type in the old namespace, and code from the first assembly would crash at runtime (with a MissingMemberException or similar) when trying to run 🥲

As in, it's a binary breaking change.

EDIT: we could consider it, but it'd have to be in a major release.

[Abrynos]

I didn't think that. But I thought of yet another possible solution: Add the [CLSCompliant(true)] attribute just to the ObservableObject class.

[teo-tsirpanis]

@Abrynos do you care about CLS compliance? If not you can just add [assembly:CLSCompliant(false)].

[Abrynos]

do you care about CLS compliance?

As per my original issue:

This we do not want to change.

[Sergio0694]

Ok but why? You cited the analyzer as the reason for enabling that. Other than making that (optional) analyzer happy, is there any specific reason why you want to keep this assembly marked as CLS compliant?

[Abrynos]

Yes. For simplicity I'll just say "my employer" 😅

[Youssef1313]

FYI https://github.com/dotnet/roslyn-analyzers/pull/6008