Mark Microsoft.AspNetCore.Http.Abstractions CLS Compliant

[aspnet-hello]

From @NightOwl888 on Thursday, September 14, 2017 10:13:09 AM

Not sure how this was missed, but one of our goals is to make our library CLS compliant, but it is not possible when dependencies we expose on our public API (namely HttpRequest and HttpResponse) are not marked CLS compliant.

I have tried all versions of Microsoft.AspNetCore.Http.Abstractions (including 2.0.0, although I am not sure I can use it when targeting .NET Standard 1.5), but none of them seems to be marked with the CLSCompliantAttribute. Is there some reason for this, or was it just overlooked?

Copied from original issue: aspnet/HttpAbstractions#938

[aspnet-hello]

From @Tratcher on Thursday, September 14, 2017 10:57:46 AM

@Eilon we don't do this for any ASP.NET Core assemblies, correct?

[aspnet-hello]

From @Eilon on Sunday, September 17, 2017 9:30:03 PM

It's come up a small handful of times, but the priority was never high enough to go do it because the value wasn't clear. I'm sure most of the assemblies in ASP.NET/EF Core could be marked as such without any problem. However, once we mark it as such, it's hard to go back - it would be a breaking change.

So, the ultimate question is, where is CLS compliance actually required? So far we haven't seen the concrete need from it, so we end up not doing it.

[aspnet-hello]

From @NightOwl888 on Sunday, September 17, 2017 9:57:43 PM

So, the ultimate question is, where is CLS compliance actually required?

Basically, if you don't mark a library CLS compliant, types from that library (whether they are actually considered CLS compliant or not by the compiler) cannot be used in public interfaces of other CLS compliant assemblies without generating a "type is not CLS compliant" warning.

It is unclear to me why you would not want to mark these assemblies CLS compliant, since they will be used by Visual Basic and perhaps other languages. Visual Basic does not have case sensitivity for public member names, so it seems like a good idea to use CLS compliance as insurance you don't violate rules such as these. Not that you are violating any of the rules in your assembly, but not declaring it CLS compliant means that consumers will have to deal with these warnings.

That said, removing the attribute from the one assembly we have that requires it or suppressing the warning explicitly are also options to remove this warning.

What it boils down to I guess is why was this decision made? It seems like marking CLS compliant should be the standard across all Microsoft libraries, since when you do they can be used in public interfaces of both CLS compliant and non-CLS compliant assemblies without generating warnings, where not using this attribute will cause warnings for CLS compliant consumers.

The result is that marking consuming classes with this attribute generates warnings that have no value to the consumer. Since the purpose of the attribute is to ensure you get warnings when you violate the CLS rules, these warnings are unwelcome from an assembly that we don't own. Again, we can suppress them to make them go away, but why should this be necessary?

[aspnet-hello]

From @Eilon on Tuesday, September 19, 2017 7:48:11 PM

Features don't show up by default - they have to be built, reviewed, and tested 😄

But still to be honest it isn't clear what value there is to mark something as CLSCompliant - aside from enabling other assemblies to also be marked as CLSCompliant (which is merely circular logic).

Here are all the past discussion I found regarding this:

1. Make Microsoft.Extensions.* CLS compliant #1716
2. https://github.com/aspnet/Coherence-Signed/issues/336
3. CLS compliancy aspnet/Configuration#308
4. Microsoft.Extensions.Logging.Abstractions not marked CLSCompliant aspnet/Logging#500
5. CLS compliance aspnet/Options#92
6. Make assemblies CSLCompliant  efcore#954

And in none of them was there any reason aside from "to avoid the warning that would enable other assemblies to be CLS-compliant", which is the circular logic argument.

As far as reasons not to:

1. It's work. Work takes time.
2. It'd be a breaking change to remove it (thus it's potentially more work to maintain our existing codebase because more developers would have to be aware of marking new members as CLSCompliant(false) when new types or members are added that are not compliant.

So we're really looking for concrete data on the value that this would add because we have to weigh that against other features that are planned.

[aspnet-hello]

From @j2jensen on Friday, October 6, 2017 3:02:48 PM

