From 825bda2556fc888e7dceef119c52cfbc0069fe8d Mon Sep 17 00:00:00 2001 From: Georg Semmler Date: Fri, 18 Jun 2021 18:26:38 +0200 Subject: [PATCH] Address review comments --- diesel/src/macros/mod.rs | 2 +- diesel_cli/src/cli.rs | 4 +-- diesel_cli/src/main.rs | 4 +-- diesel_cli/src/print_schema.rs | 33 ++++++++++++++----- diesel_cli/tests/print_schema.rs | 11 +------ .../postgres/expected.rs | 15 ++++++--- .../postgres/expected.rs | 15 ++++++--- 7 files changed, 50 insertions(+), 34 deletions(-) diff --git a/diesel/src/macros/mod.rs b/diesel/src/macros/mod.rs index ee61f73f44ca..12fa206aecc6 100644 --- a/diesel/src/macros/mod.rs +++ b/diesel/src/macros/mod.rs @@ -616,7 +616,7 @@ macro_rules! __diesel_table_impl { },)+], ) => { $($meta)* - #[allow(unused_imports)] + #[allow(unused_imports, dead_code)] pub mod $table_name { pub use self::columns::*; $($imports)* diff --git a/diesel_cli/src/cli.rs b/diesel_cli/src/cli.rs index 1ebf99f3148f..fac12313d2bc 100644 --- a/diesel_cli/src/cli.rs +++ b/diesel_cli/src/cli.rs @@ -221,9 +221,7 @@ pub fn build_cli() -> App<'static, 'static> { ) .arg( Arg::with_name("generate-custom-type-definitions") - .long("generate-custom-type-definitions") - .takes_value(true) - .possible_values(&["true", "false"]) + .long("no-generate-missing-sql-type-definitions") .help("Generate SQL type definitions for types not provided by diesel"), ); diff --git a/diesel_cli/src/main.rs b/diesel_cli/src/main.rs index 9c845a6ef630..13eec9822f0e 100644 --- a/diesel_cli/src/main.rs +++ b/diesel_cli/src/main.rs @@ -562,8 +562,8 @@ fn run_infer_schema(matches: &ArgMatches) -> Result<(), Box bool { self.types.iter().find(|c| c.rust_name == tpe).is_some() } } impl Display for CustomTypeList { - fn fmt(&self, out: &mut Formatter<'_>) -> fmt::Result { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { + if self.types.is_empty() { + return Ok(()); + } for t in &self.types { match self.backend { #[cfg(feature = "postgres")] Backend::Pg => { if self.with_docs { - writeln!(out, "/// The `{}` SQL type", t.rust_name)?; + writeln!(f, "/// A module containing custom SQL type definitions")?; + writeln!(f, "///")?; + writeln!(f, "/// (Automatically generated by Diesel.)")?; + } + let mut out = PadAdapter::new(f); + writeln!(out, "pub mod sql_types {{")?; + if self.with_docs { + writeln!(out, "/// The `{}` SQL type", t.sql_name)?; writeln!(out, "///")?; writeln!(out, "/// (Automatically generated by Diesel.)")?; } @@ -255,13 +267,16 @@ impl Display for CustomTypeList { writeln!(out, "#[postgres(type_name = \"{}\")]", t.sql_name)?; writeln!(out, "pub struct {};", t.rust_name)?; writeln!(out)?; + writeln!(f, "}}")?; } #[cfg(feature = "sqlite")] Backend::Sqlite => { - unreachable!("We only generate a closed set of types for sqlite") + let _ = (&f, self.with_docs, t); } #[cfg(feature = "mysql")] - Backend::Mysql => todo!(), + Backend::Mysql => { + let _ = (&f, self.with_docs, t); + } } } Ok(()) @@ -362,16 +377,18 @@ impl<'a> Display for TableDefinition<'a> { } } + #[cfg(feature = "postgres")] for col in &self.table.column_data { - dbg!(&col.rust_name); - if dbg!(self.custom_type_defs.contains(&col.ty.rust_name)) { + if self.custom_type_defs.contains(&col.ty.rust_name) { if !has_written_import { writeln!(out, "use diesel::sql_types::*;")?; } - writeln!(out, "use super::{};", col.ty.rust_name)?; + writeln!(out, "use super::sql_types::{};", col.ty.rust_name)?; has_written_import = true; } } + #[cfg(not(feature = "postgres"))] + let _ = self.custom_type_defs; if has_written_import { writeln!(out)?; diff --git a/diesel_cli/tests/print_schema.rs b/diesel_cli/tests/print_schema.rs index 014ada318cba..bb5dc27ceae5 100644 --- a/diesel_cli/tests/print_schema.rs +++ b/diesel_cli/tests/print_schema.rs @@ -171,21 +171,12 @@ fn schema_file_is_relative_to_project_root() { assert!(p.has_file("src/schema.rs")); } -#[test] -#[cfg(feature = "postgres")] -fn schema_file_contains_custom_types() { - test_print_schema( - "schema_file_contains_custom_types", - vec!["--generate-custom-type-definitions", "true"], - ) -} - #[test] #[cfg(feature = "postgres")] fn print_schema_disabling_custom_type_works() { test_print_schema( "print_schema_disabling_custom_type_works", - vec!["--generate-custom-type-definitions", "false"], + vec!["--no-generate-missing-sql-type-definitions"], ) } diff --git a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs index bba1a2d2c13c..2d014c18d149 100644 --- a/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_default_is_to_generate_custom_types/postgres/expected.rs @@ -1,15 +1,20 @@ // @generated automatically by Diesel CLI. -/// The `MyType` SQL type +/// A module containing custom SQL type definitions /// /// (Automatically generated by Diesel.) -#[derive(diesel::SqlType)] -#[postgres(type_name = "my_type")] -pub struct MyType; +pub mod sql_types { + /// The `my_type` SQL type + /// + /// (Automatically generated by Diesel.) + #[derive(diesel::SqlType)] + #[postgres(type_name = "my_type")] + pub struct MyType; +} diesel::table! { use diesel::sql_types::*; - use super::MyType; + use super::sql_types::MyType; /// Representation of the `custom_types` table. /// diff --git a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs index 272165b8e5e8..58379e4d98d8 100644 --- a/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs +++ b/diesel_cli/tests/print_schema/print_schema_type_renaming/postgres/expected.rs @@ -1,15 +1,20 @@ // @generated automatically by Diesel CLI. -/// The `UserJob` SQL type +/// A module containing custom SQL type definitions /// /// (Automatically generated by Diesel.) -#[derive(diesel::SqlType)] -#[postgres(type_name = "user_job")] -pub struct UserJob; +pub mod sql_types { + /// The `user_job` SQL type + /// + /// (Automatically generated by Diesel.) + #[derive(diesel::SqlType)] + #[postgres(type_name = "user_job")] + pub struct UserJob; +} diesel::table! { use diesel::sql_types::*; - use super::UserJob; + use super::sql_types::UserJob; /// Representation of the `users` table. ///