[pureions]

I took the code of paval's PR #3 and resolved the merge conflicts against main and removed the code to fix the oneOf problem with enum messages because it has been already fixed in 5b2adbc.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 92.15686% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.43%. Comparing base (2679b90) to head (3a2a592).

Files with missing lines	Patch %	Lines
asyncapi-rust/tests/integration_test.rs	90.90%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #17      +/-   ##
==========================================
+ Coverage   89.29%   89.43%   +0.14%     
==========================================
  Files           6        6              
  Lines         962     1013      +51     
==========================================
+ Hits          859      906      +47     
- Misses        103      107       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mlilback]

Thanks for rebasing #3 and carrying this forward — the messages = [...] parsing in extract_operation follows the existing bracketed!/parse_terminated pattern correctly, cargo clippy --all-targets and cargo test are clean on the head, and the new tests pass.

I found one issue that I think needs fixing before merge, plus some cleanups. Details are in the inline comments; summary:

Blocking

1. HashSet dedup in the channel-messages codegen makes the generated AsyncAPI JSON non-deterministic across builds (lib.rs:954). This affects everyone using the crate, not just this feature.

Spec-validity gaps in the new feature
2. Types in messages = [...] aren't validated against #[asyncapi_messages(...)], so refs can point into a components section that doesn't exist (lib.rs:944, :1024).
3. Operation message refs aren't deduplicated, unlike the channel side (lib.rs:1031).
4. Refs can point at an undeclared channel (lib.rs:1032) — pre-existing, but now multiplied per message.

Unrelated leftovers from #3 — please revert/drop
5. message_integration.rs: changing UserJoin to a newtype variant drops the type const from its payload schema, so the example now publishes a schema serde's own output would fail.
6. examples/test_oneof.rs and examples/schema_inspection.rs are debugging scratch files.
7. Indentation churn inside quote! at lib.rs:514.

Follow-up, not blocking
8. Generic types in messages = [...] produce an unparsable-tokens error rather than a clear diagnostic (asyncapi_spec_attrs.rs:553).

[mlilback]

Non-deterministic generated output. HashSet uses RandomState, so the order in which the channel_messages.insert(...) blocks are emitted varies per compilation. Verified empirically: rebuilding examples/message_integration.rs six times produced two different channel-message orderings (Status, Error, UserJoin, ... vs UserJoin, ChatMessage, ..., Status, Error).

Since Channel.messages is an IndexMap serialized in insertion order, the emitted AsyncAPI JSON changes between builds — which breaks reproducible builds, golden-file/spec diffs, and any checked-in generated spec.

