Skip to content

Commit d2d8ae6

Browse files
borsbors
committed
Auto merge of #39214 - estebank:fix-labels-without-msg, r=nikomatsakis
Fix multiple labels when some don't have message The diagnostic emitter now accounts for labels with no text message, presenting the underline on its own, without drawing the line for the non existing message below it. Go from ``` error: foo --> test.rs:3:6 | 3 | a { b { c } d } | ----^^^^^^^---- | | | | | `b` is a good letter | ``` to ``` error: foo --> test.rs:3:6 | 3 | a { b { c } d } | ----^^^^^^^---- | | | `b` is a good letter ``` from ``` error: foo --> test.rs:3:6 | 3 | a { b { c } d } | ^^^^-------^^^^ | | | | | | `a` is a good letter ``` to ``` error: foo --> test.rs:3:6 | 3 | a { b { c } d } | ^^^^-------^^^^ `a` is a good letter ``` and from ``` error: foo --> test.rs:3:6 | 3 | a { b { c } d } | ^^^^-------^^^^ | | | | | | ``` to ``` error: foo --> test.rs:3:6 | 3 | a { b { c } d } | ^^^^-------^^^^ ``` r? @nikomatsakis cc @jonathandturner, @GuillaumeGomez, @nrc
2 parents 65b17f5 + 469ecef commit d2d8ae6

File tree

3 files changed

+470
-30
lines changed

3 files changed

+470
-30
lines changed

‎src/librustc_errors/emitter.rs

+47-30
Original file line numberDiff line numberDiff line change
@@ -339,43 +339,56 @@ impl EmitterWriter {
339339
// which is...less weird, at least. In fact, in general, if
340340
// the rightmost span overlaps with any other span, we should
341341
// use the "hang below" version, so we can at least make it
342-
// clear where the span *starts*.
342+
// clear where the span *starts*. There's an exception for this
343+
// logic, when the labels do not have a message:
344+
//
345+
// fn foo(x: u32) {
346+
// --------------
347+
// |
348+
// x_span
349+
//
350+
// instead of:
351+
//
352+
// fn foo(x: u32) {
353+
// --------------
354+
// | |
355+
// | x_span
356+
// <EMPTY LINE>
357+
//
343358
let mut annotations_position = vec![];
344359
let mut line_len = 0;
345360
let mut p = 0;
346361
let mut ann_iter = annotations.iter().peekable();
347362
while let Some(annotation) = ann_iter.next() {
348-
let is_line = if let AnnotationType::MultilineLine(_) = annotation.annotation_type {
349-
true
350-
} else {
351-
false
352-
};
353363
let peek = ann_iter.peek();
354364
if let Some(next) = peek {
355-
let next_is_line = if let AnnotationType::MultilineLine(_) = next.annotation_type {
356-
true
357-
} else {
358-
false
359-
};
360-
361-
if overlaps(next, annotation) && !is_line && !next_is_line {
365+
if overlaps(next, annotation) && !annotation.is_line() && !next.is_line()
366+
&& annotation.has_label()
367+
{
368+
// This annotation needs a new line in the output.
362369
p += 1;
363370
}
364371
}
365372
annotations_position.push((p, annotation));
366373
if let Some(next) = peek {
367-
let next_is_line = if let AnnotationType::MultilineLine(_) = next.annotation_type {
368-
true
369-
} else {
370-
false
371-
};
372374
let l = if let Some(ref label) = next.label {
373375
label.len() + 2
374376
} else {
375377
0
376378
};
377-
if (overlaps(next, annotation) || next.end_col + l > annotation.start_col)
378-
&& !is_line && !next_is_line
379+
if (overlaps(next, annotation) // Do not allow two labels to be in the same line
380+
|| next.end_col + l > annotation.start_col) // if they overlap including
381+
// padding, to avoid situations like:
382+
//
383+
// fn foo(x: u32) {
384+
// -------^------
385+
// | |
386+
// fn_spanx_span
387+
//
388+
&& !annotation.is_line() // Do not add a new line if this annotation or the
389+
&& !next.is_line() // next are vertical line placeholders.
390+
&& annotation.has_label() // Both labels must have some text, otherwise
391+
&& next.has_label() // they are not overlapping.
379392
{
380393
p += 1;
381394
}
@@ -412,6 +425,17 @@ impl EmitterWriter {
412425
return;
413426
}
414427

428+
// Write the colunmn separator.
429+
//
430+
// After this we will have:
431+
//
432+
// 2 | fn foo() {
433+
// |
434+
// |
435+
// |
436+
// 3 |
437+
// 4 | }
438+
// |
415439
for pos in 0..line_len + 1 {
416440
draw_col_separator(buffer, line_offset + pos + 1, width_offset - 2);
417441
buffer.putc(line_offset + pos + 1,
@@ -472,7 +496,8 @@ impl EmitterWriter {
472496
Style::UnderlineSecondary
473497
};
474498
let pos = pos + 1;
475-
if pos > 1 {
499+
500+
if pos > 1 && annotation.has_label() {
476501
for p in line_offset + 1..line_offset + pos + 1 {
477502
buffer.putc(p,
478503
code_offset + annotation.start_col,
@@ -550,16 +575,8 @@ impl EmitterWriter {
550575
// | | something about `foo`
551576
// | something about `fn foo()`
552577
annotations_position.sort_by(|a, b| {
553-
fn len(a: &Annotation) -> usize {
554-
// Account for usize underflows
555-
if a.end_col > a.start_col {
556-
a.end_col - a.start_col
557-
} else {
558-
a.start_col - a.end_col
559-
}
560-
}
561578
// Decreasing order
562-
len(a.1).cmp(&len(b.1)).reverse()
579+
a.1.len().cmp(&b.1.len()).reverse()
563580
});
564581

565582
// Write the underlines.

‎src/librustc_errors/snippet.rs

+35
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,15 @@ impl Annotation {
151151
}
152152
}
153153

154+
/// Wether this annotation is a vertical line placeholder.
155+
pub fn is_line(&self) -> bool {
156+
if let AnnotationType::MultilineLine(_) = self.annotation_type {
157+
true
158+
} else {
159+
false
160+
}
161+
}
162+
154163
pub fn is_multiline(&self) -> bool {
155164
match self.annotation_type {
156165
AnnotationType::Multiline(_) |
@@ -161,6 +170,32 @@ impl Annotation {
161170
}
162171
}
163172

173+
pub fn len(&self) -> usize {
174+
// Account for usize underflows
175+
if self.end_col > self.start_col {
176+
self.end_col - self.start_col
177+
} else {
178+
self.start_col - self.end_col
179+
}
180+
}
181+
182+
pub fn has_label(&self) -> bool {
183+
if let Some(ref label) = self.label {
184+
// Consider labels with no text as effectively not being there
185+
// to avoid weird output with unnecessary vertical lines, like:
186+
//
187+
// X | fn foo(x: u32) {
188+
// | -------^------
189+
// | | |
190+
// | |
191+
// |
192+
//
193+
// Note that this would be the complete output users would see.
194+
label.len() > 0
195+
} else {
196+
false
197+
}
198+
}
164199
}
165200

166201
#[derive(Debug)]

0 commit comments

Comments
 (0)