Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions slicec/examples/parse.rs

This file was deleted.

34 changes: 7 additions & 27 deletions slicec/src/compilation_state.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// Copyright (c) ZeroC, Inc.

use crate::ast::Ast;
use crate::diagnostic_emitter::{emit_totals, DiagnosticEmitter};
use crate::diagnostics::{get_totals, Diagnostic, Diagnostics};
use crate::diagnostics::{Diagnostics, PrintableDiagnostic};
use crate::slice_file::SliceFile;
use crate::slice_options::{DiagnosticFormat, SliceOptions};
use crate::slice_options::SliceOptions;

#[derive(Debug, Default)]
pub struct CompilationState {
Expand Down Expand Up @@ -43,29 +42,10 @@ impl CompilationState {
}
}

/// This function is the exit point of the compiler.
/// It emits diagnostics to the console, along with the total number of warning/errors emitted.
/// After this it returns whether any errors were emitted.
pub fn emit_diagnostics(self, options: &SliceOptions) -> bool {
let diagnostics = self.diagnostics.into_updated(&self.ast, &self.files, options);
let (total_warnings, total_errors) = get_totals(&diagnostics);

// Print any diagnostics to the console, along with the total number of warnings and errors emitted.
let mut stderr = console::Term::stderr();
let mut emitter = DiagnosticEmitter::new(&mut stderr, options, &self.files);
DiagnosticEmitter::emit_diagnostics(&mut emitter, diagnostics).expect("failed to emit diagnostics");

// Only emit the summary message if we're writing human-readable output.
if options.diagnostic_format == DiagnosticFormat::Human {
emit_totals(total_warnings, total_errors).expect("failed to emit totals");
}

total_errors != 0
}

/// Consumes this `CompilationState` and returns the diagnostics it contains.
/// This method exists to simplify the testing of diagnostic emission.
pub fn into_diagnostics(self, options: &SliceOptions) -> Vec<Diagnostic> {
self.diagnostics.into_updated(&self.ast, &self.files, options)
pub fn get_printable_diagnostics(&self, options: &SliceOptions) -> Vec<PrintableDiagnostic> {
self.diagnostics
.iter()
.map(|diagnostic| crate::diagnostics::convert_diagnostic(diagnostic, options, self))
.collect()
}
}
111 changes: 55 additions & 56 deletions slicec/src/diagnostic_emitter.rs
Original file line number Diff line number Diff line change
@@ -1,138 +1,137 @@
// Copyright (c) ZeroC, Inc.

use crate::diagnostics::{Diagnostic, DiagnosticLevel};
use crate::slice_file::{SliceFile, Span};
use crate::diagnostics::{DiagnosticLevel, PrintableDiagnostic, Snippet};
use crate::slice_options::{DiagnosticFormat, SliceOptions};
use serde::ser::SerializeStruct;
use serde::Serializer;
Comment thread
InsertCreativityHere marked this conversation as resolved.
use std::io::{Result, Write};
use std::path::Path;

#[derive(Debug)]
pub struct DiagnosticEmitter<'a, T: Write> {
pub struct DiagnosticEmitter<'a> {
/// Reference to the output that diagnostics should be emitted to.
output: &'a mut T,
output: &'a mut dyn Write,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched from using a generic T: Write to a dynamic dyn Write.
This makes the typing for DiagnosticEmitter easier, since it no longer has a generic type.

And in reality, it's not like we care about DiagnosticEmitter<StdOut> vs DiagnosticEmitter<Vec<u8>>...
As long as we can write to something it's fine. No need to tie the underlying write-able thing to the type of DiagnosticEmitter.

/// Can specify `json` to serialize errors as JSON or `human` to pretty-print them.
diagnostic_format: DiagnosticFormat,
/// If true, diagnostic output will not be styled with colors (only used in `human` format).
disable_color: bool,
Comment on lines -17 to -18
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We disable color way earlier in the program now, instead of right before diagnostic emission.
Otherwise there were other things still being emitted in color.

/// Provides the emitter access to the slice files that were compiled so it can extract snippets from them.
files: &'a [SliceFile],
}

