extend MultiSpan to carry labels

remove the old logic that prevents them from being overlapping

Author: nikomatsakis

src/libsyntax/codemap.rs

@@ -32,8 +32,6 @@ use serialize::{Encodable, Decodable, Encoder, Decoder};
 
 use ast::Name;
 
-use errors::emitter::MAX_HIGHLIGHT_LINES;
-
 // _____________________________________________________________________________
 // Pos, BytePos, CharPos
 //
@@ -132,13 +130,21 @@ pub struct Span {
     pub expn_id: ExpnId
 }
 
-/// Spans are converted to MultiSpans just before error reporting, either automatically,
-/// generated by line grouping, or manually constructed.
-/// In the latter case care should be taken to ensure that spans are ordered, disjoint,
-/// and point into the same FileMap.
+/// A collection of spans. Each span can optionally have an associated
+/// label. The spans do not have to be disjoint or have any other
+/// particular relationship: in fact, they can even be from distinct
+/// files. When the error is reported, the text referenced by the
+/// spans will be printed, with highlights or labels in the
+/// appropriate places.
 #[derive(Clone)]
 pub struct MultiSpan {
-    pub spans: Vec<Span>
+    pub spans: Vec<SpanString>
+}
+
+#[derive(Clone, Debug)]
+pub struct SpanString {
+    pub span: Span,
+    pub label: Option<String>,
 }
 
 pub const DUMMY_SP: Span = Span { lo: BytePos(0), hi: BytePos(0), expn_id: NO_EXPANSION };
@@ -284,55 +290,28 @@ impl MultiSpan {
         MultiSpan { spans: Vec::new() }
     }
 
+    pub fn from_labels(vec: Vec<(Span, String)>) -> MultiSpan {
+        MultiSpan {
+            spans: vec.into_iter()
+                      .map(|(span, string)| SpanString {
+                          span: span,
+                          label: Some(string)
+                      })
+                      .collect()
+        }
+    }
+
     pub fn to_span_bounds(&self) -> Span {
         assert!(!self.spans.is_empty());
-        let Span { lo, expn_id, .. } = *self.spans.first().unwrap();
-        let Span { hi, .. } = *self.spans.last().unwrap();
+        let Span { lo, expn_id, .. } = self.spans.first().unwrap().span;
+        let Span { hi, .. } = self.spans.last().unwrap().span;
         Span { lo: lo, hi: hi, expn_id: expn_id }
     }
-
-    /// Merges or inserts the given span into itself.
-    pub fn push_merge(&mut self, mut sp: Span) {
-        let mut idx_merged = None;
-
-        for idx in 0.. {
-            let cur = match self.spans.get(idx) {
-                Some(s) => *s,
-                None => break,
-            };
-            // Try to merge with a contained Span
-            if let Some(union) = cur.merge(sp) {
-                self.spans[idx] = union;
-                sp = union;
-                idx_merged = Some(idx);
-                break;
-            }
-            // Or insert into the first sorted position
-            if sp.hi <= cur.lo {
-                self.spans.insert(idx, sp);
-                idx_merged = Some(idx);
-                break;
-            }
-        }
-        if let Some(idx) = idx_merged {
-            // Merge with spans trailing the insertion/merging position
-            while (idx + 1) < self.spans.len() {
-                if let Some(union) = self.spans[idx + 1].merge(sp) {
-                    self.spans[idx] = union;
-                    self.spans.remove(idx + 1);
-                } else {
-                    break;
-                }
-            }
-        } else {
-            self.spans.push(sp);
-        }
-    }
 }
 
 impl From<Span> for MultiSpan {
     fn from(span: Span) -> MultiSpan {
-        MultiSpan { spans: vec![span] }
+        MultiSpan { spans: vec![SpanString { span: span, label: None }] }
     }
 }
 
@@ -1156,53 +1135,6 @@ impl CodeMap {
         }
     }
 
