Skip to content

New lint against indirect method calls #11193

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

Open
not-my-profile opened this issue Jul 20, 2023 · 1 comment
Open

New lint against indirect method calls #11193

not-my-profile opened this issue Jul 20, 2023 · 1 comment
Labels
A-lint Area: New lints

Comments

@not-my-profile
Copy link
Contributor

not-my-profile commented Jul 20, 2023

What it does

It would flag method calls from the standard library that actually only call another method, e.g:

  • .into() -> use Type::from(..) instead
  • .try_into() -> use Type::try_from(..) instead
  • .parse() -> use Type::from_str(..) instead
  • etc. ... (there are probably more of these?)

Advantage

You can easily jump to the definition of the conversion code with rust-analyzer. Because if you go to definition of e.g. .into() with rust-analyzer it will just take you to the definition of the blanket implementation:

impl<T, U> const Into<U> for T
where
    U: ~const From<T>,
{
    /// Calls `U::from(self)`.
    ///
    /// That is, this conversion is whatever the implementation of
    /// <code>[From]&lt;T&gt; for U</code> chooses to do.
    #[inline]
    fn into(self) -> U {
        U::from(self)
    }
}

which is technically correct but not very helpful since you're probably trying to get to the definition of the actual conversion code (ref rust-lang/rust-analyzer#15315) . Especially in large code bases it can be hard to manually find where a trait is implemented.

Drawbacks

It makes code more verbose and can easily lead to false positives since Clippy isn't type-aware (however implementing methods called into or try_into outside of the std traits is probably an anti-pattern anyway since it's confusing.)

Example

let a: Foo = b.into();

Could be written as:

let a: Foo = Foo::from(b);
@not-my-profile not-my-profile added the A-lint Area: New lints label Jul 20, 2023
@Centri3
Copy link
Member

Centri3 commented Jul 20, 2023

This should probably be a restriction lint if it's added. The advantage is definitely nice but it makes code a lot more verbose.

Clippy isn't type-aware

This isn't true, there are may queries clippy has access to that can determine if it's calling a method or a trait method; if that trait method is a diagnostic item, check if it's that, otherwise get the called method's path and check that

If a lint doesn't do this, it's a bug and should be reported ^^ Most recently: https://github.com/rust-lang/rust-clippy/pull/11188/files#diff-9d8f5ebe3dc9b6e459da6b540ddacf6feb2b05628f9a05edfbc0fead31edc684R292

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants