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

DeriveActiveEnum is incompatible with deny(missing_docs) attribute #1472

Closed
emirror-de opened this issue Feb 10, 2023 · 6 comments · Fixed by #1522 or #1531
Closed

DeriveActiveEnum is incompatible with deny(missing_docs) attribute #1472

emirror-de opened this issue Feb 10, 2023 · 6 comments · Fixed by #1522 or #1531
Assignees

Comments

@emirror-de
Copy link
Contributor

Description

When using sea-orm = 0.11, compilation throws an error when using DeriveActiveEnum with #![deny(missing_docs)].

Steps to Reproduce

Given the following example:

//! Hello World
#![deny(missing_docs)]

use sea_orm::entity::prelude::*;

/// This is my documentation.
#[derive(DeriveActiveEnum, EnumIter)]
#[sea_orm(rs_type = "i32", db_type = "Integer")]
pub enum MyEnum {
    /// This values means something.
    SomeValue = 1,
}

Expected Behavior

The code should compile.

Actual Behavior

cargo check answers:

    Checking sea-orm-test v0.1.0 (/tmp/sea-orm-test)
error: missing documentation for a struct
 --> src/lib.rs:7:10
  |
7 | #[derive(DeriveActiveEnum, EnumIter)]
  |          ^^^^^^^^^^^^^^^^
  |
note: the lint level is defined here
 --> src/lib.rs:2:9
  |
2 | #![deny(missing_docs)]
  |         ^^^^^^^^^^^^
  = note: this error originates in the derive macro `DeriveActiveEnum` (in Nightly builds, run with -Z macro-backtrace for more info)

error: could not compile `sea-orm-test` due to previous error

Reproduces How Often

Happens always.

Versions

sea-orm-test v0.1.0 (/tmp/sea-orm-test)
└── sea-orm v0.11.0
    ├── sea-orm-macros v0.11.0 (proc-macro)
    ├── sea-query v0.28.3
    │   ├── sea-query-derive v0.3.0 (proc-macro)
    ├── sea-strum v0.23.0
    │   └── sea-strum_macros v0.23.0 (proc-macro)
@tyt2y3
Copy link
Member

tyt2y3 commented Feb 11, 2023

Hi there, thank you for your suggestion. This is a common complaint for the linter, so in general you can't put #![deny(missing_docs)] everywhere because another library may also break you.

Ideally, the linter should be able to ignore #[automatically_derived] types.

That say, we can sprinkle some #[doc = "generated by sea-orm-macros"] in places.

rust-lang/rust#78490

@emirror-de
Copy link
Contributor Author

Thanks for the hint, I was not aware of the rust-lang issue.

So as far as I can see, it would also be an option to add #[allow(missing_docs)] to the generated types. But imo adding your suggested doc attribute would make more sense.

If I would want to add these changes, this would affect the -codegen and -macro?

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 13, 2023

Yes, but I think changing the macros is more important - you can't workaround that otherwise. A PR would be welcomed.

For the codegen, I think it totally depends on your philosophy. IMO you can leave them undocumented. Because it is no better for the cli to generate some stub docs /// FIXME This is the Cake Entity right?

@emirror-de
Copy link
Contributor Author

For the codegen, I think it totally depends on your philosophy. IMO you can leave them undocumented. Because it is no better for the cli to generate some stub docs /// FIXME This is the Cake Entity right?

Ah, didn't know that codegen is for the cli. That sounds reasonable, leaving models generated by CLI undocumented adds some implicit requirement for the user to document their code.

I am happy to add a draft PR as soon as I have a spare minute.

@emirror-de
Copy link
Contributor Author

emirror-de commented Mar 5, 2023

I added the PR linked above as draft as base for further discussions, if required.

@tyt2y3
Copy link
Member

tyt2y3 commented Mar 10, 2023

Released in 0.11.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants