rust-lang / rust-lang/rust-clippy

Lint for unintentionally using default trait implementations

Open
#14,728 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-lint
Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

What it does

This lint would trigger when a trait such as std::hash::Hasher was implemented but relying on the slow delgated method (write) without implementing the performance improving specialised methods (write_u8).

I have thought of three ways for this lint to trigger:

  1. A restriction lint that the user has to enable for the specific impl block, called something like missing_overrides
  2. A pedantic lint which triggers when a trait implementation is delegating all the methods it has implemented, but some default methods are being provided.
  3. A pedantic lint which triggers when a trait impl block is relying on default implemented methods. This would be noisy and expect to be expected in quite a lot of places.
Advantage
  • Newtypes which are implementing traits via delegation do not miss out on the fast path implementations (ie: std::hash::Hasher)
  • Traits which default implementations just return errors don't accidentally turn a compile time issue to a runtime issue (ie: serde::de::Visitor)
Drawbacks

Method 1 would require knowledge of the lint to know to enable it, which probably means the user is going to implement all the default provided methods correctly anyway. Method 2 and 3 could be too noisy, even for pedantic.

Example
#![warn(clippy::pedantic)]

use std::hash::Hasher;

struct NewType(std::hash::DefaultHasher);

impl Hasher for NewType {
    fn write(&mut self, bytes: &[u8]) {
        self.0.write(bytes)
    }

    fn finish(&self) -> u64 {
        self.0.finish()
    }
}

Could be written as:

use std::hash::Hasher;

struct NewType(std::hash::DefaultHasher);

impl Hasher for NewType {
    fn write(&mut self, bytes: &[u8]) {
        self.0.write(bytes)
    }

    fn write_u8(&mut self, val: u8) {
        self.0.write_u8(val);
    }

    fn write_u16(&mut self, val: u16) {
        self.0.write_u16(val);
    }

    fn write_u32(&mut self, val: u32) {
        self.0.write_u32(val);
    }

    // etc, etc, etc

    fn finish(&self) -> u64 {
        self.0.finish()
    }
}

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

No implementation files, tests, or entry points are named. First resolve which of the three proposed lint behaviors and noise levels is intended; done means a defined lint scope and behavior that addresses delegated default methods without imposing an unspecified level of false positives.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
devtools
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.