From e60651fc3ed2979e7cee86f05c2409e4a9b7ff5a Mon Sep 17 00:00:00 2001 From: Kaspar Lyngsie Date: Sun, 6 Sep 2026 17:10:41 +0200 Subject: [PATCH 1/2] feat: no silent drops --- README.md | 2 +- crates/oapi-codegen/src/config.rs | 131 ++- crates/oapi-codegen/src/console.rs | 3 + crates/oapi-codegen/src/coverage.rs | 886 ++++++++++++++++++ crates/oapi-codegen/src/diagnostic.rs | 50 + crates/oapi-codegen/src/error.rs | 15 +- crates/oapi-codegen/src/lib.rs | 2 + crates/oapi-codegen/src/loader.rs | 2 + crates/oapi-codegen/src/lower/paths.rs | 125 ++- crates/oapi-codegen/src/lower/schema.rs | 19 +- crates/oapi-codegen/tests/check_mode.rs | 165 ++++ .../tests/diagnostics_operations.rs | 314 +++++++ docs/configuration.md | 9 +- docs/design.md | 44 + 14 files changed, 1748 insertions(+), 19 deletions(-) create mode 100644 crates/oapi-codegen/src/coverage.rs create mode 100644 crates/oapi-codegen/src/diagnostic.rs create mode 100644 crates/oapi-codegen/tests/diagnostics_operations.rs diff --git a/README.md b/README.md index 802f491..8302033 100644 --- a/README.md +++ b/README.md @@ -47,7 +47,7 @@ differ: - Typed status-code response enums instead of response structs. - Token-based generation (`quote`/`syn`), not user-overridable `text/template`. - Blocking `reqwest` client — no async runtime forced on consumers. -- Reuses your `oapi-codegen` YAML config — unknown keys are ignored. +- Reuses your `oapi-codegen` YAML config — unknown keys produce warnings and are ignored. - Explicit over implicit: `--config-file` is required, and an output path must be given via `--output-file` or the config's `output:` key; empty generation fails loudly (Go defaults these and prints to stdout). - Fails fast where Go assumes: where `oapi-codegen` silently defaults, guesses, or ignores an ambiguity, this generator diff --git a/crates/oapi-codegen/src/config.rs b/crates/oapi-codegen/src/config.rs index e9b4d4f..f2eb3c2 100644 --- a/crates/oapi-codegen/src/config.rs +++ b/crates/oapi-codegen/src/config.rs @@ -5,15 +5,18 @@ use std::path::Path; use std::path::PathBuf; use serde::Deserialize; +use serde::Serialize; +use crate::diagnostic::Warning; +use crate::diagnostic::pointer; +use crate::diagnostic::report_warnings; use crate::error::Error; use crate::error::Result; /// A generator configuration, mirroring the keys used by `oapi-codegen`. /// -/// Unknown keys are ignored so that existing `oapi-codegen` configurations can be used -/// as-is. Only the subset relevant to this tool is interpreted. -#[derive(Debug, Default, Clone, Deserialize)] +/// Loading reports warnings for unknown keys and ignores them for compatibility. +#[derive(Debug, Default, Clone, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct Config { /// Target module/package name (informational for the Rust generator). @@ -32,7 +35,7 @@ pub struct Config { } /// The set of artifacts a configuration requests. -#[derive(Debug, Default, Clone, Deserialize)] +#[derive(Debug, Default, Clone, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct Generate { /// Generate data models (structs/enums) from component schemas. @@ -74,7 +77,7 @@ pub(crate) const DEFAULT_RESPONSE_SUFFIX: &str = "response"; pub(crate) const TYPE_NAME_SUFFIX_KEY: &str = "type-name-suffix"; /// Output tuning options. -#[derive(Debug, Default, Clone, Deserialize)] +#[derive(Debug, Default, Clone, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct OutputOptions { /// Keep schemas that are not referenced (no pruning). @@ -131,6 +134,19 @@ impl Config { source, }; })?; + let value: serde_yaml::Value = serde_yaml::from_str(&text).map_err(|source| { + return Error::ParseConfig { + path: path.display().to_string(), + source, + }; + })?; + let warnings = configuration_warnings(&value).map_err(|source| { + return Error::ParseConfig { + path: path.display().to_string(), + source, + }; + })?; + report_warnings(&path.display().to_string(), &warnings); let config: Config = serde_yaml::from_str(&text).map_err(|source| { return Error::ParseConfig { path: path.display().to_string(), @@ -141,10 +157,115 @@ impl Config { } } +fn configuration_warnings(value: &serde_yaml::Value) -> serde_yaml::Result> { + let shape = serde_yaml::to_value(Config::default())?; + let mut warnings = Vec::new(); + collect_unknown_keys(value, &shape, "", &mut warnings); + return Ok(warnings); +} + +fn collect_unknown_keys( + value: &serde_yaml::Value, + shape: &serde_yaml::Value, + parent: &str, + warnings: &mut Vec, +) { + let (Some(mapping), Some(fields)) = (value.as_mapping(), shape.as_mapping()) else { + return; + }; + // Empty default mappings contain user-defined keys. + if fields.is_empty() { + return; + } + for (key, value) in mapping { + let Some(name) = key.as_str() else { + continue; + }; + let path = pointer(parent, name); + if let Some(field) = fields.get(key) { + collect_unknown_keys(value, field, &path, warnings); + } else { + warnings.push(Warning::new(path, "unknown configuration key is ignored")); + } + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn unknown_configuration_keys_warn() -> serde_yaml::Result<()> { + for (yaml, paths) in [ + ("packge: demo", vec!["/packge"]), + ("generate: {modles: true}", vec!["/generate/modles"]), + ("output-options: {skip-prun: true}", vec!["/output-options/skip-prun"]), + ("a~/b: secret", vec!["/a~0~1b"]), + ("generate: {'~1/': true}", vec!["/generate/~01~1"]), + ( + "packge: demo\ngenerate: {modles: true}\noutput-options: {skip-prun: true}", + vec!["/packge", "/generate/modles", "/output-options/skip-prun"], + ), + ] { + let value = serde_yaml::from_str(yaml)?; + let expected: Vec<_> = paths + .into_iter() + .map(|path| { + return Warning::new(path, "unknown configuration key is ignored"); + }) + .collect(); + assert_eq!(configuration_warnings(&value)?, expected, "{yaml}"); + assert!(serde_yaml::from_value::(value).is_ok(), "{yaml}"); + } + return Ok(()); + } + + #[test] + fn known_configuration_keys_do_not_warn() -> serde_yaml::Result<()> { + for yaml in [ + "{}", + "package: demo\noutput: generated.rs", + "generate: {models: true, std-http-server: true, client: true, embedded-spec: false, server-urls: true}", + "import-mapping: {'./other.yaml': other, 'a~/b': module}", + "output-options: {include-tags: [pets], response-type-suffix: Resp, type-name-suffix: Alt}", + ] { + let value = serde_yaml::from_str(yaml)?; + assert!(configuration_warnings(&value)?.is_empty(), "{yaml}"); + assert!(serde_yaml::from_value::(value).is_ok(), "{yaml}"); + } + let defaults = serde_yaml::to_value(Config::default())?; + assert!(configuration_warnings(&defaults)?.is_empty()); + return Ok(()); + } + + #[test] + fn invalid_configuration_values_remain_deserialization_errors() -> serde_yaml::Result<()> { + for yaml in ["null", "generate: null", "output-options: null", "import-mapping: null"] { + let value = serde_yaml::from_str(yaml)?; + assert!(configuration_warnings(&value)?.is_empty(), "{yaml}"); + } + for yaml in [ + "false", + "[]", + "generate: wrong", + "generate: {models: wrong}", + "output-options: []", + "output-options: {include-tags: false}", + "import-mapping: {other: []}", + ] { + let value = serde_yaml::from_str(yaml)?; + assert!(configuration_warnings(&value)?.is_empty(), "{yaml}"); + assert!(serde_yaml::from_value::(value).is_err(), "{yaml}"); + } + let value = serde_yaml::from_str("generate: {modles: true, models: wrong}")?; + assert_eq!( + configuration_warnings(&value)?, + vec![Warning::new("/generate/modles", "unknown configuration key is ignored")] + ); + assert!(serde_yaml::from_value::(value).is_err()); + return Ok(()); + } + #[test] fn config_keys_match_serde_names() { let yaml = diff --git a/crates/oapi-codegen/src/console.rs b/crates/oapi-codegen/src/console.rs index c6ab02d..ff9250a 100644 --- a/crates/oapi-codegen/src/console.rs +++ b/crates/oapi-codegen/src/console.rs @@ -241,6 +241,9 @@ pub fn report_install_failed(dep: &Dependency, detail: &str) { /// Build the context-specific hints shown after an error message. fn hints_for(err: &Error) -> Vec { match err { + Error::InvalidSpec { .. } => { + return vec!["Use the OpenAPI 3.0 spelling and location for this key.".to_owned()]; + } Error::ReadSpec { path, source } => { return io_read_hints("spec file", path, source.kind()); } diff --git a/crates/oapi-codegen/src/coverage.rs b/crates/oapi-codegen/src/coverage.rs new file mode 100644 index 0000000..7eaa155 --- /dev/null +++ b/crates/oapi-codegen/src/coverage.rs @@ -0,0 +1,886 @@ +//! OpenAPI 3.0 object keys and diagnostics before typed deserialization. + +use serde_yaml::Value; + +use crate::diagnostic::Warning; +use crate::diagnostic::pointer; +use crate::diagnostic::report_warnings; +use crate::error::Error; +use crate::error::Result; +use crate::lower::validate::Diagnostics; + +const MAX_DEPTH: usize = 128; +const CONSTRAINT_KEYS: &[&str] = &[ + "multipleOf", + "maximum", + "exclusiveMaximum", + "minimum", + "exclusiveMinimum", + "maxLength", + "minLength", + "pattern", + "maxItems", + "minItems", + "uniqueItems", + "maxProperties", + "minProperties", +]; + +macro_rules! catalogue { + ($context:expr; $($pattern:pat => $fields:expr),* $(,)?) => { + match $context { + $($pattern => const { $fields },)* + } + }; +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Context { + Document, + Info, + Contact, + License, + Server, + ServerVariable, + Paths, + PathItem, + Operation, + Components, + Schema, + PropertySchema, + Parameter, + Header, + RequestBody, + Responses, + Response, + MediaType, + Encoding, + Example, + Link, + Callback, + SecurityScheme, + OAuthFlows, + OAuthFlow, + ExternalDocs, + Tag, + Discriminator, + Xml, +} + +#[derive(Debug, Clone, Copy)] +enum Traversal { + Literal, + SchemaOrBool, + Object(Context), + Map(Context), + Array(Context), +} + +#[derive(Debug, Clone, Copy)] +enum Handling { + Read, + Annotation, + Unsupported(&'static str), +} + +#[derive(Debug, Clone, Copy)] +struct Field { + name: &'static str, + traversal: Traversal, + handling: Handling, +} + +const fn read(name: &'static str, traversal: Traversal) -> Field { + return Field { + name, + traversal, + handling: Handling::Read, + }; +} + +const fn annotation(name: &'static str, traversal: Traversal) -> Field { + return Field { + name, + traversal, + handling: Handling::Annotation, + }; +} + +const fn unsupported(name: &'static str, traversal: Traversal, reason: &'static str) -> Field { + return Field { + name, + traversal, + handling: Handling::Unsupported(reason), + }; +} + +fn fields(context: Context) -> &'static [Field] { + use Context::*; + use Traversal::*; + + return catalogue! { context; + Document => &[ + read("openapi", Literal), + annotation("info", Object(Info)), + read("servers", Array(Server)), + read("paths", Object(Paths)), + read("components", Object(Components)), + read("security", Literal), + annotation("tags", Array(Tag)), + annotation("externalDocs", Object(ExternalDocs)), + ], + Info => &[ + annotation("title", Literal), + annotation("description", Literal), + annotation("termsOfService", Literal), + annotation("contact", Object(Contact)), + annotation("license", Object(License)), + annotation("version", Literal), + ], + Contact => &[ + annotation("name", Literal), + annotation("url", Literal), + annotation("email", Literal), + ], + License => &[annotation("name", Literal), annotation("url", Literal)], + Server => &[ + read("url", Literal), + read("description", Literal), + read("variables", Map(ServerVariable)), + ], + ServerVariable => &[ + read("enum", Literal), + read("default", Literal), + read("description", Literal), + ], + PathItem => &[ + annotation("summary", Literal), + annotation("description", Literal), + read("get", Object(Operation)), + read("put", Object(Operation)), + read("post", Object(Operation)), + read("delete", Object(Operation)), + read("options", Object(Operation)), + read("head", Object(Operation)), + read("patch", Object(Operation)), + read("trace", Object(Operation)), + unsupported("servers", Array(Server), "path-level server overrides are not implemented"), + read("parameters", Array(Parameter)), + ], + Operation => &[ + read("tags", Literal), + read("summary", Literal), + read("description", Literal), + annotation("externalDocs", Object(ExternalDocs)), + read("operationId", Literal), + read("parameters", Array(Parameter)), + read("requestBody", Object(RequestBody)), + read("responses", Object(Responses)), + unsupported("callbacks", Map(Callback), "callback operations are not generated"), + unsupported("deprecated", Literal, "operation deprecation is not emitted"), + read("security", Literal), + unsupported("servers", Array(Server), "operation-level server overrides are not implemented"), + ], + Components => &[ + read("schemas", Map(Schema)), + read("responses", Map(Response)), + read("parameters", Map(Parameter)), + annotation("examples", Map(Example)), + read("requestBodies", Map(RequestBody)), + read("headers", Map(Header)), + read("securitySchemes", Map(SecurityScheme)), + unsupported("links", Map(Link), "response links are not generated"), + unsupported("callbacks", Map(Callback), "callback operations are not generated"), + ], + Schema | PropertySchema => &[ + annotation("title", Literal), + read("multipleOf", Literal), + read("maximum", Literal), + read("exclusiveMaximum", Literal), + read("minimum", Literal), + read("exclusiveMinimum", Literal), + read("maxLength", Literal), + read("minLength", Literal), + read("pattern", Literal), + read("maxItems", Literal), + read("minItems", Literal), + read("uniqueItems", Literal), + read("maxProperties", Literal), + read("minProperties", Literal), + read("required", Literal), + read("enum", Literal), + read("type", Literal), + read("allOf", Array(Schema)), + read("oneOf", Array(Schema)), + read("anyOf", Array(Schema)), + read("not", Object(Schema)), + read("items", Object(Schema)), + read("properties", Map(PropertySchema)), + read("additionalProperties", SchemaOrBool), + read("description", Literal), + read("format", Literal), + read("default", Literal), + read("nullable", Literal), + read("discriminator", Object(Discriminator)), + read("readOnly", Literal), + read("writeOnly", Literal), + unsupported("xml", Object(Xml), "XML serialization is not implemented"), + annotation("externalDocs", Object(ExternalDocs)), + annotation("example", Literal), + read("deprecated", Literal), + ], + Parameter => &[ + read("name", Literal), + read("in", Literal), + read("description", Literal), + read("required", Literal), + unsupported("deprecated", Literal, "parameter deprecation is not emitted"), + unsupported("allowEmptyValue", Literal, "allowEmptyValue is not implemented"), + read("style", Literal), + read("explode", Literal), + unsupported("allowReserved", Literal, "allowReserved is not implemented"), + read("schema", Object(Schema)), + annotation("example", Literal), + annotation("examples", Map(Example)), + read("content", Map(MediaType)), + ], + Header => &[ + read("description", Literal), + read("required", Literal), + unsupported("deprecated", Literal, "header deprecation is not emitted"), + unsupported("allowEmptyValue", Literal, "header allowEmptyValue is not implemented"), + unsupported("allowReserved", Literal, "header allowReserved is not implemented"), + unsupported("style", Literal, "response-header serialization styles are not implemented"), + unsupported("explode", Literal, "response-header explode is not implemented"), + read("schema", Object(Schema)), + annotation("example", Literal), + annotation("examples", Map(Example)), + read("content", Map(MediaType)), + ], + RequestBody => &[ + annotation("description", Literal), + read("content", Map(MediaType)), + read("required", Literal), + ], + Response => &[ + read("description", Literal), + read("headers", Map(Header)), + read("content", Map(MediaType)), + unsupported("links", Map(Link), "response links are not generated"), + ], + MediaType => &[ + read("schema", Object(Schema)), + annotation("example", Literal), + annotation("examples", Map(Example)), + unsupported("encoding", Map(Encoding), "per-property body encoding is not implemented"), + ], + Encoding => &[ + read("contentType", Literal), + read("headers", Map(Header)), + read("style", Literal), + read("explode", Literal), + read("allowReserved", Literal), + ], + Example => &[ + annotation("summary", Literal), + annotation("description", Literal), + annotation("value", Literal), + annotation("externalValue", Literal), + ], + Link => &[ + read("operationRef", Literal), + read("operationId", Literal), + read("parameters", Literal), + read("requestBody", Literal), + annotation("description", Literal), + read("server", Object(Server)), + ], + SecurityScheme => &[ + read("type", Literal), + read("description", Literal), + read("name", Literal), + read("in", Literal), + read("scheme", Literal), + annotation("bearerFormat", Literal), + read("flows", Object(OAuthFlows)), + read("openIdConnectUrl", Literal), + ], + OAuthFlows => &[ + read("implicit", Object(OAuthFlow)), + read("password", Object(OAuthFlow)), + read("clientCredentials", Object(OAuthFlow)), + read("authorizationCode", Object(OAuthFlow)), + ], + OAuthFlow => &[ + read("authorizationUrl", Literal), + read("tokenUrl", Literal), + read("refreshUrl", Literal), + read("scopes", Literal), + ], + ExternalDocs => &[annotation("description", Literal), annotation("url", Literal)], + Tag => &[ + annotation("name", Literal), + annotation("description", Literal), + annotation("externalDocs", Object(ExternalDocs)), + ], + Discriminator => &[ + unsupported("propertyName", Literal, "discriminator dispatch uses shapes rather than this property"), + read("mapping", Literal), + ], + Xml => &[ + annotation("name", Literal), + annotation("namespace", Literal), + annotation("prefix", Literal), + annotation("attribute", Literal), + annotation("wrapped", Literal), + ], + Paths | Responses | Callback => &[], + }; +} + +impl Context { + fn references(self) -> bool { + return matches!( + self, + Self::Schema + | Self::PropertySchema + | Self::PathItem + | Self::Parameter + | Self::Header + | Self::RequestBody + | Self::Response + | Self::Example + | Self::Link + | Self::SecurityScheme + | Self::Callback + ); + } +} + +struct Sweep<'a> { + document: &'a str, + warnings: Vec, + problems: Diagnostics, +} + +pub(crate) fn check(document: &str, value: &Value) -> Result<()> { + let sweep = inspect(document, value); + report_warnings(document, &sweep.warnings); + return sweep.problems.into_result(); +} + +fn inspect<'a>(document: &'a str, value: &Value) -> Sweep<'a> { + let mut sweep = Sweep { + document, + warnings: Vec::new(), + problems: Diagnostics::new(), + }; + sweep.object(value, Context::Document, "", 0); + return sweep; +} + +impl Sweep<'_> { + fn invalid(&mut self, path: &str, reason: impl Into) { + self.problems.push(Error::InvalidSpec { + document: self.document.to_owned(), + path: path.to_owned(), + reason: reason.into(), + }); + } + + fn warn(&mut self, path: &str, message: impl Into) { + self.warnings.push(Warning::new(path, message)); + } + + fn object(&mut self, value: &Value, context: Context, path: &str, depth: usize) { + if depth > MAX_DEPTH { + self.invalid(path, format!("OpenAPI object nesting exceeds {MAX_DEPTH} levels")); + return; + } + let Some(mapping) = value.as_mapping() else { + self.invalid(path, format!("{context:?} must be an object")); + return; + }; + let reference = context.references() && mapping.contains_key("$ref"); + for (key, child) in mapping { + let key = match key { + Value::String(key) => key.clone(), + Value::Number(number) if context == Context::Responses => number.to_string(), + _ => { + self.invalid(path, "OpenAPI object keys must be strings"); + continue; + } + }; + let at = pointer(path, &key); + if key == "$ref" && context.references() { + if child.as_str().is_none() { + self.invalid(&at, "$ref must be a string"); + } + continue; + } + if reference && context != Context::PathItem { + self.warn(&at, "siblings of $ref are ignored in OpenAPI 3.0"); + } + if key.starts_with("x-") { + self.extension(&key, context, &at); + continue; + } + let dynamic = match context { + Context::Paths if key.starts_with('/') => Some(Context::PathItem), + Context::Responses if response_key(&key) => Some(Context::Response), + Context::Callback => Some(Context::PathItem), + _ => None, + }; + if let Some(next) = dynamic { + self.object(child, next, &at, depth + 1); + continue; + } + let Some(field) = fields(context).iter().find(|field| return field.name == key) else { + self.invalid(&at, format!("unknown OpenAPI 3.0 key `{key}` in {context:?}")); + continue; + }; + if !reference { + if let Handling::Unsupported(reason) = field.handling + && child.as_bool() != Some(false) + { + self.warn(&at, reason); + } + self.value_notes(context, &key, child, &at); + } + self.walk(child, field.traversal, &at, depth + 1); + } + if !reference { + self.object_notes(value, context, path); + } + } + + fn walk(&mut self, value: &Value, traversal: Traversal, path: &str, depth: usize) { + match traversal { + Traversal::Literal => {} + Traversal::SchemaOrBool if value.is_bool() => {} + Traversal::SchemaOrBool => self.object(value, Context::Schema, path, depth), + Traversal::Object(context) => self.object(value, context, path, depth), + Traversal::Map(context) => { + if let Some(mapping) = value.as_mapping() { + for (name, child) in mapping { + let Some(name) = name.as_str() else { + self.invalid(path, "OpenAPI map names must be strings"); + continue; + }; + self.object(child, context, &pointer(path, name), depth); + } + } else { + self.invalid(path, "this OpenAPI field must be a map"); + } + } + Traversal::Array(context) => { + if let Some(sequence) = value.as_sequence() { + for (index, child) in sequence.iter().enumerate() { + self.object(child, context, &pointer(path, &index.to_string()), depth); + } + } else { + self.invalid(path, "this OpenAPI field must be an array"); + } + } + } + } + + fn extension(&mut self, key: &str, context: Context, path: &str) { + if key.starts_with("x-go-") { + return; + } + let handled = match context { + Context::PropertySchema => matches!( + key, + "x-rust-type" + | "x-rust-name" + | "x-rust-derive" + | "x-rust-serde-skip" + | "x-omitempty" + | "x-order" + | "x-deprecated-reason" + | "x-enum-varnames" + | "x-enumNames" + ), + Context::Schema => matches!( + key, + "x-rust-type" + | "x-rust-name" + | "x-rust-derive" + | "x-deprecated-reason" + | "x-enum-varnames" + | "x-enumNames" + ), + Context::Operation => key == "x-rust-name", + _ => false, + }; + if !handled { + self.warn(path, "this extension is not implemented here and is ignored"); + } + } + + fn value_notes(&mut self, context: Context, key: &str, value: &Value, path: &str) { + if matches!(context, Context::Document | Context::Operation) + && key == "security" + && let Some(requirements) = value.as_sequence() + { + if requirements.len() > 1 { + self.warn(path, "security alternatives are flattened into a list of scheme names"); + } + if requirements.iter().any(|requirement| { + return requirement.as_mapping().is_some_and(|schemes| { + return schemes.values().any(|scopes| { + return scopes.as_sequence().is_some_and(|scopes| return !scopes.is_empty()); + }); + }); + }) { + self.warn(path, "security scopes are not enforced by the generated code"); + } + } + if matches!(context, Context::Schema | Context::PropertySchema) { + match key { + "oneOf" => self.warn(path, "oneOf uses first-match deserialization, not exclusive matching"), + "anyOf" => self.warn(path, "anyOf retains only the first matching representation"), + "allOf" + if value.as_sequence().is_some_and(|members| { + return match members.as_slice() { + [] => false, + [member] => member.get("$ref").is_none(), + _ => true, + }; + }) => + { + self.warn( + path, + "allOf merges properties rather than validating every member independently", + ); + } + "nullable" if value.as_bool() == Some(true) => { + self.warn( + path, + "null and absent values are not distinguished in every schema position", + ); + } + "default" if context == Context::Schema => { + self.warn( + path, + "defaults are applied only at supported property and query-parameter uses", + ); + } + _ => {} + } + } + } + + fn object_notes(&mut self, value: &Value, context: Context, path: &str) { + match context { + Context::RequestBody if value.get("required").and_then(Value::as_bool) != Some(true) => { + self.warn( + path, + "optional request bodies are generated as required when a body type is emitted", + ); + } + Context::MediaType if value.get("schema").is_none() => { + self.warn(path, "a media entry without a schema does not generate a body type"); + } + Context::Schema | Context::PropertySchema => self.schema_notes(value, context, path), + _ => {} + } + } + + fn schema_notes(&mut self, value: &Value, context: Context, path: &str) { + let kind = value.get("type").and_then(Value::as_str); + if let Some(kind) = kind + && !matches!(kind, "string" | "integer" | "number" | "boolean" | "object" | "array") + { + self.invalid( + &pointer(path, "type"), + format!("`{kind}` is not an OpenAPI 3.0 schema type"), + ); + } + if context == Context::Schema && CONSTRAINT_KEYS.iter().any(|key| return value.get(*key).is_some()) { + self.warn( + path, + "constraints are enforced only at supported field uses, not on type aliases or array items", + ); + } + if value.get("x-rust-derive").is_some() && value.get("x-rust-type").is_none() { + self.warn( + &pointer(path, "x-rust-derive"), + "x-rust-derive requires x-rust-type and is otherwise ignored", + ); + } + if matches!(kind, Some("number" | "boolean")) && value.get("enum").is_some() { + self.warn( + &pointer(path, "enum"), + "number and boolean enum restrictions are not enforced", + ); + } + if let Some(format) = value.get("format").and_then(Value::as_str) { + let handled = match kind { + Some("string") => matches!(format, "date" | "date-time" | "byte" | "binary" | "password" | "uuid"), + Some("integer") => matches!(format, "int32" | "int64"), + Some("number") => matches!(format, "float" | "double"), + _ => false, + }; + if !handled { + self.warn( + &pointer(path, "format"), + "this format is not implemented and the base type is used", + ); + } + } + if kind == Some("object") + && value.get("additionalProperties").and_then(Value::as_bool) == Some(false) + && value + .get("properties") + .and_then(Value::as_mapping) + .is_none_or(serde_yaml::Mapping::is_empty) + { + self.warn( + &pointer(path, "additionalProperties"), + "an object without declared properties becomes a map and does not reject additional properties", + ); + } + } +} + +fn response_key(key: &str) -> bool { + if key == "default" { + return true; + } + let mut bytes = key.bytes(); + return matches!(bytes.next(), Some(b'1'..=b'5')) + && matches!( + (bytes.next(), bytes.next(), bytes.next()), + (Some(b'0'..=b'9'), Some(b'0'..=b'9'), None) | (Some(b'X'), Some(b'X'), None) + ); +} + +#[cfg(test)] +mod tests { + use super::*; + + fn inspect_yaml(yaml: &str) -> Sweep<'static> { + let value = serde_yaml::from_str(yaml).expect("valid YAML"); + return inspect("spec.yaml", &value); + } + + #[test] + fn unknown_keys_are_rejected_in_nested_objects() { + for yaml in [ + "inf: {}", + "info: {titel: Demo}", + "components: {schemas: {Widget: {type: string, const: x}}}", + "paths: {/widgets: {get: {operationID: list}}}", + "components: {schemas: {Widget: {properties: {name: {typo: string}}}}}", + "components: {schemas: {Widget: {xml: {nam: widget}}}}", + "components: {examples: {payload: {externalvalue: url}}}", + "components: {securitySchemes: {auth: {flows: {password: {tokenURL: url}}}}}", + "paths: {widgets: {}}", + "paths: {/widgets: {get: {responses: {20X: {description: wrong}}}}}", + ] { + let sweep = inspect_yaml(yaml); + assert!(!sweep.problems.is_empty(), "accepted {yaml}"); + } + } + + #[test] + fn invalid_structures_and_schema_types_are_rejected() { + for yaml in [ + "info: []", + "components: {schemas: []}", + "servers: {}", + "components: {schemas: {Widget: {type: stirng}}}", + "components: {schemas: {Widget: {type: 'null'}}}", + "components: {schemas: {Widget: {$ref: 42, type: string}}}", + "components: {schemas: {Widget: {xml: false}}}", + "components: {schemas: {Widget: {items: true}}}", + "components: {schemas: {Widget: {additionalProperties: []}}}", + ] { + assert!(!inspect_yaml(yaml).problems.is_empty(), "accepted {yaml}"); + } + for value in ["true", "false", "{type: string}"] { + let yaml = format!("components: {{schemas: {{Widget: {{type: object, additionalProperties: {value}}}}}}}"); + assert!(inspect_yaml(&yaml).problems.is_empty(), "{yaml}"); + } + } + + #[test] + fn supported_shapes_and_documented_annotations_are_quiet() { + let sweep = inspect_yaml( + " +openapi: 3.0.3 +info: + title: Demo + version: 1.0.0 + contact: {name: Support, url: 'https://example.com', email: support@example.com} + license: {name: MIT, url: 'https://example.com/license'} +tags: [{name: widgets, externalDocs: {url: 'https://example.com/widgets'}}] +paths: {} +components: + schemas: + Widget: + type: object + title: A widget + description: The model. + required: [id] + properties: + id: {type: string, example: {arbitrary: true}} +", + ); + assert!(sweep.problems.is_empty()); + assert!(sweep.warnings.is_empty(), "{:?}", sweep.warnings); + } + + #[test] + fn header_flags_are_recognized_and_default_values_are_quiet() { + for flag in [false, true] { + let yaml = format!( + "components: {{headers: {{X-Test: {{schema: {{type: string}}, allowEmptyValue: {flag}, allowReserved: {flag}}}}}}}" + ); + let sweep = inspect_yaml(&yaml); + assert!(sweep.problems.is_empty()); + assert_eq!(sweep.warnings.is_empty(), !flag); + } + } + + #[test] + fn deeply_nested_objects_stop_at_the_inspection_limit() { + let mut value = Value::Mapping(serde_yaml::Mapping::new()); + for _ in 0..MAX_DEPTH + 1 { + let mut mapping = serde_yaml::Mapping::new(); + mapping.insert(Value::String("items".to_owned()), value); + value = Value::Mapping(mapping); + } + let mut sweep = inspect_yaml("{}"); + sweep.object(&value, Context::Schema, "/schema", 0); + let error = sweep.problems.into_result().expect_err("nesting limit"); + assert!(error.to_string().contains("nesting exceeds")); + } + + #[test] + fn literal_maps_and_user_names_are_not_spec_objects() { + let sweep = inspect_yaml( + " +components: + schemas: + x-model: + type: object + properties: + const: {type: string} + example: {const: arbitrary, typo: {anything: true}} + default: {unevaluatedProperties: false} + enum: [{other: {anything: true}}] + discriminator: {propertyName: kind, mapping: {arbitrary: '#/components/schemas/x-model'}} + examples: + x-example: {value: {notAnOpenAPIKey: true}} + links: + next: {parameters: {arbitrary: '$response.body#/id'}, requestBody: {anything: true}} + securitySchemes: + auth: + type: oauth2 + flows: + password: {tokenUrl: /token, scopes: {arbitrary: description}} +security: [{arbitrary: [custom]}] +", + ); + assert!(sweep.problems.is_empty()); + assert!( + !sweep + .warnings + .iter() + .any(|warning| return warning.path.contains("typo")) + ); + } + + #[test] + fn unsupported_features_warn_and_their_children_are_still_checked() { + let sweep = inspect_yaml( + "paths: {/widgets: {get: {callbacks: {event: {'{$request.body#/url}': {post: {responses: {default: {description: ok}}}}}}}}}", + ); + assert!(sweep.problems.is_empty()); + assert!( + sweep + .warnings + .iter() + .any(|warning| return warning.path.ends_with("/callbacks")) + ); + let invalid = inspect_yaml( + "paths: {/widgets: {get: {callbacks: {event: {'{$request.body#/url}': {post: {typo: true}}}}}}}", + ); + assert!(!invalid.problems.is_empty()); + } + + #[test] + fn references_report_ignored_siblings() { + let sweep = + inspect_yaml("components: {schemas: {Widget: {$ref: '#/components/schemas/Other', description: ignored}}}"); + assert!(sweep.problems.is_empty()); + assert!( + sweep + .warnings + .iter() + .any(|warning| return warning.message.contains("siblings")) + ); + let invalid = + inspect_yaml("components: {schemas: {Widget: {$ref: '#/components/schemas/Other', requird: [id]}}}"); + assert!(!invalid.problems.is_empty()); + } + + #[test] + fn extensions_are_opaque_but_unhandled_extensions_warn() { + let sweep = inspect_yaml( + "x-vendor: {arbitrary: true}\nx-go-custom: true\ncomponents: {schemas: {Widget: {type: string, x-rust-type: 'String'}}}", + ); + assert!(sweep.problems.is_empty()); + assert_eq!(sweep.warnings.len(), 1); + assert_eq!(sweep.warnings.first().expect("warning").path, "/x-vendor"); + } + + #[test] + fn pointers_escape_property_and_path_names() { + let sweep = inspect_yaml("paths: {'/a~b': {get: {typo: true}}}"); + let error = sweep.problems.into_result().expect_err("unknown key"); + assert!(matches!(error, Error::InvalidSpec { path, .. } if path == "/paths/~1a~0b/get/typo")); + } + + #[test] + fn response_keys_allow_exact_codes_ranges_and_default() { + for key in ["100", "200", "599", "2XX", "default"] { + assert!(response_key(key), "{key}"); + } + for key in ["20", "2000", "600", "2xx", "20X", "foo"] { + assert!(!response_key(key), "{key}"); + } + let sweep = inspect_yaml("paths: {/widgets: {get: {responses: {200: {description: ok}}}}}"); + assert!(sweep.problems.is_empty()); + } + + #[test] + fn schema_limitations_are_reported_without_changing_generation() { + for (schema, keyword) in [ + ("{type: number, enum: [1.5]}", "enum"), + ("{type: string, format: custom}", "format"), + ("{type: string, nullable: true}", "nullable"), + ("{oneOf: [{type: string}, {type: integer}]}", "oneOf"), + ("{anyOf: [{type: string}, {type: integer}]}", "anyOf"), + ("{type: object, additionalProperties: false}", "additionalProperties"), + ( + "{allOf: [{type: object, properties: {name: {type: string}}, additionalProperties: false}]}", + "allOf", + ), + ] { + let yaml = format!("components: {{schemas: {{Widget: {schema}}}}}"); + let sweep = inspect_yaml(&yaml); + assert!(sweep.problems.is_empty(), "{schema}"); + assert!( + sweep + .warnings + .iter() + .any(|warning| return warning.path.ends_with(keyword)), + "{schema}", + ); + } + } +} diff --git a/crates/oapi-codegen/src/diagnostic.rs b/crates/oapi-codegen/src/diagnostic.rs new file mode 100644 index 0000000..98716dc --- /dev/null +++ b/crates/oapi-codegen/src/diagnostic.rs @@ -0,0 +1,50 @@ +use anstream::eprintln; +use owo_colors::OwoColorize; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct Warning { + pub(crate) path: String, + pub(crate) message: String, +} + +impl Warning { + pub(crate) fn new(path: impl Into, message: impl Into) -> Self { + return Self { + path: path.into(), + message: message.into(), + }; + } +} + +pub(crate) fn report_warnings(document: &str, warnings: &[Warning]) { + for warning in warnings { + eprintln!( + "{} {document}: {}: {}", + "warning:".yellow().bold(), + warning.path, + warning.message + ); + } +} + +pub(crate) fn pointer(parent: &str, key: &str) -> String { + return format!("{parent}/{}", key.replace('~', "~0").replace('/', "~1")); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn pointer_escapes_literal_keys() { + for (parent, key, expected) in [ + ("", "generate", "/generate"), + ("/generate", "modles", "/generate/modles"), + ("", "a~/b", "/a~0~1b"), + ("/a~0~1b", "~1/", "/a~0~1b/~01~1"), + ("", "", "/"), + ] { + assert_eq!(pointer(parent, key), expected); + } + } +} diff --git a/crates/oapi-codegen/src/error.rs b/crates/oapi-codegen/src/error.rs index 9694283..5ad0f5f 100644 --- a/crates/oapi-codegen/src/error.rs +++ b/crates/oapi-codegen/src/error.rs @@ -4,6 +4,15 @@ #[derive(Debug)] #[non_exhaustive] pub enum Error { + /// An OpenAPI object contains an invalid key or structure. + InvalidSpec { + /// The source document. + document: String, + /// The JSON pointer to the invalid entry. + path: String, + /// Why the entry is invalid. + reason: String, + }, /// The spec file cannot be read from disk. ReadSpec { /// Path that cannot be read. @@ -411,6 +420,9 @@ pub enum Error { impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { + Error::InvalidSpec { document, path, reason } => { + return write!(f, "{document}#{path}: {reason}"); + } Error::ReadSpec { path, source } => { return write!(f, "failed to read spec file `{path}`: {source}"); } @@ -594,7 +606,8 @@ impl std::error::Error for Error { Error::InvalidGeneratedCode { source } => return Some(source), // `Validation` holds problems at the same level and wraps no cause. // It has no single `source`. `Display` shows the problems instead. - Error::Validation { .. } + Error::InvalidSpec { .. } + | Error::Validation { .. } | Error::Unimplemented(_) | Error::UnownedOutput { .. } | Error::UnsplittableOutput { .. } diff --git a/crates/oapi-codegen/src/lib.rs b/crates/oapi-codegen/src/lib.rs index 3213328..60325ef 100644 --- a/crates/oapi-codegen/src/lib.rs +++ b/crates/oapi-codegen/src/lib.rs @@ -7,7 +7,9 @@ pub mod cli; pub mod config; +mod coverage; pub mod deps; +mod diagnostic; pub mod emit; pub mod error; pub mod filter; diff --git a/crates/oapi-codegen/src/loader.rs b/crates/oapi-codegen/src/loader.rs index c063827..94a1396 100644 --- a/crates/oapi-codegen/src/loader.rs +++ b/crates/oapi-codegen/src/loader.rs @@ -90,6 +90,7 @@ impl Spec { // with a message that names a YAML shape and not a version. check_spec_version(&document, &value)?; check_top_level_keys(&value)?; + crate::coverage::check(&document, &value)?; let inner: OpenAPI = serde_yaml::from_value(value).map_err(|source| { return Error::ParseSpec { path: document.clone(), @@ -140,6 +141,7 @@ impl Spec { // before the typed parse for the same reason. check_spec_version(file, &value)?; check_top_level_keys(&value)?; + crate::coverage::check(file, &value)?; let parsed: OpenAPI = serde_yaml::from_value(value).map_err(|source| { return Error::ParseRefFile { file: file.to_owned(), diff --git a/crates/oapi-codegen/src/lower/paths.rs b/crates/oapi-codegen/src/lower/paths.rs index d28b96d..764a0d6 100644 --- a/crates/oapi-codegen/src/lower/paths.rs +++ b/crates/oapi-codegen/src/lower/paths.rs @@ -45,6 +45,7 @@ use openapiv3::Operation as OasOperation; use openapiv3::Parameter; use openapiv3::ParameterData; use openapiv3::ParameterSchemaOrContent; +use openapiv3::PathStyle; use openapiv3::QueryStyle; use openapiv3::ReferenceOr; use openapiv3::RequestBody; @@ -55,6 +56,8 @@ use openapiv3::SchemaKind; use openapiv3::StatusCode; use openapiv3::Type; +use crate::diagnostic::Warning; +use crate::diagnostic::report_warnings; use crate::error::Error; use crate::error::Result; use crate::ir::Body; @@ -332,8 +335,9 @@ impl Lowerer<'_> { // the template. Driving the loop above from the template alone will // otherwise silently drop such a parameter from the generated signature, // producing a handler that omits a required input. + let mut seen = Vec::new(); for parameter in params { - let Parameter::Path { parameter_data, .. } = ¶meter.value else { + let Parameter::Path { parameter_data, style } = ¶meter.value else { continue; }; if !placeholders @@ -346,6 +350,20 @@ impl Lowerer<'_> { name: parameter_data.name.clone(), }); } + if seen.contains(¶meter_data.name.as_str()) { + continue; + } + seen.push(parameter_data.name.as_str()); + if !matches!(style, PathStyle::Simple) { + self.warn( + parameter.origin.as_deref(), + &format!("{method} {path}"), + format!( + "path parameter `{}` uses unsupported style `{style:?}`. Generated code uses `simple` encoding", + parameter_data.name + ), + ); + } } return Ok(path_params); } @@ -499,7 +517,14 @@ impl Lowerer<'_> { }); } let element = match &array.items { - Some(ReferenceOr::Item(item)) => scalar_type(&item.schema_kind), + Some(ReferenceOr::Item(item)) => { + self.warn_scalar_enum( + origin, + &format!("{method} {path} query parameter `{name}` items"), + &item.schema_kind, + ); + scalar_type(&item.schema_kind) + } Some(ReferenceOr::Reference { reference }) if ref_file_part(reference).is_some() => { return Err(Error::UnsupportedOperation { method: method.to_owned(), @@ -511,6 +536,11 @@ impl Lowerer<'_> { } Some(ReferenceOr::Reference { reference }) => { let item = self.spec.resolve_schema(origin, reference)?; + self.warn_scalar_enum( + origin, + &format!("{method} {path} query parameter `{name}` items"), + &item.schema_kind, + ); scalar_type(&item.schema_kind) } None => { @@ -537,6 +567,20 @@ impl Lowerer<'_> { reason: format!("query parameter `{name}` must be a scalar or an array of scalars"), }; })?; + self.warn_scalar_enum( + origin, + &format!("{method} {path} query parameter `{name}`"), + &schema.schema_kind, + ); + if !matches!(style, QueryStyle::Form) { + self.warn( + origin, + &format!("{method} {path}"), + format!( + "query parameter `{name}` uses unsupported scalar style `{style:?}`. Generated code uses `form` encoding" + ), + ); + } return Ok(ty); } @@ -674,6 +718,11 @@ impl Lowerer<'_> { reason: format!("{kind_label} `{name}` uses a `byte`/`binary` format, which is not supported"), }); } + self.warn_scalar_enum( + origin, + &format!("{method} {path} {kind_label} `{name}`"), + &schema.schema_kind, + ); return Ok(ty); } @@ -784,9 +833,30 @@ impl Lowerer<'_> { reason: format!("cookie parameter `{name}` uses a `byte`/`binary` format, which is not supported"), }); } + self.warn_scalar_enum( + origin, + &format!("{method} {path} cookie parameter `{name}`"), + &schema.schema_kind, + ); return Ok(ty); } + fn warn_scalar_enum(&self, origin: Option<&str>, context: &str, kind: &SchemaKind) { + let has_enum = match kind { + SchemaKind::Type(Type::String(schema)) => !schema.enumeration.is_empty(), + SchemaKind::Type(Type::Integer(schema)) => !schema.enumeration.is_empty(), + _ => false, + }; + if has_enum { + self.warn( + origin, + context, + "the declared `enum` is ignored. Generated code uses the base scalar type without enum validation" + .to_owned(), + ); + } + } + /// Collect every supported content type a body declares, deduplicated by /// [`BodyKind`] and ordered by the caller's `priority` (requests and /// responses differ — see [`REQUEST_BODY_PRIORITY`] / @@ -798,19 +868,47 @@ impl Lowerer<'_> { &self, content: &'m indexmap::IndexMap, priority: &[BodyKind], + origin: Option<&str>, + context: &str, ) -> Vec<(BodyKind, &'m openapiv3::MediaType)> { let mut selected = Vec::new(); for &wanted in priority { + let mut first = None; for (name, media) in content { if media_type_kind(name) == Some(wanted) { - selected.push((wanted, media)); - break; + match first { + Some(first) => self.warn( + origin, + context, + format!("media type `{name}` is ignored because `{first}` is the first representation of the same body kind"), + ), + None => { + selected.push((wanted, media)); + first = Some(name); + } + } + } + } + } + if !selected.is_empty() { + for name in content.keys() { + if !media_type_kind(name).is_some_and(|kind| return priority.contains(&kind)) { + self.warn(origin, context, format!("unsupported media type `{name}` is ignored")); } } } return selected; } + fn warn(&self, origin: Option<&str>, context: &str, message: String) { + let document = self.spec.source().display().to_string(); + let message = match origin { + Some(origin) => format!("{message} (resolved from `{origin}`)"), + None => message, + }; + report_warnings(&document, &[Warning::new(context, message)]); + } + /// Lower a selected body media entry into a typed [`Body`] for the given /// content kind. Text bodies must be `string`. form bodies must reference a /// named object schema. JSON reuses the existing body-type mapping. @@ -1002,6 +1100,11 @@ impl Lowerer<'_> { reason: format!("path parameter `{name}` must be a scalar type"), }; })?; + self.warn_scalar_enum( + origin, + &format!("{method} {path} path parameter `{name}`"), + &schema.schema_kind, + ); return Ok(ty); } @@ -1032,7 +1135,12 @@ impl Lowerer<'_> { (resolved.value, resolved.origin) } }; - let supported = self.supported_bodies(&body.content, &REQUEST_BODY_PRIORITY); + let supported = self.supported_bodies( + &body.content, + &REQUEST_BODY_PRIORITY, + origin.as_deref(), + &format!("{method} {path} request body"), + ); if supported.is_empty() { if body.content.is_empty() { return Ok(None); @@ -1426,7 +1534,12 @@ impl Lowerer<'_> { origin: Option<&str>, response: &OasResponse, ) -> Result> { - let supported = self.supported_bodies(&response.content, &RESPONSE_BODY_PRIORITY); + let supported = self.supported_bodies( + &response.content, + &RESPONSE_BODY_PRIORITY, + origin, + &format!("{method} {path} {location}"), + ); if supported.is_empty() && !response.content.is_empty() { return Err(Error::UnsupportedContentType { method: method.to_owned(), diff --git a/crates/oapi-codegen/src/lower/schema.rs b/crates/oapi-codegen/src/lower/schema.rs index 3156b50..a3387f1 100644 --- a/crates/oapi-codegen/src/lower/schema.rs +++ b/crates/oapi-codegen/src/lower/schema.rs @@ -189,11 +189,11 @@ impl Mapper<'_> { ty, }) } - SchemaKind::Any(_) => Item::Alias(Alias { + SchemaKind::Any(schema) => Item::Alias(Alias { name: self.type_name_ident(name), doc: doc_of(data), deprecated: deprecation_of(data, name)?, - ty: RustType::Value, + ty: self.unconstrained_type(name, schema), }), SchemaKind::Not { .. } => { return Err(Error::UnsupportedSchema { @@ -749,7 +749,7 @@ impl Mapper<'_> { RustType::Named(hint.to_owned()) } } - SchemaKind::Any(_) => RustType::Value, + SchemaKind::Any(schema) => self.unconstrained_type(hint, schema), SchemaKind::Not { .. } => { return Err(Error::UnsupportedSchema { path: hint.to_owned(), @@ -772,6 +772,19 @@ impl Mapper<'_> { return Ok(RustType::Named(hint.to_owned())); } + fn unconstrained_type(&self, name: &str, schema: &openapiv3::AnySchema) -> RustType { + if *schema != openapiv3::AnySchema::default() { + crate::diagnostic::report_warnings( + &self.spec.source().display().to_string(), + &[crate::diagnostic::Warning::new( + name, + "this schema combination is not implemented and becomes an unconstrained JSON value", + )], + ); + } + return RustType::Value; + } + /// Element type for an object used purely as a map (`additionalProperties`). fn additional_properties_type(&mut self, hint: &str, obj: &ObjectType) -> Result { let element = match &obj.additional_properties { diff --git a/crates/oapi-codegen/tests/check_mode.rs b/crates/oapi-codegen/tests/check_mode.rs index 7d907bc..a9629a9 100644 --- a/crates/oapi-codegen/tests/check_mode.rs +++ b/crates/oapi-codegen/tests/check_mode.rs @@ -174,6 +174,171 @@ fn stderr(output: &Output) -> String { return String::from_utf8_lossy(&output.stderr).into_owned(); } +#[test] +fn unknown_configuration_keys_warn_without_changing_the_output() { + let dir = TestDir::new("unknown-config"); + assert_eq!(code(&dir.run(false)), SUCCESS); + let original = read(&dir.output()); + dir.write( + "config.yaml", + "package: demo\nextra: true\ngenerate:\n models: true\n modles: true\noutput-options:\n skip-prun: true\n", + ); + let output = dir.run(true); + assert_eq!(code(&output), SUCCESS, "{}", stderr(&output)); + for pointer in ["/extra", "/generate/modles", "/output-options/skip-prun"] { + assert!(stderr(&output).contains(pointer), "{}", stderr(&output)); + } + assert_eq!(read(&dir.output()), original); +} + +#[test] +fn invalid_configuration_values_keep_their_parse_errors() { + let dir = TestDir::new("invalid-config"); + for configuration in ["null", "generate: null", "generate:\n models: wrong\n modles: true\n"] { + dir.write("config.yaml", configuration); + let output = dir.run(false); + assert_eq!(code(&output), FAILURE, "{}", stderr(&output)); + assert!( + stderr(&output).contains("failed to parse config"), + "{}", + stderr(&output) + ); + assert!(!dir.output().exists()); + } +} + +#[test] +fn unknown_spec_keys_fail_before_any_output_is_replaced() { + let dir = TestDir::new("unknown-spec"); + assert_eq!(code(&dir.run(false)), SUCCESS); + let original = read(&dir.output()); + dir.write( + "spec.yaml", + &SPEC.replace("type: object", "type: object\n requird: [name]\n const: {}"), + ); + for check in [false, true] { + let output = dir.run(check); + assert_eq!(code(&output), FAILURE, "{}", stderr(&output)); + assert!( + stderr(&output).contains("/components/schemas/Widget/requird"), + "{}", + stderr(&output) + ); + assert!( + stderr(&output).contains("/components/schemas/Widget/const"), + "{}", + stderr(&output) + ); + assert_eq!(read(&dir.output()), original); + } +} + +#[test] +fn ignored_features_warn_during_generation_and_check_mode() { + let dir = TestDir::new("unsupported-note"); + dir.write( + "spec.yaml", + &SPEC.replace("type: object", "type: object\n xml: {name: widget}"), + ); + for check in [false, true] { + let output = dir.run(check); + assert_eq!(code(&output), SUCCESS, "{}", stderr(&output)); + assert!( + stderr(&output).contains("/components/schemas/Widget/xml"), + "{}", + stderr(&output) + ); + assert!( + stderr(&output).contains("XML serialization is not implemented"), + "{}", + stderr(&output) + ); + } +} + +#[test] +fn referenced_documents_use_the_same_key_diagnostics() { + let dir = TestDir::new("referenced-key"); + dir.write("config.yaml", PACKAGE_CONFIG); + dir.write( + "spec.yaml", + "openapi: 3.0.3\ninfo: {title: Root, version: 1.0.0}\npaths:\n /widgets:\n get:\n responses:\n '200': {$ref: 'other.yaml#/components/responses/Widget'}\n", + ); + dir.write( + "other.yaml", + "openapi: 3.0.3\ninfo: {title: Other, version: 1.0.0}\npaths: {}\ncomponents:\n responses:\n Widget: {description: ok, contnet: {}}\n", + ); + let output = dir.run(false); + assert_eq!(code(&output), FAILURE, "{}", stderr(&output)); + assert!( + stderr(&output).contains("other.yaml#/components/responses/Widget/contnet"), + "{}", + stderr(&output) + ); + assert!(!dir.output().exists()); +} + +#[test] +fn referenced_document_warnings_are_emitted_once_per_loaded_document() { + let dir = TestDir::new("referenced-note"); + dir.write("config.yaml", PACKAGE_CONFIG); + dir.write( + "spec.yaml", + "openapi: 3.0.3\ninfo: {title: Root, version: 1.0.0}\npaths:\n /widgets:\n get:\n responses:\n '200': {$ref: 'other.yaml#/components/responses/Widget'}\n '201': {$ref: 'other.yaml#/components/responses/Widget'}\n", + ); + dir.write( + "other.yaml", + "openapi: 3.0.3\ninfo: {title: Other, version: 1.0.0}\npaths: {}\nx-vendor: true\ncomponents:\n responses:\n Widget: {description: ok}\n", + ); + let output = dir.run(false); + assert_eq!(code(&output), SUCCESS, "{}", stderr(&output)); + assert_eq!(stderr(&output).matches("/x-vendor").count(), 1, "{}", stderr(&output)); +} + +#[test] +fn json_documents_and_all_library_entry_points_reject_unknown_keys() { + let dir = TestDir::new("json-unknown"); + dir.write( + "spec.yaml", + r#"{"openapi":"3.0.3","info":{"title":"Demo","version":"1.0.0"},"paths":{},"components":{"schemas":{"Widget":{"type":"string","const":"x"}}}}"#, + ); + let spec = dir.join("spec.yaml"); + let config = oapi_codegen::Config::load(&dir.join("config.yaml")).expect("valid configuration"); + assert!(oapi_codegen::generate_models_string(&spec).is_err()); + assert!(oapi_codegen::generate(&spec, &config).is_err()); + assert!(oapi_codegen::generate_package(&spec, &config, &dir.output()).is_err()); + let output = dir.run(false); + assert_eq!(code(&output), FAILURE, "{}", stderr(&output)); + assert!( + stderr(&output).contains("/components/schemas/Widget/const"), + "{}", + stderr(&output) + ); + assert!(!dir.output().exists()); +} + +#[test] +fn unconstrained_schema_fallback_warns_but_an_empty_schema_does_not() { + let dir = TestDir::new("schema-fallback"); + let preamble = "openapi: 3.0.3\ninfo: {title: Demo, version: 1.0.0}\npaths: {}\ncomponents:\n schemas:\n"; + dir.write("spec.yaml", &format!("{preamble} Widget: {{}}\n")); + let empty = dir.run(false); + assert_eq!(code(&empty), SUCCESS, "{}", stderr(&empty)); + assert!(!stderr(&empty).contains("warning:"), "{}", stderr(&empty)); + dir.write( + "spec.yaml", + &format!("{preamble} Widget: {{type: string, minimum: 1}}\n"), + ); + let constrained = dir.run(false); + assert_eq!(code(&constrained), SUCCESS, "{}", stderr(&constrained)); + assert!( + stderr(&constrained).contains("unconstrained JSON value"), + "{}", + stderr(&constrained) + ); + assert!(read(&dir.output()).contains("pub type Widget = serde_json::Value;")); +} + /// Read `path`, which every case here has already generated. fn read(path: &Path) -> String { return std::fs::read_to_string(path).unwrap_or_else(|err| panic!("reading `{}` failed: {err}", path.display())); diff --git a/crates/oapi-codegen/tests/diagnostics_operations.rs b/crates/oapi-codegen/tests/diagnostics_operations.rs new file mode 100644 index 0000000..b75aba5 --- /dev/null +++ b/crates/oapi-codegen/tests/diagnostics_operations.rs @@ -0,0 +1,314 @@ +use std::collections::BTreeMap; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use std::process::Output; + +const CONFIG: &str = "package: demo\ngenerate:\n models: true\n std-http-server: true\n"; + +const SPEC: &str = "\ +openapi: 3.0.3 +info: + title: Demo + version: 1.0.0 +paths: + /items: + post: + operationId: createItem + requestBody: + required: true + content: + application/json: + schema: + type: string + responses: + '200': + description: The item + content: + application/json: + schema: + type: string +"; + +struct TestDir { + path: PathBuf, +} + +impl TestDir { + fn new(name: &str) -> Self { + let elapsed = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_else(|err| panic!("the system clock must be after the Unix epoch: {err}")); + let path = std::env::temp_dir().join(format!( + "oapi-codegen-diagnostics-operations-{name}-{}-{}", + std::process::id(), + elapsed.as_nanos() + )); + std::fs::create_dir_all(path.join("generated")) + .unwrap_or_else(|err| panic!("cannot create the test directory: {err}")); + let dir = Self { path }; + dir.write("config.yaml", CONFIG); + return dir; + } + + fn write(&self, name: &str, contents: &str) { + std::fs::write(self.path.join(name), contents) + .unwrap_or_else(|err| panic!("cannot write the test input: {err}")); + } + + fn run(&self) -> Output { + return Command::new(env!("CARGO_BIN_EXE_oapi-codegen")) + .arg("--config-file") + .arg(self.path.join("config.yaml")) + .arg("--output-file") + .arg(self.path.join("generated/output.rs")) + .arg(self.path.join("spec.yaml")) + .output() + .unwrap_or_else(|err| panic!("cannot run the generator: {err}")); + } + + fn generated(&self) -> BTreeMap { + return read_tree(&self.path.join("generated")); + } +} + +impl Drop for TestDir { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.path); + } +} + +fn read_tree(path: &Path) -> BTreeMap { + let mut files = BTreeMap::new(); + for entry in std::fs::read_dir(path).unwrap_or_else(|err| panic!("cannot read the generated directory: {err}")) { + let path = entry + .unwrap_or_else(|err| panic!("cannot read the generated entry: {err}")) + .path(); + if path.is_dir() { + files.extend(read_tree(&path)); + } else { + let contents = + std::fs::read_to_string(&path).unwrap_or_else(|err| panic!("cannot read the generated file: {err}")); + files.insert(path, contents); + } + } + return files; +} + +fn successful_stderr(output: &Output) -> String { + let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + assert!(output.status.success(), "{stderr}"); + return stderr; +} + +#[test] +fn mixed_unsupported_media_warns_without_changing_generated_bodies() { + let dir = TestDir::new("mixed-media"); + dir.write("spec.yaml", SPEC); + successful_stderr(&dir.run()); + let original = dir.generated(); + let spec = SPEC + .replace( + "\n application/json:", + "\n application/xml:\n schema:\n type: integer\n application/json:", + ) + .replace( + "\n application/json:", + "\n application/octet-stream:\n schema:\n type: integer\n application/json:", + ); + dir.write("spec.yaml", &spec); + let stderr = successful_stderr(&dir.run()); + assert!( + stderr.contains("unsupported media type `application/xml` is ignored"), + "{stderr}" + ); + assert!( + stderr.contains("unsupported media type `application/octet-stream` is ignored"), + "{stderr}" + ); + assert!(stderr.contains("post /items request body"), "{stderr}"); + assert!(stderr.contains("post /items `200` response"), "{stderr}"); + assert_eq!(dir.generated(), original); +} + +#[test] +fn duplicate_body_kinds_warn_and_keep_the_first_representation() { + let dir = TestDir::new("duplicate-media"); + let first = SPEC.replace("application/json:", "application/vnd.first+json:"); + dir.write("spec.yaml", &first); + successful_stderr(&dir.run()); + let original = dir.generated(); + let spec = first + .replace( + " type: string\n responses:", + " type: string\n application/json:\n schema:\n type: integer\n responses:", + ) + + " application/json:\n schema:\n type: integer\n"; + dir.write("spec.yaml", &spec); + let stderr = successful_stderr(&dir.run()); + let message = "media type `application/json` is ignored because `application/vnd.first+json` is the first representation of the same body kind"; + assert_eq!(stderr.matches(message).count(), 2, "{stderr}"); + assert_eq!(dir.generated(), original); +} + +#[test] +fn resolved_response_media_warns_with_the_origin_file() { + let dir = TestDir::new("referenced-media"); + dir.write( + "spec.yaml", + "openapi: 3.0.3\ninfo:\n title: Demo\n version: 1.0.0\npaths:\n /items:\n get:\n responses:\n '200':\n $ref: 'responses.yaml#/components/responses/Item'\n", + ); + let response = "openapi: 3.0.3\ninfo:\n title: Responses\n version: 1.0.0\npaths: {}\ncomponents:\n responses:\n Item:\n description: The item\n content:\n application/json:\n schema:\n type: string\n"; + dir.write("responses.yaml", response); + successful_stderr(&dir.run()); + let original = dir.generated(); + dir.write( + "responses.yaml", + &format!("{response} application/xml:\n schema:\n type: integer\n application/vnd.second+json:\n schema:\n type: integer\n"), + ); + let stderr = successful_stderr(&dir.run()); + assert!(stderr.contains("get /items `200` response"), "{stderr}"); + assert!(stderr.contains("resolved from `responses.yaml`"), "{stderr}"); + assert!( + stderr.contains("unsupported media type `application/xml` is ignored"), + "{stderr}" + ); + assert!( + stderr.contains("media type `application/vnd.second+json` is ignored"), + "{stderr}" + ); + assert_eq!(dir.generated(), original); +} + +#[test] +fn scalar_styles_warn_but_scalar_explode_settings_do_not() { + let dir = TestDir::new("scalar-styles"); + let spec = "openapi: 3.0.3\ninfo:\n title: Demo\n version: 1.0.0\npaths:\n /items/{id}:\n get:\n parameters:\n - in: path\n name: id\n required: true\n style: simple\n schema:\n type: string\n - in: query\n name: search\n style: form\n schema:\n type: string\n - in: header\n name: X-Item\n style: simple\n explode: true\n schema:\n type: string\n - in: cookie\n name: item\n style: form\n explode: false\n schema:\n type: string\n responses:\n '204':\n description: No content\n"; + dir.write("spec.yaml", spec); + let stderr = successful_stderr(&dir.run()); + assert!(!stderr.contains("unsupported scalar style"), "{stderr}"); + assert!(!stderr.contains("unsupported style"), "{stderr}"); + assert!(!stderr.contains("explode"), "{stderr}"); + let original = dir.generated(); + for style in ["label", "matrix"] { + let changed = spec.replacen("style: simple", &format!("style: {style}"), 1).replacen( + "style: form", + "style: spaceDelimited", + 1, + ); + dir.write("spec.yaml", &changed); + let stderr = successful_stderr(&dir.run()); + assert!( + stderr.contains("path parameter `id` uses unsupported style"), + "{stderr}" + ); + assert!( + stderr.contains("query parameter `search` uses unsupported scalar style"), + "{stderr}" + ); + assert_eq!(dir.generated(), original); + } +} + +#[test] +fn unsupported_only_content_still_fails_without_replacing_output() { + let dir = TestDir::new("unsupported-only"); + dir.write("spec.yaml", SPEC); + successful_stderr(&dir.run()); + let original = dir.generated(); + for indentation in [" ", " "] { + let changed = SPEC.replace( + &format!("\n{indentation}application/json:"), + &format!("\n{indentation}application/xml:"), + ); + dir.write("spec.yaml", &changed); + let output = dir.run(); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!(!output.status.success(), "{stderr}"); + assert!(stderr.contains("application/xml"), "{stderr}"); + assert!( + !stderr.contains("unsupported media type `application/xml` is ignored"), + "{stderr}" + ); + assert_eq!(dir.generated(), original); + } +} + +#[test] +fn resolved_parameter_style_warns_with_the_origin_file() { + let dir = TestDir::new("referenced-parameter"); + dir.write( + "spec.yaml", + "openapi: 3.0.3\ninfo:\n title: Demo\n version: 1.0.0\npaths:\n /items/{id}:\n get:\n parameters:\n - $ref: 'parameters.yaml#/components/parameters/Id'\n responses:\n '204':\n description: No content\n", + ); + let parameter = "openapi: 3.0.3\ninfo:\n title: Parameters\n version: 1.0.0\npaths: {}\ncomponents:\n parameters:\n Id:\n in: path\n name: id\n required: true\n style: simple\n schema:\n type: string\n"; + dir.write("parameters.yaml", parameter); + successful_stderr(&dir.run()); + let original = dir.generated(); + dir.write("parameters.yaml", ¶meter.replace("style: simple", "style: matrix")); + let stderr = successful_stderr(&dir.run()); + assert!(stderr.contains("get /items/{id}"), "{stderr}"); + assert!( + stderr.contains("path parameter `id` uses unsupported style"), + "{stderr}" + ); + assert!(stderr.contains("resolved from `parameters.yaml`"), "{stderr}"); + assert_eq!(dir.generated(), original); +} + +#[test] +fn scalar_parameter_enums_warn_without_changing_generated_types() { + for location in ["path", "query", "header", "cookie"] { + for (kind, values) in [("string", "[one, two]"), ("integer", "[1, 2]")] { + let dir = TestDir::new(&format!("enum-{location}-{kind}")); + let path = if location == "path" { "/items/{value}" } else { "/items" }; + let spec = format!( + "openapi: 3.0.3\ninfo:\n title: Demo\n version: 1.0.0\npaths:\n {path}:\n get:\n parameters:\n - in: {location}\n name: value\n required: true\n schema:\n type: {kind}\n responses:\n '204':\n description: No content\n" + ); + dir.write("spec.yaml", &spec); + let stderr = successful_stderr(&dir.run()); + assert!(!stderr.contains("the declared `enum` is ignored"), "{stderr}"); + let original = dir.generated(); + dir.write( + "spec.yaml", + &spec.replace( + &format!("type: {kind}"), + &format!("type: {kind}\n enum: {values}"), + ), + ); + let stderr = successful_stderr(&dir.run()); + assert!(stderr.contains(&format!("{location} parameter `value`")), "{stderr}"); + assert_eq!(stderr.matches("the declared `enum` is ignored").count(), 1, "{stderr}"); + assert_eq!(dir.generated(), original); + } + } +} + +#[test] +fn resolved_parameter_schema_enums_warn_for_scalars_and_array_items() { + for array in [false, true] { + let dir = TestDir::new(if array { "enum-ref-array" } else { "enum-ref-scalar" }); + dir.write( + "spec.yaml", + "openapi: 3.0.3\ninfo:\n title: Demo\n version: 1.0.0\npaths:\n /items:\n get:\n parameters:\n - $ref: 'parameters.yaml#/components/parameters/Value'\n responses:\n '204':\n description: No content\n", + ); + let schema = if array { + " type: array\n items:\n $ref: '#/components/schemas/Value'" + } else { + " $ref: '#/components/schemas/Value'" + }; + let parameter = format!( + "openapi: 3.0.3\ninfo:\n title: Parameters\n version: 1.0.0\npaths: {{}}\ncomponents:\n parameters:\n Value:\n in: query\n name: value\n schema:\n{schema}\n schemas:\n Value:\n type: string\n" + ); + dir.write("parameters.yaml", ¶meter); + let stderr = successful_stderr(&dir.run()); + assert!(!stderr.contains("the declared `enum` is ignored"), "{stderr}"); + let original = dir.generated(); + dir.write("parameters.yaml", &format!("{parameter} enum: [one, two]\n")); + let stderr = successful_stderr(&dir.run()); + assert!(stderr.contains("get /items query parameter `value`"), "{stderr}"); + assert!(stderr.contains("resolved from `parameters.yaml`"), "{stderr}"); + assert_eq!(stderr.matches("the declared `enum` is ignored").count(), 1, "{stderr}"); + assert_eq!(dir.generated(), original); + } +} diff --git a/docs/configuration.md b/docs/configuration.md index 061a6bb..235135b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -23,9 +23,12 @@ For a full, auto-generated reference of every flag and argument, see ## Config file -Keys mirror [`oapi-codegen`](https://github.com/oapi-codegen/oapi-codegen)'s -YAML config; unknown keys are ignored, so an existing Go config can be reused -as-is. Only the subset below is interpreted. +Keys mirror the YAML configuration of +[`oapi-codegen`](https://github.com/oapi-codegen/oapi-codegen). +Unknown keys produce warnings but do not stop generation. +This also applies inside `generate` and `output-options`. +An invalid value for a recognized key remains an error. +The names inside `import-mapping` are file names, not configuration keys. | Key | Type | Purpose | | ---------------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------ | diff --git a/docs/design.md b/docs/design.md index abb86bc..2478a9d 100644 --- a/docs/design.md +++ b/docs/design.md @@ -275,6 +275,50 @@ A document must also declare no `webhooks:` key. That key carries operations, and the generator emits no handler for them. Silence about the key reads as "the document declares no such operation", so the generator rejects the key. +## Diagnose unhandled spec content + +The generator inspects OpenAPI object keys before typed deserialization. +The inspection covers the root document and each referenced document that the +loader reads. It runs before filtering, so an excluded operation cannot hide an +invalid key. + +The catalogue in `crates/oapi-codegen/src/coverage.rs` distinguishes handled keys, annotations, and +unsupported features. An unknown key is an error with the document name and a +JSON pointer. An invalid object, map, or array structure is also an error. +The generator reports independent key errors together, before it writes +output. + +A valid but unimplemented feature produces a warning. Examples include callbacks, +XML serialization, response links, server overrides, and body encoding options. +Existing errors remain errors. For example, an unsupported body type or a `not` +schema still stops generation. + +Annotations that have no generated representation remain intentionally omitted. +These include document information, schema titles, external documentation, and +examples. Compatibility extensions with an `x-go-` prefix are also intentionally +ignored. Other unhandled extensions produce warnings. + +Property names, schema names, security scheme names, and media types are data, +not fixed OpenAPI keys. Payloads in `example`, `default`, and example `value` +fields are also data. The inspection does not interpret their contents as +OpenAPI objects. However, it still inspects objects inside unsupported features, +such as callback operations. + +Warnings also identify several limits of the current translation. These include +first-match unions, merged `allOf` members, nullability, and unconstrained fallback +types. Body selection reports discarded media entries. +The warnings expose these limits without changing the generated types. +The catalogue is not a complete OpenAPI value validator. Lowering still applies +its own value and combination checks. + +Warnings go to stderr for library calls and CLI commands, including `--check`. +A warning alone does not change the exit code. Drift and generation errors still +make `--check` fail. + +Configuration diagnostics use the serialized shape of `Config::default()`. +This keeps recognized configuration keys in the Rust types rather than in a +second catalogue. Dynamic `import-mapping` entries do not enter this comparison. + ## A body must declare a content type the generator can represent A request body must declare one of `application/json`, From 8ca7e2ec07a3b270cc4978857aecc99285a0525d Mon Sep 17 00:00:00 2001 From: Kaspar Lyngsie Date: Sun, 6 Sep 2026 20:19:10 +0200 Subject: [PATCH 2/2] fix: decouple e2e toolchain from Docker image releases --- crates/oapi-codegen/tests/integration/client/Dockerfile | 7 ++++--- crates/oapi-codegen/tests/integration/server/Dockerfile | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/oapi-codegen/tests/integration/client/Dockerfile b/crates/oapi-codegen/tests/integration/client/Dockerfile index f3c33b6..ab10a16 100644 --- a/crates/oapi-codegen/tests/integration/client/Dockerfile +++ b/crates/oapi-codegen/tests/integration/client/Dockerfile @@ -4,10 +4,11 @@ # start; pre-building with `--no-run` means startup goes straight to the tests. # The build context is the repository root (see docker-compose.yml). -# Kept in sync with rust-toolchain.toml by the Makefile: a mismatch makes -# rustup download a second toolchain inside the image on every build. +# Rust toolchains can be available before matching Docker image tags. +FROM rust:bookworm AS builder ARG RUST_VERSION=1.97.1 -FROM rust:${RUST_VERSION}-bookworm AS builder +RUN rustup toolchain install "${RUST_VERSION}" --profile minimal --no-self-update \ + && rustup default "${RUST_VERSION}" WORKDIR /src COPY . . RUN cargo test --no-run --locked -p bookstore-example --features client --test e2e diff --git a/crates/oapi-codegen/tests/integration/server/Dockerfile b/crates/oapi-codegen/tests/integration/server/Dockerfile index 69fa306..608d7b3 100644 --- a/crates/oapi-codegen/tests/integration/server/Dockerfile +++ b/crates/oapi-codegen/tests/integration/server/Dockerfile @@ -1,10 +1,11 @@ # Build the bookstore server binary from the workspace, then run it from a slim # runtime image. The build context is the repository root (see docker-compose.yml). -# Kept in sync with rust-toolchain.toml by the Makefile: a mismatch makes -# rustup download a second toolchain inside the image on every build. +# Rust toolchains can be available before matching Docker image tags. +FROM rust:bookworm AS builder ARG RUST_VERSION=1.97.1 -FROM rust:${RUST_VERSION}-bookworm AS builder +RUN rustup toolchain install "${RUST_VERSION}" --profile minimal --no-self-update \ + && rustup default "${RUST_VERSION}" WORKDIR /src COPY . . RUN cargo build --locked -p bookstore-example --bin bookstore-server