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

Suggest usage of array and Vec .as_slice() and .as_mut_slice() methods #8405

Closed
wants to merge 5 commits into from
Closed

Suggest usage of array and Vec .as_slice() and .as_mut_slice() methods #8405

wants to merge 5 commits into from

Conversation

nindalf
Copy link
Contributor

@nindalf nindalf commented Feb 8, 2022 •
edited
Loading

implements lint suggested in #7633

  • Followed lint naming conventions
  • Added passing UI tests (including committed .stderr file)
  • cargo test passes locally
  • Executed cargo dev update_lints
  • Added lint documentation
  • Run cargo dev fmt

Please write a short comment explaining your change (or "none" for internal only changes)

changelog: Add lint [`manual_slice`]

@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 @llogiq (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 Feb 8, 2022
@nindalf nindalf changed the title Suggest usage of array and Vec .as_slice() and as_mut_slice() methods Suggest usage of array and Vec .as_slice() and .as_mut_slice() methods Feb 8, 2022
nindalf
nindalf commented Feb 8, 2022
View reviewed changes
clippy_lints/src/manual_slice.rs
/// ```
#[clippy::version = "1.60.0"]
pub MANUAL_SLICE,
restriction,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure about this.

Alexendoo
Alexendoo reviewed Feb 9, 2022
View reviewed changes
clippy_lints/src/manual_slice.rs
let snip = snippet_with_context(cx, object.span, ctxt, "..", &mut app).0;
let suggested_method = match mutability {
Mutability::Not => "to_slice()",
Mutability::Mut => "to_mut_slice()",
Copy link
Member

Choose a reason for hiding this comment

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

Should be as_slice/as_mut_slice

@camsteffen
Copy link
Contributor

A new lint called deref_by_slicing is being added in #8218. I'm pretty sure this lint is the same thing except that it suggests as_slice() instead of *. And I think the solution here is to have both suggestions appear in deref_by_slicing. This could be done by @Jarcho in #8218 or it can be done afterwards as an enhancement.

Also there has been a lot of discussion in #8218 about how this lint should be categoriezed (summary: restriction).

@nindalf
Copy link
Contributor Author

nindalf commented Feb 9, 2022

@camsteffen fair enough, shall I abandon this PR and close the attached task?

@camsteffen
Copy link
Contributor

Yeah you can close this PR but let's leave the issue open until we have a lint that suggests as_slice().

@nindalf
Copy link
Contributor Author

nindalf commented Feb 9, 2022 •
edited
Loading

Ok, makes sense.

@nindalf nindalf closed this Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Alexendoo Alexendoo Alexendoo left review comments

Assignees

@llogiq llogiq

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nindalf @rust-highfive @camsteffen @Alexendoo @llogiq