Better Obsoletion

[terrajobst]

No description provided.

[terrajobst]

@ericstj

You should probably address browser visiblity for hiding the api from intellisense.

What do you mean? Presumably, you believe the existing mechanism isn't good enough?

[terrajobst]

would it be a goal or non-goal to mark APIs which were never shipped in stable packages as experimental? Or even wider - have an Experimental attribute which is similar to Obsolete but works in opposite way. You are marking something as non-committal. i.e. library haven't committed on yet and it may be gone when library will move from -beta to -stable.

We have only briefly talked about that. It's an interesting idea that might make it easier for us to ship previews and inform customers what's not stable yet. However, this warrants more thought and I'll consider this out-of-scope for the discussion of obsoletion.

[MeikTranel]

I agree with almost everything said. That being said.. while agreeing with the idea that breaking the obsolete attribute would be madness, i sure hope you are really sure about the naming of the Deprecated attribute.

The word obsolete and deprecated may be equivalently used in the english language... but to the best of my knowledge in other languages such as the german language "obsolete" implies that something becomes no longer required, while deprecation implies a plea not to use.

I am by no means an expert on languages so please feel free to chime in, non-english speakers. I am also by no means asking you not to design english-first. It's just a consideration worth taking in imho

[terrajobst]

I agree to almost anything said. That said.. while agreeing with the idea that breaking the obsolete attribute is madness, i sure hope you are really sure about the naming of the DeprecatedAttribute.

If we would be starting from scratch, I'd push for the name of DeprecatedAttribute because it does indeed sound stronger than obsolete. At the same time, obsolete is a well understood concept in .NET and is already recognized by the compilers. I don't think inventing a new concept here is worth the cost.

[PathogenDavid]

"obsolete" implies that something becomes no longer required, while deprecation implies a plea not to use.

FWIW, this is true in English as well. It's software developers that tend to use them interchangeably.

[MeikTranel]

Maybe we could still have the best of both worlds by allowing both URL or Message in the constructor. We could tag the message attribute as obsolete or leave it (not every library author has the time or resources to host a URL for error codes) and defaulting the root URL to msdocs and the error code to the generic obsolete diagnosis. That way we don't have any massive breaking changes but still allow for a better system to take its place.

[KathleenDollard]

Could you still suppress all Obsolete attributed things - new style and old style?

If we are touching this attribute, should we consider anything else - like a level or reason. Or is everything the same level of STOP, FIX THIS RIGHT NOW

[terrajobst]

@KathleenDollard

Could you still suppress all Obsolete attributed things - new style and old style?

Good point. I've added an open issue with 6b146af.

If we are touching this attribute, should we consider anything else - like a level or reason. Or is everything the same level of STOP, FIX THIS RIGHT NOW

Hmm, I don't think that's super useful at the level of the diagnostic. This should more more in the linked documentation page IMHO.

[ericstj]

@ericstj

You should probably address browser visiblity for hiding the api from intellisense.

What do you mean? Presumably, you believe the existing mechanism isn't good enough?

Not necessarily, but I didn't notice any mention of it in the doc and it seemed relevant to what you were discussing. EG: explain why just hiding the API from intellisense isn't good enough to obsolete an API.

[pascalberger]

Why is providing coder fixers not a goal?

While I agree with the reasons listed to not provide code fixes, I still think it would be a nice opt-in feature for cases where there is a 1:1 replacement. Instead of not providing code fixes I would prefer to have the possibility to define a replacement member and let the author of the obsolete attribute decide if a specific case is an appropriate use case for a code fix or not.

[poke]

@pascalberger

I would prefer to have the possibility to define a replacement member

As mentioned in the document, for most obsoletions, just finding a replacement that is mostly compatible will not work. It is a lot more likely that migrating to a newer functionality requires non-trivial changes throughout the code base. A fixer that only operates locally wouldn't really be helpful there.

@ericstj

explain why just hiding the API from intellisense isn't good enough to obsolete an API.

Hiding things from IntelliSense only works for new code that is written, and also only when that isn't e.g. copy pasted from some other place (the described example use case covers this exaxtly).

One point of obsoletion warnings is to tell authors of an exisiting code bases that APIs they are already using are obsolete.