impl<'a, T: Write> DiagnosticEmitter<'a, T> {
pub fn new(output: &'a mut T, slice_options: &SliceOptions, files: &'a [SliceFile]) -> Self {
impl<'a> DiagnosticEmitter<'a> {
pub fn new(output: &'a mut dyn Write, slice_options: &SliceOptions) -> Self {
DiagnosticEmitter {
output,
diagnostic_format: slice_options.diagnostic_format,
disable_color: slice_options.disable_color,
files,
}
}

pub fn emit_diagnostics(&mut self, diagnostics: Vec<Diagnostic>) -> Result<()> {
// Disable colors if the user requested no colors.
if self.disable_color {
console::set_colors_enabled(false);
console::set_colors_enabled_stderr(false);
pub fn get_totals(diagnostics: &[PrintableDiagnostic]) -> (usize, usize) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be a free function at the bottom of diagnostic.rs, now it's a static function on DiagnosticEmitter.
Just feels tidier to keep it here, since this was only used when emitting diagnostics.

let (mut total_warnings, mut total_errors) = (0, 0);

for diagnostic in diagnostics {
match diagnostic.level {
DiagnosticLevel::Error => total_errors += 1,
DiagnosticLevel::Warning => total_warnings += 1,
DiagnosticLevel::Allowed => {}
}
}

(total_warnings, total_errors)
}

pub fn emit_totals(total_warnings: usize, total_errors: usize) -> Result<()> {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be a free function at the bottom of this file, now it's a static function on DiagnosticEmitter.
Just feels tidier, and saves an import in the places it's used.

// Totals are always printed to stdout.
let stdout = &mut console::Term::stdout();

if total_warnings > 0 {
let warnings = console::style("Warnings").yellow().bold();
writeln!(stdout, "{warnings}: Compilation generated {total_warnings} warning(s)")?;
}
if total_errors > 0 {
let failed = console::style("Failed").red().bold();
writeln!(stdout, "{failed}: Compilation failed with {total_errors} error(s)")?;
}

Ok(())
}

pub fn emit_diagnostics(&mut self, diagnostics: &[PrintableDiagnostic]) -> Result<()> {
// Emit the diagnostics in whatever form the user requested.
match self.diagnostic_format {
DiagnosticFormat::Human => self.emit_diagnostics_in_human(diagnostics),
DiagnosticFormat::Json => self.emit_diagnostics_in_json(diagnostics),
}
}

fn emit_diagnostics_in_human(&mut self, diagnostics: Vec<Diagnostic>) -> Result<()> {
fn emit_diagnostics_in_human(&mut self, diagnostics: &[PrintableDiagnostic]) -> Result<()> {
for diagnostic in diagnostics {
// Style the prefix. Note that for `Notes` we do not insert a newline since they should be "attached"
// to the previously emitted diagnostic.
let code = diagnostic.code();
let prefix = match diagnostic.level() {
let code = &diagnostic.code;
let prefix = match &diagnostic.level {
DiagnosticLevel::Error => console::style(format!("error [{code}]")).red().bold(),
DiagnosticLevel::Warning => console::style(format!("warning [{code}]")).yellow().bold(),
DiagnosticLevel::Allowed => continue,
};

// Emit the message with the prefix.
writeln!(self.output, "{prefix}: {}", console::style(diagnostic.message()).bold())?;
writeln!(self.output, "{prefix}: {}", console::style(&diagnostic.message).bold())?;

// If the diagnostic contains a span, show a snippet containing the offending code.
if let Some(span) = diagnostic.span() {
self.emit_snippet(span)?;
// If the diagnostic contains a snippet of the offending code, display it.
if let Some(snippet) = &diagnostic.snippet {
self.emit_snippet(snippet)?;
}

// If the diagnostic contains notes, display them.
for note in diagnostic.notes() {
for note in &diagnostic.notes {
writeln!(
self.output,
"{}: {}",
console::style("note").blue().bold(),
console::style(&note.message).bold(),
)?;

if let Some(span) = &note.span {
self.emit_snippet(span)?;
if let Some(snippet) = &note.snippet {
self.emit_snippet(snippet)?;
}
}
}
Ok(())
}

fn emit_diagnostics_in_json(&mut self, diagnostics: Vec<Diagnostic>) -> Result<()> {
fn emit_diagnostics_in_json(&mut self, diagnostics: &[PrintableDiagnostic]) -> Result<()> {
// Write each diagnostic as a single line of JSON.
for diagnostic in diagnostics {
let severity = match diagnostic.level() {
let severity = match diagnostic.level {
DiagnosticLevel::Error => "error",
DiagnosticLevel::Warning => "warning",
DiagnosticLevel::Allowed => continue,
};

let mut serializer = serde_json::Serializer::new(&mut *self.output);
let mut state = serializer.serialize_struct("Diagnostic", 5)?;
state.serialize_field("message", &diagnostic.message())?;
state.serialize_field("message", &diagnostic.message)?;
state.serialize_field("severity", severity)?;
state.serialize_field("span", &diagnostic.span())?;
state.serialize_field("notes", diagnostic.notes())?;
state.serialize_field("error_code", diagnostic.code())?;
state.serialize_field("span", &diagnostic.snippet)?;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should either encode just the span like we did before (but as diagnostic.snippet.span`, or encode the entire snippet.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR currently does option 1.
If you look at the implementation where snippet is encoded, all it actually encodes is the span.

Personally, I'd lean towards option 2 (encode the full snippet (span + text),
but wasn't sure what other opinions would be.

state.serialize_field("notes", &diagnostic.notes)?;
state.serialize_field("error_code", &diagnostic.code)?;
state.end()?;
writeln!(self.output)?; // Separate each diagnostic by a newline character.
}
Ok(())
}

fn emit_snippet(&mut self, span: &Span) -> Result<()> {
fn emit_snippet(&mut self, snippet: &Snippet) -> Result<()> {
// Display the file name and line row and column where the error began.
writeln!(
self.output,
" {} {}:{}:{}",
console::style("-->").blue().bold(),
Path::new(&span.file).display(),
span.start.row,
span.start.col,
Path::new(&snippet.span.file).display(),
snippet.span.start.row,
snippet.span.start.col,
)?;

// Display the line of code where the error occurred.
let file = self.files.iter().find(|f| f.relative_path == span.file).unwrap();
writeln!(self.output, "{}", file.get_snippet(span.start, span.end))?;
writeln!(self.output, "{}", snippet.text)?;

Ok(())
}
}

pub fn emit_totals(total_warnings: usize, total_errors: usize) -> Result<()> {
// Totals are always printed to stdout.
let stdout = &mut console::Term::stdout();

if total_warnings > 0 {
let warnings = console::style("Warnings").yellow().bold();
writeln!(stdout, "{warnings}: Compilation generated {total_warnings} warning(s)")?;
}
if total_errors > 0 {
let failed = console::style("Failed").red().bold();
writeln!(stdout, "{failed}: Compilation failed with {total_errors} error(s)")?;
}

Ok(())
}
Loading