-    /// Groups and sorts spans by lines into `MultiSpan`s, where `push` adds them to their group,
-    /// specifying the unification behaviour for overlapping spans.
-    /// Spans overflowing a line are put into their own one-element-group.
-    pub fn custom_group_spans<F>(&self, mut spans: Vec<Span>, push: F) -> Vec<MultiSpan>
-        where F: Fn(&mut MultiSpan, Span)
-    {
-        spans.sort_by(|a, b| a.lo.cmp(&b.lo));
-        let mut groups = Vec::<MultiSpan>::new();
-        let mut overflowing = vec![];
-        let mut prev_expn = ExpnId(!2u32);
-        let mut prev_file = !0usize;
-        let mut prev_line = !0usize;
-        let mut err_size = 0;
-
-        for sp in spans {
-            let line = self.lookup_char_pos(sp.lo).line;
-            let line_hi = self.lookup_char_pos(sp.hi).line;
-            if line != line_hi {
-                overflowing.push(sp.into());
-                continue
-            }
-            let file = self.lookup_filemap_idx(sp.lo);
-
-            if err_size < MAX_HIGHLIGHT_LINES && sp.expn_id == prev_expn && file == prev_file {
-                // `push` takes care of sorting, trimming, and merging
-                push(&mut groups.last_mut().unwrap(), sp);
-                if line != prev_line {
-                    err_size += 1;
-                }
-            } else {
-                groups.push(sp.into());
-                err_size = 1;
-            }
-            prev_expn = sp.expn_id;
-            prev_file = file;
-            prev_line = line;
-        }
-        groups.extend(overflowing);
-        groups
-    }
-
-    /// Groups and sorts spans by lines into `MultiSpan`s, merging overlapping spans.
-    /// Spans overflowing a line are put into their own one-element-group.
-    pub fn group_spans(&self, spans: Vec<Span>) -> Vec<MultiSpan> {
-        self.custom_group_spans(spans, |msp, sp| msp.push_merge(sp))
-    }
-
     pub fn get_filemap(&self, filename: &str) -> Rc<FileMap> {
         for fm in self.files.borrow().iter() {
             if filename == fm.name {
@@ -1719,62 +1651,4 @@ r"blork2.rs:2:1: 2:12
 ";
         assert_eq!(sstr, res_str);
     }
-
-    #[test]
-    fn t13() {
-        // Test that collecting multiple spans into line-groups works correctly
-        let cm = CodeMap::new();
-        let inp  =      "_aaaaa__bbb\nvv\nw\nx\ny\nz\ncccccc__ddddee__";
-        let sp1  =      " ~~~~~     \n  \n \n \n \n \n                ";
-        let sp2  =      "           \n  \n \n \n \n~\n                ";
-        let sp3  =      "        ~~~\n~~\n \n \n \n \n                ";
-        let sp4  =      "           \n  \n \n \n \n \n~~~~~~          ";
-        let sp5  =      "           \n  \n \n \n \n \n        ~~~~    ";
-        let sp6  =      "           \n  \n \n \n \n \n          ~~~~  ";
-        let sp_merge =  "           \n  \n \n \n \n \n        ~~~~~~  ";
-        let sp7  =      "           \n ~\n \n \n \n \n                ";
-        let sp8  =      "           \n  \n~\n \n \n \n                ";
-        let sp9  =      "           \n  \n \n~\n \n \n                ";
-        let sp10 =      "           \n  \n \n \n~\n \n                ";
-
-        let span = |sp, expected| {
-            let sp = span_from_selection(inp, sp);
-            assert_eq!(&cm.span_to_snippet(sp).unwrap(), expected);
-            sp
-        };
-
-        cm.new_filemap_and_lines("blork.rs", inp);
-        let sp1 = span(sp1, "aaaaa");
-        let sp2 = span(sp2, "z");
-        let sp3 = span(sp3, "bbb\nvv");
-        let sp4 = span(sp4, "cccccc");
-        let sp5 = span(sp5, "dddd");
-        let sp6 = span(sp6, "ddee");
-        let sp7 = span(sp7, "v");
-        let sp8 = span(sp8, "w");
-        let sp9 = span(sp9, "x");
-        let sp10 = span(sp10, "y");
-        let sp_merge = span(sp_merge, "ddddee");
-
-        let spans = vec![sp5, sp2, sp4, sp9, sp10, sp7, sp3, sp8, sp1, sp6];
-
-        macro_rules! check_next {
-            ($groups: expr, $expected: expr) => ({
-                let actual = $groups.next().map(|g|&g.spans[..]);
-                let expected = $expected;
-                println!("actual:\n{:?}\n", actual);
-                println!("expected:\n{:?}\n", expected);
-                assert_eq!(actual, expected.as_ref().map(|x|&x[..]));
-            });
-        }
-
-        let _groups = cm.group_spans(spans.clone());
-        let it = &mut _groups.iter();
-
-        check_next!(it, Some([sp1, sp7, sp8, sp9, sp10, sp2]));
-        // New group because we're exceeding MAX_HIGHLIGHT_LINES
-        check_next!(it, Some([sp4, sp_merge]));
-        check_next!(it, Some([sp3]));
-        check_next!(it, None::<[Span; 0]>);
-    }
 }

src/libsyntax/errors/emitter.rs

@@ -243,8 +243,8 @@ impl EmitterWriter {
                        -> io::Result<()>
     {
         let mut snippet_data = SnippetData::new(self.cm.clone());
-        for &span in &msp.spans {
-            snippet_data.push(span, None);
+        for span_str in &msp.spans {
+            snippet_data.push(span_str.span, span_str.label.clone());
         }
         let rendered_lines = snippet_data.render_lines();
         for rendered_line in &rendered_lines {
@@ -380,6 +380,7 @@ impl Destination {
                    -> io::Result<()> {
         match style {
             Style::FileNameLine => {
+                self.start_attr(term::Attr::Bold)?;
                 self.start_attr(term::Attr::ForegroundColor(term::color::BRIGHT_BLUE))?;
             }
             Style::Quotation => {
@@ -562,54 +563,6 @@ dummy.txt:12         tolv
         assert_eq!(suggest.splice_lines(&cm), expected);
     }
 
-    #[test]
-    fn test_multiple_span_splice() {
-        // Test that a `MultiSpan` containing multiple spans splices substitions on
-        // several lines correctly
-        let cm = CodeMap::new();
-        let inp = "aaaaabbbbBB\nZZ\nZZ\nCCCDDDDDdddddeee";
-        let sp1 = "     ~~~~~~\n  \n  \n                ";
-        let sp2 = "           \n  \n  \n~~~~~~          ";
-        let sp3 = "           \n  \n  \n        ~~~     ";
-        let sp4 = "           \n  \n  \n           ~~~~ ";
-
-        let span_eq = |sp, eq| assert_eq!(&cm.span_to_snippet(sp).unwrap(), eq);
-
-        cm.new_filemap_and_lines("blork.rs", inp);
-        let sp1 = span_from_selection(inp, sp1);
-        let sp2 = span_from_selection(inp, sp2);
-        let sp3 = span_from_selection(inp, sp3);
-        let sp4 = span_from_selection(inp, sp4);
-        span_eq(sp1, "bbbbBB");
-        span_eq(sp2, "CCCDDD");
-        span_eq(sp3, "ddd");
-        span_eq(sp4, "ddee");
-
-        let substitutes: Vec<String> = ["1", "2", "3", "4"].iter().map(|x|x.to_string()).collect();
-        let expected = "aaaaa1\nZZ\nZZ\n2DD34e";
-
-        let test = |msp| {
-            let suggest = CodeSuggestion {
-                msp: msp,
-                substitutes: substitutes.clone(),
-            };
-            let actual = suggest.splice_lines(&cm);
-            assert_eq!(actual, expected);
-        };
-        test(MultiSpan { spans: vec![sp1, sp2, sp3, sp4] });
-
-        // Test ordering and merging by `MultiSpan::push`
-        let mut msp = MultiSpan::new();
-        msp.push_merge(sp2);
-        msp.push_merge(sp1);
-        assert_eq!(&msp.spans, &[sp1, sp2]);
-        msp.push_merge(sp4);
-        assert_eq!(&msp.spans, &[sp1, sp2, sp4]);
-        msp.push_merge(sp3);
-        assert_eq!(&msp.spans, &[sp1, sp2, sp3, sp4]);
-        test(msp);
-    }
-
     #[test]
     fn test_multispan_highlight() {
         let data = Arc::new(Mutex::new(Vec::new()));

src/libsyntax/errors/json.rs

@@ -20,7 +20,7 @@
 // FIXME spec the JSON output properly.
 
 
-use codemap::{Span, MultiSpan, CodeMap};
+use codemap::{Span, SpanString, MultiSpan, CodeMap};
 use diagnostics::registry::Registry;
 use errors::{Level, DiagnosticBuilder, SubDiagnostic, RenderSpan, CodeSuggestion};
 use errors::emitter::Emitter;
@@ -179,7 +179,7 @@ impl<'a> Diagnostic<'a> {
 
 impl DiagnosticSpan {
     fn from_multispan(msp: &MultiSpan, je: &JsonEmitter) -> Vec<DiagnosticSpan> {
-        msp.spans.iter().map(|span| {
+        msp.spans.iter().map(|&SpanString { span, .. }| {
             let start = je.cm.lookup_char_pos(span.lo);
             let end = je.cm.lookup_char_pos(span.hi);
             DiagnosticSpan {
@@ -190,7 +190,7 @@ impl DiagnosticSpan {
                 line_end: end.line,
                 column_start: start.col.0 + 1,
                 column_end: end.col.0 + 1,
-                text: DiagnosticSpanLine::from_span(span, je),
+                text: DiagnosticSpanLine::from_span(&span, je),
             }
         }).collect()
     }
@@ -203,7 +203,7 @@ impl DiagnosticSpan {
                 DiagnosticSpan::from_multispan(msp, je)
             }
             RenderSpan::FileLine(ref msp) => {
-                msp.spans.iter().map(|span| {
+                msp.spans.iter().map(|&SpanString { span, .. }| {
                     let start = je.cm.lookup_char_pos(span.lo);
                     let end = je.cm.lookup_char_pos(span.hi);
                     DiagnosticSpan {
@@ -214,7 +214,7 @@ impl DiagnosticSpan {
                         line_end: end.line,
                         column_start: 0,
                         column_end: 0,
-                        text: DiagnosticSpanLine::from_span(span, je),
+                        text: DiagnosticSpanLine::from_span(&span, je),
                     }
                 }).collect()
             }

src/libsyntax/errors/mod.rs

@@ -106,6 +106,7 @@ impl CodeSuggestion {
         let mut buf = String::new();
 
         for (sp, substitute) in self.msp.spans.iter().zip(self.substitutes.iter()) {
+            let sp = sp.span;
             let cur_lo = cm.lookup_char_pos(sp.lo);
             if prev_hi.line == cur_lo.line {
                 push_trailing(&mut buf, prev_line, &prev_hi, Some(&cur_lo));