From ff48c87f97e95fbb7d1255a9d43dada6966c2aae Mon Sep 17 00:00:00 2001 From: RAprogramm Date: Sat, 4 Jul 2026 14:31:59 +0700 Subject: [PATCH] #184 fix: emit feature-dependent derives at expansion time instead of consumer cfgs --- .github/workflows/ci.yml | 5 ++ crates/entity-derive-impl/Cargo.toml | 10 +++ crates/entity-derive-impl/src/entity/dto.rs | 79 ++++++++++++++++--- .../src/entity/new_entity.rs | 7 +- .../src/entity/projection.rs | 14 +++- crates/entity-derive-impl/src/entity/query.rs | 15 +++- crates/entity-derive-impl/src/entity/row.rs | 7 +- .../entity-derive-impl/src/utils/tracing.rs | 36 ++++----- crates/entity-derive/Cargo.toml | 10 +-- 9 files changed, 141 insertions(+), 42 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a66452..cbc5e88 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -206,6 +206,11 @@ jobs: - name: Run doctests run: cargo test --doc --all-features || true + - name: Test garde backend (garde without validate) + run: | + cargo nextest run -p entity-derive-impl --no-default-features --features garde --profile ci --no-tests=pass + cargo nextest run -p entity-derive --no-default-features --features postgres,garde --test garde --profile ci --no-tests=pass + - name: Upload test results if: always() uses: actions/upload-artifact@v7 diff --git a/crates/entity-derive-impl/Cargo.toml b/crates/entity-derive-impl/Cargo.toml index 174c7ed..dce638a 100644 --- a/crates/entity-derive-impl/Cargo.toml +++ b/crates/entity-derive-impl/Cargo.toml @@ -32,6 +32,16 @@ default = [ "projections" ] +# Facade-mirrored gates: when off, generated code omits the matching +# `cfg`/`cfg_attr` entirely so consumer crates never see cfgs for +# features they do not declare (unexpected_cfgs) and never silently +# lose the repository impl. +postgres = [] +api = [] +validate = [] +garde = [] +tracing = [] + # Generates `{Entity}Event` enum and lifecycle event helpers. Required # transitively by `streams` because the NOTIFY payload uses the event type. events = [] diff --git a/crates/entity-derive-impl/src/entity/dto.rs b/crates/entity-derive-impl/src/entity/dto.rs index 81ad447..d0ac211 100644 --- a/crates/entity-derive-impl/src/entity/dto.rs +++ b/crates/entity-derive-impl/src/entity/dto.rs @@ -72,18 +72,35 @@ fn generate_create_dto(entity: &EntityDef) -> TokenStream { } }); + let extra_derives = dto_extra_derives(); let marker = marker::generated(); quote! { #marker #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] - #[cfg_attr(feature = "validate", derive(validator::Validate))] - #[cfg_attr(all(feature = "garde", not(feature = "validate")), derive(garde::Validate))] + #extra_derives #vis struct #name { #(#field_defs),* } } } +/// Backend-dependent derives for generated DTOs. +/// +/// Emitted only for features enabled on the facade at expansion time, +/// so consumer crates never receive `cfg_attr`s for features they do +/// not declare (`unexpected_cfgs`). +fn dto_extra_derives() -> TokenStream { + let mut derives = TokenStream::new(); + if cfg!(feature = "api") { + derives.extend(quote! { #[derive(utoipa::ToSchema)] }); + } + if cfg!(feature = "validate") { + derives.extend(quote! { #[derive(validator::Validate)] }); + } else if cfg!(feature = "garde") { + derives.extend(quote! { #[derive(garde::Validate)] }); + } + derives +} + /// Build the `#[garde(...)]` attribute for a DTO field. /// /// Translates the typed validation constraints into garde rules; @@ -127,8 +144,11 @@ fn garde_attr(field: &FieldDef, option_depth: usize) -> TokenStream { inner }; + if !cfg!(feature = "garde") || cfg!(feature = "validate") { + return TokenStream::new(); + } let tokens: TokenStream = body.parse().expect("garde rules are valid tokens"); - quote! { #[cfg_attr(all(feature = "garde", not(feature = "validate")), garde(#tokens))] } + quote! { #[garde(#tokens)] } } fn generate_update_dto(entity: &EntityDef) -> TokenStream { @@ -162,6 +182,11 @@ fn generate_update_dto(entity: &EntityDef) -> TokenStream { } }); + let version_garde_skip = if cfg!(feature = "garde") && !cfg!(feature = "validate") { + quote! { #[garde(skip)] } + } else { + TokenStream::new() + }; let version_field = entity.version_field().map(|f| { let vt = f.ty(); quote! { @@ -169,19 +194,18 @@ fn generate_update_dto(entity: &EntityDef) -> TokenStream { /// /// The UPDATE only applies when the row still carries this /// version; on mismatch the call fails with a conflict. - #[cfg_attr(all(feature = "garde", not(feature = "validate")), garde(skip))] + #version_garde_skip pub expected_version: #vt, } }); + let extra_derives = dto_extra_derives(); let marker = marker::generated(); quote! { #marker #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] - #[cfg_attr(feature = "validate", derive(validator::Validate))] - #[cfg_attr(all(feature = "garde", not(feature = "validate")), derive(garde::Validate))] + #extra_derives #vis struct #name { #(#field_defs,)* #version_field @@ -204,17 +228,22 @@ fn generate_response_dto(entity: &EntityDef) -> TokenStream { quote! { pub #n: #t } }); + let extra_derives_api = if cfg!(feature = "api") { + quote! { #[derive(utoipa::ToSchema)] } + } else { + TokenStream::new() + }; let marker = marker::generated(); quote! { #marker #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] + #extra_derives_api #vis struct #name { #(#field_defs),* } } } -#[cfg(test)] +#[cfg(all(test, feature = "garde", not(feature = "validate")))] mod garde_tests { use quote::quote; use syn::DeriveInput; @@ -265,8 +294,34 @@ mod garde_tests { } #[test] - fn validate_takes_precedence_when_both_enabled() { + fn garde_derive_emitted_without_cfg_wrapper() { let code = generate(&validated_entity()).to_string(); - assert!(code.contains("all (feature = \"garde\" , not (feature = \"validate\"))")); + assert!(code.contains("derive (garde :: Validate)")); + assert!(!code.contains("cfg_attr")); + } +} + +#[cfg(all(test, feature = "garde", feature = "validate"))] +mod garde_precedence_tests { + use syn::DeriveInput; + + use super::*; + + #[test] + fn validate_wins_over_garde() { + let input: DeriveInput = syn::parse_quote! { + #[entity(table = "users")] + pub struct User { + #[id] + pub id: uuid::Uuid, + #[field(create, response)] + #[validate(email)] + pub email: String, + } + }; + let entity = EntityDef::from_derive_input(&input).unwrap(); + let code = generate(&entity).to_string(); + assert!(code.contains("validator :: Validate")); + assert!(!code.contains("garde")); } } diff --git a/crates/entity-derive-impl/src/entity/new_entity.rs b/crates/entity-derive-impl/src/entity/new_entity.rs index f7cca91..c2b292f 100644 --- a/crates/entity-derive-impl/src/entity/new_entity.rs +++ b/crates/entity-derive-impl/src/entity/new_entity.rs @@ -86,11 +86,16 @@ fn generate_new_struct(entity: &EntityDef) -> TokenStream { }); let marker = crate::utils::marker::generated(); + let api_derive = if cfg!(feature = "api") { + quote! { #[derive(utoipa::ToSchema)] } + } else { + TokenStream::new() + }; quote! { #marker #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] + #api_derive #vis struct #new_name { #(#field_defs),* } diff --git a/crates/entity-derive-impl/src/entity/projection.rs b/crates/entity-derive-impl/src/entity/projection.rs index 337dafe..deab474 100644 --- a/crates/entity-derive-impl/src/entity/projection.rs +++ b/crates/entity-derive-impl/src/entity/projection.rs @@ -92,12 +92,22 @@ fn generate_projection(entity: &EntityDef, proj: &super::parse::ProjectionDef) - .collect(); let marker = marker::generated(); + let api_derive = if cfg!(feature = "api") { + quote! { #[derive(utoipa::ToSchema)] } + } else { + TokenStream::new() + }; + let from_row_derive = if cfg!(feature = "postgres") { + quote! { #[derive(sqlx::FromRow)] } + } else { + TokenStream::new() + }; quote! { #marker #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] - #[cfg_attr(feature = "postgres", derive(sqlx::FromRow))] + #api_derive + #from_row_derive #vis struct #proj_name { #(#field_defs),* } diff --git a/crates/entity-derive-impl/src/entity/query.rs b/crates/entity-derive-impl/src/entity/query.rs index 69e805e..061f9cc 100644 --- a/crates/entity-derive-impl/src/entity/query.rs +++ b/crates/entity-derive-impl/src/entity/query.rs @@ -44,6 +44,15 @@ use proc_macro2::TokenStream; use quote::{format_ident, quote}; +/// `#[derive(utoipa::ToSchema)]` when the facade `api` feature is on. +fn api_schema_derive() -> proc_macro2::TokenStream { + if cfg!(feature = "api") { + quote::quote! { #[derive(utoipa::ToSchema)] } + } else { + proc_macro2::TokenStream::new() + } +} + use super::parse::{EntityDef, FilterType}; use crate::utils::marker; @@ -84,6 +93,7 @@ pub fn generate(entity: &EntityDef) -> TokenStream { }) .collect(); + let api_derive = api_schema_derive(); let marker = marker::generated(); let filter_name = entity.ident_with("", "Filter"); @@ -95,7 +105,7 @@ pub fn generate(entity: &EntityDef) -> TokenStream { #marker #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] + #api_derive #vis struct #query_name { #(#field_defs,)* #sort_field @@ -141,6 +151,7 @@ fn generate_sort_enum(entity: &EntityDef) -> (TokenStream, TokenStream) { } let doc = format!("Sortable columns for [`{}`] queries.", entity.name()); + let api_derive = api_schema_derive(); let marker = marker::generated(); let sort_enum = quote! { @@ -148,7 +159,7 @@ fn generate_sort_enum(entity: &EntityDef) -> (TokenStream, TokenStream) { #[doc = #doc] #[derive(Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "snake_case")] - #[cfg_attr(feature = "api", derive(utoipa::ToSchema))] + #api_derive #vis enum #sort_name { #(#variants,)* } diff --git a/crates/entity-derive-impl/src/entity/row.rs b/crates/entity-derive-impl/src/entity/row.rs index c91574a..45381ee 100644 --- a/crates/entity-derive-impl/src/entity/row.rs +++ b/crates/entity-derive-impl/src/entity/row.rs @@ -74,11 +74,16 @@ pub fn generate(entity: &EntityDef) -> TokenStream { }); let marker = marker::generated(); + let from_row_derive = if cfg!(feature = "postgres") { + quote! { #[derive(sqlx::FromRow)] } + } else { + TokenStream::new() + }; quote! { #marker #[derive(Debug, Clone)] - #[cfg_attr(feature = "postgres", derive(sqlx::FromRow))] + #from_row_derive #vis struct #row_name { #(#field_defs),* } } } diff --git a/crates/entity-derive-impl/src/utils/tracing.rs b/crates/entity-derive-impl/src/utils/tracing.rs index 354f121..6f48744 100644 --- a/crates/entity-derive-impl/src/utils/tracing.rs +++ b/crates/entity-derive-impl/src/utils/tracing.rs @@ -37,7 +37,8 @@ use proc_macro2::TokenStream; use quote::quote; -/// Generate a `#[cfg_attr(feature = "tracing", …instrument…)]` attribute. +/// Generate a `#[tracing::instrument(...)]` attribute when the +/// `tracing` feature is enabled at expansion time (nothing otherwise). /// /// # Arguments /// @@ -48,38 +49,35 @@ use quote::quote; /// /// # Returns /// -/// A `TokenStream` ready to splice in front of an `async fn`. If the -/// consumer crate does not enable the `tracing` feature, the attribute -/// expands to nothing. +/// A `TokenStream` ready to splice in front of an `async fn`; empty +/// when the facade's `tracing` feature is off, so consumer crates never +/// see a cfg for a feature they do not declare. #[must_use] pub fn instrument(entity_name: &str, op: &str) -> TokenStream { + if !cfg!(feature = "tracing") { + return TokenStream::new(); + } quote! { - #[cfg_attr( - feature = "tracing", - ::tracing::instrument( - skip_all, - fields(entity = #entity_name, op = #op), - err(Debug) - ) + #[::tracing::instrument( + skip_all, + fields(entity = #entity_name, op = #op), + err(Debug) )] } } -#[cfg(test)] +#[cfg(all(test, feature = "tracing"))] mod tests { use super::*; #[test] - fn emits_cfg_attr_gate() { + fn emits_plain_instrument_attribute() { let tokens = instrument("User", "create").to_string(); assert!( - tokens.contains("cfg_attr"), - "must be feature-gated: {tokens}" - ); - assert!( - tokens.contains("feature = \"tracing\""), - "wrong gate: {tokens}" + !tokens.contains("cfg_attr"), + "expansion-time gating must not leak cfgs: {tokens}" ); + assert!(tokens.contains("instrument")); } #[test] diff --git a/crates/entity-derive/Cargo.toml b/crates/entity-derive/Cargo.toml index 1130bb3..0c47c59 100644 --- a/crates/entity-derive/Cargo.toml +++ b/crates/entity-derive/Cargo.toml @@ -31,7 +31,7 @@ default = [ ] # Database dialects -postgres = ["entity-core/postgres"] +postgres = ["entity-core/postgres", "entity-derive-impl/postgres"] clickhouse = ["entity-core/clickhouse"] mongodb = ["entity-core/mongodb"] @@ -41,12 +41,12 @@ streams = ["entity-core/streams", "events"] outbox = ["entity-core/outbox", "events"] # HTTP handlers + OpenAPI generation. -api = [] +api = ["entity-derive-impl/api"] # `validator::Validate` integration on DTOs. -validate = [] +validate = ["entity-derive-impl/validate"] # `garde::Validate` integration on DTOs (mutually exclusive with `validate`). -garde = [] +garde = ["entity-derive-impl/garde"] # Lifecycle event types (`{Entity}Event::Created` / `Updated` / etc.). events = ["entity-derive-impl/events"] @@ -73,7 +73,7 @@ projections = ["entity-derive-impl/projections"] # Opt-in: emit `#[tracing::instrument]` on every generated async method. # Users who enable this must also depend on `tracing` directly so the # generated `::tracing::instrument` resolves. -tracing = ["entity-core/tracing"] +tracing = ["entity-core/tracing", "entity-derive-impl/tracing"] [dependencies] entity-core = { path = "../entity-core", version = "0.10", features = ["serde"] }