From c44d51c465551eb8decbd45f1625bbadd64ec355 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Thu, 30 May 2019 15:47:19 +1000 Subject: [PATCH 1/7] Add Header struct --- codespan-reporting/src/emitter.rs | 116 +++++++++++++++++------------- 1 file changed, 68 insertions(+), 48 deletions(-) diff --git a/codespan-reporting/src/emitter.rs b/codespan-reporting/src/emitter.rs index c2d1e59..2d0e23a 100644 --- a/codespan-reporting/src/emitter.rs +++ b/codespan-reporting/src/emitter.rs @@ -74,46 +74,7 @@ pub fn emit( codemap: &CodeMap>, diagnostic: &Diagnostic, ) -> io::Result<()> { - let severity_color = config.severity_color(diagnostic.severity); - let gutter_spec = ColorSpec::new().set_fg(Some(config.gutter_color)).clone(); - let header_message_spec = ColorSpec::new().set_bold(true).set_intense(true).clone(); - let header_primary_spec = ColorSpec::new() - .set_bold(true) - .set_intense(true) - .set_fg(Some(severity_color)) - .clone(); - - // Diagnostic header - // - // ``` - // error[E0001]: Unexpected type in `+` application - // ``` - - // Write severity name - // - // ``` - // error - // ``` - writer.set_color(&header_primary_spec)?; - write!(writer, "{}", severity_name(diagnostic.severity))?; - if let Some(code) = &diagnostic.code { - // Write error code - // - // ``` - // [E0001] - // ``` - write!(writer, "[{}]", code)?; - } - - // Write diagnostic message - // - // ``` - // : Unexpected type in `+` application - // ``` - writer.set_color(&header_message_spec)?; - write!(writer, ": {}", diagnostic.message)?; - write!(writer, "\n")?; - writer.reset()?; + Header::new(diagnostic).emit(&mut writer, config)?; for label in &diagnostic.labels { match codemap.find_file(label.span.start()) { @@ -124,6 +85,7 @@ pub fn emit( } }, Some(file) => { + let gutter_spec = ColorSpec::new().set_fg(Some(config.gutter_color)).clone(); let (start_line, start_column) = file.location(label.span.start()).expect("location_start"); let (end_line, _) = file.location(label.span.end()).expect("location_end"); @@ -282,13 +244,71 @@ pub fn emit( Ok(()) } -/// A string that explains this diagnostic severity. -fn severity_name(severity: Severity) -> &'static str { - match severity { - Severity::Bug => "bug", - Severity::Error => "error", - Severity::Warning => "warning", - Severity::Note => "note", - Severity::Help => "help", +/// Diagnostic header +/// +/// ``` +/// error[E0001]: Unexpected type in `+` application +/// ``` +#[derive(Copy, Clone, Debug)] +struct Header<'doc> { + severity: Severity, + code: Option<&'doc str>, + message: &'doc str, +} + +impl<'doc> Header<'doc> { + fn new(diagnostic: &'doc Diagnostic) -> Header<'doc> { + Header { + severity: diagnostic.severity, + code: diagnostic.code.as_ref().map(String::as_str), + message: &diagnostic.message, + } + } + + fn severity_name(&self) -> &'static str { + match self.severity { + Severity::Bug => "bug", + Severity::Error => "error", + Severity::Warning => "warning", + Severity::Help => "help", + Severity::Note => "note", + } + } + + fn emit(&self, writer: &mut impl WriteColor, config: &Config) -> io::Result<()> { + let message_spec = ColorSpec::new().set_bold(true).set_intense(true).clone(); + let primary_spec = ColorSpec::new() + .set_bold(true) + .set_intense(true) + .set_fg(Some(config.severity_color(self.severity))) + .clone(); + + // Write severity name + // + // ``` + // error + // ``` + writer.set_color(&primary_spec)?; + write!(writer, "{}", self.severity_name())?; + if let Some(code) = &self.code { + // Write error code + // + // ``` + // [E0001] + // ``` + write!(writer, "[{}]", code)?; + } + + // Write diagnostic message + // + // ``` + // : Unexpected type in `+` application + // ``` + writer.set_color(&message_spec)?; + write!(writer, ": {}", self.message)?; + write!(writer, "\n")?; + writer.reset()?; + + Ok(()) } } From 1c14e952be297c980fda9b2fbafce9be80b28657 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Thu, 30 May 2019 18:01:54 +1000 Subject: [PATCH 2/7] Split up message body into components --- codespan-reporting/src/emitter.rs | 431 +++++++++++++++++------------- 1 file changed, 248 insertions(+), 183 deletions(-) diff --git a/codespan-reporting/src/emitter.rs b/codespan-reporting/src/emitter.rs index 2d0e23a..22d9f5a 100644 --- a/codespan-reporting/src/emitter.rs +++ b/codespan-reporting/src/emitter.rs @@ -1,8 +1,8 @@ -use codespan::{CodeMap, LineIndex, RawIndex}; +use codespan::{ByteSpan, CodeMap, FileMap, LineIndex, RawIndex}; use std::io; use termcolor::{Color, ColorSpec, WriteColor}; -use crate::{Diagnostic, LabelStyle, Severity}; +use crate::{Diagnostic, Label, LabelStyle, Severity}; #[derive(Clone, Debug)] pub struct Config { @@ -50,22 +50,6 @@ impl Config { Severity::Help => self.help_color, } } - - /// The style used to mark the labelled section of code. - pub fn label_color(&self, severity: Severity, label_style: LabelStyle) -> Color { - match label_style { - LabelStyle::Primary => self.severity_color(severity), - LabelStyle::Secondary => self.secondary_color, - } - } - - /// The character used for the underlined section of code. - fn underline_char(&self, label_style: LabelStyle) -> char { - match label_style { - LabelStyle::Primary => self.primary_mark, - LabelStyle::Secondary => self.secondary_mark, - } - } } pub fn emit( @@ -78,186 +62,30 @@ pub fn emit( for label in &diagnostic.labels { match codemap.find_file(label.span.start()) { - None => { - if !label.message.is_empty() { - write!(writer, "- {}", label.message)?; - write!(writer, "\n")?; - } - }, + None => SimpleMessage::new(&label.message).emit(&mut writer, config)?, Some(file) => { - let gutter_spec = ColorSpec::new().set_fg(Some(config.gutter_color)).clone(); - let (start_line, start_column) = - file.location(label.span.start()).expect("location_start"); - let (end_line, _) = file.location(label.span.end()).expect("location_end"); - let start_line_span = file.line_span(start_line).expect("line_span"); - let end_line_span = file.line_span(end_line).expect("line_span"); - - // Use the length of the last line number as the gutter padding - let gutter_padding = format!("{}", end_line.number()).len(); - - // File name - // - // ``` - // ┌╴ :2:9 - // ``` - - // Write gutter - writer.set_color(&gutter_spec)?; - write!(writer, "{: >width$} ┌╴ ", "", width = gutter_padding)?; - writer.reset()?; - - // Write file name - write!( - writer, - "{file}:{line}:{column}", - file = file.name(), - line = start_line.number(), - column = start_column.number(), - )?; - write!(writer, "\n")?; - - // Source code snippet - // - // ``` - // │ - // 2 │ (+ test "") - // │ ^^ Expected integer but got string - // ╵ - // ``` - - // Write line number and gutter - writer.set_color(&gutter_spec)?; - write!(writer, "{: >width$} │ ", "", width = gutter_padding)?; - write!(writer, "\n")?; - write!( - writer, - "{: >width$} │ ", - start_line.number(), - width = gutter_padding, - )?; - writer.reset()?; - - // Write source prefix before marked section - let source_prefix = file - .src_slice(start_line_span.with_end(label.span.start())) - .expect("prefix"); - write!(writer, "{}", source_prefix)?; - - let label_color = config.label_color(diagnostic.severity, label.style); - let label_spec = ColorSpec::new().set_fg(Some(label_color)).clone(); - - // Write marked section - let mark_len = if start_line == end_line { - // Single line - - // Write marked source section - let marked_source = file.src_slice(label.span).expect("marked_source"); - writer.set_color(&label_spec)?; - write!(writer, "{}", marked_source)?; - writer.reset()?; - marked_source.len() - } else { - // Multiple lines - - // Write marked source section - let marked_source = file - .src_slice(start_line_span.with_start(label.span.start())) - .expect("start_of_marked"); - writer.set_color(&label_spec)?; - write!(writer, "{}", marked_source)?; - - for line_index in ((start_line.to_usize() + 1)..end_line.to_usize()) - .map(|i| LineIndex::from(i as RawIndex)) - { - // Write line number and gutter - writer.set_color(&gutter_spec)?; - write!( - writer, - "{: >width$} │ ", - line_index.number(), - width = gutter_padding, - )?; - - // Write marked source section - let line_span = file.line_span(line_index).expect("marked_line_span"); - let marked_source = file - .src_slice(line_span) - .expect("marked_source") - .trim_end_matches(|ch: char| ch == '\r' || ch == '\n'); - writer.set_color(&label_spec)?; - write!(writer, "{}", marked_source)?; - write!(writer, "\n")?; - } - - // Write line number and gutter - writer.set_color(&gutter_spec)?; - write!( - writer, - "{: >width$} │ ", - end_line.number(), - width = gutter_padding, - )?; - - // Write marked source section - let marked_source = file - .src_slice(end_line_span.with_end(label.span.end())) - .expect("marked_source"); - writer.set_color(&label_spec)?; - write!(writer, "{}", marked_source)?; - writer.reset()?; - marked_source.len() - }; - - // Write source suffix after marked section - let source = file - .src_slice(end_line_span.with_start(label.span.end())) - .expect("suffix") - .trim_end_matches(|ch: char| ch == '\r' || ch == '\n'); - write!(writer, "{}", source)?; - write!(writer, "\n")?; - - // Write underline gutter - writer.set_color(&gutter_spec)?; - write!(writer, "{: >width$} │ ", "", width = gutter_padding)?; - writer.reset()?; - - // Write underline and label - writer.set_color(&label_spec)?; - write!(writer, "{: >width$}", "", width = source_prefix.len())?; - for _ in 0..mark_len { - write!(writer, "{}", config.underline_char(label.style))?; - } - if !label.message.is_empty() { - write!(writer, " {}", label.message)?; - write!(writer, "\n")?; - } - writer.reset()?; - - // Write final gutter - writer.set_color(&gutter_spec)?; - write!(writer, "{: >width$} ╵", "", width = gutter_padding)?; - write!(writer, "\n")?; - writer.reset()?; + MarkedSource::new(file, &diagnostic, &label).emit(&mut writer, config)? }, } } + Ok(()) } /// Diagnostic header /// -/// ``` +/// ```text /// error[E0001]: Unexpected type in `+` application /// ``` #[derive(Copy, Clone, Debug)] -struct Header<'doc> { +struct Header<'a> { severity: Severity, - code: Option<&'doc str>, - message: &'doc str, + code: Option<&'a str>, + message: &'a str, } -impl<'doc> Header<'doc> { - fn new(diagnostic: &'doc Diagnostic) -> Header<'doc> { +impl<'a> Header<'a> { + fn new(diagnostic: &'a Diagnostic) -> Header<'a> { Header { severity: diagnostic.severity, code: diagnostic.code.as_ref().map(String::as_str), @@ -312,3 +140,240 @@ impl<'doc> Header<'doc> { Ok(()) } } + +/// A simple message +/// +/// ```text +/// - Expected integer but got string +/// ``` +struct SimpleMessage<'a> { + message: &'a str, +} + +impl<'a> SimpleMessage<'a> { + fn new(message: &'a str) -> SimpleMessage<'a> { + SimpleMessage { message } + } + + fn emit(&self, writer: &mut impl WriteColor, _config: &Config) -> io::Result<()> { + if !self.message.is_empty() { + write!(writer, "- {}", self.message)?; + write!(writer, "\n")?; + } + + Ok(()) + } +} + +/// A marked section of source code +/// +/// ```text +/// ┌╴ :2:9 +/// │ +/// 2 │ (+ test "") +/// │ ^^ Expected integer but got string +/// ╵ +/// ``` +struct MarkedSource<'a, S: AsRef> { + file: &'a FileMap, + span: ByteSpan, + message: &'a str, + severity: Option, +} + +impl<'a, S: AsRef> MarkedSource<'a, S> { + fn new( + file: &'a FileMap, + diagnostic: &Diagnostic, + label: &'a Label, + ) -> MarkedSource<'a, S> { + MarkedSource { + file, + span: label.span, + message: &label.message, + severity: match label.style { + LabelStyle::Primary => Some(diagnostic.severity), + LabelStyle::Secondary => None, + }, + } + } + + fn label_color(&self, config: &Config) -> Color { + match self.severity { + None => config.secondary_color, + Some(severity) => config.severity_color(severity), + } + } + + fn underline_char(&self, config: &Config) -> char { + match self.severity { + Some(_) => config.primary_mark, + None => config.secondary_mark, + } + } + + fn emit(&self, writer: &mut impl WriteColor, config: &Config) -> io::Result<()> { + let gutter_spec = ColorSpec::new().set_fg(Some(config.gutter_color)).clone(); + let label_color = self.label_color(config); + let label_spec = ColorSpec::new().set_fg(Some(label_color)).clone(); + + let (start_line, start_column) = self + .file + .location(self.span.start()) + .expect("location_start"); + let (end_line, _) = self.file.location(self.span.end()).expect("location_end"); + let start_line_span = self.file.line_span(start_line).expect("line_span"); + let end_line_span = self.file.line_span(end_line).expect("line_span"); + + // Use the length of the last line number as the gutter padding + let gutter_padding = format!("{}", end_line.number()).len(); + + // File name + // + // ``` + // ┌╴ :2:9 + // ``` + + // Write gutter + writer.set_color(&gutter_spec)?; + write!(writer, "{: >width$} ┌╴ ", "", width = gutter_padding)?; + writer.reset()?; + + // Write file name + write!( + writer, + "{file}:{line}:{column}", + file = self.file.name(), + line = start_line.number(), + column = start_column.number(), + )?; + write!(writer, "\n")?; + + // Source code snippet + // + // ``` + // │ + // 2 │ (+ test "") + // │ ^^ Expected integer but got string + // ╵ + // ``` + + // Write line number and gutter + writer.set_color(&gutter_spec)?; + write!(writer, "{: >width$} │ ", "", width = gutter_padding)?; + write!(writer, "\n")?; + write!( + writer, + "{: >width$} │ ", + start_line.number(), + width = gutter_padding, + )?; + writer.reset()?; + + // Write source prefix before marked section + let source_prefix = self + .file + .src_slice(start_line_span.with_end(self.span.start())) + .expect("prefix"); + write!(writer, "{}", source_prefix)?; + + // Write marked section + let mark_len = if start_line == end_line { + // Single line + + // Write marked source section + let marked_source = self.file.src_slice(self.span).expect("marked_source"); + writer.set_color(&label_spec)?; + write!(writer, "{}", marked_source)?; + writer.reset()?; + marked_source.len() + } else { + // Multiple lines + + // Write marked source section + let marked_source = self + .file + .src_slice(start_line_span.with_start(self.span.start())) + .expect("start_of_marked"); + writer.set_color(&label_spec)?; + write!(writer, "{}", marked_source)?; + + for line_index in ((start_line.to_usize() + 1)..end_line.to_usize()) + .map(|i| LineIndex::from(i as RawIndex)) + { + // Write line number and gutter + writer.set_color(&gutter_spec)?; + write!( + writer, + "{: >width$} │ ", + line_index.number(), + width = gutter_padding, + )?; + + // Write marked source section + let line_span = self.file.line_span(line_index).expect("marked_line_span"); + let marked_source = self + .file + .src_slice(line_span) + .expect("marked_source") + .trim_end_matches(|ch: char| ch == '\r' || ch == '\n'); + writer.set_color(&label_spec)?; + write!(writer, "{}", marked_source)?; + write!(writer, "\n")?; + } + + // Write line number and gutter + writer.set_color(&gutter_spec)?; + write!( + writer, + "{: >width$} │ ", + end_line.number(), + width = gutter_padding, + )?; + + // Write marked source section + let marked_source = self + .file + .src_slice(end_line_span.with_end(self.span.end())) + .expect("marked_source"); + writer.set_color(&label_spec)?; + write!(writer, "{}", marked_source)?; + writer.reset()?; + marked_source.len() + }; + + // Write source suffix after marked section + let source = self + .file + .src_slice(end_line_span.with_start(self.span.end())) + .expect("suffix") + .trim_end_matches(|ch: char| ch == '\r' || ch == '\n'); + write!(writer, "{}", source)?; + write!(writer, "\n")?; + + // Write underline gutter + writer.set_color(&gutter_spec)?; + write!(writer, "{: >width$} │ ", "", width = gutter_padding)?; + writer.reset()?; + + // Write underline and label + writer.set_color(&label_spec)?; + write!(writer, "{: >width$}", "", width = source_prefix.len())?; + for _ in 0..mark_len { + write!(writer, "{}", self.underline_char(config))?; + } + if !self.message.is_empty() { + write!(writer, " {}", self.message)?; + write!(writer, "\n")?; + } + writer.reset()?; + + // Write final gutter + writer.set_color(&gutter_spec)?; + write!(writer, "{: >width$} ╵", "", width = gutter_padding)?; + write!(writer, "\n")?; + writer.reset()?; + + Ok(()) + } +} From f87fcaa92506c2bfeae6f34bf5db7b56acb332bf Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Thu, 30 May 2019 21:11:26 +1000 Subject: [PATCH 3/7] Make diagnostics look closer to what the LSP expects --- codespan-lsp/src/lib.rs | 39 +++----- codespan-reporting/examples/emit.rs | 32 ++++--- codespan-reporting/src/diagnostic.rs | 76 +++++----------- codespan-reporting/src/emitter.rs | 127 ++++++++++++++------------- codespan-reporting/src/lib.rs | 2 +- 5 files changed, 115 insertions(+), 161 deletions(-) diff --git a/codespan-lsp/src/lib.rs b/codespan-lsp/src/lib.rs index be97066..7b2a81a 100644 --- a/codespan-lsp/src/lib.rs +++ b/codespan-lsp/src/lib.rs @@ -180,8 +180,6 @@ pub fn make_lsp_diagnostic( where F: FnMut(&str) -> Result, { - use codespan_reporting::LabelStyle; - let find_file = |index| { code_map .find_file(index) @@ -189,37 +187,22 @@ where }; // We need a position for the primary error so take the span from the first primary label - let (primary_file_map, primary_label_range) = { - let first_primary_label = diagnostic - .labels - .iter() - .find(|label| label.style == LabelStyle::Primary); - - match first_primary_label { - Some(label) => { - let file_map = find_file(label.span.start())?; - (Some(file_map), byte_span_to_range(&file_map, label.span)?) - }, - None => (None, UNKNOWN_RANGE), - } - }; + let primary_file_map = find_file(diagnostic.primary_label.span.start())?; + let primary_label_range = byte_span_to_range(&primary_file_map, diagnostic.primary_label.span)?; let related_information = diagnostic - .labels + .secondary_labels .into_iter() .map(|label| { - let (file_map, range) = match primary_file_map { - // If the label's span does not point anywhere, assume it comes from the same file - // as the primary label - Some(file_map) if label.span.start() == ByteIndex::none() => { - (file_map, UNKNOWN_RANGE) - }, - Some(_) | None => { - let file_map = find_file(label.span.start())?; - let range = byte_span_to_range(file_map, label.span)?; + // If the label's span does not point anywhere, assume it comes from the same file + // as the primary label + let (file_map, range) = if label.span.start() == ByteIndex::none() { + (primary_file_map, UNKNOWN_RANGE) + } else { + let file_map = find_file(label.span.start())?; + let range = byte_span_to_range(file_map, label.span)?; - (file_map, range) - }, + (file_map, range) }; let uri = codespan_name_to_file(file_map.name()) diff --git a/codespan-reporting/examples/emit.rs b/codespan-reporting/examples/emit.rs index e0e53c7..8f48e78 100644 --- a/codespan-reporting/examples/emit.rs +++ b/codespan-reporting/examples/emit.rs @@ -2,7 +2,7 @@ use structopt::StructOpt; use codespan::{CodeMap, Span}; use codespan_reporting::termcolor::StandardStream; -use codespan_reporting::{emit, ColorArg, Diagnostic, Label, Severity}; +use codespan_reporting::{emit, ColorArg, Diagnostic, Label}; #[derive(Debug, StructOpt)] #[structopt(name = "emit")] @@ -31,25 +31,23 @@ fn main() { let file_map = code_map.add_filemap("test".into(), source.to_string()); let str_start = file_map.byte_index(3.into(), 6.into()).unwrap(); - let error = Diagnostic::new(Severity::Error, "Unexpected type in `+` application") - .with_label( - Label::new_primary(Span::from_offset(str_start, 2.into())) - .with_message("Expected integer but got string"), - ) - .with_label( - Label::new_secondary(Span::from_offset(str_start, 2.into())) - .with_message("Expected integer but got string"), - ) - .with_code("E0001"); + let error = Diagnostic::new_error( + "Unexpected type in `+` application", + Label::new( + Span::from_offset(str_start, 2.into()), + "Expected integer but got string", + ), + ) + .with_code("E0001") + .with_secondary_labels(vec![Label::new( + Span::from_offset(str_start, 2.into()), + "Expected integer but got string", + )]); let line_start = file_map.byte_index(2.into(), 3.into()).unwrap(); - let warning = Diagnostic::new( - Severity::Warning, + let warning = Diagnostic::new_warning( "`+` function has no effect unless its result is used", - ) - .with_label( - Label::new_primary(Span::from_offset(line_start, 27.into())) - .with_message("Value discarded"), + Label::new(Span::from_offset(line_start, 27.into()), "Value discarded"), ); let diagnostics = [error, warning]; diff --git a/codespan-reporting/src/diagnostic.rs b/codespan-reporting/src/diagnostic.rs index 4262b06..426490f 100644 --- a/codespan-reporting/src/diagnostic.rs +++ b/codespan-reporting/src/diagnostic.rs @@ -6,18 +6,7 @@ use serde::{Deserialize, Serialize}; use crate::Severity; -/// A style for the label -#[derive(Copy, Clone, PartialEq, Debug)] -#[cfg_attr(feature = "memory_usage", derive(heapsize_derive::HeapSizeOf))] -#[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] -pub enum LabelStyle { - /// The main focus of the diagnostic - Primary, - /// Supporting labels that may help to isolate the cause of the diagnostic - Secondary, -} - -/// A label describing an underlined region of code associated with a diagnostic +/// A label describing an underlined region of code associated with a diagnostic. #[derive(Clone, Debug)] #[cfg_attr(feature = "memory_usage", derive(heapsize_derive::HeapSizeOf))] #[cfg_attr(feature = "serialization", derive(Serialize, Deserialize))] @@ -26,31 +15,15 @@ pub struct Label { pub span: ByteSpan, /// A message to provide some additional information for the underlined code. pub message: String, - /// The style to use for the label. - pub style: LabelStyle, } impl Label { - pub fn new(span: ByteSpan, style: LabelStyle) -> Label { + pub fn new(span: ByteSpan, mesaage: impl Into) -> Label { Label { span, - message: String::new(), - style, + message: mesaage.into(), } } - - pub fn new_primary(span: ByteSpan) -> Label { - Label::new(span, LabelStyle::Primary) - } - - pub fn new_secondary(span: ByteSpan) -> Label { - Label::new(span, LabelStyle::Secondary) - } - - pub fn with_message(mut self, message: impl Into) -> Label { - self.message = message.into(); - self - } } /// Represents a diagnostic message and associated child messages. @@ -62,41 +35,43 @@ pub struct Diagnostic { pub severity: Severity, /// An optional code that identifies this diagnostic. pub code: Option, - /// The main message associated with this diagnostic + /// The main message associated with this diagnostic. pub message: String, - /// The labelled spans marking the regions of code that cause this - /// diagnostic to be raised - pub labels: Vec