Skip to content

Commit 5fd22b3

Browse files
committed
Optionally indent snippet_block relative to an Expr
1 parent a6f310e commit 5fd22b3

File tree

4 files changed

+96
-35
lines changed

4 files changed

+96
-35
lines changed

clippy_lints/src/attrs.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use crate::reexport::*;
44
use crate::utils::{
5-
is_present_in_source, last_line_of_span, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
5+
first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
66
span_lint_and_then, without_block_comments,
77
};
88
use if_chain::if_chain;
@@ -261,7 +261,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Attributes {
261261
_ => {},
262262
}
263263
}
264-
let line_span = last_line_of_span(cx, attr.span);
264+
let line_span = first_line_of_span(cx, attr.span);
265265

266266
if let Some(mut sugg) = snippet_opt(cx, line_span) {
267267
if sugg.contains("#[") {

clippy_lints/src/collapsible_if.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ fn check_if(cx: &EarlyContext<'_>, expr: &ast::Expr) {
9595

9696
fn block_starts_with_comment(cx: &EarlyContext<'_>, expr: &ast::Block) -> bool {
9797
// We trim all opening braces and whitespaces and then check if the next string is a comment.
98-
let trimmed_block_text = snippet_block(cx, expr.span, "..")
98+
let trimmed_block_text = snippet_block(cx, expr.span, "..", None)
9999
.trim_start_matches(|c: char| c.is_whitespace() || c == '{')
100100
.to_owned();
101101
trimmed_block_text.starts_with("//") || trimmed_block_text.starts_with("/*")
@@ -116,7 +116,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
116116
block.span,
117117
"this `else { if .. }` block can be collapsed",
118118
"try",
119-
snippet_block_with_applicability(cx, else_.span, "..", &mut applicability).into_owned(),
119+
snippet_block_with_applicability(cx, else_.span, "..", Some(block.span), &mut applicability).into_owned(),
120120
applicability,
121121
);
122122
}
@@ -146,7 +146,7 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &
146146
format!(
147147
"if {} {}",
148148
lhs.and(&rhs),
149-
snippet_block(cx, content.span, ".."),
149+
snippet_block(cx, content.span, "..", Some(expr.span)),
150150
),
151151
Applicability::MachineApplicable, // snippet
152152
);

clippy_lints/src/matches.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ fn report_single_match_single_pattern(
434434
) {
435435
let lint = if els.is_some() { SINGLE_MATCH_ELSE } else { SINGLE_MATCH };
436436
let els_str = els.map_or(String::new(), |els| {
437-
format!(" else {}", expr_block(cx, els, None, ".."))
437+
format!(" else {}", expr_block(cx, els, None, "..", Some(expr.span)))
438438
});
439439
span_lint_and_sugg(
440440
cx,
@@ -447,7 +447,7 @@ fn report_single_match_single_pattern(
447447
"if let {} = {} {}{}",
448448
snippet(cx, arms[0].pat.span, ".."),
449449
snippet(cx, ex.span, ".."),
450-
expr_block(cx, &arms[0].body, None, ".."),
450+
expr_block(cx, &arms[0].body, None, "..", Some(expr.span)),
451451
els_str,
452452
),
453453
Applicability::HasPlaceholders,
@@ -523,17 +523,21 @@ fn check_match_bool(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], e
523523
(false, false) => Some(format!(
524524
"if {} {} else {}",
525525
snippet(cx, ex.span, "b"),
526-
expr_block(cx, true_expr, None, ".."),
527-
expr_block(cx, false_expr, None, "..")
526+
expr_block(cx, true_expr, None, "..", Some(expr.span)),
527+
expr_block(cx, false_expr, None, "..", Some(expr.span))
528528
)),
529529
(false, true) => Some(format!(
530530
"if {} {}",
531531
snippet(cx, ex.span, "b"),
532-
expr_block(cx, true_expr, None, "..")
532+
expr_block(cx, true_expr, None, "..", Some(expr.span))
533533
)),
534534
(true, false) => {
535535
let test = Sugg::hir(cx, ex, "..");
536-
Some(format!("if {} {}", !test, expr_block(cx, false_expr, None, "..")))
536+
Some(format!(
537+
"if {} {}",
538+
!test,
539+
expr_block(cx, false_expr, None, "..", Some(expr.span))
540+
))
537541
},
538542
(true, true) => None,
539543
};

clippy_lints/src/utils/mod.rs

Lines changed: 81 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ use rustc_hir::Node;
4444
use rustc_hir::*;
4545
use rustc_lint::{LateContext, Level, Lint, LintContext};
4646
use rustc_span::hygiene::{ExpnKind, MacroKind};
47+
use rustc_span::source_map::original_sp;
4748
use rustc_span::symbol::{self, kw, Symbol};
4849
use rustc_span::{BytePos, Pos, Span, DUMMY_SP};
4950
use smallvec::SmallVec;
@@ -541,11 +542,17 @@ pub fn snippet_opt<T: LintContext>(cx: &T, span: Span) -> Option<String> {
541542
///
542543
/// # Example
543544
/// ```rust,ignore
544-
/// snippet_block(cx, expr.span, "..")
545+
/// snippet_block(cx, expr.span, "..", None)
545546
/// ```
546-
pub fn snippet_block<'a, T: LintContext>(cx: &T, span: Span, default: &'a str) -> Cow<'a, str> {
547+
pub fn snippet_block<'a, T: LintContext>(
548+
cx: &T,
549+
span: Span,
550+
default: &'a str,
551+
indent_relative_to: Option<Span>,
552+
) -> Cow<'a, str> {
547553
let snip = snippet(cx, span, default);
548-
trim_multiline(snip, true)
554+
let indent = indent_relative_to.and_then(|s| indent_of(cx, s));
555+
trim_multiline(snip, true, indent)
549556
}
550557

