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.

Sign up for GitHub

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

Jump to bottom

New lint bytes_count_to_len #8375

Closed
wants to merge 6 commits into from
Closed

New lint bytes_count_to_len #8375

wants to merge 6 commits into from

Conversation

chaseruskin
Copy link
Contributor

@chaseruskin chaseruskin commented Jan 31, 2022 •
edited
Loading

Fixes #8083

changelog:
• adds new lint [`bytes_count_to_len`] to consider replacing .bytes().count() with .len()

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 31, 2022
Manishearth
Manishearth requested changes Feb 3, 2022
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start, some suggestions

clippy_lints/src/bytes_count_to_len.rs
#[clippy::version = "1.60.0"]
pub BYTES_COUNT_TO_LEN,
complexity,
"Using bytest().count() when len() performs the same functionality"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Using bytest().count() when len() performs the same functionality"
"Using bytes().count() when len() performs the same functionality"

clippy_lints/src/bytes_count_to_len.rs
if_chain! {
//check for method call called "count"
if let hir::ExprKind::MethodCall(count_path, count_args, _) = &expr.kind;
if count_path.ident.name == rustc_span::sym::count;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use match_def_path here and below

And we should ensure the type is str

@bors
Copy link
Contributor

bors commented Feb 11, 2022

☔ The latest upstream changes (presumably #8411) made this pull request unmergeable. Please resolve the merge conflicts.

@giraffate
Copy link
Contributor

@c-rus ping from triage. Can you have any question to work on this?

@giraffate
Copy link
Contributor

@c-rus ping from triage. According the triage procedure, I'm closing this because 2 weeks have passed with no activity. If you have more time to work on this, feel free to reopen this.

@giraffate giraffate closed this Apr 13, 2022
@giraffate giraffate added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Apr 13, 2022
@kyoto7250
Copy link
Contributor

I'm interested in this PR.
Can I take over and create another PR?

@giraffate
Copy link
Contributor

Yes, thanks!

@kyoto7250 kyoto7250 mentioned this pull request Apr 16, 2022
Merged
@kyoto7250
Copy link
Contributor

I opened a PR that took over.

#8711

bors added a commit that referenced this pull request Apr 19, 2022
@bors
Auto merge of #8711 - kyoto7250:new-lint-bytes-count-to-len, r=giraffate
e17b97c 
Take over: New lint bytes count to len

take over #8375
close #8083

This PR adds new lint about  considering replacing `.bytes().count()` with `.len()`.

Thank you in advance.

---

r! `@Manishearth`

changelog: adds new lint [`bytes_count_to_len`] to consider replacing `.bytes().count()` with `.len()`
@blyxyas blyxyas removed the S-inactive-closed Status: Closed due to inactivity label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Manishearth Manishearth Manishearth requested changes

Assignees

@Manishearth Manishearth

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Lint suggesting to replace str::bytes().count() with str::len()
7 participants
@chaseruskin @rust-highfive @bors @giraffate @kyoto7250 @Manishearth @blyxyas