Refactor Diagnostic Creation by InsertCreativityHere · Pull Request #770 · icerpc/slicec

InsertCreativityHere · 2026-04-15T14:50:20Z

This PR:

Removes the implement_diagnostic_functions macro we used to implement functions for diagnostic types. Now we just implement the functions like normal.
Adds more typed constructors to create diagnostics, in anticipation of adding a new Info diagnostic kind, which will be very different from the existing Error and Lints.

None of the actual error messages or codes should be changed. Just moved the implementation from a macro to functions.

Copilot

Pull request overview

Refactors how diagnostics are constructed and identified across slicec, replacing the prior macro-based implementation with explicit constructors/methods and updating call sites (including tests) to use the new typed constructors.

Changes:

Replaces Diagnostic::new(...) usage with typed constructors Diagnostic::error(...) / Diagnostic::lint(...) throughout validators, parsers, patchers, CLI, and tests.
Removes the macro that generated diagnostic code/message functions and implements these as regular methods on Error/Lint.
Centralizes allowable lint identifiers into a shared constant for attribute/CLI validation.

Reviewed changes

Copilot reviewed 52 out of 52 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
slicec/src/diagnostics/diagnostic.rs	Introduces typed constructors and adjusts code/level behavior.
slicec/src/diagnostics/errors.rs	Replaces macro-generated code/message with explicit methods.
slicec/src/diagnostics/lints.rs	Replaces macro-generated name/message/default-level with explicit methods.
slicec/src/diagnostics/mod.rs	Removes macro and reorganizes imports/exports accordingly.
slicec/src/grammar/attributes/allow.rs	Adds shared `ALLOWABLE_LINT_IDENTIFIERS` constant and updates validation.
slicec/src/slice_options.rs	Updates CLI `--allow` parser to use shared allowable-lints constant.
slicec/src/utils/file_util.rs	Updates duplicate-file lint + IO error creation to typed constructors.
slicec/src/main.rs	Updates generator-response diagnostics to typed constructors.
slicec/src/validators/type_aliases.rs	Updates error construction to `Diagnostic::error`.
slicec/src/validators/structs.rs	Updates error construction to `Diagnostic::error`.
slicec/src/validators/parameters.rs	Updates streamed-member errors to `Diagnostic::error`.
slicec/src/validators/operations.rs	Updates doc-comment lint construction to `Diagnostic::lint`.
slicec/src/validators/members.rs	Updates tag-related errors to `Diagnostic::error`.
slicec/src/validators/identifiers.rs	Updates shadowing/redefinition errors to `Diagnostic::error`.
slicec/src/validators/enums.rs	Updates enum validation errors to `Diagnostic::error`.
slicec/src/validators/dictionary.rs	Updates dictionary key-type errors to `Diagnostic::error`.
slicec/src/validators/cycle_detection.rs	Updates cycle error construction to `Diagnostic::error`.
slicec/src/validators/comments.rs	Updates doc-comment lint construction to `Diagnostic::lint`.
slicec/src/validators/attribute.rs	Updates attribute error construction to `Diagnostic::error`.
slicec/src/patchers/type_ref_patcher.rs	Updates mapped errors and deprecation lints to typed constructors.
slicec/src/patchers/mod.rs	Updates unknown-attribute errors to `Diagnostic::error`.
slicec/src/patchers/comment_link_patcher.rs	Updates broken-link lints to `Diagnostic::lint`.
slicec/src/parsers/mod.rs	Minor reordering + updates module-level syntax error construction to `Diagnostic::error`.
slicec/src/parsers/slice/mod.rs	Updates parse error conversion to `Diagnostic::error`.
slicec/src/parsers/slice/grammar.rs	Updates parser-emitted errors to `Diagnostic::error`.
slicec/src/parsers/preprocessor/mod.rs	Updates preprocessor parse error conversion to `Diagnostic::error`.
slicec/src/parsers/comments/mod.rs	Updates comment-parse lints to `Diagnostic::lint`.
slicec/src/grammar/attributes/mod.rs	Updates attribute error construction and imports to match refactor.
slicec/src/grammar/attributes/sliced_format.rs	Updates invalid-argument error construction to `Diagnostic::error`.
slicec/src/grammar/attributes/compress.rs	Updates invalid-argument error construction to `Diagnostic::error`.
slicec/tests/typealias_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/tag_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/structs/tags.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/structs/container.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/sequence_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/scope_resolution_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/redefinition_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/preprocessor_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/parser_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/module_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/interfaces/operations.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/interfaces/mod.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/interfaces/inheritance.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/identifier_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/files/io.rs	Updates tests to use `Diagnostic::lint`.
slicec/tests/enums/mod.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/enums/container.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/dictionaries/value_type.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/dictionaries/key_type.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/cycle_tests.rs	Updates tests to use `Diagnostic::error`.
slicec/tests/comment_tests.rs	Updates tests to use `Diagnostic::error` / `Diagnostic::lint`.
slicec/tests/attribute_tests.rs	Updates tests to use `Diagnostic::error` / `Diagnostic::lint`.

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

