Report Which Plugin a Diagnostic came from by InsertCreativityHere · Pull Request #784 · icerpc/slicec

InsertCreativityHere · 2026-05-29T18:41:31Z

This PR adds a field to our Diagnostic types tracking which plugin (if any) a diagnostic came from.
In the case of multiple plugins reporting the same error, we still only emit a single error, but it's now more clear that it was reported by multiple plugins.

The main advantage to doing this is that errors about the plugin itself (like providing a bad option) will clearly identify which plugin had the problem.

Here is sample output from the console:

And for JSON output, we now include a new "reported_by":[...] field.
The Task will need to be updated to properly handle this field in a follow-up PR.

InsertCreativityHere · 2026-05-29T18:42:28Z

 /// An annotated version 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(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq)]


We implement PartialEq by hand now, so that we exclude the reported_by from the equality check.

InsertCreativityHere · 2026-05-29T18:43:48Z

    });

+    // If the diagnostic was reported by a plugin, we just use the filename of the plugin (not its entire path).
+    let reported_by = diagnostic.plugin.as_deref().map_or("slicec", |plugin_path| {


I went back and forth on this, but I decided that in the case where the diagnostic is issued by slicec, it's worthwhile to still explicitly say that this came from slicec, and not a plugin.

It also makes the implementation cleaner and keep the error format 100% consistent, instead of being a little different for plugin diagnostics vs those about bad syntax for example.

Note that AnnotatedDiagnostic is the "finished" form of a diagnostic.
These are always de-duplicated, and are what we're actually going to print to the user.

The normal Diagnostic type is what gets reported throughout the code, and before we exit, we convert the Diagnostics to AnnotatedDiagnostics so that we can print them.

Therefore Diagnostic can have at most one reporter (since only one thing reported it).
But AnnotatedDiagnostic can have multiple (since duplicate diagnostics will of been merged together).

InsertCreativityHere · 2026-05-29T18:47:49Z

+            if let Some(original) = annotated_diagnostics.iter_mut().find(|d| **d == converted) {
+                original.reported_by.extend(converted.reported_by);
+            } else {
                annotated_diagnostics.push(converted);


Previously, we completely threw away duplicate diagnostics.
Now if we encounter a duplicate, we update the original's reported_by before throwing away the duplicate now.

InsertCreativityHere · 2026-05-29T18:48:17Z

    /// present in the returned [`Vec`].
    pub fn get_annotated_diagnostics(&self, options: &SliceOptions) -> Vec<AnnotatedDiagnostic> {
-        let mut annotated_diagnostics = Vec::new();
+        let mut annotated_diagnostics = Vec::<AnnotatedDiagnostic>::new();


I have no idea why, but the compiler insists on putting a type-hint here now.
It seems unnecessary to me, but who am I to argue with the compiler...

InsertCreativityHere · 2026-05-29T18:48:42Z

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

+            // Emit what the diagnostic was reported by.
+            writeln!(self.output, "  Reported by: [{}]", diagnostic.reported_by.join(", "))?;


See the diagnostic_output_test or my PR description for an example of what the output looks like.

InsertCreativityHere · 2026-05-29T18:50:14Z

-            diagnostics.extend(generator_diagnostics.into_inner());
+            // Store generator's diagnostics for later emission, after setting the plugin that reported them.
+            for mut generator_diagnostic in generator_diagnostics.into_inner() {
+                generator_diagnostic.plugin = Some(generator.path.clone());


Where we actually set plugin.
This is after we've finished (or tried) to run the plugin and are storing its diagnostics.

InsertCreativityHere · 2026-05-29T18:51:48Z


    // Construct and push the converted 'slicec' `Diagnostic`.
-    let converted_diagnostic = slicec::diagnostics::Diagnostic { kind, span, scope, notes };
+    let converted_diagnostic = slicec::diagnostics::Diagnostic { kind, span, scope, notes, plugin: None };


We construct the diagnostic in one-shot here, so had to update it to include the new field.
We manually set it to it's default value (None).

Rust doesn't have the idea of default field values like other languages.

InsertCreativityHere · 2026-05-29T18:52:35Z

@@ -156,9 +180,13 @@ error [E008]: invalid enum 'E': enums must contain at least one enumerator
        // Assert
        assert_eq!(converted_diagnostics.len(), 4);
        assert_eq!(converted_diagnostics[0].message, "This is a test");


Check that the reported_bys are correct after merging.

Copilot

Pull request overview

This PR adds diagnostic provenance so emitted diagnostics indicate whether they came from slicec or one or more plugins, including JSON output support.

Changes:

Adds plugin/reporting metadata to diagnostics and annotated diagnostics.
Emits Reported by in human output and reported_by in JSON output.
Updates diagnostic output tests and duplicate diagnostic merging behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`slicec/src/diagnostics/diagnostic.rs`	Adds optional plugin source tracking to core diagnostics.
`slicec/src/diagnostics/annotated_diagnostic.rs`	Converts plugin paths into displayed reporter names.
`slicec/src/compilation_state.rs`	Merges duplicate annotated diagnostics while aggregating reporters.
`slicec/src/diagnostic_emitter.rs`	Emits reporter information in human and JSON diagnostic formats.
`slicec/src/main.rs`	Tags plugin-generated diagnostics with the generator path.
`slicec/src/slice_file_converter.rs`	Initializes converted plugin response diagnostics with no direct plugin source.
`slicec/tests/diagnostic_output_tests.rs`	Updates expected output and duplicate merging tests for reporter metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…I guess...

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

bernardnormier · 2026-05-29T21:16:44Z

    /// Any additional information that should be reported alongside this diagnostic's main message.
    pub notes: Vec<Note>,
+
+    /// The plugin that reported this diagnostic, or `None` if this diagnostic was reported by 'slicec' itself.


We use the term plugin and not generator in slicec?

InsertCreativityHere added 3 commits May 29, 2026 13:22

Implemented the tracking of which plugin reported what.

f298fca

Slight tweaks.

00f863d

Fixed some tests.

70180fa

InsertCreativityHere commented May 29, 2026

View reviewed changes

rustfmt hates my comment styles...

3dd9f6f

InsertCreativityHere requested a review from Copilot May 29, 2026 18:56

Copilot started reviewing on behalf of InsertCreativityHere May 29, 2026 18:56 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread slicec/src/compilation_state.rs

Comment thread slicec/src/diagnostics/annotated_diagnostic.rs

InsertCreativityHere added 2 commits May 29, 2026 15:05

Fallback to the full path if we can't get the filestem of the plugin …

7f0081b

…I guess...

Formatting and a sneaky test file.

0a099ca

InsertCreativityHere requested review from bernardnormier, Copilot and externl May 29, 2026 21:04

Copilot started reviewing on behalf of InsertCreativityHere May 29, 2026 21:04 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

bernardnormier approved these changes May 29, 2026

View reviewed changes

Conversation

InsertCreativityHere commented May 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants