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

cmp_owned false positives - proc_macro Ident #4874

Closed
elichai opened this issue Dec 2, 2019 · 2 comments · Fixed by #5701
Closed

cmp_owned false positives - proc_macro Ident #4874

elichai opened this issue Dec 2, 2019 · 2 comments · Fixed by #5701
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied

Comments

@elichai
Copy link
Contributor

elichai commented Dec 2, 2019

Hi,
I noticed a false positive when comparing a string with ident.to_string()
clippy suggests you don't need to add to_string() and you can instead dereference the ident.

  1. You can't dereference it because there's no reference involved.
  2. there's no PartialEq implementation between &str/String and Ident.

Example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=b6ae15acb40aee8fe582f1620ac569d7

@elichai elichai changed the title cmp_owned false positives cmp_owned false positives - proc_macro Ident Dec 2, 2019
@flip1995 flip1995 added C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels Dec 2, 2019
@ghost
Copy link

ghost commented Jan 8, 2020

Your example is:

extern crate proc_macro2;
use proc_macro2::*;

fn main() {
    let a1 = Ident::new("Hi", Span::call_site());
    if "Hi" == a1.to_string() {
        println!("yay");
    } else {
        println!(":(");
    }
}

proc-macro2 has impl<T: ?Sized> PartialEq<T> for Ident where T: AsRef<str> so you can write
a1 == "Hi" to avoid creating the string.

The reason for the bug is that the lint assumes that PartialEq is implemented symmetrically i.e. T: PartialEq<U> implies U: PartialEq<T>. The PartialEq docs actually require this so maybe proc-macro2 is missing an impl.

@elichai
Copy link
Contributor Author

elichai commented Jan 8, 2020

Hi, Nice catch :)
bad I'm not sure the solution you're proposing is even possible: https://play.rust-lang.org/?gist=75e6e15847489b8c6ee7c8af6507dd66

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants