Skip to content

Commit

Permalink
Simplify reindent_multiline() signature
Browse files Browse the repository at this point in the history
- `reindent_multiline()` always returns the result of
  `reindent_multiline_inner()` which returns a `String`. Make
  `reindent_multiline()` return a `String` as well, instead of a
  systematically owned `Cow<'_, str>`.
- There is no reason for `reindent_multiline()` to force a caller to
  build a `Cow<'_, str>` instead of passing a `&str` directly,
  especially considering that a `String` will always be returned.

Also, both the input parameter and return value (of type `Cow<'_, str>`)
shared the same (elided) lifetime for no reason: this worked only because
the result was always the `Cow::Owned` variant which is compatible with
any lifetime.

As a consequence, the signature changes from:

```rust
fn reindent_multiline(s: Cow<'_, str>, …) -> Cow<'_, str> { … }
```

to

```rust
fn reindent_multiline(s: &str, …) -> String { … }
```
  • Loading branch information
samueltardieu committed Jan 30, 2025
1 parent ad05bc0 commit ca00f88
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 63 deletions.
11 changes: 5 additions & 6 deletions clippy_lints/src/copies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ use rustc_session::impl_lint_pass;
use rustc_span::hygiene::walk_chain;
use rustc_span::source_map::SourceMap;
use rustc_span::{Span, Symbol};
use std::borrow::Cow;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -248,18 +247,18 @@ fn lint_branches_sharing_code<'tcx>(
let first_line_span = first_line_of_span(cx, expr.span);
let replace_span = first_line_span.with_hi(span.hi());
let cond_span = first_line_span.until(first_block.span);
let cond_snippet = reindent_multiline(snippet(cx, cond_span, "_"), false, None);
let cond_snippet = reindent_multiline(&snippet(cx, cond_span, "_"), false, None);
let cond_indent = indent_of(cx, cond_span);
let moved_snippet = reindent_multiline(snippet(cx, span, "_"), true, None);
let moved_snippet = reindent_multiline(&snippet(cx, span, "_"), true, None);
let suggestion = moved_snippet.to_string() + "\n" + &cond_snippet + "{";
let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, cond_indent);
let suggestion = reindent_multiline(&suggestion, true, cond_indent);
(replace_span, suggestion.to_string())
});
let end_suggestion = res.end_span(last_block, sm).map(|span| {
let moved_snipped = reindent_multiline(snippet(cx, span, "_"), true, None);
let moved_snipped = reindent_multiline(&snippet(cx, span, "_"), true, None);
let indent = indent_of(cx, expr.span.shrink_to_hi());
let suggestion = "}\n".to_string() + &moved_snipped;
let suggestion = reindent_multiline(Cow::Borrowed(&suggestion), true, indent);
let suggestion = reindent_multiline(&suggestion, true, indent);

let span = span.with_hi(last_block.span.hi());
// Improve formatting if the inner block has indention (i.e. normal Rust formatting)
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ impl<'tcx> LateLintPass<'tcx> for HashMapPass {
format!(
"match {map_str}.entry({key_str}) {{\n{indent_str} {entry}::{then_entry} => {}\n\
{indent_str} {entry}::{else_entry} => {}\n{indent_str}}}",
reindent_multiline(then_str.into(), true, Some(4 + indent_str.len())),
reindent_multiline(else_str.into(), true, Some(4 + indent_str.len())),
reindent_multiline(&then_str, true, Some(4 + indent_str.len())),
reindent_multiline(&else_str, true, Some(4 + indent_str.len())),
entry = map_ty.entry_path(),
)
}
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/if_not_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::Span;
use std::borrow::Cow;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -107,7 +106,7 @@ fn make_sugg<'a>(
els_span: Span,
default: &'a str,
indent_relative_to: Option<Span>,
) -> Cow<'a, str> {
) -> String {
let cond_inner_snip = snippet(sess, cond_inner, default);
let els_snip = snippet(sess, els_span, default);
let indent = indent_relative_to.and_then(|s| indent_of(sess, s));
Expand All @@ -130,5 +129,5 @@ fn make_sugg<'a>(
_ => String::new(),
};

reindent_multiline(suggestion.into(), true, indent)
reindent_multiline(&suggestion, true, indent)
}
2 changes: 1 addition & 1 deletion clippy_lints/src/matches/manual_unwrap_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ fn lint<'tcx>(
or_body_snippet: &str,
indent: usize,
) {
let reindented_or_body = reindent_multiline(or_body_snippet.into(), true, Some(indent));
let reindented_or_body = reindent_multiline(or_body_snippet, true, Some(indent));

let mut app = Applicability::MachineApplicable;
let suggestion = sugg::Sugg::hir_with_context(cx, scrutinee, expr.span.ctxt(), "..", &mut app).maybe_par();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,13 @@ pub(super) fn check<'tcx>(
};

let suggestion_source = reindent_multiline(
format!(
&format!(
"std::path::Path::new({})
.extension()
.map_or(false, |ext| ext.eq_ignore_ascii_case(\"{}\"))",
recv_source,
ext_str.strip_prefix('.').unwrap()
)
.into(),
),
true,
Some(indent_of(cx, call_span).unwrap_or(0) + 4),
);
Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/methods/filter_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use rustc_lint::LateContext;
use rustc_middle::ty::adjustment::Adjust;
use rustc_span::Span;
use rustc_span::symbol::{Ident, Symbol, sym};
use std::borrow::Cow;

use super::{MANUAL_FILTER_MAP, MANUAL_FIND_MAP, OPTION_FILTER_MAP, RESULT_FILTER_MAP};

Expand Down Expand Up @@ -302,7 +301,7 @@ pub(super) fn check(
filter_span.with_hi(expr.span.hi()),
"`filter` for `Some` followed by `unwrap`",
"consider using `flatten` instead",
reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, map_span)).into_owned(),
reindent_multiline("flatten()", true, indent_of(cx, map_span)),
Applicability::MachineApplicable,
);

Expand All @@ -316,7 +315,7 @@ pub(super) fn check(
filter_span.with_hi(expr.span.hi()),
"`filter` for `Ok` followed by `unwrap`",
"consider using `flatten` instead",
reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, map_span)).into_owned(),
reindent_multiline("flatten()", true, indent_of(cx, map_span)),
Applicability::MachineApplicable,
);

Expand Down
5 changes: 2 additions & 3 deletions clippy_lints/src/methods/iter_filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use rustc_hir as hir;
use rustc_hir::QPath;
use rustc_span::Span;
use rustc_span::symbol::{Ident, Symbol, sym};
use std::borrow::Cow;

///
/// Returns true if the expression is a method call to `method_name`
Expand Down Expand Up @@ -181,7 +180,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir
filter_span.with_hi(expr.span.hi()),
"`filter` for `is_ok` on iterator over `Result`s",
"consider using `flatten` instead",
reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, filter_span)).into_owned(),
reindent_multiline("flatten()", true, indent_of(cx, filter_span)),
Applicability::HasPlaceholders,
),
Some(FilterType::IsSome) => span_lint_and_sugg(
Expand All @@ -190,7 +189,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, filter_arg: &hir
filter_span.with_hi(expr.span.hi()),
"`filter` for `is_some` on iterator over `Option`",
"consider using `flatten` instead",
reindent_multiline(Cow::Borrowed("flatten()"), true, indent_of(cx, filter_span)).into_owned(),
reindent_multiline("flatten()", true, indent_of(cx, filter_span)),
Applicability::HasPlaceholders,
),
}
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/manual_ok_or.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ pub(super) fn check<'tcx>(
&& let Some(err_arg_snippet) = err_arg.span.get_source_text(cx)
&& let Some(indent) = indent_of(cx, expr.span)
{
let reindented_err_arg_snippet = reindent_multiline(err_arg_snippet.as_str().into(), true, Some(indent + 4));
let reindented_err_arg_snippet = reindent_multiline(err_arg_snippet.as_str(), true, Some(indent + 4));
span_lint_and_sugg(
cx,
MANUAL_OK_OR,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/needless_continue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ fn suggestion_snippet_for_continue_inside_else(cx: &EarlyContext<'_>, data: &Lin
.iter()
.map(|stmt| {
let span = cx.sess().source_map().stmt_span(stmt.span, data.loop_block.span);
let snip = snippet_block(cx, span, "..", None).into_owned();
let snip = snippet_block(cx, span, "..", None);
snip.lines()
.map(|line| format!("{}{line}", " ".repeat(indent)))
.collect::<Vec<_>>()
Expand Down
10 changes: 2 additions & 8 deletions clippy_lints/src/redundant_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::declare_lint_pass;
use rustc_span::Span;
use std::borrow::Cow;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -164,14 +163,9 @@ fn extract_else_block(mut block: &str) -> String {
block.trim_end().to_string()
}

fn make_sugg<'a>(
cx: &EarlyContext<'_>,
els_span: Span,
default: &'a str,
indent_relative_to: Option<Span>,
) -> Cow<'a, str> {
fn make_sugg(cx: &EarlyContext<'_>, els_span: Span, default: &str, indent_relative_to: Option<Span>) -> String {
let extracted = extract_else_block(&snippet(cx, els_span, default));
let indent = indent_relative_to.and_then(|s| indent_of(cx, s));

reindent_multiline(extracted.into(), false, indent)
reindent_multiline(&extracted, false, indent)
}
5 changes: 2 additions & 3 deletions clippy_lints/src/unit_types/unit_arg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,15 @@ fn fmt_stmts_and_call(
stmts_and_call.push(call_snippet_with_replacements);
stmts_and_call = stmts_and_call
.into_iter()
.map(|v| reindent_multiline(v.into(), true, Some(call_expr_indent)).into_owned())
.map(|v| reindent_multiline(&v, true, Some(call_expr_indent)))
.collect();

let mut stmts_and_call_snippet = stmts_and_call.join(&format!("{}{}", ";\n", " ".repeat(call_expr_indent)));
// expr is not in a block statement or result expression position, wrap in a block
let parent_node = cx.tcx.parent_hir_node(call_expr.hir_id);
if !matches!(parent_node, Node::Block(_)) && !matches!(parent_node, Node::Stmt(_)) {
let block_indent = call_expr_indent + 4;
stmts_and_call_snippet =
reindent_multiline(stmts_and_call_snippet.into(), true, Some(block_indent)).into_owned();
stmts_and_call_snippet = reindent_multiline(&stmts_and_call_snippet, true, Some(block_indent));
stmts_and_call_snippet = format!(
"{{\n{}{}\n{}}}",
" ".repeat(block_indent),
Expand Down
52 changes: 23 additions & 29 deletions clippy_utils/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ pub fn expr_block(
&& let ExprKind::Block(block, _) = expr.kind
&& block.rules != BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided)
{
format!("{code}")
code
} else {
// FIXME: add extra indent for the unsafe blocks:
// original code: unsafe { ... }
Expand Down Expand Up @@ -420,11 +420,10 @@ pub fn position_before_rarrow(s: &str) -> Option<usize> {
}

/// Reindent a multiline string with possibility of ignoring the first line.
#[expect(clippy::needless_pass_by_value)]
pub fn reindent_multiline(s: Cow<'_, str>, ignore_first: bool, indent: Option<usize>) -> Cow<'_, str> {
let s_space = reindent_multiline_inner(&s, ignore_first, indent, ' ');
pub fn reindent_multiline(s: &str, ignore_first: bool, indent: Option<usize>) -> String {
let s_space = reindent_multiline_inner(s, ignore_first, indent, ' ');
let s_tab = reindent_multiline_inner(&s_space, ignore_first, indent, '\t');
reindent_multiline_inner(&s_tab, ignore_first, indent, ' ').into()
reindent_multiline_inner(&s_tab, ignore_first, indent, ' ')
}

fn reindent_multiline_inner(s: &str, ignore_first: bool, indent: Option<usize>, ch: char) -> String {
Expand Down Expand Up @@ -552,42 +551,37 @@ pub fn snippet_opt(sess: &impl HasSession, span: Span) -> Option<String> {
/// } // aligned with `if`
/// ```
/// Note that the first line of the snippet always has 0 indentation.
pub fn snippet_block<'a>(
sess: &impl HasSession,
span: Span,
default: &'a str,
indent_relative_to: Option<Span>,
) -> Cow<'a, str> {
pub fn snippet_block(sess: &impl HasSession, span: Span, default: &str, indent_relative_to: Option<Span>) -> String {
let snip = snippet(sess, span, default);
let indent = indent_relative_to.and_then(|s| indent_of(sess, s));
reindent_multiline(snip, true, indent)
reindent_multiline(&snip, true, indent)
}

/// Same as `snippet_block`, but adapts the applicability level by the rules of
/// `snippet_with_applicability`.
pub fn snippet_block_with_applicability<'a>(
pub fn snippet_block_with_applicability(
sess: &impl HasSession,
span: Span,
default: &'a str,
default: &str,
indent_relative_to: Option<Span>,
applicability: &mut Applicability,
) -> Cow<'a, str> {
) -> String {
let snip = snippet_with_applicability(sess, span, default, applicability);
let indent = indent_relative_to.and_then(|s| indent_of(sess, s));
reindent_multiline(snip, true, indent)
reindent_multiline(&snip, true, indent)
}

pub fn snippet_block_with_context<'a>(
pub fn snippet_block_with_context(
sess: &impl HasSession,
span: Span,
outer: SyntaxContext,
default: &'a str,
default: &str,
indent_relative_to: Option<Span>,
app: &mut Applicability,
) -> (Cow<'a, str>, bool) {
) -> (String, bool) {
let (snip, from_macro) = snippet_with_context(sess, span, outer, default, app);
let indent = indent_relative_to.and_then(|s| indent_of(sess, s));
(reindent_multiline(snip, true, indent), from_macro)
(reindent_multiline(&snip, true, indent), from_macro)
}

/// Same as `snippet_with_applicability`, but first walks the span up to the given context.
Expand Down Expand Up @@ -745,11 +739,11 @@ mod test {

#[test]
fn test_reindent_multiline_single_line() {
assert_eq!("", reindent_multiline("".into(), false, None));
assert_eq!("...", reindent_multiline("...".into(), false, None));
assert_eq!("...", reindent_multiline(" ...".into(), false, None));
assert_eq!("...", reindent_multiline("\t...".into(), false, None));
assert_eq!("...", reindent_multiline("\t\t...".into(), false, None));
assert_eq!("", reindent_multiline("", false, None));
assert_eq!("...", reindent_multiline("...", false, None));
assert_eq!("...", reindent_multiline(" ...", false, None));
assert_eq!("...", reindent_multiline("\t...", false, None));
assert_eq!("...", reindent_multiline("\t\t...", false, None));
}

#[test]
Expand All @@ -764,7 +758,7 @@ mod test {
y
} else {
z
}".into(), false, None));
}", false, None));
assert_eq!("\
if x {
\ty
Expand All @@ -774,7 +768,7 @@ mod test {
\ty
} else {
\tz
}".into(), false, None));
}", false, None));
}

#[test]
Expand All @@ -791,7 +785,7 @@ mod test {
} else {
z
}".into(), false, None));
}", false, None));
}

#[test]
Expand All @@ -807,6 +801,6 @@ mod test {
y
} else {
z
}".into(), true, Some(8)));
}", true, Some(8)));
}
}

0 comments on commit ca00f88

Please sign in to comment.