Based on https://stackoverflow.com/questions/570452/what-is-the-clscompliant-attribute-in-net it seems the point is that people can leverage your library from other languages that target the .NET CLR.

such as C#, C++/CLI, Eiffel, F#, IronPython, IronRuby, PowerBuilder, Visual Basic, Visual COBOL, and Windows PowerShell.

-- https://docs.microsoft.com/en-us/dotnet/standard/language-independence-and-language-independent-components

The reason people are using a circular-sounding argument is that in order for one library to be CLS Compliant, it needs to avoid using other libraries that aren't. So if your assembly isn't CLS Compliant then you're preventing a IronPython developer from using any library that even indirectly leverages HttpAbstractions.

The requirements for the assembly to be CLS Compliant seem pretty benign to me, but you do have a point that if you ended up determining that you really needed a public-facing ulong then you'd be introducing a breaking change to anyone who depended on your CLS Compliance. On the other hand, if you don't make yourself CLS Compliant in the first place, then you're preventing those same people from using your library in the first place; I'm not sure which is worse.

[aspnet-hello]

From @j2jensen on Friday, October 6, 2017 3:09:42 PM

Reading further, it appears that not marking your library CLS Compliant doesn't necessarily prevent these other languages from being able to use it (supposing your library actually follows the rules). But by marking your library CLS Compliant, you'll help ensure that you get warnings if you ever accidentally add code that is not compliant (like having two public members whose names only differ by capitalization).

In this sense, if you ever removed the CLS Compliant attribute from your library, you wouldn't truly be introducing a breaking change--people could still continue using the library in their IronPython project, e.g.. But by not having the attribute there you're more likely to introduce an actual breaking change without realizing it. For example, someone's Visual Basic code might break because you add a property that they can't disambiguate.

[aspnet-hello]

From @NightOwl888 on Friday, October 6, 2017 5:32:15 PM

Thanks @j2jensen

I ran an experiment, since the MSDN docs are not clear what happens when a language that requires full CLS compliance consumes a library that is marked CLS compliant that exposes types from one that is not marked CLS compliant.

Basically, if the non-CLS compliant library doesn't actually have any CLS violations, it works fine.

However, if a member of the non-CLS compliant library exposes both a protected field named foo and a public property named Foo, when subclassing the class in VB, the protected field disappears from the API.

So, as a consumer of the library who is intending to be CLS compliant exposing a non-CLS compliant type, there is effectively no way to guarantee that someday the non-CLS compliant library won't change in a way that causes APIs to disappear (a breaking change) for certain languages. While today it may be CLS compliant, it might not be tomorrow. The only way we can guarantee that our library won't ever break for certain .NET languages is if all types that we expose (including those we don't own) are CLS compliant.

If Microsoft.AspNetCore.Http.Abstractions is not marked CLSCompliant(true), there are really no good alternatives for us:

1. Not marking our library CLSCompliant(true) means that we won't get any warnings if we break a CLS compliance rule. Since we are porting from Java and many of the design decisions that were made often break the CLS rules, this is a non-starter.
2. Suppressing the warning we get because Microsoft.AspNetCore.Http.Abstractions isn't marked CLS compliant is a dangerous proposition because at any point in time a change could be made to the types we expose from that library that breaks CLS compliance and causes consumers of certain languages to lose APIs.
3. Marking the members that publicly expose Microsoft.AspNetCore.Http.Abstractions types CLSCompliant(false) effectively is the same thing as ignoring it - it means those members will disappear from our public API for some languages if the library we don't control changes in a way that breaks CLS compliance. In our case, this is not an acceptable solution because we have no way to provide a CLS compliant alternative HttpRequest or HttpResponse type.

I am not even sure if option 2 is a realistic option because MSDN specifically states:

If an assembly is marked as CLS-compliant, any publicly exposed type in the assembly that is not CLS-compliant must be marked with CLSCompliantAttribute using a false argument.

And as I already stated, that option is not viable because it is for one of our main APIs and we need to ensure it fully works for all consumers.

[aspnet-hello]

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.