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

use_self still gives false positives #6902

Closed
Tracked by #79
imp opened this issue Mar 14, 2021 · 7 comments · Fixed by #9454
Closed
Tracked by #79

use_self still gives false positives #6902

imp opened this issue Mar 14, 2021 · 7 comments · Fixed by #9454
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@imp
Copy link
Contributor

imp commented Mar 14, 2021

Now that #6818 is fixed and the fix (#6833) has made its way to the nightly there are still cases of false positives:

Lint name: use_self

I tried this code:

#![deny(clippy::use_self)]
use serde::Deserialize;

#[derive(Debug, Deserialize)]
#[serde(untagged)]
enum Direction {
    North,
}

fn main() {
    let d = Direction::North;

    println!("{:?}", d);
}

I expected to see this happen: clippy clean - no warning/errors

Instead, this happened:

    Checking playground v0.0.1 (/playground)
error: unnecessary structure name repetition
 --> src/main.rs:6:6
  |
6 |   enum Direction {
  |  ______^
7 | |     North,
  | |____^ help: use the applicable keyword: `Self`
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![deny(clippy::use_self)]
  |         ^^^^^^^^^^^^^^^^
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_self

error: aborting due to previous error

error: could not compile `playground`

To learn more, run the command again with --verbose.

Interestingly enough it the code generated by #[serde(untagged)] that triggers this lint. If this line is commented out then clippy seems to be happy

Meta

  • cargo clippy -V: clippy 0.1.52 (2021-03-13 acca818)

  • rustc -Vv:

rustc 1.52.0-nightly (acca81892 2021-03-13)
binary: rustc
commit-hash: acca818928654807ed3bc1ce0e97df118f8716c8
commit-date: 2021-03-13
host: x86_64-unknown-linux-gnu
release: 1.52.0-nightly
LLVM version: 12.0.0
@imp imp added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Mar 14, 2021
@Y-Nak
Copy link
Contributor

Y-Nak commented Apr 12, 2021

This issue would be automatically fixed when rust-lang/rust#84122 is addressed.

@barafael
Copy link

barafael commented Mar 1, 2022

For reference, here are two cases in which the same false positive occurs:

  • smart-default with the SmartDefault derive macro
  • num-derive with the FromPrimitive and ToPrimitive derive macros
    • This PR on num-derive proposes to stop using the type name and instead use Self in the generated code, solving the problem inside num-derive: Fix #47 rust-num/num-derive#49

@carols10cents
Copy link
Member

carols10cents commented Apr 6, 2022

This problem has now made it to the clippy that's in beta (specifically 1.61.0-beta.1 (0f231250b 2022-04-05))

Here's a reproduction, this project has 0 warnings with the clippy with Rust 1.60 but has warnings with the clippy with 1.61.0-beta.1:

Cargo.toml:

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
serde = { version = "1.0", features = ["derive"] }

src/lib.rs:

#![warn(clippy::use_self)]

use serde::Serialize;

#[derive(Serialize)]
pub enum Status {
    Pass,
    Fail,
}

warning:

error: unnecessary structure name repetition
 --> src/lib.rs:6:10
  |
6 | pub enum Status {
  |          ^^^^^^ help: use the applicable keyword: `Self`
  |
  = note: `-D clippy::use-self` implied by `-D warnings`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_self

cargo expand shows the code serde is generating is:

#![feature(prelude_import)]
#![warn(clippy::use_self)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
use serde::Serialize;
pub enum Status {
    Pass,
    Fail,
}
#[doc(hidden)]
#[allow(non_upper_case_globals, unused_attributes, unused_qualifications)]
const _: () = {
    #[allow(unused_extern_crates, clippy::useless_attribute)]
    extern crate serde as _serde;
    #[automatically_derived]
    impl _serde::Serialize for Status {
        fn serialize<__S>(
            &self,
            __serializer: __S,
        ) -> _serde::__private::Result<__S::Ok, __S::Error>
        where
            __S: _serde::Serializer,
        {
            match *self {
                Status::Pass => {
                    _serde::Serializer::serialize_unit_variant(__serializer, "Status", 0u32, "Pass")
                }
                Status::Fail => {
                    _serde::Serializer::serialize_unit_variant(__serializer, "Status", 1u32, "Fail")
                }
            }
        }
    }
};

which does contain the repetition in the match, but I think this is still a clippy bug because clippy didn't warn about this with the previous version and it's pointing to code that definitely shouldn't use Self.

@kleinesfilmroellchen
Copy link

I can reproduce this as well. Shouldn't clippy not apply this lint (and others) on impls and functions marked as #[automatically_derived]?

@tyranron
Copy link

Now it fires on stable 😭

bors bot pushed a commit to boa-dev/boa that referenced this issue May 20, 2022
This fixes the CI after the upgrade to Rust 1.61. I had to "allow" the `use_self` lint, due to rust-lang/rust-clippy#8845 and rust-lang/rust-clippy#6902.

It removed some false negatives, though, so I fixed some of the usage.
Razican added a commit to boa-dev/boa that referenced this issue Jun 8, 2022
This fixes the CI after the upgrade to Rust 1.61. I had to "allow" the `use_self` lint, due to rust-lang/rust-clippy#8845 and rust-lang/rust-clippy#6902.

It removed some false negatives, though, so I fixed some of the usage.
@repi
Copy link

repi commented Aug 13, 2022

The use_self lint is fantastic and we've run it multiple times manually to fix up our code with it over the last year or so, but even as of latest 1.63 these false positives on enums that use serde Serialize/Deserialize prevent us from enabling and enforcing use of the lint. Would be great if could be solved!

@daxpedda
Copy link
Contributor

I believe this should now be fixable with the help of #8694.

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-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
8 participants