[terrajobst]

@ericstj

Not necessarily, but I didn't notice any mention of it in the doc and it seemed relevant to what you were discussing. EG: explain why just hiding the API from intellisense isn't good enough to obsolete an API.

Got it. I've clarified this in 74707d7.

@pascalberger

While I agree with the reasons listed to not provide code fixes, I still think it would be a nice opt-in feature for cases where there is a 1:1 replacement. Instead of not providing code fixes I would prefer to have the possibility to define a replacement member and let the author of the obsolete attribute decide if a specific case is an appropriate use case for a code fix or not.

Code fixers are an independent feature. On case by case basis, we can decide to add one if we want. It's just not in scope of this feature to enable that, i.e. I don't see a point in recording the alternative member as part of the obsolete API. The cases where a single one exists are rare and docs are better at explaining this. That being said, the fact that we're adding specific diagnostic IDs also make it easier for code fix providers to scan for violations without having to re-implement the analysis.

[rainersigwald]

One of the major concerns with the existing attribute (and the reason we haven't applied it in MSBuild) is that adding any warning is a build-breaking change for users who have warnings-as-errors. What's the plan to resolve that problem in the core libraries?

[terrajobst]

One of the major concerns with the existing attribute (and the reason we haven't applied it in MSBuild) is that adding any warning is a build-breaking change for users who have warnings-as-errors. What's the plan to resolve that problem in the core libraries?

.NET Core has a lower bar and we'd accept this. The entire proposal is to alow customers to apply supressions in a more targeted fashion. By definition, obsoletion is about introducing new diagnostics. However, they only kick-in when people target a later version of the platform. My position is that this is an acceptable breaking change, especially for customers that turn on "warnings as errors" because they asked to be broken. The work around is: disable "warn as error" until you stabilized your build.

[svick]

This might be only tangentially related, but it seems it's important that people will be able to reach the relevant documentation page. Will they?

First of all, if they use dotnet build, I don't think there is anything in the error message to indicate that such a page exists.

And even if they use VS, I think the UI is quite subtle ("light blue color in the Code column means I can click it?") and when they do figure that out, they might be discouraged from clicking by previous bad experiences (e.g. MicrosoftDocs/feedback#1303 or when the link just sends you to Bing).

I'm not sure if there's anything that can be done here, but I think it's worth thinking about.

[terrajobst]

@svick

This might be only tangentially related, but it seems it's important that people will be able to reach the relevant documentation page. Will they?

First of all, if they use dotnet build, I don't think there is anything in the error message to indicate that such a page exists.

And even if they use VS, I think the UI is quite subtle ("light blue color in the Code column means I can click it?") and when they do figure that out, they might be discouraged from clicking by previous bad experiences (e.g. MicrosoftDocs/feedback#1303 or when the link just sends you to Bing).

I'm not sure if there's anything that can be done here, but I think it's worth thinking about.

That's a fair point. I don't think including the URL when printing diagnostics on the command line would be bad, but I also don't think it will be super useful either. The CI build isn't the right place to find out that you're using something obsoleted, that should be when you're doing development in the inner loop. And while there are some people who do that entirely using a text editor and dotnet build, the 87.3412% of developers do that from inside an IDE (yes, I made that number up).

I agree with you that the IDE affordances for navigating to the provided URL seem too subtle. One thing I've been doing in my own analyzer is adding a code fixer like About DE002... which opens the browser. Having a generalized actions to promote this inside the the IDE would be useful. I've filed dotnet/roslyn#42103 for this.

[terrajobst]

While there are still some open design issues, I'll merge this because we agreed that we'll tackle this for .NET 5. Thanks everyone for the feedback:

Remaining issues (listed in the doc too):

• Should the compiler suppress all diagnostics from ObsoleteAttribute if the existing generic diagnostic ID is suppressed (e.g. CS0618)? If not, the developer has no way to turn this feature off entirely.
• We need to decide on guidance to avoid ID conflicts with the community. We should talk to the Roslyn analyzer team for best practices. Stake in the ground could be that we should use a prefix like BCL that is owned by a single party and responsible to avoid duplicated numbers. Other parties would need to choose a different prefix.
• Close the loop with XML Serializer team and lock down what we should do.