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

Warn on push_str with a single-character string literal #5875

Closed
camelid opened this issue Aug 7, 2020 · 5 comments · Fixed by #5881
Closed

Warn on push_str with a single-character string literal #5875

camelid opened this issue Aug 7, 2020 · 5 comments · Fixed by #5881
Labels
A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy

Comments

@camelid
Copy link
Member

camelid commented Aug 7, 2020

What it does

Warns when using push_str with a single-character string literal, and push with a char would work fine.

Categories (optional)

  • Kind: clippy::style and clippy::perf (maybe clippy::pedantic?)

What is the advantage of the recommended code over the original code?

  • It's more obvious what's going on; after all, you're not trying to push a string, you're trying to push a character
  • It should be more performant and reduce binary size to push a char, since it's just a number and will be inlined, than to push a &'static str, which must be stored in the binary and accessed via a pointer

Drawbacks

None.

Example

let mut string = String::new();
string.push_str("R");

Could be written as:

let mut string = String::new();
string.push('R');
@camelid camelid added the A-lint Area: New lints label Aug 7, 2020
@camelid
Copy link
Member Author

camelid commented Aug 7, 2020

@rustbot modify labels to +A-style +A-performance

@camelid
Copy link
Member Author

camelid commented Aug 7, 2020

Hmm, rustbot's not responding...

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Aug 8, 2020
wiomoc added a commit to wiomoc/rust-clippy that referenced this issue Aug 9, 2020
wiomoc added a commit to wiomoc/rust-clippy that referenced this issue Aug 9, 2020
@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented Aug 12, 2020

@rustbot modify labels: -A-style

Okay, seems to work, just had to add rustbot with write permissions to this repo.

@rustbot rustbot added L-style Lint: Belongs in the style lint group and removed L-style Lint: Belongs in the style lint group labels Aug 12, 2020
bors added a commit that referenced this issue Aug 16, 2020
…lip1995

Lint `push_str` with a single-character string literal

Fixes #5875
changelog:  `* [single_char_push_str]`
@bors bors closed this as completed in 72d2c2e Aug 16, 2020
hegza pushed a commit to hegza/rust-clippy that referenced this issue Aug 25, 2020
@camelid
Copy link
Member Author

camelid commented Aug 30, 2020

Correct me if I'm wrong, but this lint does not seem to working on the latest nightly. With this code:

// src/main.rs

fn main() {
    let mut string = String::new();
    string.push_str("R");
}

When I run nightly clippy, I don't get any warnings:

$ cargo +nightly clippy
    Checking clippy-issue-5875 v0.1.0 (~/clippy-issue-5875)
    Finished dev [unoptimized + debuginfo] target(s) in 0.11s

Output of cargo clippy --version:

clippy 0.0.212 (04488afe3 2020-08-24)

@ebroto
Copy link
Member

ebroto commented Aug 30, 2020

@camelid if I'm not mistaken, the clippy component is built now from the rust-lang/rust repo copy and not from this repo. The latest sync was merged yesterday (see here), so I think it should be available on the next nightly.

cc @flip1995 for any imprecision with the component release process, I don't want to lie to anyone :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants