Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking Issue for #![feature(deprecated_suggestion)] #94785

Open
1 of 3 tasks
jhpratt opened this issue Mar 9, 2022 · 12 comments
Open
1 of 3 tasks

Tracking Issue for #![feature(deprecated_suggestion)] #94785

jhpratt opened this issue Mar 9, 2022 · 12 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC L-deprecated Lint: deprecated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jhpratt
Copy link
Member

jhpratt commented Mar 9, 2022

Feature gate: #![feature(deprecated_suggestion)]

This is a tracking issue for suggested replacements on deprecations. This has been used as part of the #[rustc_deprecated] attribute since 2019-01-31. The suggestions are machine-applicable, in that cargo fix is capable of making the replacement without further user intervention. As such, the suggestion must be valid Rust (though this is not enforced).

Public API

#![feature(deprecated_suggestion)]

struct Foo;

impl Foo {
    #[deprecated(suggestion = "baz")]
    fn bar() {}
    fn baz() {}
}

Steps / History

Unresolved Questions

  • None yet.
@jhpratt jhpratt added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 9, 2022
@tgross35
Copy link
Contributor

tgross35 commented Jun 6, 2023

Are there any blockers to stabilizing at least the attribute? It seems like cargo fix works fine with it and the messages seem good.

@jhpratt
Copy link
Member Author

jhpratt commented Jun 6, 2023

Not sure what you mean by "at least the attribute", as that's literally the entire feature. But there are no blockers unless T-libs-api would like an ACP/RFC for this. It's been used as part of the standard library for 4+ years.

@tgross35
Copy link
Contributor

tgross35 commented Jun 6, 2023

Is suggestion enforcement validity desired? It would be a nice sanity check.

#![allow(unused)]
#![feature(deprecated_suggestion)]

struct Foo;

impl Foo {
    #[deprecated(suggestion = "new_foo")]
    fn new() -> Foo { todo!() }
    #[deprecated(suggestion = "some invalid path")]
    fn bar() {}
    fn baz() {}
}

fn new_foo() -> Foo { todo!() }

fn main() {
    // Suggests `Foo::new_foo();` which doesn't exist
    Foo::new();
    // Suggests invalid `Foo::some invalid path();`
    Foo::bar();
}

Maybe unquoted identifiers could be verified (must be brought into scope) but quoted string literals could be suggested as-is, if the current behavior is desirable for some reason (which is what I meant by at least the attribute).

@jhpratt
Copy link
Member Author

jhpratt commented Jun 6, 2023

It might be worth looking into whether the resulting code could be verified. It may be a fair amount of effort.

@workingjubilee
Copy link
Member

While reviewing #113365 it became at least somewhat apparent that suggestions like these make a lot of our existing notes, which don't add meaningful context, but are just manual suggestions, redundant. It would be nice to trim out the notes!

...except doing so triggers error E0543? That seems weird.

@Nemo157
Copy link
Member

Nemo157 commented Aug 18, 2023

That error is part of staged_api, it could be updated to check for either note or suggestion.

@workingjubilee
Copy link
Member

workingjubilee commented Aug 20, 2023

Question: Do we want suggestions to be applied via rustfix that may be valid Rust but fail to typecheck due to a signature change? Several of the deprecations were due to that reason.

@madsmtm
Copy link
Contributor

madsmtm commented Sep 2, 2023

A concern: I don't think the suggestion shows up in rustdoc? And maybe rustdoc should be able to generate a link to the new functionality automatically, though I'm unsure how it would know how to?

@SamB
Copy link

SamB commented Oct 25, 2023

Question: Do we want suggestions to be applied via rustfix that may be valid Rust but fail to typecheck due to a signature change?

Worse: what if the arguments got shuffled but still typecheck? The name suggestion doesn't do a great job of communicating the possible danger.

@workingjubilee
Copy link
Member

Oh yeah, I can just imagine the nightmare that is a library with 2+ lerp functions and deprecated(suggestion) there...

@jhpratt
Copy link
Member Author

jhpratt commented Oct 28, 2023

Right now, the only thing this feature is useful for is simple renaming of methods. Anything else it cannot handle.

@lolbinarycat
Copy link
Contributor

Worse: what if the arguments got shuffled but still typecheck? The name suggestion doesn't do a great job of communicating the possible danger.

perhaps renamed_to or alias_of would be better?

@fmease fmease added the L-deprecated Lint: deprecated label Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC L-deprecated Lint: deprecated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants