Skip to content

Commit 469ecef

Browse files
committed
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 ``` and 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 ```
1 parent f0b4207 commit 469ecef

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
@@ -335,43 +335,56 @@ impl EmitterWriter {
335335
// which is...less weird, at least. In fact, in general, if
336336
// the rightmost span overlaps with any other span, we should
337337
// use the "hang below" version, so we can at least make it
338-
// clear where the span *starts*.
338+
// clear where the span *starts*. There's an exception for this
339+
// logic, when the labels do not have a message:
340+
//
341+
// fn foo(x: u32) {
342+
// --------------
343+
// |
344+
// x_span
345+
//
346+
// instead of:
347+
//
348+
// fn foo(x: u32) {
349+
// --------------
350+
// | |
351+
// | x_span
352+
// <EMPTY LINE>
353+
//
339354
let mut annotations_position = vec![];
340355
let mut line_len = 0;
341356
let mut p = 0;
342357
let mut ann_iter = annotations.iter().peekable();
343358
while let Some(annotation) = ann_iter.next() {
344-
let is_line = if let AnnotationType::MultilineLine(_) = annotation.annotation_type {
345-
true
346-
} else {
347-
false
348-
};
349359
let peek = ann_iter.peek();
350360
if let Some(next) = peek {
351-
let next_is_line = if let AnnotationType::MultilineLine(_) = next.annotation_type {
352-
true
353-
} else {
354-
false
355-
};
356-
357-
if overlaps(next, annotation) && !is_line && !next_is_line {
361+
if overlaps(next, annotation) && !annotation.is_line() && !next.is_line()
362+
&& annotation.has_label()
363+
{
364+
// This annotation needs a new line in the output.
358365
p += 1;
359366
}
360367
}
361368
annotations_position.push((p, annotation));
362369
if let Some(next) = peek {
363-
let next_is_line = if let AnnotationType::MultilineLine(_) = next.annotation_type {
364-
true
365-
} else {
366-
false
367-
};
368370
let l = if let Some(ref label) = next.label {
369371
label.len() + 2
370372
} else {
371373
0
372374
};
373-
if (overlaps(next, annotation) || next.end_col + l > annotation.start_col)
374-
&& !is_line && !next_is_line
375+
if (overlaps(next, annotation) // Do not allow two labels to be in the same line
376+
|| next.end_col + l > annotation.start_col) // if they overlap including
377+
// padding, to avoid situations like:
378+
//
379+
// fn foo(x: u32) {
380+
// -------^------
381+
// | |
382+
// fn_spanx_span
383+
//
384+
&& !annotation.is_line() // Do not add a new line if this annotation or the
385+
&& !next.is_line() // next are vertical line placeholders.
386+
&& annotation.has_label() // Both labels must have some text, otherwise
387+
&& next.has_label() // they are not overlapping.
375388
{
376389
p += 1;
377390
}
@@ -408,6 +421,17 @@ impl EmitterWriter {
408421
return;
409422
}
410423

424+
// Write the colunmn separator.
425+
//
426+
// After this we will have:
427+
//
428+
// 2 | fn foo() {
429+
// |
430+
// |
431+
// |
432+
// 3 |
433+
// 4 | }
434+
// |
411435
for pos in 0..line_len + 1 {
412436
draw_col_separator(buffer, line_offset + pos + 1, width_offset - 2);
413437
buffer.putc(line_offset + pos + 1,
@@ -468,7 +492,8 @@ impl EmitterWriter {
468492
Style::UnderlineSecondary
469493
};
470494
let pos = pos + 1;
471-
if pos > 1 {
495+
496+
if pos > 1 && annotation.has_label() {
472497
for p in line_offset + 1..line_offset + pos + 1 {
473498
buffer.putc(p,
474499
code_offset + annotation.start_col,
@@ -546,16 +571,8 @@ impl EmitterWriter {
546571
// | | something about `foo`
547572
// | something about `fn foo()`
548573
annotations_position.sort_by(|a, b| {
549-
fn len(a: &Annotation) -> usize {
550-
// Account for usize underflows
551-
if a.end_col > a.start_col {
552-
a.end_col - a.start_col
553-
} else {
554-
a.start_col - a.end_col
555-
}
556-
}
557574
// Decreasing order
558-
len(a.1).cmp(&len(b.1)).reverse()
575+
a.1.len().cmp(&b.1.len()).reverse()
559576
});
560577

561578
// 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)