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

Lint for soft-deprecated {integer}::max_value and min_value #5841

Closed
tesuji opened this issue Jul 25, 2020 · 6 comments
Closed

Lint for soft-deprecated {integer}::max_value and min_value #5841

tesuji opened this issue Jul 25, 2020 · 6 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@tesuji
Copy link
Contributor

tesuji commented Jul 25, 2020

What it does

Currently, Rust have three way to getting min/max of an integers:

  1. module constants: std::i32::MAX
  2. type constants: i32::MAX
  3. const methods: i32::max_value()

(3) is soft-deprecated and replaced by 2) when possible.

T-libs doesn't want to deprecate 1) and 3) now until next Rust edition,
but we have no tools to enforce style for 2),
it is great to have such a lint in Clippy.

Categories (optional)

  • Kind: style

Drawbacks

None.

@tesuji tesuji added the A-lint Area: New lints label Jul 25, 2020
@flip1995 flip1995 added L-style Lint: Belongs in the style lint group L-suggestion Lint: Improving, adding or fixing lint suggestions good-first-issue These issues are a good way to get started with Clippy labels Jul 29, 2020
@kangalio
Copy link

I'll try to do this

@kangalio
Copy link

kangalio commented Aug 1, 2020

I'm a bit stuck because I can't properly figure out a way to detect "indirect" min_value()/max_value() calls.

impl Foo {
    type Int = i32;
    fn min_value() -> Int {
        Self::Int::min_value() // lint
    }
}
Foo::min_value(); // shouldn't lint!
Foo::Int::min_value(); // lint

type Int = i32;
let _ = Int::min_value(); // lint

I'm helpless with the rustc internals and its documentation is hardly helping.

@flip1995
Copy link
Member

flip1995 commented Aug 1, 2020

You have to compare the path of the functions. You can search the Clippy codebase for match_qpath to find out how it is used.

@kangalio
Copy link

kangalio commented Aug 1, 2020

As far as I can tell, match_qpath is for matching a path 1:1. However, I need a way to "resolve" paths to the actual function and call against that. I don't see any way of using match_qpath that would catch all three cases I've outlined above

@flip1995
Copy link
Member

flip1995 commented Aug 2, 2020

Ah I see. This may work with

pub fn fn_def_id(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<DefId> {

which you can then compare to the def_id of the min_value function. This can be done with

pub fn match_def_path<'tcx>(cx: &LateContext<'tcx>, did: DefId, syms: &[&str]) -> bool {

@phansch
Copy link
Member

phansch commented Jan 11, 2021

There has been some progress on the Rust side of things (tracking issue). Rust has recently added support for 'soft-deprecation' of items: rust-lang/rust#79877. The next step will be to soft-deprecate the old items in std directly.

Considering that, I don't think it makes sense to implement this in Clippy anymore and I'm going to go ahead and close this issue.

Thanks everyone for your interest in working on this lint 💙

@phansch phansch closed this as completed Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy L-style Lint: Belongs in the style lint group L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants