RFC: #[deprecated] for Everyone

[llogiq]

Library authors need a way of evolving their API painlessly. This is it.

Rendered

[seanmonstar]

I have never found value in a 'since' label for deprecations. How long an
item has been deprecated isn't too useful to know. It would be useful to
know when a deprecated item will be completely removed. Though, I wouldn't
want that to be a requirement, either.

Also, I would find it useful to be able to mark structs, enums, modules,
and even public fields as deprecated as well.

On Sat, Sep 5, 2015, 1:15 AM llogiq notifications@github.com wrote:

Rendered
https://github.com/llogiq/rfcs/blob/deprecate/text/0000-deprecation.md

—
Reply to this email directly or view it on GitHub
#1270 (comment).

[sfackler]

The since label would be useful for tooling, though. If you have a crate that has a wide version constraint on a dependency, it may be using a deprecated method to ensure that it works with older versions of the crate so it might not make sense for rustc to warn about deprecated items that were deprecated in the middle of the supported version range.

[llogiq]

@seanmonstar Good point about since; I mainly added it as a proxy for when the item will be removed. Since the latter is in the future, it would probably be wrong anyway, and I would mostly expect items that have been deprecated in earlier versions to be removed earlier, too. Perhaps we should make since optional and/or require either reason or surrogate be present. I shall add this to the alternatives.

And yes, I see types and public fields as API items, too. I forgot that item is a reserved word in the Rust compiler. How do I call it to avoid this misunderstanding? For now I just add them to the text.

[olivren]

A deprecate annotation is very desirable, yes.

It seems odd to me that the since field can accept a version range. I would expect it to be a precise version. I don't even understand what it would mean for a function to be deprecated since version 3.*.

I think it is fine to make the since field mandatory, because even if it is not a super useful information, its content is trivial and unambiguous to write. A library author who cares about annotating deprecated items in the first place will not be annoyed by typing the next version number of his library. Also, this information can give a sense of when the item will be removed, in particular when the project has an official deprecation policy.

I would not introduce an hypothetical until field (the version the item will be removed). If the project has a deprecation policy, then this information can be derived from the since field itself. If it doesn't have one, I don't see how the library author would be able to give a value to this field. In any case the content of this field would be unreliable.

I like the idea of the surrogate field, it makes it possible to generate a clickable link in the rustdoc. Sometimes, a deprecated item can be replaced by more than one item, either because there are multiple valid alternatives, or because the deprecated item should be replaced by a combination of other items. I propose that surrogate can contain multiple values.

[llogiq]

@olivren Thank you; those are good suggestions. I figure I was a bit inconsistent in my thinking; It is a proxy for the probability of the item being removed, with the benefit that the removal is in the future, and thus hard to predict, whereas the deprecation is in the present/past, and thus doesn't require a chrystal ball.

[bluss]

The staging_api attribute is called deprecated (with a d). Your attribute is that one, right? Or are you proposing a new and parallel attribute?

[llogiq]

Well, I initially skipped the final d to allow for parallel implementations (because the semantics are a tiny bit different), but that probably doesn't make sense. I'm changing it to #[deprecated] everywhere to reduce confusion.

[mrhota]

Bikeshed/nitpick alert :)

since is defined to contain the version of the crate at the time of deprecating the item, following the semver scheme. It makes no sense to put a version number higher than the current newest version here, and this is not checked

Why "require" or mention semver? As mentioned in the final sentence the field won't be checked by rustc. Version schemes should be left to the developer's tastes and possibly external tools.

[Gankra]

@mrhota because cargo uses semver to decide what version of a crate to use -- in particular when resolving multiple dependencies with different version requirements. It's an important aspect of reproducible/reliable builds, which deprecation is part of.

[mrhota]

@gankro You're right. I only mention it because I'm not sure why semver is mentioned if the field isn't going to be checked. Moreover, the field can be checked by cargo, which is probably the proper place for the check anyway, since it's part of the manifest spec.

Cargo, and cargo alone, should enforce the usage of semver. If I'm not using cargo, then it shouldn't matter, right?

[mrhota]

@llogiq

On use of a deprecated item, rustc will warn of the deprecation. Note that during Cargo builds, warnings on dependencies get silenced. Note that while this has the upside of keeping things tidy, it has a downside when it comes to deprecation:

(Is that how Cargo works now?)

