[Experimental] attribute should yield warnings, not errors

[jcouv]

Relates to C#/roslyn work at dotnet/roslyn#68702

[jcouv]

@terrajobst for review

[jcouv]

@terrajobst for review. Thanks

[jcouv]

@terrajobst for review. Thanks

[terrajobst]

No, we want this to be errors, same reason we wanted this for platform level preview features.

[RussKie]

No, we want this to be errors, same reason we wanted this for platform level preview features.

But if it's an error it won't be suppressible, will it?

[terrajobst]

[Obsolete(IsError = true)] is still suppressible too

[jcouv]

[Obsolete(IsError = true)] is still suppressible too

I don't think so. Only warnings are suppressible.

#pragma warning disable CS0619
C.M(); // Error CS0619	'C.M()' is obsolete: 'message'

public class C
{
    [System.Obsolete("message", error: true)]
    public static void M()
    {
    }
}

[terrajobst]

Ah, I forgot how that works. Turns out, you can work it around :-)

using System;

public class C
{
    [Obsolete("message", error: true)]
    public static void M1()
    {
    }
    
    [Obsolete]
    public static void M2()
    {
        M1(); // Error CS0619 'C.M()' is obsolete: 'message'
    }
}

Also, our platform preview feature is an error that is meant to be suppressed, so it has prior art.

[jcouv]

FWIW, the warning from [Experimental] is not suppressed in obsolete members. This follows the behavior of the Windows Experimental attribute

Regardless, suppression pragmas will not suppress errors, so [Experimental] is implemented as a warning. Unless I misunderstood how folks should use [Experimental] (ie. opt-in by suppressing it) this document should be updated to reflect that.

[terrajobst]

FWIW, the warning from [Experimental] is not suppressed in obsolete members. This follows the behavior of the Windows Experimental attribute

Right, but it's suppressed in members marked as Experimental themselves, right?

Regardless, suppression pragmas will not suppress errors, so [Experimental] is implemented as a warning.

Is this a difference between compiler diagnostics and analyzers then? Because this will produce errors that can be suppressed using #pragma.

#pragma warning disable CA2252 // This API requires opting into preview features
    private static INumber<T> Add<T>(INumber<T> x, INumber<T> y)
        where T: INumber<T>
    {
        // ...
    }
#pragma warning restore CA2252 // This API requires opting into preview features

[jcouv]

Right, but it's suppressed in members marked as Experimental themselves, right?

It's not. The attribute follows the Windows Experimental attribute, which doesn't do that. You need to use an actual suppression.

Is this a difference between compiler diagnostics and analyzers then?

Errors prevent the compilation from succeeding (we will not emit bits) and cannot be suppressed. Warnings don't prevent emitting.

[stephentoub]

Is this a difference between compiler diagnostics and analyzers then?

Errors prevent the compilation from succeeding (we will not emit bits) and cannot be suppressed. Warnings don't prevent emitting.

But an analyzer configured at error severity can still be suppressed:

[jcouv]

But an analyzer configured at error severity can still be suppressed

Good point. The suppression comes before the promotion to error.

[jcouv]

Is this doc change ready to merge? It's been open for 3 weeks

[jcouv]

@terrajobst Is this one-line doc change ready to merge? It's been open for 4 weeks

[terrajobst]

My preference is that using experimental APIs is an error that can be suppressed rather than a warning because we know that many of our customers have a sea of warnings that they don't pay attention to.

The goal of this feature is to allow us (and the library ecosystem at large) to ship stable software with unstable components in them, which is similar to the platform-preview feature. My concern is that if only raise a warning, many people inadvertently take a dependency on experimental features which puts pressure on the producers of these APIs to do this less to avoid mistakes.

An error is disruptive and forces the person to take action, a warning isn't.

I understand the concerns that this isn't how compiler errors work today but as we said, this is how analyzers work today and there are many more analyzer diagnostics than compiler diagnostics. From a user's standpoint, they don't necessarily know which diagnostic comes from the compiler and which one comes from an analyzer. Since analyzer errors can be suppressed today (and user experiences have been built that depend on this) it seems strange to me to say that we believe doing that for diagnostics raised by the compiler would be wrong.

Or is the concern implementation complexity/cost?

[jcouv]

In the language and compiler, errors cannot be suppressed. We're extremely unlikely to change that. Please bring to LDM if you want to discuss that.
You/Stephen correctly pointed out that warnings can be reported as errors. Those can only be suppressed because they are actually warnings (ie. not blocking).

[jaredpar]

I understand the concerns that this isn't how compiler errors work today but as we said, this is how analyzers work today and there are many more analyzer diagnostics than compiler diagnostics.

Errors cannot be suppressed no matter what their origin: compiler, analyzer or generator. If there is a diagnostic that can be suppressed it's because that diagnostic is a warning. If it appears as an error it's due it being elevated as such.

[terrajobst]

I understand the concerns that this isn't how compiler errors work today but as we said, this is how analyzers work today and there are many more analyzer diagnostics than compiler diagnostics.

Errors cannot be suppressed no matter what their origin: compiler, analyzer or generator. If there is a diagnostic that can be suppressed it's because that diagnostic is a warning. If it appears as an error it's due it being elevated as such.

Let me try to understand what you're saying. I think what you're saying is that if an analyzer declares a diagnostic to be of type error in its definition, then it can't be suppressed. However, if the diagnostic is defined to be a warning and then modified in, say, an editor config file, to be an error, then it can be suppressed just fine.

Is this something we can do for compiler diagnostics as well? Like have the diagnostic be a warning but then have the default editor config upgrade it to an error?

[jcouv]

Is this something we can do for compiler diagnostics as well? Like have the diagnostic be a warning but then have the default editor config upgrade it to an error?

Yes, it is possible to report the warning as error. The difficulty with how to control that.
There are many ways of affecting the severity of diagnostics (editorconfig, command-line, rules, etc). Whatever mechanism we use to promote this warning to error should allow for all those mechanisms to work (ie. allow suppression).
This will need some investigation/discussion. We'd rather not hardcode such a default rule in the compiler but it isn't out of the question

Also, there is a problem with diagnostic IDs, since they are variable (user-provided) in this scenario

[jcouv]

Closing as subsumed by https://github.com/dotnet/designs/pull/300/files