Skip to content

Conversation

@aedank0
Copy link

@aedank0 aedank0 commented Nov 15, 2025

Adds new lint clones_into_boxed_slices which detects clone-like functions followed by into_boxed_... instead of using Box::from(...).

Closes #15951, closes #15373

changelog: new lint: [clones_into_boxed_slices]

@rustbot rustbot added needs-fcp S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 15, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2025

r? @llogiq

rustbot has assigned @llogiq.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@github-actions
Copy link

github-actions bot commented Nov 15, 2025

Lintcheck changes for 1ef6ca9

Lint Added Removed Changed
clippy::clones_into_boxed_slices 14 0 0

This comment will be updated if you push new changes

Copy link
Member

@samueltardieu samueltardieu left a comment

Choose a reason for hiding this comment

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

Interesting lint. It should probably be located inside the methods directory though.

Also, it should check the MSRV to ensure that the proposed fixes are actionable. For example .as_c_str() didn't exist before Rust 1.20.

View changes since this review

let mut sugg = Sugg::hir_with_context(cx, inner, full_span.ctxt(), placeholder, &mut applicability);

let inner_ty = cx.typeck_results().expr_ty(inner);
let mut ref_count = count_refs(inner_ty);
Copy link
Member

Choose a reason for hiding this comment

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

You can use peel_and_count_ty_refs from clippy_utils::ty:

Suggested change
let mut ref_count = count_refs(inner_ty);
let mut ref_count = peel_and_count_ty_refs(inner_ty).1 as isize;

Comment on lines +75 to +82
while ref_count > 0 {
sugg = sugg.deref();
ref_count -= 1;
}
while ref_count < 0 {
sugg = sugg.addr();
ref_count += 1;
}
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
while ref_count > 0 {
sugg = sugg.deref();
ref_count -= 1;
}
while ref_count < 0 {
sugg = sugg.addr();
ref_count += 1;
}
for _ in 0..ref_count {
sugg = sugg.deref();
}
for _ in 0..-ref_count {
sugg = sugg.addr();
}

Comment on lines +102 to +106
ty.is_slice()
|| ty.is_str()
|| ty.is_diag_item(cx, sym::cstr_type)
|| ty.is_diag_item(cx, sym::Path)
|| ty.is_diag_item(cx, sym::OsStr)
Copy link
Member

Choose a reason for hiding this comment

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

You should avoid doing multiple lookups:

Suggested change
ty.is_slice()
|| ty.is_str()
|| ty.is_diag_item(cx, sym::cstr_type)
|| ty.is_diag_item(cx, sym::Path)
|| ty.is_diag_item(cx, sym::OsStr)
ty.is_slice() || ty.is_str() || matches!(ty.opt_diag_name(cx.tcx), Some(sym::cstr_type | sym::Path | sym::OsStr))

Comment on lines +117 to +124
&& [
sym::into_boxed_str,
sym::into_boxed_slice,
sym::into_boxed_path,
sym::into_boxed_c_str,
sym::into_boxed_os_str,
]
.contains(&second_method_path.ident.name)
Copy link
Member

Choose a reason for hiding this comment

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

Matching by name is not enough and can lead to false positives. You should ensure that this is an inherent method call and that the receiver type is the one you expect.

As an alternative, since those methods are not diag items, you could add them to clippy_utils::paths.

if second_method_path.ident.name == sym::into_boxed_path && !inner_ty.is_diag_item(cx, sym::Path) {
// PathBuf's from(...) can convert from other str types,
// so Path::new(...) must be used to assure resulting Box is the correct type
show_lint(cx, full_span, arg, true, Some("Path::new("), "...", Some(")"));
Copy link
Member

Choose a reason for hiding this comment

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

show_lint() should know that this suggestion should be degraded to MaybeIncorrect at most, because Path may not be have been imported.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 15, 2025
) {
let mut applicability = Applicability::MachineApplicable;

while let ExprKind::AddrOf(BorrowKind::Ref, _, expr) | ExprKind::Unary(UnOp::Deref, expr) = inner.kind {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if someone puts curly brackets around a subexpression, such as &{ foo.to_string() }? That would be an ExprKind::Block with one return expression, defeating this loop.

impl<'tcx> LateLintPass<'tcx> for ClonesIntoBoxedSlices {
fn check_expr(&mut self, cx: &LateContext<'tcx>, second_method: &'tcx Expr<'_>) {
// Is the second method into_boxed_...?
if let ExprKind::MethodCall(second_method_path, first_method, _, _) = second_method.kind
Copy link
Contributor

Choose a reason for hiding this comment

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

This would certainly be simplified by putting this lint into methods. The matching is basically already done there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-fcp S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lint for .clone().into_boxed_str() Lint suggestion: indirect_boxing

4 participants