InsertCreativityHere · 2026-04-15T14:55:29Z


+use crate::slice_file::Span;
+use serde::Serialize;
+


The conventional ordering for top-level Rust statements is pub mod, mod, pub use, use.
So I moved these use statements to where they should be.

InsertCreativityHere · 2026-04-15T14:55:49Z

-
-/// A macro that implements the `code` and `message` functions for [Lint] and [Error] enums.
-macro_rules! implement_diagnostic_functions {
-    (Lint, $(($kind:ident, $message:expr $(, $variant:ident)* )),*) => {


Probably the fanciest macro we had in slicec.

InsertCreativityHere · 2026-04-15T14:59:24Z

 use super::*;

+/// All the valid arguments that can be supplied to the `allow` metadata, or the `--allow` command line option.
+pub const ALLOWABLE_LINT_IDENTIFIERS: [&str; 6] = [


This used to be generated by the macro, now we just write it by hand.
It does live in a different module though, so had to update it's scoping where we use it.

InsertCreativityHere · 2026-04-15T15:00:41Z

 pub use sliced_format::*;

 use super::Attributables;
-use crate::diagnostics::{Diagnostic, Diagnostics, Error, Lint};


Lint was in scope to see the ALLOWABLE_LINT_IDENTIFIERS. Now that it lives in this module ancestry, no longer needed.

InsertCreativityHere · 2026-04-15T15:01:15Z

-impl From<Error> for DiagnosticKind {
-    fn from(error: Error) -> Self {
-        DiagnosticKind::Error(error)
-    }
-}
-
-impl From<Lint> for DiagnosticKind {
-    fn from(lint: Lint) -> Self {
-        DiagnosticKind::Lint(lint)
-    }
-}


These are no longer needed now that we have dedicated constructors for error and lint, instead of using From/Into casts.

InsertCreativityHere · 2026-04-15T15:01:40Z

+// We only export the parsers and keep all the other logic private.
+pub use self::comments::parser::CommentParser;
+pub use self::preprocessor::parser::Preprocessor;
+pub use self::slice::parser::Parser;


pub use should come after mod statements.

InsertCreativityHere · 2026-04-15T15:07:09Z

+            Self::MalformedDocComment { message } => format!("malformed doc comment: {message}"),
+
+            Self::IncorrectDocComment { message } => format!("incorrect doc comment: {message}"),
+
+            Self::BrokenDocLink { message } => format!("broken doc link: {message}"),
+        }


This is the one place where the messages changed. I added short little prefixes before emitting the messages. Before we just emitted {message} verbatim. No prefix.

Copilot

Pull request overview

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

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

externl

Looks good to me!

externl · 2026-04-15T15:33:35Z


+    /// Creates a new error `Diagnostic` from the provided [`Error`].
+    /// The newly created `Diagnostic` has no `span`, `scope`, or `notes` set.
+    pub fn error(error: Error) -> Self {


Shouldn't these be something like from_error and from_info?

Yes you're right. My current names are not very Rusty...
Fixed!

InsertCreativityHere added 5 commits April 15, 2026 10:37

All the diagnostic creation has been updated.

ce21d0f

Moved the 'ALLOWABLE_LINT_IDENTIFIERS' to the 'allow' module.

d58dbd6

Removed the 'implement_diagnostic_functions_for' macro.

19fcf08

Keep the old 'Allowed' name for now.

ad51705

Splitting this off into it's own PR for reviewability.

1f5e93d

InsertCreativityHere requested a review from Copilot April 15, 2026 14:50

Copilot started reviewing on behalf of InsertCreativityHere April 15, 2026 14:50 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread slicec/src/grammar/attributes/allow.rs

Comment thread slicec/src/diagnostics/lints.rs

Comment thread slicec/src/diagnostics/lints.rs

Comment thread slicec/src/diagnostics/lints.rs

Comment thread slicec/src/diagnostics/diagnostic.rs

Comment thread slicec/src/diagnostics/errors.rs

Remove 'From' impls.

3972569

InsertCreativityHere commented Apr 15, 2026

View reviewed changes

Update test messages.

0766cb9

InsertCreativityHere commented Apr 15, 2026

View reviewed changes

InsertCreativityHere requested review from Copilot and externl April 15, 2026 15:07

Copilot started reviewing on behalf of InsertCreativityHere April 15, 2026 15:07 View session

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread slicec/src/diagnostics/lints.rs

Comment thread slicec/src/diagnostics/errors.rs Outdated

Remove trailing comma

373a9f1

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

externl approved these changes Apr 15, 2026

View reviewed changes

Use better constructor names.

28e1253

InsertCreativityHere merged commit c63d0a5 into icerpc:main Apr 15, 2026
7 checks passed

Conversation

InsertCreativityHere commented Apr 15, 2026

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!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

externl left a comment

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants