Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add support for conditional cfg_attr for primary key #215

Open
nikessel opened this issue Aug 22, 2024 · 4 comments
Open
Assignees
Labels
long-term Long term feature

Comments

@nikessel
Copy link

nikessel commented Aug 22, 2024

I have a use case where I need to conditionally add the proc macro attributes to a model based on a feature. This compiles:

#[cfg_attr(
feature = "ssr",
native_db,
native_model(id = 1, version = 1)
)]
#[derive(Clone, Debug, Deserialize, Serialize, Getters, Dissolve, new)]
pub struct User {
#[primary_key]
// #[cfg_attr(feature = "ssr", primary_key)]
pub id: IdType,
}

However, if I comment in the #[cfg_attr(feature = "ssr", primary_key)], the proc macro panics with: "Primary key is not set", even though native_db is only added to the model if "ssr" is enabled.

I would have created a pull request, but proc macros are a bit beyond my ability. For now I've forked the project and added a workaround:

pub(crate) fn primary_key(&self) -> KeyDefinition<()> {
    // self.primary_key.clone().expect("Primary key is not set")
    self.primary_key.clone().unwrap_or_else(|| {
        KeyDefinition::new_field(
            self.struct_name.clone(),
            Ident::new("id", Span::call_site()),
            (),
        )
    })
}
@vincent-herlemont
Copy link
Owner

vincent-herlemont commented Aug 22, 2024

⚠️ First of all: I strongly, strongly, strongly advise against commenting out the line that checks for the presence of the primary key!! ^^ The program will not run if the primary key is not provided. This panic is expected.

For the error, this isn't related to native_db, it seems to me. The proc_macro only applies to the block of code that immediately follows it. If a comment slips in between, the next block is the comment, so it can't work.

By reversing (as shown below), the commented code and the proc_macro (#[primary_key]), you shouldn't encounter the error.

#[cfg_attr(
   feature = "ssr",
   native_db,
   native_model(id = 1, version = 1)
)]

#[derive(Clone, Debug, Deserialize, Serialize, Getters, Dissolve, new)]
pub struct User {]
   // #[cfg_attr(feature = "ssr", primary_key)]
   #[primary_key]
   pub id: IdType,
}

@nikessel
Copy link
Author

Thank you for getting back to me so quickly, I really appreciate it.

I'm aware that commenting out the primary key check is a very bad idea, but as long as I remember to add the primary key and add tests, then it should be fine for development.

I tried removing the commented line for both cases, that did not change the outcome.

I've set up a minimal project to illustrate the issue:

Cargo.toml

[package]
name = "t"
version = "0.1.0"
edition = "2021"

[dependencies]
native_db = { version = "0.7.1", optional = true }
native_model = {version = "0.4.19", optional = true }
serde = { version = "1.0.208", features = ["derive"] }

[features]
default = []
enable_native_db = ["native_db", "native_model"]

main.rs

#[cfg(feature = "enable_native_db")]
mod enable_native_db {
    pub use native_db::{native_db, ToKey};
    pub use native_model::{native_model, Model};
}

use serde::{Deserialize, Serialize};

#[cfg(feature = "enable_native_db")]
use enable_native_db::*;

#[cfg_attr(feature = "enable_native_db", native_db, native_model(id = 1, version = 1))]
#[derive(Serialize, Deserialize)]
struct Foo {
    #[cfg_attr(feature = "enable_native_db", primary_key)]
    id: i32,
}

fn main() {
    println!("Hello, world!");
}

This compiles just fine if I don't enable the feature. But if I compile it with cargo build -F enable_native_db, then I get the error:

error: custom attribute panicked
  --> src/main.rs:11:29
   |
11 | #[cfg_attr(feature = "enable_native_db", native_db, native_model(id = 1, version = 1))]
   |                             ^^^^^^^^^
   |
   = help: message: Primary key is not set

Then the other option is to change the annotation to this:

#[primary_key]

so the code is this:

#[cfg(feature = "enable_native_db")]
mod enable_native_db {
    pub use native_db::{native_db, ToKey};
    pub use native_model::{native_model, Model};
}

use serde::{Deserialize, Serialize};

#[cfg(feature = "enable_native_db")]
use enable_native_db::*;

#[cfg_attr(feature = "enable_native_db", native_db, native_model(id = 1, version = 1))]
#[derive(Serialize, Deserialize)]
struct Foo {
    #[primary_key]
    id: i32,
}

fn main() {
    println!("Hello, world!");
}

But now we have a dependency on native_db even if enable_native_db is not enabled:

cargo build:

error: cannot find attribute `primary_key` in this scope
  --> src/main.rs:14:7
   |
14 |     #[primary_key]
   |       ^^^^^^^^^^^

cargo build -F enable_native_db -> now compiles just fine

So I'm unable to define the model in a way such that native_db is only added to the model if the enable_native_db feature is enabled

If I use my fork instead, then this compiles:

Cargo.toml

[package]
name = "t"
version = "0.1.0"
edition = "2021"

[dependencies]
native_db = { git = "https://github.com/niklass-l/native_db", optional = true }
native_model = {version = "0.4.19", optional = true }
serde = { version = "1.0.208", features = ["derive"] }

[features]
default = []
enable_native_db = ["native_db", "native_model"]

main.rs

#[cfg(feature = "enable_native_db")]
mod ssr {
    pub use native_db::{native_db, ToKey};
    pub use native_model::{native_model, Model};
}

use serde::{Deserialize, Serialize};

#[cfg(feature = "enable_native_db")]
use ssr::*;

#[cfg_attr(
    feature = "enable_native_db",
    native_db,
    native_model(id = 1, version = 1)
)]
#[derive(Serialize, Deserialize)]
struct Foo {
    #[cfg_attr(feature = "enable_native_db", primary_key)]
    id: i32,
}

fn main() {
    println!("Hello, world!");
}

cargo build -F enable_native_db -> compiles
cargo build -> compiles

I would also note that it does not seem to be an issue with proc macros + conditional attributes in general, since I've been using it with the fake crate and that has been working just fine.
I'm open to the idea that I've gotten something wrong though, don't really know all that much about proc macros

@vincent-herlemont
Copy link
Owner

Thank you for your explanation, I understand better now. I had never tested native_db in this way using cfg_attr. It is true that there is a problem; I will investigate it when I have some time.

For now, I can suggest this workaround—it’s not very elegant, but it gets the job done:

#[cfg_attr(feature = "enable_native_db", native_db, native_model(id = 1, version = 1))]
#[derive(Serialize, Deserialize, PartialEq, Debug)]
struct Foo {
    #[cfg(not(feature = "enable_native_db"))]
    id: i32,

    #[cfg(feature = "enable_native_db")]
    #[primary_key]
    id: i32,
}

@nikessel
Copy link
Author

Thank you! That's much safer than my solution at least, I'll go with that.
I'm sure you have a lot to see to, I would understand if it's not worth the effort to fix this in favor of more important stuff. At least there is a solution.

@vincent-herlemont vincent-herlemont self-assigned this Oct 6, 2024
@vincent-herlemont vincent-herlemont added the long-term Long term feature label Oct 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
long-term Long term feature
Projects
None yet
Development

No branches or pull requests

2 participants