Skip to content

Commit ba6ce40

Browse files
committed
keep track of lines which formatting was disabled in order to prevent indentation which would cause code right-shifting
1 parent e1f334d commit ba6ce40

File tree

7 files changed

+129
-79
lines changed

7 files changed

+129
-79
lines changed

src/comment.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,7 +620,7 @@ impl<'a> CommentRewrite<'a> {
620620
let mut config = self.fmt.config.clone();
621621
config.set().format_doc_comments(false);
622622
match ::format_code_block(&self.code_block_buffer, &config) {
623-
Some(ref s) => trim_custom_comment_prefix(s),
623+
Some(ref s) => trim_custom_comment_prefix(&s.snippet),
624624
None => trim_custom_comment_prefix(&self.code_block_buffer),
625625
}
626626
}

src/formatting.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,8 @@ impl<'a, T: FormatHandler + 'a> FormatContext<'a, T> {
173173
if visitor.macro_rewrite_failure {
174174
self.report.add_macro_format_failure();
175175
}
176+
self.report
177+
.add_non_formatted_ranges(visitor.skipped_range.clone());
176178

177179
self.handler
178180
.handle_formatted_file(path, visitor.buffer.to_owned(), &mut self.report)

src/lib.rs

Lines changed: 66 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -144,22 +144,56 @@ impl From<io::Error> for ErrorKind {
144144
}
145145
}
146146

147+
/// Result of formatting a snippet of code along with ranges of lines that didn't get formatted,
148+
/// i.e., that got returned as they were originally.
149+
#[derive(Debug)]
150+
struct FormattedSnippet {
151+
snippet: String,
152+
non_formatted_ranges: Vec<(usize, usize)>,
153+
}
154+
155+
impl FormattedSnippet {
156+
/// In case the snippet needed to be wrapped in a function, this shifts down the ranges of
157+
/// non-formatted code.
158+
fn unwrap_code_block(&mut self) {
159+
self.non_formatted_ranges
160+
.iter_mut()
161+
.for_each(|(low, high)| {
162+
*low -= 1;
163+
*high -= 1;
164+
});
165+
}
166+
167+
/// Returns true if the line n did not get formatted.
168+
fn is_line_non_formatted(&self, n: usize) -> bool {
169+
self.non_formatted_ranges
170+
.iter()
171+
.any(|(low, high)| *low <= n && n <= *high)
172+
}
173+
}
174+
147175
/// Reports on any issues that occurred during a run of Rustfmt.
148176
///
149177
/// Can be reported to the user via its `Display` implementation of `print_fancy`.
150178
#[derive(Clone)]
151179
pub struct FormatReport {
152180
// Maps stringified file paths to their associated formatting errors.
153181
internal: Rc<RefCell<(FormatErrorMap, ReportedErrors)>>,
182+
non_formatted_ranges: Vec<(usize, usize)>,
154183
}
155184

156185
impl FormatReport {
157186
fn new() -> FormatReport {
158187
FormatReport {
159188
internal: Rc::new(RefCell::new((HashMap::new(), ReportedErrors::default()))),
189+
non_formatted_ranges: Vec::new(),
160190
}
161191
}
162192

193+
fn add_non_formatted_ranges(&mut self, mut ranges: Vec<(usize, usize)>) {
194+
self.non_formatted_ranges.append(&mut ranges);
195+
}
196+
163197
fn append(&self, f: FileName, mut v: Vec<FormattingError>) {
164198
self.track_errors(&v);
165199
self.internal
@@ -349,37 +383,44 @@ impl fmt::Display for FormatReport {
349383

350384
/// Format the given snippet. The snippet is expected to be *complete* code.
351385
/// When we cannot parse the given snippet, this function returns `None`.
352-
fn format_snippet(snippet: &str, config: &Config) -> Option<String> {
386+
fn format_snippet(snippet: &str, config: &Config) -> Option<FormattedSnippet> {
353387
let mut config = config.clone();
354-
let out = panic::catch_unwind(|| {
388+
panic::catch_unwind(|| {
355389
let mut out: Vec<u8> = Vec::with_capacity(snippet.len() * 2);
356390
config.set().emit_mode(config::EmitMode::Stdout);
357391
config.set().verbose(Verbosity::Quiet);
358392
config.set().hide_parse_errors(true);
359-
let formatting_error = {
393+
394+
let (formatting_error, result) = {
360395
let input = Input::Text(snippet.into());
361396
let mut session = Session::new(config, Some(&mut out));
362397
let result = session.format(input);
363-
session.errors.has_macro_format_failure
364-
|| session.out.as_ref().unwrap().is_empty() && !snippet.is_empty()
365-
|| result.is_err()
398+
(
399+
session.errors.has_macro_format_failure
400+
|| session.out.as_ref().unwrap().is_empty() && !snippet.is_empty()
401+
|| result.is_err(),
402+
result,
403+
)
366404
};
367405
if formatting_error {
368406
None
369407
} else {
370-
Some(out)
408+
String::from_utf8(out).ok().map(|snippet| FormattedSnippet {
409+
snippet,
410+
non_formatted_ranges: result.unwrap().non_formatted_ranges,
411+
})
371412
}
372413
})
373-
.ok()??; // The first try operator handles the error from catch_unwind,
374-
// whereas the second one handles None from the closure.
375-
String::from_utf8(out).ok()
414+
// Discard panics encountered while formatting the snippet
415+
// The ? operator is needed to remove the extra Option
416+
.ok()?
376417
}
377418

378419
/// Format the given code block. Mainly targeted for code block in comment.
379420
/// The code block may be incomplete (i.e. parser may be unable to parse it).
380421
/// To avoid panic in parser, we wrap the code block with a dummy function.
381422
/// The returned code block does *not* end with newline.
382-
fn format_code_block(code_snippet: &str, config: &Config) -> Option<String> {
423+
fn format_code_block(code_snippet: &str, config: &Config) -> Option<FormattedSnippet> {
383424
const FN_MAIN_PREFIX: &str = "fn main() {\n";
384425

385426
fn enclose_in_main_block(s: &str, config: &Config) -> String {
@@ -412,13 +453,18 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option<String> {
412453
config_with_unix_newline
413454
.set()
414455
.newline_style(NewlineStyle::Unix);
415-
let formatted = format_snippet(&snippet, &config_with_unix_newline)?;
456+
let mut formatted = format_snippet(&snippet, &config_with_unix_newline)?;
457+
// Remove wrapping main block
458+
formatted.unwrap_code_block();
416459

417460
// Trim "fn main() {" on the first line and "}" on the last line,
418461
// then unindent the whole code block.
419-
let block_len = formatted.rfind('}').unwrap_or(formatted.len());
462+
let block_len = formatted
463+
.snippet
464+
.rfind('}')
465+
.unwrap_or(formatted.snippet.len());
420466
let mut is_indented = true;
421-
for (kind, ref line) in LineClasses::new(&formatted[FN_MAIN_PREFIX.len()..block_len]) {
467+
for (kind, ref line) in LineClasses::new(&formatted.snippet[FN_MAIN_PREFIX.len()..block_len]) {
422468
if !is_first {
423469
result.push('\n');
424470
} else {
@@ -451,7 +497,10 @@ fn format_code_block(code_snippet: &str, config: &Config) -> Option<String> {
451497
result.push_str(trimmed_line);
452498
is_indented = !kind.is_string() || line.ends_with('\\');
453499
}
454-
Some(result)
500+
Some(FormattedSnippet {
501+
snippet: result,
502+
non_formatted_ranges: formatted.non_formatted_ranges,
503+
})
455504
}
456505

457506
/// A session is a run of rustfmt across a single or multiple inputs.
@@ -571,10 +620,10 @@ mod unit_tests {
571620

572621
fn test_format_inner<F>(formatter: F, input: &str, expected: &str) -> bool
573622
where
574-
F: Fn(&str, &Config) -> Option<String>,
623+
F: Fn(&str, &Config) -> Option<FormattedSnippet>,
575624
{
576625
let output = formatter(input, &Config::default());
577-
output.is_some() && output.unwrap() == expected
626+
output.is_some() && output.unwrap().snippet == expected
578627
}
579628

580629
#[test]

src/macros.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,7 @@ impl MacroBranch {
13231323
config.set().max_width(new_width);
13241324

13251325
// First try to format as items, then as statements.
1326-
let new_body = match ::format_snippet(&body_str, &config) {
1326+
let new_body_snippet = match ::format_snippet(&body_str, &config) {
13271327
Some(new_body) => new_body,
13281328
None => {
13291329
let new_width = new_width + config.tab_spaces();
@@ -1334,15 +1334,23 @@ impl MacroBranch {
13341334
}
13351335
}
13361336
};
1337-
let new_body = wrap_str(new_body, config.max_width(), shape)?;
1337+
let new_body = wrap_str(
1338+
new_body_snippet.snippet.to_string(),
1339+
config.max_width(),
1340+
shape,
1341+
)?;
13381342

13391343
// Indent the body since it is in a block.
13401344
let indent_str = body_indent.to_string(&config);
13411345
let mut new_body = LineClasses::new(new_body.trim_right())
1346+
.enumerate()
13421347
.fold(
13431348
(String::new(), true),
1344-
|(mut s, need_indent), (kind, ref l)| {
1345-
if !l.is_empty() && need_indent {
1349+
|(mut s, need_indent), (i, (kind, ref l))| {
1350+
if !l.is_empty()
1351+
&& need_indent
1352+
&& !new_body_snippet.is_line_non_formatted(i + 1)
1353+
{
13461354
s += &indent_str;
13471355
}
13481356
(s + l + "\n", !kind.is_string() || l.ends_with('\\'))

src/visitor.rs

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use source_map::{LineRangeUtils, SpanUtils};
2929
use spanned::Spanned;
3030
use utils::{
3131
self, contains_skip, count_newlines, inner_attributes, mk_sp, ptr_vec_to_ref_vec,
32-
rewrite_ident, trim_left_preserve_layout, DEPR_SKIP_ANNOTATION,
32+
rewrite_ident, DEPR_SKIP_ANNOTATION,
3333
};
3434
use {ErrorKind, FormatReport, FormattingError};
3535

@@ -71,6 +71,8 @@ pub struct FmtVisitor<'a> {
7171
pub is_if_else_block: bool,
7272
pub snippet_provider: &'a SnippetProvider<'a>,
7373
pub line_number: usize,
74+
/// List of 1-based line ranges which were annotated with skip
75+
/// Both bounds are inclusifs.
7476
pub skipped_range: Vec<(usize, usize)>,
7577
pub macro_rewrite_failure: bool,
7678
pub(crate) report: FormatReport,
@@ -109,8 +111,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
109111
self.format_missing(stmt.span.hi());
110112
}
111113
ast::StmtKind::Local(..) | ast::StmtKind::Expr(..) | ast::StmtKind::Semi(..) => {
112-
if contains_skip(get_attrs_from_stmt(stmt)) {
113-
self.push_skipped_with_span(stmt.span());
114+
let attrs = get_attrs_from_stmt(stmt);
115+
if contains_skip(attrs) {
116+
self.push_skipped_with_span(attrs, stmt.span());
114117
} else {
115118
let shape = self.shape();
116119
let rewrite = self.with_context(|ctx| stmt.rewrite(&ctx, shape));
@@ -120,7 +123,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
120123
ast::StmtKind::Mac(ref mac) => {
121124
let (ref mac, _macro_style, ref attrs) = **mac;
122125
if self.visit_attrs(attrs, ast::AttrStyle::Outer) {
123-
self.push_skipped_with_span(stmt.span());
126+
self.push_skipped_with_span(attrs, stmt.span());
124127
} else {
125128
self.visit_mac(mac, None, MacroPosition::Statement);
126129
}
@@ -328,14 +331,14 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
328331
// For use items, skip rewriting attributes. Just check for a skip attribute.
329332
ast::ItemKind::Use(..) => {
330333
if contains_skip(attrs) {
331-
self.push_skipped_with_span(item.span());
334+
self.push_skipped_with_span(attrs.as_slice(), item.span());
332335
return;
333336
}
334337
}
335338
// Module is inline, in this case we treat it like any other item.
336339
_ if !is_mod_decl(item) => {
337340
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
338-
self.push_skipped_with_span(item.span());
341+
self.push_skipped_with_span(item.attrs.as_slice(), item.span());
339342
return;
340343
}
341344
}
@@ -354,7 +357,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
354357
}
355358
_ => {
356359
if self.visit_attrs(&item.attrs, ast::AttrStyle::Outer) {
357-
self.push_skipped_with_span(item.span());
360+
self.push_skipped_with_span(item.attrs.as_slice(), item.span());
358361
return;
359362
}
360363
}
@@ -471,7 +474,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
471474
skip_out_of_file_lines_range_visitor!(self, ti.span);
472475

473476
if self.visit_attrs(&ti.attrs, ast::AttrStyle::Outer) {
474-
self.push_skipped_with_span(ti.span());
477+
self.push_skipped_with_span(ti.attrs.as_slice(), ti.span());
475478
return;
476479
}
477480

@@ -515,7 +518,7 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
515518
skip_out_of_file_lines_range_visitor!(self, ii.span);
516519

517520
if self.visit_attrs(&ii.attrs, ast::AttrStyle::Outer) {
518-
self.push_skipped_with_span(ii.span());
521+
self.push_skipped_with_span(ii.attrs.as_slice(), ii.span());
519522
return;
520523
}
521524

@@ -574,16 +577,9 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
574577
}
575578

576579
#[allow(clippy::needless_pass_by_value)]
577-
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>, is_skipped: bool) {
580+
fn push_rewrite_inner(&mut self, span: Span, rewrite: Option<String>) {
578581
if let Some(ref s) = rewrite {
579582
self.push_str(s);
580-
} else if is_skipped {
581-
// in case the code block (e.g., inside a macro or a doc) is skipped a minimum of
582-
// leading whitespaces is trimmed so that the code layout is kept but allows it to
583-
// be indented as necessary
584-
let snippet =
585-
trim_left_preserve_layout(self.snippet(span), &self.block_indent, self.config);
586-
self.push_str(&snippet);
587583
} else {
588584
let snippet = self.snippet(span);
589585
self.push_str(snippet);
@@ -593,13 +589,20 @@ impl<'b, 'a: 'b> FmtVisitor<'a> {
593589

594590
pub fn push_rewrite(&mut self, span: Span, rewrite: Option<String>) {
595591
self.format_missing_with_indent(source!(self, span).lo());
596-
self.push_rewrite_inner(span, rewrite, false);
592+
self.push_rewrite_inner(span, rewrite);
597593
}
598594

599-
pub fn push_skipped_with_span(&mut self, span: Span) {
600-
self.format_missing_with_indent(source!(self, span).lo());
601-
let lo = self.line_number + 1;
602-
self.push_rewrite_inner(span, None, true);
595+
pub fn push_skipped_with_span(&mut self, attrs: &[ast::Attribute], item_span: Span) {
596+
self.format_missing_with_indent(source!(self, item_span).lo());
597+
// do not take into account the lines with attributes as part of the skipped range
598+
let attrs_end = attrs
599+
.iter()
600+
.map(|attr| self.source_map.lookup_char_pos(attr.span().hi()).line)
601+
.max()
602+
.unwrap_or(1);
603+
// Add 1 to get the line past the last attribute
604+
let lo = attrs_end + 1;
605+
self.push_rewrite_inner(item_span, None);
603606
let hi = self.line_number + 1;
604607
self.skipped_range.push((lo, hi));
605608
}

tests/source/issue-3105.rs

Lines changed: 0 additions & 30 deletions
This file was deleted.

0 commit comments

Comments
 (0)