From 48560eb1e26ab3fa8486796aefd6113e59df8191 Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Wed, 15 Apr 2026 15:55:17 -0400 Subject: [PATCH 1/5] Did everything except for the emitter so far. --- slicec/src/diagnostics/diagnostic.rs | 140 +++-------------------- slicec/src/diagnostics/mod.rs | 22 +++- slicec/src/parsers/mod.rs | 2 +- slicec/src/validators/cycle_detection.rs | 8 +- slicec/src/validators/dictionary.rs | 2 +- 5 files changed, 42 insertions(+), 132 deletions(-) diff --git a/slicec/src/diagnostics/diagnostic.rs b/slicec/src/diagnostics/diagnostic.rs index c85e2695..ed7233ae 100644 --- a/slicec/src/diagnostics/diagnostic.rs +++ b/slicec/src/diagnostics/diagnostic.rs @@ -1,34 +1,23 @@ // Copyright (c) ZeroC, Inc. -use super::{Error, Lint, Note}; -use crate::ast::Ast; -use crate::grammar::{attributes, Attributable, Entity}; -use crate::slice_file::{SliceFile, Span}; -use crate::slice_options::SliceOptions; +use super::{DiagnosticKind, Error, Lint, Note}; +use crate::slice_file::Span; /// A diagnostic is a message that is reported to the user during compilation. -/// It can either hold an [Error] or a [Lint]. #[derive(Debug)] pub struct Diagnostic { - kind: DiagnosticKind, - level: DiagnosticLevel, - span: Option, - scope: Option, - notes: Vec, + pub kind: DiagnosticKind, + pub span: Option, + pub scope: Option, + pub notes: Vec, } impl Diagnostic { /// Creates a new `Diagnostic` directly from a [`DiagnosticKind`]. /// The newly created `Diagnostic` has no `span`, `scope`, or `notes` set. pub fn new(kind: DiagnosticKind) -> Self { - let level = match &kind { - DiagnosticKind::Error(_) => DiagnosticLevel::Error, - DiagnosticKind::Lint(lint) => lint.default_diagnostic_level(), - }; - Diagnostic { kind, - level, span: None, scope: None, notes: Vec::new(), @@ -63,27 +52,6 @@ impl Diagnostic { } } - /// Returns the [level](DiagnosticLevel) of this diagnostic. - /// Note that this value may change after the diagnostic is reported, since levels can be altered by attributes. - pub fn level(&self) -> DiagnosticLevel { - self.level - } - - /// Returns the [Span] of this diagnostic if it has one. - pub fn span(&self) -> Option<&Span> { - self.span.as_ref() - } - - /// Returns the scope of this diagnostic if it has one. - pub fn scope(&self) -> Option<&String> { - self.scope.as_ref() - } - - /// Returns any [Notes](Note) associated with this diagnostic. - pub fn notes(&self) -> &[Note] { - &self.notes - } - pub fn set_span(mut self, span: &Span) -> Self { self.span = Some(span.to_owned()); self @@ -102,36 +70,11 @@ impl Diagnostic { self } - pub fn extend_notes>(mut self, iter: I) -> Self { - self.notes.extend(iter); - self - } - pub fn push_into(self, diagnostics: &mut Diagnostics) { diagnostics.0.push(self); } } -#[derive(Debug)] -pub enum DiagnosticKind { - Error(Error), - Lint(Lint), -} - -/// Diagnostic levels describe the severity of a diagnostic, and how the compiler should react to their emission. -#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] -pub enum DiagnosticLevel { - #[rustfmt::skip] // See https://github.com/rust-lang/rustfmt/issues/5801 - /// Diagnostics with the `Error` level will be emitted and will cause compilation to fail with a non-zero exit code. - Error, - - /// Diagnostics with the `Warning` level will be emitted, but will not influence the exit code of the compiler. - Warning, - - /// Diagnostics with the `Allowed` level will be suppressed and will not emit any message. - Allowed, -} - #[derive(Debug, Default)] pub struct Diagnostics(Vec); @@ -141,65 +84,12 @@ impl Diagnostics { Self::default() } - pub fn extend(&mut self, other: Diagnostics) { - self.0.extend(other.0); - } - /// Returns true if this contains any diagnostics that are errors. pub fn has_errors(&self) -> bool { let mut diagnostics = self.0.iter(); diagnostics.any(|diagnostic| matches!(diagnostic.kind, DiagnosticKind::Error(_))) } - /// Returns true if this contains no diagnostics. - pub fn is_empty(&self) -> bool { - self.0.is_empty() - } - - /// Returns the diagnostics this struct contains after it has patched and updated them. - /// Lint levels can be configured via attributes or command line options, but these aren't applied until this runs. - pub fn into_updated(mut self, ast: &Ast, files: &[SliceFile], options: &SliceOptions) -> Vec { - // Helper function that checks whether a lint should be allowed according to the provided identifiers. - fn is_lint_allowed_by<'b>(mut identifiers: impl Iterator, lint: &Lint) -> bool { - identifiers.any(|identifier| identifier == "All" || identifier == lint.lint_name()) - } - - // Helper function that checks whether a lint is allowed by attributes on the provided entity. - fn is_lint_allowed_by_attributes(attributable: &(impl Attributable + ?Sized), lint: &Lint) -> bool { - let attributes = attributable.all_attributes().into_iter(); - let mut allowed = attributes.filter_map(|a| a.downcast::()); - allowed.any(|allow| is_lint_allowed_by(allow.allowed_lints.iter(), lint)) - } - - for diagnostic in &mut self.0 { - // If this diagnostic is a lint, update its diagnostic level. Errors always have a level of `Error`. - if let DiagnosticKind::Lint(lint) = &diagnostic.kind { - // Check if the lint is allowed by an `--allow` flag passed on the command line. - if is_lint_allowed_by(options.allowed_lints.iter(), lint) { - diagnostic.level = DiagnosticLevel::Allowed; - } - - // If the diagnostic has a span, check if it's affected by an `allow` attribute on its file. - if let Some(span) = diagnostic.span() { - let file = files.iter().find(|f| f.relative_path == span.file).expect("no file"); - if is_lint_allowed_by_attributes(file, lint) { - diagnostic.level = DiagnosticLevel::Allowed; - } - } - - // If the diagnostic has a scope, check if it's affected by an `allow` attribute in that scope. - if let Some(scope) = diagnostic.scope() { - if let Ok(entity) = ast.find_element::(scope) { - if is_lint_allowed_by_attributes(entity, lint) { - diagnostic.level = DiagnosticLevel::Allowed; - } - } - } - } - } - self.0 - } - /// Returns the diagnostics held by this without any updates or patches. /// This should only be called by tests that want to bypass this behavior. pub fn into_inner(self) -> Vec { @@ -207,16 +97,16 @@ impl Diagnostics { } } -pub fn get_totals(diagnostics: &[Diagnostic]) -> (usize, usize) { - let (mut total_warnings, mut total_errors) = (0, 0); +impl std::ops::Deref for Diagnostics { + type Target = Vec; - for diagnostic in diagnostics { - match diagnostic.level() { - DiagnosticLevel::Error => total_errors += 1, - DiagnosticLevel::Warning => total_warnings += 1, - DiagnosticLevel::Allowed => {} - } + fn deref(&self) -> &Self::Target { + &self.0 } +} - (total_warnings, total_errors) +impl std::ops::DerefMut for Diagnostics { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } } diff --git a/slicec/src/diagnostics/mod.rs b/slicec/src/diagnostics/mod.rs index 61690c40..d7e9273d 100644 --- a/slicec/src/diagnostics/mod.rs +++ b/slicec/src/diagnostics/mod.rs @@ -4,13 +4,33 @@ mod diagnostic; mod errors; mod lints; -pub use diagnostic::*; +pub use diagnostic::{Diagnostic, Diagnostics}; pub use errors::Error; pub use lints::Lint; use crate::slice_file::Span; use serde::Serialize; +#[derive(Debug)] +pub enum DiagnosticKind { + Error(Error), + Lint(Lint), +} + +/// Diagnostic levels describe the severity of a diagnostic, and how the compiler should react to their emission. +#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub enum DiagnosticLevel { + #[rustfmt::skip] // See https://github.com/rust-lang/rustfmt/issues/5801 + /// Diagnostics with the `Error` level will be emitted and will cause compilation to fail with a non-zero exit code. + Error, + + /// Diagnostics with the `Warning` level will be emitted, but will not influence the exit code of the compiler. + Warning, + + /// Diagnostics with the `Allowed` level will be suppressed and will not emit any message. + Allowed, +} + /// Stores additional information about a diagnostic. #[derive(Serialize, Debug, Clone)] pub struct Note { diff --git a/slicec/src/parsers/mod.rs b/slicec/src/parsers/mod.rs index 442ee4a5..721a6d0b 100644 --- a/slicec/src/parsers/mod.rs +++ b/slicec/src/parsers/mod.rs @@ -25,7 +25,7 @@ pub fn parse_files(state: &mut CompilationState, symbols: &HashSet) { parse_file(file, &mut state.ast, &mut diagnostics, symbols.clone()); // Store any diagnostics that were emitted during parsing. - state.diagnostics.extend(diagnostics); + state.diagnostics.extend(diagnostics.into_inner()); } } diff --git a/slicec/src/validators/cycle_detection.rs b/slicec/src/validators/cycle_detection.rs index cd869968..c30db240 100644 --- a/slicec/src/validators/cycle_detection.rs +++ b/slicec/src/validators/cycle_detection.rs @@ -147,10 +147,10 @@ impl<'a> CycleDetector<'a> { let cycle_notes = self.dependency_stack.iter().map(|(_, field)| Self::get_note_for(field)); // Report the error. - Diagnostic::from_error(Error::InfiniteSizeCycle { type_id, cycle }) - .set_span(type_being_checked.1.span()) - .extend_notes(cycle_notes) - .push_into(self.diagnostics); + let mut error = Diagnostic::from_error(Error::InfiniteSizeCycle { type_id, cycle }); + error = error.set_span(type_being_checked.1.span()); + error.notes = cycle_notes.collect(); + self.diagnostics.push(error); } fn get_note_for(field: &Field) -> Note { diff --git a/slicec/src/validators/dictionary.rs b/slicec/src/validators/dictionary.rs index b9b10370..a0dc8b5a 100644 --- a/slicec/src/validators/dictionary.rs +++ b/slicec/src/validators/dictionary.rs @@ -42,7 +42,7 @@ fn check_dictionary_key_type(type_ref: &TypeRef) -> Option { // Convert each error into a note and add it to the struct key error. for e in errors { - error = error.add_note(e.message(), e.span()); + error = error.add_note(e.message(), e.span.as_ref()); } return Some(error); } From db37903280b2018d46d1465d2780e3564639bf90 Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Thu, 16 Apr 2026 13:39:02 -0400 Subject: [PATCH 2/5] Finished the refactoring portion. --- slicec/examples/parse.rs | 11 -- slicec/src/compilation_state.rs | 34 ++---- slicec/src/diagnostic_emitter.rs | 111 +++++++++-------- slicec/src/diagnostics/mod.rs | 5 +- .../src/diagnostics/reportable_diagnostic.rs | 112 ++++++++++++++++++ slicec/src/lib.rs | 12 ++ slicec/src/main.rs | 23 ++-- slicec/tests/attribute_tests.rs | 30 +++-- slicec/tests/diagnostic_output_tests.rs | 30 ++--- slicec/tests/test_helpers.rs | 33 +++--- 10 files changed, 248 insertions(+), 153 deletions(-) delete mode 100644 slicec/examples/parse.rs create mode 100644 slicec/src/diagnostics/reportable_diagnostic.rs diff --git a/slicec/examples/parse.rs b/slicec/examples/parse.rs deleted file mode 100644 index 115b9f51..00000000 --- a/slicec/examples/parse.rs +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright (c) ZeroC, Inc. - -use clap::Parser; -use slicec::slice_options::SliceOptions; -use std::process::exit; - -pub fn main() { - let options = SliceOptions::parse(); - let state = slicec::compile_from_options(&options); - exit(i32::from(state.emit_diagnostics(&options))); -} diff --git a/slicec/src/compilation_state.rs b/slicec/src/compilation_state.rs index 85793ea7..2ce106cb 100644 --- a/slicec/src/compilation_state.rs +++ b/slicec/src/compilation_state.rs @@ -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, ReportableDiagnostic}; use crate::slice_file::SliceFile; -use crate::slice_options::{DiagnosticFormat, SliceOptions}; +use crate::slice_options::SliceOptions; #[derive(Debug, Default)] pub struct CompilationState { @@ -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 { - self.diagnostics.into_updated(&self.ast, &self.files, options) + pub fn get_updated_diagnostics(&self, options: &SliceOptions) -> Vec { + self.diagnostics + .iter() + .map(|diagnostic| crate::diagnostics::convert_diagnostic(diagnostic, options, self)) + .collect() } } diff --git a/slicec/src/diagnostic_emitter.rs b/slicec/src/diagnostic_emitter.rs index 0d53ea2b..cb07c1db 100644 --- a/slicec/src/diagnostic_emitter.rs +++ b/slicec/src/diagnostic_emitter.rs @@ -1,42 +1,58 @@ // Copyright (c) ZeroC, Inc. -use crate::diagnostics::{Diagnostic, DiagnosticLevel}; -use crate::slice_file::{SliceFile, Span}; +use crate::diagnostics::{DiagnosticLevel, ReportableDiagnostic, Snippet}; use crate::slice_options::{DiagnosticFormat, SliceOptions}; use serde::ser::SerializeStruct; use serde::Serializer; 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, /// 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, - /// 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) -> 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: &[ReportableDiagnostic]) -> (usize, usize) { + 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<()> { + // 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: &[ReportableDiagnostic]) -> Result<()> { // Emit the diagnostics in whatever form the user requested. match self.diagnostic_format { DiagnosticFormat::Human => self.emit_diagnostics_in_human(diagnostics), @@ -44,27 +60,27 @@ impl<'a, T: Write> DiagnosticEmitter<'a, T> { } } - fn emit_diagnostics_in_human(&mut self, diagnostics: Vec) -> Result<()> { + fn emit_diagnostics_in_human(&mut self, diagnostics: &[ReportableDiagnostic]) -> 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, "{}: {}", @@ -72,18 +88,18 @@ impl<'a, T: Write> DiagnosticEmitter<'a, T> { console::style(¬e.message).bold(), )?; - if let Some(span) = ¬e.span { - self.emit_snippet(span)?; + if let Some(snippet) = ¬e.snippet { + self.emit_snippet(snippet)?; } } } Ok(()) } - fn emit_diagnostics_in_json(&mut self, diagnostics: Vec) -> Result<()> { + fn emit_diagnostics_in_json(&mut self, diagnostics: &[ReportableDiagnostic]) -> 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, @@ -91,48 +107,31 @@ impl<'a, T: Write> DiagnosticEmitter<'a, T> { 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)?; + 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(()) -} diff --git a/slicec/src/diagnostics/mod.rs b/slicec/src/diagnostics/mod.rs index d7e9273d..38990191 100644 --- a/slicec/src/diagnostics/mod.rs +++ b/slicec/src/diagnostics/mod.rs @@ -3,13 +3,14 @@ mod diagnostic; mod errors; mod lints; +mod reportable_diagnostic; pub use diagnostic::{Diagnostic, Diagnostics}; pub use errors::Error; pub use lints::Lint; +pub use reportable_diagnostic::*; use crate::slice_file::Span; -use serde::Serialize; #[derive(Debug)] pub enum DiagnosticKind { @@ -32,7 +33,7 @@ pub enum DiagnosticLevel { } /// Stores additional information about a diagnostic. -#[derive(Serialize, Debug, Clone)] +#[derive(Debug, Clone)] pub struct Note { pub message: String, pub span: Option, diff --git a/slicec/src/diagnostics/reportable_diagnostic.rs b/slicec/src/diagnostics/reportable_diagnostic.rs new file mode 100644 index 00000000..aeade0bb --- /dev/null +++ b/slicec/src/diagnostics/reportable_diagnostic.rs @@ -0,0 +1,112 @@ +// Copyright (c) ZeroC, Inc. + +use crate::compilation_state::CompilationState; +use crate::diagnostics::{Diagnostic, DiagnosticKind, DiagnosticLevel, Lint}; +use crate::grammar::{attributes, Attributable, Entity}; +use crate::slice_file::{SliceFile, Span}; +use crate::slice_options::SliceOptions; +use serde::Serialize; + +#[derive(Debug, Clone)] +pub struct ReportableDiagnostic { + pub message: String, + pub level: DiagnosticLevel, + pub code: String, + pub snippet: Option, + pub notes: Vec, +} + +#[derive(Debug, Clone, Serialize)] +pub struct ReportableNote { + pub message: String, + #[serde(rename = "span")] + pub snippet: Option, +} + +#[derive(Debug, Clone)] +pub struct Snippet { + pub span: Span, + pub text: String, +} +// TODO should we send the text snippet in JSON diagnostic format? Or just the range? +impl Serialize for Snippet { + fn serialize(&self, serializer: S) -> Result { + self.span.serialize(serializer) + } +} + +pub fn convert_diagnostic( + diagnostic: &Diagnostic, + options: &SliceOptions, + compilation_state: &CompilationState, +) -> ReportableDiagnostic { + let notes = diagnostic.notes.iter().map(|n| ReportableNote { + message: n.message.clone(), + snippet: get_snippet(&n.span, &compilation_state.files), + }); + + ReportableDiagnostic { + message: diagnostic.message(), + level: get_diagnostic_level_for(diagnostic, options, compilation_state), + code: diagnostic.code().to_owned(), + snippet: get_snippet(&diagnostic.span, &compilation_state.files), + notes: notes.collect(), + } +} + +fn get_diagnostic_level_for( + diagnostic: &Diagnostic, + options: &SliceOptions, + compilation_state: &CompilationState, +) -> DiagnosticLevel { + // Only lints can have their diagnostic levels changed (through attributes or command-line options). + // For other kinds of diagnostics, we can immediately return their levels. + let lint = match &diagnostic.kind { + DiagnosticKind::Error(_) => return DiagnosticLevel::Error, + DiagnosticKind::Lint(lint) => lint, + }; + + // Helper function that checks whether a lint should be allowed according to the provided identifiers. + fn is_lint_allowed_by<'b>(mut identifiers: impl Iterator, lint: &Lint) -> bool { + identifiers.any(|identifier| identifier == "All" || identifier == lint.lint_name()) + } + + // Helper function that checks whether a lint is allowed by attributes on the provided entity. + fn is_lint_allowed_by_attributes(attributable: &(impl Attributable + ?Sized), lint: &Lint) -> bool { + let attributes = attributable.all_attributes().into_iter(); + let mut allowed = attributes.filter_map(|a| a.downcast::()); + allowed.any(|allow| is_lint_allowed_by(allow.allowed_lints.iter(), lint)) + } + + // Check if the lint is allowed by an `--allow` flag passed on the command line. + if is_lint_allowed_by(options.allowed_lints.iter(), lint) { + return DiagnosticLevel::Allowed; + } + + // If the diagnostic has a span, check if it's affected by an `allow` attribute on its file. + if let Some(span) = &diagnostic.span { + let file = compilation_state.files.iter().find(|f| f.relative_path == span.file); + if is_lint_allowed_by_attributes(file.unwrap(), lint) { + return DiagnosticLevel::Allowed; + } + } + + // If the diagnostic has a scope, check if it's affected by an `allow` attribute in that scope. + if let Some(scope) = &diagnostic.scope { + if let Ok(entity) = compilation_state.ast.find_element::(scope) { + if is_lint_allowed_by_attributes(entity, lint) { + return DiagnosticLevel::Allowed; + } + } + } + + // Otherwise, we just return the default diagnostic level for this lint. + lint.default_diagnostic_level() +} + +fn get_snippet(span: &Option, files: &[SliceFile]) -> Option { + let span = span.clone()?; + let snippet_file = files.iter().find(|file| file.relative_path == span.file)?; + let text = snippet_file.get_snippet(span.start, span.end); + Some(Snippet { span, text }) +} diff --git a/slicec/src/lib.rs b/slicec/src/lib.rs index 705f6b87..b31786f5 100644 --- a/slicec/src/lib.rs +++ b/slicec/src/lib.rs @@ -24,6 +24,12 @@ pub fn compile_from_options(options: &SliceOptions) -> CompilationState { // Create an instance of `CompilationState` for holding all the compiler's state. let mut state = CompilationState::create(); + // Disable colored output if the user requested we do so. + if options.disable_color { + console::set_colors_enabled(false); + console::set_colors_enabled_stderr(false); + } + // Recursively resolve any Slice files contained in the paths specified by the user. state.files = file_util::resolve_files_from(options, &mut state.diagnostics); @@ -38,6 +44,12 @@ pub fn compile_from_strings(inputs: &[&str], options: Option<&SliceOptions>) -> // Create an instance of `CompilationState` for holding all the compiler's state. let mut state = CompilationState::create(); + // Disable colored output if the user requested we do so. + if options.map(|opts| opts.disable_color).unwrap_or_default() { + console::set_colors_enabled(false); + console::set_colors_enabled_stderr(false); + } + // Create a Slice file from each of the strings. for (i, &input) in inputs.iter().enumerate() { let slice_file = SliceFile::new(format!("string-{i}"), input.to_owned(), false); diff --git a/slicec/src/main.rs b/slicec/src/main.rs index d8ceacc9..ae2289bf 100644 --- a/slicec/src/main.rs +++ b/slicec/src/main.rs @@ -10,7 +10,6 @@ use clap::Parser; use slice_codec::decoder::Decoder; use slice_codec::encoder::Encoder; -use slicec::compilation_state::CompilationState; use slicec::diagnostic_emitter::DiagnosticEmitter; use slicec::diagnostics::Diagnostics; use slicec::slice_options::{DiagnosticFormat, Plugin, SliceOptions}; @@ -182,17 +181,13 @@ fn main() -> ExitCode { let slice_options = SliceOptions::parse(); // Perform the compilation. - let compilation_state = slicec::compile_from_options(&slice_options); - let CompilationState { - ast, - mut diagnostics, - files, - } = compilation_state; + let mut compilation_state = slicec::compile_from_options(&slice_options); + let diagnostics = &mut compilation_state.diagnostics; // Only invoke the plugins if there were no errors in the Slice files. if !diagnostics.has_errors() { // Encode the request which will be sent to each of the code-generation plugins. - let encoded_request = match encode_generate_code_request(&files) { + let encoded_request = match encode_generate_code_request(&compilation_state.files) { Ok(result) => result, Err(error) => { eprintln!("Critical error: failed to encode request payload!\n{error:?}"); @@ -215,22 +210,22 @@ fn main() -> ExitCode { .and_then(|payload| handle_generator_response(payload, &slice_options.output_dir)) // Returns any diagnostics if the payload successfully decoded. .unwrap_or_else(|err| convert_generator_error_to_diagnostic(generator, err)); - diagnostics.extend(generator_diagnostics); // Store the generator's diagnostics for later emission. + diagnostics.extend(generator_diagnostics.into_inner()); // Store generator's diagnostics for later emission. } } // Process the diagnostics (filter out allowed lints, and update diagnostic levels as necessary). - let updated_diagnostics = diagnostics.into_updated(&ast, &files, &slice_options); - let (warning_count, error_count) = slicec::diagnostics::get_totals(&updated_diagnostics); + let updated_diagnostics = compilation_state.get_updated_diagnostics(&slice_options); // 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, &slice_options, &files); - DiagnosticEmitter::emit_diagnostics(&mut emitter, updated_diagnostics).expect("failed to emit diagnostics"); + let mut emitter = DiagnosticEmitter::new(&mut stderr, &slice_options); + emitter.emit_diagnostics(&updated_diagnostics).expect("failed to emit diagnostics"); + let (warning_count, error_count) = DiagnosticEmitter::get_totals(&updated_diagnostics); // Only emit the summary message if we're writing human-readable output. if slice_options.diagnostic_format == DiagnosticFormat::Human { - slicec::diagnostic_emitter::emit_totals(warning_count, error_count).expect("failed to emit totals"); + DiagnosticEmitter::emit_totals(warning_count, error_count).expect("failed to emit totals"); } // Finished. diff --git a/slicec/tests/attribute_tests.rs b/slicec/tests/attribute_tests.rs index 37f2d658..512731eb 100644 --- a/slicec/tests/attribute_tests.rs +++ b/slicec/tests/attribute_tests.rs @@ -4,11 +4,12 @@ mod test_helpers; mod attributes { use crate::test_helpers::*; - use slicec::diagnostics::{Diagnostic, Error, Lint}; + use slicec::diagnostics::{Diagnostic, DiagnosticLevel, Error, Lint}; use slicec::grammar::attributes::*; mod allow { use super::*; + use slicec::slice_options::SliceOptions; use test_case::test_case; #[test] @@ -92,6 +93,7 @@ mod attributes { #[test_case("IncorrectDocComment", [0, 1]; "incorrect_doc_comment")] fn allow_only_specified_lints(arguments: &str, expected_indexes: [usize; L]) { // Arrange + let options = SliceOptions::default(); let slice = format!( " [[allow({arguments})]] @@ -109,10 +111,11 @@ mod attributes { ); // Act - let diagnostics = parse_for_diagnostics(slice); + let compilation_state = parse(slice, Some(&options)); + let updated_diagnostics = compilation_state.get_updated_diagnostics(&options); // Assert - let mut all_lints = vec![ + let all_expected_lints = vec![ Diagnostic::from_lint(Lint::Deprecated { identifier: "S".to_owned(), reason: Some("test".to_owned()), @@ -124,16 +127,17 @@ mod attributes { message: "comment has a 'returns' tag, but only operations can return".to_owned(), }), ]; - // Filter out any lints that should be allowed by the supplied test arguments. - let mut index = 0; - all_lints.retain(|_| { - index += 1; - expected_indexes.contains(&(index - 1)) - }); - let expected: [Diagnostic; L] = all_lints.try_into().unwrap(); - - // Check that only the correct warnings were emitted. - check_diagnostics(diagnostics, expected); + for (index, lint) in updated_diagnostics.iter().enumerate() { + assert!(lint.message == all_expected_lints[index].message()); + assert!(lint.code == all_expected_lints[index].code()); + + // Check that the correct lints were marked as `Warning` or `Allowed` accordingly. + let expected_level = match expected_indexes.contains(&index) { + true => DiagnosticLevel::Warning, + false => DiagnosticLevel::Allowed, + }; + assert!(lint.level == expected_level); + } } } diff --git a/slicec/tests/diagnostic_output_tests.rs b/slicec/tests/diagnostic_output_tests.rs index 5cb5c80c..226d9741 100644 --- a/slicec/tests/diagnostic_output_tests.rs +++ b/slicec/tests/diagnostic_output_tests.rs @@ -28,13 +28,13 @@ mod output { // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.diagnostics.into_updated(&state.ast, &state.files, &options); + let diagnostics = state.get_updated_diagnostics(&options); let mut output: Vec = Vec::new(); - let mut emitter = DiagnosticEmitter::new(&mut output, &options, &state.files); + let mut emitter = DiagnosticEmitter::new(&mut output, &options); // Act - emitter.emit_diagnostics(diagnostics).unwrap(); + emitter.emit_diagnostics(&diagnostics).unwrap(); // Assert let expected = concat!( @@ -72,13 +72,13 @@ mod output { // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.diagnostics.into_updated(&state.ast, &state.files, &options); + let diagnostics = state.get_updated_diagnostics(&options); let mut output: Vec = Vec::new(); - let mut emitter = DiagnosticEmitter::new(&mut output, &options, &state.files); + let mut emitter = DiagnosticEmitter::new(&mut output, &options); // Act - emitter.emit_diagnostics(diagnostics).unwrap(); + emitter.emit_diagnostics(&diagnostics).unwrap(); // Assert let expected = "\ @@ -129,13 +129,13 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.diagnostics.into_updated(&state.ast, &state.files, &options); + let diagnostics = state.get_updated_diagnostics(&options); let mut output: Vec = Vec::new(); - let mut emitter = DiagnosticEmitter::new(&mut output, &options, &state.files); + let mut emitter = DiagnosticEmitter::new(&mut output, &options); // Act - emitter.emit_diagnostics(diagnostics).unwrap(); + emitter.emit_diagnostics(&diagnostics).unwrap(); // Assert assert_eq!("", String::from_utf8(output).unwrap()); @@ -162,13 +162,13 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.diagnostics.into_updated(&state.ast, &state.files, &options); + let diagnostics = state.get_updated_diagnostics(&options); let mut output: Vec = Vec::new(); - let mut emitter = DiagnosticEmitter::new(&mut output, &options, &state.files); + let mut emitter = DiagnosticEmitter::new(&mut output, &options); // Act - emitter.emit_diagnostics(diagnostics).unwrap(); + emitter.emit_diagnostics(&diagnostics).unwrap(); // Assert: Only one of the two lints should be allowed. let expected = concat!( @@ -189,13 +189,13 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator }; let state = parse(slice, Some(&options)); - let diagnostics = state.diagnostics.into_updated(&state.ast, &state.files, &options); + let diagnostics = state.get_updated_diagnostics(&options); let mut output: Vec = Vec::new(); - let mut emitter = DiagnosticEmitter::new(&mut output, &options, &state.files); + let mut emitter = DiagnosticEmitter::new(&mut output, &options); // Act - emitter.emit_diagnostics(diagnostics).unwrap(); + emitter.emit_diagnostics(&diagnostics).unwrap(); // Assert let expected = "\ diff --git a/slicec/tests/test_helpers.rs b/slicec/tests/test_helpers.rs index 4052efb5..8019bcc6 100644 --- a/slicec/tests/test_helpers.rs +++ b/slicec/tests/test_helpers.rs @@ -8,7 +8,7 @@ use slicec::ast::Ast; use slicec::compilation_state::CompilationState; use slicec::compile_from_strings; -use slicec::diagnostics::{Diagnostic, DiagnosticLevel}; +use slicec::diagnostics::Diagnostic; use slicec::slice_options::SliceOptions; /// This function parses the provided Slice file. @@ -35,7 +35,7 @@ pub fn parse_for_ast(slice: impl Into) -> Ast { #[must_use] pub fn parse_multiple_for_ast(slice: &[&str]) -> Ast { let compilation_state = compile_from_strings(slice, None); - if compilation_state.diagnostics.has_errors() { + if !compilation_state.diagnostics.is_empty() { panic!("{:?}", compilation_state.diagnostics); } compilation_state.ast @@ -51,15 +51,10 @@ pub fn parse_for_diagnostics(slice: impl Into) -> Vec { /// Each string is treated as a separate Slice file by the parser. #[must_use] pub fn parse_multiple_for_diagnostics(slice: &[&str]) -> Vec { - let compilation_state = compile_from_strings(slice, None); - let slice_options = SliceOptions::default(); - - let mut diagnostics = compilation_state.into_diagnostics(&slice_options); - diagnostics.retain(|diagnostic| diagnostic.level() != DiagnosticLevel::Allowed); - diagnostics + compile_from_strings(slice, None).diagnostics.into_inner() } -/// Asserts that the provided slice parses okay, producing no errors. +/// Asserts that the provided slice parses okay, producing no diagnostics. pub fn assert_parses(slice: impl Into) { let diagnostics = parse_for_diagnostics(slice); let expected: [Diagnostic; 0] = []; // Compiler needs the type hint. @@ -115,17 +110,25 @@ pub fn check_diagnostics(diagnostics: Vec, expected: } // If a span was provided, check that it matches. - if expect.span().is_some() && expect.span() != diagnostic.span() { + if expect.span.is_some() && expect.span != diagnostic.span { eprintln!("diagnostic spans didn't match:"); - eprintln!("\texpected: \"{:?}\"", expect.span()); - eprintln!("\t but got: \"{:?}\"", diagnostic.span()); + eprintln!("\texpected: \"{:?}\"", expect.span); + eprintln!("\t but got: \"{:?}\"", diagnostic.span); + failed = true; + } + + // If a scope was provided, check that it matches. + if expect.scope.is_some() && expect.scope != diagnostic.scope { + eprintln!("diagnostic scopes didn't match:"); + eprintln!("\texpected: \"{:?}\"", expect.scope); + eprintln!("\t but got: \"{:?}\"", diagnostic.scope); failed = true; } // If notes were provided, check that they match. - if !expect.notes().is_empty() { - let expected_notes = expect.notes(); - let emitted_notes = diagnostic.notes(); + if !expect.notes.is_empty() { + let expected_notes = expect.notes; + let emitted_notes = diagnostic.notes; if expected_notes.len() != emitted_notes.len() { eprintln!( "Expected {} notes, but got {}.", From 02df3381841b712ff21c710bbbd64530d6af93a5 Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Mon, 20 Apr 2026 11:06:39 -0400 Subject: [PATCH 3/5] Better comment and changed the name. --- slicec/src/compilation_state.rs | 4 ++-- slicec/src/diagnostic_emitter.rs | 10 +++++----- slicec/src/diagnostics/diagnostic.rs | 8 ++++---- slicec/src/diagnostics/mod.rs | 4 ++-- ...ortable_diagnostic.rs => printable_diagnostic.rs} | 12 ++++++------ slicec/src/main.rs | 6 +++--- slicec/tests/attribute_tests.rs | 3 +-- slicec/tests/diagnostic_output_tests.rs | 10 +++++----- 8 files changed, 28 insertions(+), 29 deletions(-) rename slicec/src/diagnostics/{reportable_diagnostic.rs => printable_diagnostic.rs} (94%) diff --git a/slicec/src/compilation_state.rs b/slicec/src/compilation_state.rs index 2ce106cb..2db5a29f 100644 --- a/slicec/src/compilation_state.rs +++ b/slicec/src/compilation_state.rs @@ -1,7 +1,7 @@ // Copyright (c) ZeroC, Inc. use crate::ast::Ast; -use crate::diagnostics::{Diagnostics, ReportableDiagnostic}; +use crate::diagnostics::{Diagnostics, PrintableDiagnostic}; use crate::slice_file::SliceFile; use crate::slice_options::SliceOptions; @@ -42,7 +42,7 @@ impl CompilationState { } } - pub fn get_updated_diagnostics(&self, options: &SliceOptions) -> Vec { + pub fn get_printable_diagnostics(&self, options: &SliceOptions) -> Vec { self.diagnostics .iter() .map(|diagnostic| crate::diagnostics::convert_diagnostic(diagnostic, options, self)) diff --git a/slicec/src/diagnostic_emitter.rs b/slicec/src/diagnostic_emitter.rs index cb07c1db..1958987a 100644 --- a/slicec/src/diagnostic_emitter.rs +++ b/slicec/src/diagnostic_emitter.rs @@ -1,6 +1,6 @@ // Copyright (c) ZeroC, Inc. -use crate::diagnostics::{DiagnosticLevel, ReportableDiagnostic, Snippet}; +use crate::diagnostics::{DiagnosticLevel, PrintableDiagnostic, Snippet}; use crate::slice_options::{DiagnosticFormat, SliceOptions}; use serde::ser::SerializeStruct; use serde::Serializer; @@ -22,7 +22,7 @@ impl<'a> DiagnosticEmitter<'a> { } } - pub fn get_totals(diagnostics: &[ReportableDiagnostic]) -> (usize, usize) { + pub fn get_totals(diagnostics: &[PrintableDiagnostic]) -> (usize, usize) { let (mut total_warnings, mut total_errors) = (0, 0); for diagnostic in diagnostics { @@ -52,7 +52,7 @@ impl<'a> DiagnosticEmitter<'a> { Ok(()) } - pub fn emit_diagnostics(&mut self, diagnostics: &[ReportableDiagnostic]) -> Result<()> { + 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), @@ -60,7 +60,7 @@ impl<'a> DiagnosticEmitter<'a> { } } - fn emit_diagnostics_in_human(&mut self, diagnostics: &[ReportableDiagnostic]) -> 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. @@ -96,7 +96,7 @@ impl<'a> DiagnosticEmitter<'a> { Ok(()) } - fn emit_diagnostics_in_json(&mut self, diagnostics: &[ReportableDiagnostic]) -> 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 { diff --git a/slicec/src/diagnostics/diagnostic.rs b/slicec/src/diagnostics/diagnostic.rs index ed7233ae..0898b818 100644 --- a/slicec/src/diagnostics/diagnostic.rs +++ b/slicec/src/diagnostics/diagnostic.rs @@ -3,7 +3,7 @@ use super::{DiagnosticKind, Error, Lint, Note}; use crate::slice_file::Span; -/// A diagnostic is a message that is reported to the user during compilation. +/// A message that is reported by the compiler to provide information and context for errors, warnings, or other issues. #[derive(Debug)] pub struct Diagnostic { pub kind: DiagnosticKind, @@ -75,11 +75,12 @@ impl Diagnostic { } } +/// Stores a collection of diagnostics. #[derive(Debug, Default)] pub struct Diagnostics(Vec); impl Diagnostics { - /// Creates a new diagnostics container that is empty. + /// Creates a new (empty) collection of diagnostics. pub fn new() -> Self { Self::default() } @@ -90,8 +91,7 @@ impl Diagnostics { diagnostics.any(|diagnostic| matches!(diagnostic.kind, DiagnosticKind::Error(_))) } - /// Returns the diagnostics held by this without any updates or patches. - /// This should only be called by tests that want to bypass this behavior. + /// Consumes this [`Diagnostics`] and returns its inner collection of [`Diagnostic`]s. pub fn into_inner(self) -> Vec { self.0 } diff --git a/slicec/src/diagnostics/mod.rs b/slicec/src/diagnostics/mod.rs index 38990191..8df4f29b 100644 --- a/slicec/src/diagnostics/mod.rs +++ b/slicec/src/diagnostics/mod.rs @@ -3,12 +3,12 @@ mod diagnostic; mod errors; mod lints; -mod reportable_diagnostic; +mod printable_diagnostic; pub use diagnostic::{Diagnostic, Diagnostics}; pub use errors::Error; pub use lints::Lint; -pub use reportable_diagnostic::*; +pub use printable_diagnostic::*; use crate::slice_file::Span; diff --git a/slicec/src/diagnostics/reportable_diagnostic.rs b/slicec/src/diagnostics/printable_diagnostic.rs similarity index 94% rename from slicec/src/diagnostics/reportable_diagnostic.rs rename to slicec/src/diagnostics/printable_diagnostic.rs index aeade0bb..d259ea6b 100644 --- a/slicec/src/diagnostics/reportable_diagnostic.rs +++ b/slicec/src/diagnostics/printable_diagnostic.rs @@ -8,16 +8,16 @@ use crate::slice_options::SliceOptions; use serde::Serialize; #[derive(Debug, Clone)] -pub struct ReportableDiagnostic { +pub struct PrintableDiagnostic { pub message: String, pub level: DiagnosticLevel, pub code: String, pub snippet: Option, - pub notes: Vec, + pub notes: Vec, } #[derive(Debug, Clone, Serialize)] -pub struct ReportableNote { +pub struct PrintableNote { pub message: String, #[serde(rename = "span")] pub snippet: Option, @@ -39,13 +39,13 @@ pub fn convert_diagnostic( diagnostic: &Diagnostic, options: &SliceOptions, compilation_state: &CompilationState, -) -> ReportableDiagnostic { - let notes = diagnostic.notes.iter().map(|n| ReportableNote { +) -> PrintableDiagnostic { + let notes = diagnostic.notes.iter().map(|n| PrintableNote { message: n.message.clone(), snippet: get_snippet(&n.span, &compilation_state.files), }); - ReportableDiagnostic { + PrintableDiagnostic { message: diagnostic.message(), level: get_diagnostic_level_for(diagnostic, options, compilation_state), code: diagnostic.code().to_owned(), diff --git a/slicec/src/main.rs b/slicec/src/main.rs index ae2289bf..5e1383be 100644 --- a/slicec/src/main.rs +++ b/slicec/src/main.rs @@ -123,7 +123,7 @@ fn handle_generator_response(response_payload: Vec, output_dir: &Option ExitCode { } // Process the diagnostics (filter out allowed lints, and update diagnostic levels as necessary). - let updated_diagnostics = compilation_state.get_updated_diagnostics(&slice_options); + let updated_diagnostics = compilation_state.get_printable_diagnostics(&slice_options); // 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, &slice_options); - emitter.emit_diagnostics(&updated_diagnostics).expect("failed to emit diagnostics"); + emitter.emit_diagnostics(&updated_diagnostics).expect("failed to emit"); let (warning_count, error_count) = DiagnosticEmitter::get_totals(&updated_diagnostics); // Only emit the summary message if we're writing human-readable output. diff --git a/slicec/tests/attribute_tests.rs b/slicec/tests/attribute_tests.rs index 512731eb..e9588960 100644 --- a/slicec/tests/attribute_tests.rs +++ b/slicec/tests/attribute_tests.rs @@ -111,8 +111,7 @@ mod attributes { ); // Act - let compilation_state = parse(slice, Some(&options)); - let updated_diagnostics = compilation_state.get_updated_diagnostics(&options); + let updated_diagnostics = parse(slice, Some(&options)).get_printable_diagnostics(&options); // Assert let all_expected_lints = vec![ diff --git a/slicec/tests/diagnostic_output_tests.rs b/slicec/tests/diagnostic_output_tests.rs index 226d9741..8135fe76 100644 --- a/slicec/tests/diagnostic_output_tests.rs +++ b/slicec/tests/diagnostic_output_tests.rs @@ -28,7 +28,7 @@ mod output { // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.get_updated_diagnostics(&options); + let diagnostics = state.get_printable_diagnostics(&options); let mut output: Vec = Vec::new(); let mut emitter = DiagnosticEmitter::new(&mut output, &options); @@ -72,7 +72,7 @@ mod output { // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.get_updated_diagnostics(&options); + let diagnostics = state.get_printable_diagnostics(&options); let mut output: Vec = Vec::new(); let mut emitter = DiagnosticEmitter::new(&mut output, &options); @@ -129,7 +129,7 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.get_updated_diagnostics(&options); + let diagnostics = state.get_printable_diagnostics(&options); let mut output: Vec = Vec::new(); let mut emitter = DiagnosticEmitter::new(&mut output, &options); @@ -162,7 +162,7 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator // Parse the Slice file. let state = parse(slice, Some(&options)); - let diagnostics = state.get_updated_diagnostics(&options); + let diagnostics = state.get_printable_diagnostics(&options); let mut output: Vec = Vec::new(); let mut emitter = DiagnosticEmitter::new(&mut output, &options); @@ -189,7 +189,7 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator }; let state = parse(slice, Some(&options)); - let diagnostics = state.get_updated_diagnostics(&options); + let diagnostics = state.get_printable_diagnostics(&options); let mut output: Vec = Vec::new(); let mut emitter = DiagnosticEmitter::new(&mut output, &options); From d66a4119ffa9dcc535f54791b476a33d6730f934 Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Mon, 20 Apr 2026 11:31:25 -0400 Subject: [PATCH 4/5] Slightly easier to review this way. --- slicec/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slicec/src/main.rs b/slicec/src/main.rs index 5e1383be..7f3dd6c7 100644 --- a/slicec/src/main.rs +++ b/slicec/src/main.rs @@ -216,12 +216,12 @@ fn main() -> ExitCode { // Process the diagnostics (filter out allowed lints, and update diagnostic levels as necessary). let updated_diagnostics = compilation_state.get_printable_diagnostics(&slice_options); + let (warning_count, error_count) = DiagnosticEmitter::get_totals(&updated_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, &slice_options); emitter.emit_diagnostics(&updated_diagnostics).expect("failed to emit"); - let (warning_count, error_count) = DiagnosticEmitter::get_totals(&updated_diagnostics); // Only emit the summary message if we're writing human-readable output. if slice_options.diagnostic_format == DiagnosticFormat::Human { From bfb024295fa6416c6e334f5ebe8dacb889947a8a Mon Sep 17 00:00:00 2001 From: Austin Henriksen Date: Mon, 20 Apr 2026 15:23:19 -0400 Subject: [PATCH 5/5] More comments in the printable_diagnostics file. --- slicec/src/diagnostics/printable_diagnostic.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/slicec/src/diagnostics/printable_diagnostic.rs b/slicec/src/diagnostics/printable_diagnostic.rs index d259ea6b..8692b8ae 100644 --- a/slicec/src/diagnostics/printable_diagnostic.rs +++ b/slicec/src/diagnostics/printable_diagnostic.rs @@ -7,6 +7,8 @@ use crate::slice_file::{SliceFile, Span}; use crate::slice_options::SliceOptions; use serde::Serialize; +/// A printable representation of a [`Diagnostic`], whose [`DiagnosticLevel`] has been computed (taking into account any +/// 'allow' attributes or command-line flags), and that has pre-extracted text snippets to display alongside messages. #[derive(Debug, Clone)] pub struct PrintableDiagnostic { pub message: String, @@ -35,6 +37,7 @@ impl Serialize for Snippet { } } +/// Creates a [`PrintableDiagnostic`] from the provided [`Diagnostic`]. pub fn convert_diagnostic( diagnostic: &Diagnostic, options: &SliceOptions, @@ -54,6 +57,7 @@ pub fn convert_diagnostic( } } +/// Returns the [`DiagnosticLevel`] that the provided [`Diagnostic`] should be emitted with. fn get_diagnostic_level_for( diagnostic: &Diagnostic, options: &SliceOptions, @@ -104,6 +108,8 @@ fn get_diagnostic_level_for( lint.default_diagnostic_level() } +/// If `span` is `Some`, this tries to extract a text snippet corresponding to the file & locations contained in the +/// span. If `span` is `None` or if the text couldn't be extracted, this returns `None`. fn get_snippet(span: &Option, files: &[SliceFile]) -> Option { let span = span.clone()?; let snippet_file = files.iter().find(|file| file.relative_path == span.file)?;