What if Cargo didn't bother silencing deprecation warnings on dependencies? I'm not sure that would be so bad. It could be potentially useful information (you can file reports with the relevant package's maintainers). Plus you'd get to KISS.

[llogiq]

Since the semver check would be done as a lint and cargo silences lints on dependencies, yes, this is how it works.

And no, this is done in rustc, not cargo.

[aturon]

@llogiq Thanks for this RFC! Definitely a worthwhile feature for the Rust ecosystem.

I have a couple concerns:

1. The use field. In my experience with the standard library, it's pretty unusual for a replacement to be as simple as a new path -- that basically amounts to moving/renaming an API. For more complicated replacements or generalizations, we've often written short snippets of code or prose to explain how to do the change. In some cases, we've pointed at different crates (when things are getting split out). I wonder: can we just have since and message, where the latter works exactly like reason does today? That tilts things toward flexibility for library authors.
2. Just a note, it can be useful for the since field to be a future version in some cases. We've talked about doing this for the standard library in cases where we know an API is going to land/be stabilized in some particular version, and we don't want the deprecation warning to fire until that version is reached (this doesn't work today, but it should!)
3. I don't see why this feature requires instant stabilization, as the RFC seems to imply. What's the thinking there?
4. I agree with @huonw's concerns about fine-grained message control; before this feature is stabilized, it'd be good to have some basic vision for how to turn off deprecation warnings in a controlled way, to make sure we've accounted for that. However, I suspect that the attribute won't need to change much, if at all -- and that any needed changes could be done through additional, optional fields. So I don't see this concern as a blocker.

[alexcrichton]

@llogiq any thoughts on @aturon's comment? The libs team discussed this during triage today and we were ok merging modulo removing the use parts for now.

[llogiq]

1. I'll move use into alternatives. We may want to introduce it later. It's not essential at this point.
2. Ok, but this will require the "since" field to be present, and a semver. I guess rustc gets the current crate version via some env variable?
3. I'm not exactly sure what you mean here: Should it be an unstable feature first? I'd be ok with that.
4. This hinges on being able to get additional infos into a lint. I had a pre-RFC on rust-internals for that, but it's not much progress since then.

[aturon]

@llogiq

For (2), I don't see why since would have to be required -- more that if it exists and is for a future version, the deprecation doesn't yet apply. FWIW, we could probably make this rustc-specific, unless people thing it'd see use in the wider ecosystem; the use case is tied to our release channels and train model.

For (3), yeah, I was saying that we could introduce it as unstable first. Though, if we end up going with the simplified design that's basically how the feature has been working forever, I don't know that we need a long stabilization process.

For (4), I'm not overly worried; we should be able to layer something on later on.

[llogiq]

@aturon: Fine with me. Unfortunately, I'm a bit short on time, so I plan to have the update some time next week.

To sum it up: since will stay optional, but if there is checked by the deprecation_syntax lint to contain a version. The deprecation lint will not warn if the contained version isn't reached yet. We may want to add a future_deprecation lint that warns for future deprecations, if not in rustc, then in clippy. The whole deprecation feature will start out as unstable (which means nightly only), but is expected to be stabilized rather soon. We do want finer grained control on deprecation warnings (e.g. by crate of the deprecated API), but that will be done in a followup RFC, once I manage to find the time to write up that lint argument RFC...

[alexcrichton]

The compiler currently has no knowledge of crate versions, actually, it's all opaque information coming from Cargo which passes through the compiler. Along those lines I don't think that the compiler itself should have any validation of the since field, but an external lint seems fine.

[llogiq]

@aturon I finally came around to update the RFC. Before we get to implement it, I have another question to you folks: Should we rename the reason field to note? Because as it is currently used, most people don't actually write the reason for deprecating their APIs there, and renaming it would leave us the option to declare a more specific reason field later on.

[ghost]

note or perhaps message seems like a good choice.

[aturon]

@llogiq I agree that reason is often a misnomer, and note sounds fine to me.

[alexcrichton]

Thanks for the update @llogiq! I'm going to go ahead and merge this for now and we can always tweak minor details like naming here and there after this is merged. Note that this will continue to be unstable to start out with and we'll stabilize it at a future date at which point the naming can be clarified.

[alexcrichton]

Tracking issue

[quodlibetor]

The rendered link in the original post here points to a file which no longer exists, and all the commits in this thread refer to an RFC that still specifies reason instead of note.

Since this is still a popular target for search engines maybe someone could update the original "rendered" link to point to https://github.com/rust-lang/rfcs/blob/master/text/1270-deprecation.md ?

[sfackler]

@quodlibetor Fixed!