Suggest an order-preserving dedup (a Vec plus a seen-set keyed on quote!(#path).to_string()), or dropping the dedup entirely since the IndexMap already dedups by message name.

[mlilback]

No validation that messages = [...] types are also declared in #[asyncapi_messages(...)].

Components messages are generated only from spec_meta.message_types (line 1129). If a type appears in an operation's messages list but not in #[asyncapi_messages(...)], the channel emits "$ref": "#/components/messages/X" and the operation emits "$ref": "#/channels/c/messages/X" while components is None.

Verified: a struct with only #[asyncapi_operation(..., messages = [Msg])] produces refs into a components section that doesn't exist. The result is a silently invalid spec with no compile-time or runtime diagnostic. A compile error would be warranted here — or auto-inclusion of these types into message_types.

[pureions]

I modified the components generation code and included all operation messages. Hence messages declared in operations always land in the components. This would make using asyncapi_operation(...) optional.
What do you think about that?

[mlilback]

Same issue as on the channel side (see the comment on line 944): the types listed here are never checked against #[asyncapi_messages(...)], so this can emit #/channels/{c}/messages/X refs for a message that never makes it into components.

[mlilback]

Operation message refs are not deduplicated. operation.messages.iter() pushes refs unconditionally, so messages = [Msg, Msg] — or two types whose asyncapi_message_names() overlap — yields duplicate entries in the operation's messages array. Verified output contained #/channels/.../messages/A and .../B twice each.

Duplicate message references in an operation aren't valid AsyncAPI. Note the channel side is deduped (line 954), so the two sides currently disagree.

[mlilback]

channel_ref is used verbatim with no check that the channel exists. With channel = "nonexistent" the macro happily emits #/channels/nonexistent/messages/A.

The dangling #/channels/X ref is pre-existing, but this PR multiplies it into one dangling ref per message. Since this feature's whole contract is "operations reference channel messages", validating that operation.channel matches a declared #[asyncapi_channel(name = ...)] becomes worth doing.

[mlilback]

Unrelated indentation regression: this block was de-indented by 4 spaces relative to the surrounding statements, and the redundant // Convert schemars RootSchema to JSON comment was reintroduced. cargo fmt doesn't format inside quote!, so this will persist. Could you revert this hunk?

[mlilback]

Minor: generic message types produce a confusing error. Parsing as Path accepts Foo<Bar> only in expression-ambiguous form, and the emitted #type_name::asyncapi_message_names() would need a turbofish anyway.

Verified that messages = [Option<Msg>] fails with "comparison operators cannot be chained" plus "proc-macro derive produced unparsable tokens". Either reject non-generic-free paths with a clear syn::Error, or emit <#type_name>::asyncapi_message_names().

[mlilback]

This change silently drops the discriminant from the payload schema. Running the example shows the generated payload for UserJoin is now just {"$ref": "#/components/schemas/UserJoin"} with no "type": {"const": "user.join"} property — while every other variant (ChatMessage, UserLeave, Status, Error) still carries its const tag.

For an internally-tagged enum the wire payload does include "type":"user.join", so the example now publishes a schema that doesn't match what serde emits; a consumer validating against it would reject valid messages.

This looks like leftover debugging from the original #3 and is unrelated to the feature — please revert it. Separately, the newtype-variant case is a real gap in derive_to_asyncapi_message's tag injection, worth its own issue.

[mlilback]

Leftover debugging scratch file. It doesn't demonstrate the feature under review and doesn't use asyncapi_rust at all (only schemars/serde_json) — it just dumps raw schema JSON, which the existing debug_schema.rs example already does. It would become part of the published crate's examples and need maintenance. Recommend dropping it.

[mlilback]

Same as schema_inspection.rs: leftover scratch file that dumps raw schema JSON rather than demonstrating the messages = [...] feature. Recommend dropping it.

[mlilback]

Marking this as request-changes to make the status explicit — my earlier review was filed as a plain comment. No new findings here; the inline comments there still stand.

The blocker is the HashSet dedup at asyncapi-rust-codegen/src/lib.rs:954, which makes the generated AsyncAPI JSON non-deterministic across builds. Everything else is either a spec-validity gap in the new feature or an unrelated leftover from #3 that should be reverted.

I've also filed #18 for the underlying newtype-variant tag-injection gap that the message_integration.rs change exposed, so that can be handled separately from this PR — here, just reverting the example back to the struct-like variant is enough.

Happy to help with any of these if you'd like.

[pureions]

Thanks for your review! I think that I addressed all points except the issue with generics.

[mlilback]

Auto-inclusion works for me as the design — thanks for taking that route.

One thing to fix before it does what you describe, though: the merge loop sits inside the if spec_meta.message_types.is_empty() { None } guard, so it never runs in exactly the case it was written for. With no #[asyncapi_messages(...)] at all, message_types is empty, we take the None branch, and the loop is dead code.

Verified on 2ce3569:

#[derive(Serialize, Deserialize, JsonSchema, ToAsyncApiMessage)]
#[serde(tag = "type")]
pub enum Msg { Echo { text: String } }

#[derive(AsyncApi)]
#[asyncapi(title = "Probe", version = "1.0.0")]
#[asyncapi_channel(name = "chat", address = "/chat")]
#[asyncapi_operation(name = "send", action = "send", channel = "chat", messages = [Msg])]
pub struct Api;

produces "$ref": "#/components/messages/Echo" in the channel with no components section in the document.

The fix is to hoist the list construction above the guard and test that instead:

     // Generate components with messages and hoisted shared schemas
-    let components_code = if spec_meta.message_types.is_empty() {
-        quote! { None }
-    } else {
-        let mut all_message_types = spec_meta.message_types.clone();
-
-        for type_name in spec_meta
-            .operations
-            .iter()
-            .flat_map(|operation| operation.messages.iter())
-        {
-            if !all_message_types.contains(type_name) {
-                all_message_types.push(type_name.clone());
-            }
+    let mut all_message_types = spec_meta.message_types.clone();
+
+    for type_name in spec_meta
+        .operations
+        .iter()
+        .flat_map(|operation| operation.messages.iter())
+    {
+        if !all_message_types.contains(type_name) {
+            all_message_types.push(type_name.clone());
         }
+    }
 
+    let components_code = if all_message_types.is_empty() {
+        quote! { None }
+    } else {
         let type_calls = all_message_types.iter().map(|type_name| {

I applied this locally: the probe above now emits a proper components.messages.Echo, the full suite passes, and cargo clippy --all-targets is clean. When message_types is non-empty the generated output is unchanged, so it only affects the branch that was previously emitting dangling refs.

One design question while you're in here, not a blocker: with auto-inclusion, #[asyncapi_messages(...)] becomes advisory, which means a typo'd type in messages = [...] now quietly lands in components rather than failing. That's a reasonable trade for the ergonomics — just want to confirm it's the behaviour you intend.

Everything else from the review checks out on my end — the ordering fix, the operation-level dedup (messages = [Msg, Msg] now yields one ref), the unknown-channel compile error, and the example/scratch-file reverts. The generics diagnostic can stay a follow-up.

[pureions]

Thanks again! I'm happy that almost everything is fixed.

One design question while you're in here, not a blocker: with auto-inclusion, #[asyncapi_messages(...)] becomes advisory, which means a typo'd type in messages = [...] now quietly lands in components rather than failing. That's a reasonable trade for the ergonomics — just want to confirm it's the behaviour you intend.

Yes that was my intention because I though if you add a message to an operation I assume that you also want to have it in the components list.

The fix is to hoist the list construction above the guard and test that instead:

Can you apply this patch or should I do it?

[mlilback]

Thanks for confirming the intent — that reasoning makes sense to me, and it matches how the messages = [...] list reads at the call site. Let's keep auto-inclusion.

Go ahead and apply the patch yourself — it's your branch and your commit. The diff in my last comment should apply as-is; it's purely a move of the all_message_types construction above the guard, with the emptiness check switched to the merged list. Ping me when it's pushed and I'll re-review.

[pureions]

I applied the patch. It's ready for review

[mlilback]

Verified on 3a2a592 — the hoist matches the patch exactly, and all findings from my earlier review are now resolved. Approving.

What I checked on this head:

• Components auto-inclusion. An operation with messages = [Msg] and no #[asyncapi_messages(...)] at all now emits a full components.messages entry per variant, each carrying its "type": {"const": ...} tag. No dangling #/components/messages/X refs.
• Dedup. messages = [Msg, Msg] yields one ref per message name on both the channel and the operation side.
• Determinism. Six forced rebuilds of examples/message_integration.rs produced identical output — this was the original blocker, and it's gone.
• Unknown channels. channel = "nope" fails at compile time with Operation 'send' references unknown channel 'nope'.
• Suite. 79 tests pass across the workspace; cargo clippy --all-targets and cargo fmt --check are clean, and all 8 CI checks are green.

The only item left from the original review is the generics diagnostic, which I've filed as #23 so it doesn't get lost — not a blocker for this PR.

Thanks for the patience on this one; it went through a lot of rounds and came out clean. Nice work carrying #3 across the line.