551558
/// Same as `snippet_block`, but adapts the applicability level by the rules of
@@ -554,27 +561,73 @@ pub fn snippet_block_with_applicability<'a, T: LintContext>(
554561
cx: &T,
555562
span: Span,
556563
default: &'a str,
564+
indent_relative_to: Option<Span>,
557565
applicability: &mut Applicability,
558566
) -> Cow<'a, str> {
559567
let snip = snippet_with_applicability(cx, span, default, applicability);
560-
trim_multiline(snip, true)
568+
let indent = indent_relative_to.and_then(|s| indent_of(cx, s));
569+
trim_multiline(snip, true, indent)
561570
}
562571

563-
/// Returns a new Span that covers the full last line of the given Span
564-
pub fn last_line_of_span<T: LintContext>(cx: &T, span: Span) -> Span {
572+
/// Returns a new Span that extends the original Span to the first non-whitespace char of the first
573+
/// line.
574+
///
575+
/// ```rust,ignore
576+
/// let x = ();
577+
/// // ^^
578+
/// // will be converted to
579+
/// let x = ();
580+
/// // ^^^^^^^^^^
581+
/// ```
582+
pub fn first_line_of_span<T: LintContext>(cx: &T, span: Span) -> Span {
583+
if let Some(first_char_pos) = first_char_in_first_line(cx, span) {
584+
span.with_lo(first_char_pos)
585+
} else {
586+
span
587+
}
588+
}
589+
590+
fn first_char_in_first_line<T: LintContext>(cx: &T, span: Span) -> Option<BytePos> {
591+
let line_span = line_span(cx, span);
592+
if let Some(snip) = snippet_opt(cx, line_span) {
593+
snip.find(|c: char| !c.is_whitespace())
594+
.map(|pos| line_span.lo() + BytePos::from_usize(pos))
595+
} else {
596+
None
597+
}
598+
}
599+
600+
/// Returns the indentation of the line of a span
601+
///
602+
/// ```rust,ignore
603+
/// let x = ();
604+
/// // ^^ -- will return 0
605+
/// let x = ();
606+
/// // ^^ -- will return 4
607+
/// ```
608+
pub fn indent_of<T: LintContext>(cx: &T, span: Span) -> Option<usize> {
609+
if let Some(snip) = snippet_opt(cx, line_span(cx, span)) {
610+
snip.find(|c: char| !c.is_whitespace())
611+
} else {
612+
None
613+
}
614+
}
615+
616+
/// Extends the span to the beginning of the spans line, incl. whitespaces.
617+
///
618+
/// ```rust,ignore
619+
/// let x = ();
620+
/// // ^^
621+
/// // will be converted to
622+
/// let x = ();
623+
/// // ^^^^^^^^^^^^^^
624+
/// ```
625+
fn line_span<T: LintContext>(cx: &T, span: Span) -> Span {
626+
let span = original_sp(span, DUMMY_SP);
565627
let source_map_and_line = cx.sess().source_map().lookup_line(span.lo()).unwrap();
566628
let line_no = source_map_and_line.line;
567-
let line_start = &source_map_and_line.sf.lines[line_no];
568-
let span = Span::new(*line_start, span.hi(), span.ctxt());
569-
if_chain! {
570-
if let Some(snip) = snippet_opt(cx, span);
571-
if let Some(first_ch_pos) = snip.find(|c: char| !c.is_whitespace());
572-
then {
573-
span.with_lo(span.lo() + BytePos::from_usize(first_ch_pos))
574-
} else {
575-
span
576-
}
577-
}
629+
let line_start = source_map_and_line.sf.lines[line_no];
630+
Span::new(line_start, span.hi(), span.ctxt())
578631
}
579632

580633
/// Like `snippet_block`, but add braces if the expr is not an `ExprKind::Block`.
@@ -584,8 +637,9 @@ pub fn expr_block<'a, T: LintContext>(
584637
expr: &Expr<'_>,
585638
option: Option<String>,
586639
default: &'a str,
640+
indent_relative_to: Option<Span>,
587641
) -> Cow<'a, str> {
588-
let code = snippet_block(cx, expr.span, default);
642+
let code = snippet_block(cx, expr.span, default, indent_relative_to);
589643
let string = option.unwrap_or_default();
590644
if expr.span.from_expansion() {
591645
Cow::Owned(format!("{{ {} }}", snippet_with_macro_callsite(cx, expr.span, default)))
@@ -600,14 +654,14 @@ pub fn expr_block<'a, T: LintContext>(
600654

601655
/// Trim indentation from a multiline string with possibility of ignoring the
602656
/// first line.
603-
pub fn trim_multiline(s: Cow<'_, str>, ignore_first: bool) -> Cow<'_, str> {
604-
let s_space = trim_multiline_inner(s, ignore_first, ' ');
605-
let s_tab = trim_multiline_inner(s_space, ignore_first, '\t');
606-
trim_multiline_inner(s_tab, ignore_first, ' ')
657+
pub fn trim_multiline(s: Cow<'_, str>, ignore_first: bool, indent: Option<usize>) -> Cow<'_, str> {
658+
let s_space = trim_multiline_inner(s, ignore_first, indent, ' ');
659+
let s_tab = trim_multiline_inner(s_space, ignore_first, indent, '\t');
660+
trim_multiline_inner(s_tab, ignore_first, indent, ' ')
607661
}
608662

609-
fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, ch: char) -> Cow<'_, str> {
610-
let x = s
663+
fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, indent: Option<usize>, ch: char) -> Cow<'_, str> {
664+
let mut x = s
611665
.lines()
612666
.skip(ignore_first as usize)
613667
.filter_map(|l| {
@@ -620,6 +674,9 @@ fn trim_multiline_inner(s: Cow<'_, str>, ignore_first: bool, ch: char) -> Cow<'_
620674
})
621675
.min()
622676
.unwrap_or(0);
677+
if let Some(indent) = indent {
678+
x = x.saturating_sub(indent);
679+
}
623680
if x > 0 {
624681
Cow::Owned(
625682
s.lines()

0 commit comments

Comments
 (0)