Skip to content

Lint for using the same expression for both length and capacity when constructing a vec/string #5955

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
sgrif opened this issue Aug 24, 2020 · 7 comments
Assignees
Labels
A-lint Area: New lints good first issue These issues are a good way to get started with Clippy

Comments

@sgrif
Copy link

sgrif commented Aug 24, 2020

What it does

This lint checks for calls to String::from_raw_parts and Vec::from_raw_parts, and triggers when the same expression is used for both the length and capacity. If folks are interested in a pull request for this lint, I am happy to do the work.

Categories (optional)

  • Kind: pedantic

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

When code is written this way, the author likely intended to use a borrowed type instead. The idea for this lint came after noticing a friend of mine consistently reaching for String and not &str, which requires inventing a capacity. The author will likely use the length in that case. Unless the string/vec was created using with_capacity or had shrink_to_fit called on it, it's unlikely that the length and capacity are the same.

Drawbacks

This is trying to catch a beginner mistake, and can potentially trigger on valid code. It's unclear how common this mistake actually is

Example

Vec::from_raw_parts(ptr, len, len)

Could be written as:

slice::from_raw_parts(ptr, len)
// or
Vec::from_raw_parts(ptr, len, cap)
@sgrif sgrif added the A-lint Area: New lints label Aug 24, 2020
@flip1995 flip1995 added the good first issue These issues are a good way to get started with Clippy label Aug 26, 2020
@longlb
Copy link
Contributor

longlb commented Sep 4, 2020

Can I give this one a try?

@ebroto
Copy link
Member

ebroto commented Sep 4, 2020

Sure, go ahead!

@ebroto
Copy link
Member

ebroto commented Sep 4, 2020

Oh, I just saw that @sgrif offered themself to implement it. It would be better to hear from them first if they started to work on it already.

@longlb
Copy link
Contributor

longlb commented Sep 4, 2020

Not a problem, I'll wait for them to reply. By the way, is there a bug in the repo currently? I just forked it and the build won't compile due to a dependency struct being private.

error[E0616]: field kind of struct TyS is private   
--> clippy_lints/src/vec.rs:130:36
    |
130 |     if let ty::Adt(_, substs) = ty.kind {
    |                                    ^^^^ private field
    |
help: a method `kind` also exists, call it with parentheses
    |
130 |     if let ty::Adt(_, substs) = ty.kind() {
    |                                        ^^

I have 187 cases of this in my terminal.

@ebroto
Copy link
Member

ebroto commented Sep 4, 2020

Thanks for the heads-up, a fix is in progress in #6007.

This happens when rustc breaks Clippy, we're working on trying to avoid this hassle to contributors :) After that PR passes the tests and is merged, the errors should be gone.

@km274
Copy link

km274 commented Jan 30, 2024

@rustbot claim

@km274
Copy link

km274 commented Jan 30, 2024

Hi 👋🏻, I'm an experienced software engineer but am new to Rust. I picked this as a first issue since it seems like it doesn't exactly need to be done in a rush 😋. I'll get to work in my spare time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good first issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

5 participants