From aaa6007d271cc502c7969be08955426c25adc334 Mon Sep 17 00:00:00 2001 From: C J Silverio Date: Tue, 2 Apr 2024 18:18:35 -0700 Subject: [PATCH 1/2] fix(keywords): extend the list of restricted names, with a test I observed that the list of restricted words didn't cover some attractive words when I attempted to generate a Rust client for a schema that used keywords like `async` as field names. - Extended the list of names in `is_restricted()`. - Added a short yaml schema that uses all of the restricted words as field names. Put this fixture to work in a test, to ensure no regressions. The list is undoubtedly incomplete, but there is a very clear place to add new ones (thank you!) and now a test to validate it. --- libninja/tests/basic/main.rs | 21 ++++++++++++++ libninja/tests/spec/keywords.yaml | 48 +++++++++++++++++++++++++++++++ mir_rust/src/lib.rs | 18 +++++++----- 3 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 libninja/tests/spec/keywords.yaml diff --git a/libninja/tests/basic/main.rs b/libninja/tests/basic/main.rs index 102e8c6..37d3299 100644 --- a/libninja/tests/basic/main.rs +++ b/libninja/tests/basic/main.rs @@ -13,6 +13,7 @@ use ln_core::{OutputConfig, PackageConfig}; const BASIC: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/spec/basic.yaml"); const RECURLY: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/spec/recurly.yaml"); +const KEYWORDS: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/tests/spec/keywords.yaml"); const EXAMPLE: &str = include_str!("link_create_token.rs"); @@ -74,3 +75,23 @@ pub fn test_build_full_library_recurly() -> Result<()> { }; generate_library(spec, opts) } + +#[test] +pub fn test_keywords_schema() -> Result<()> { + let yaml = File::open(KEYWORDS).unwrap(); + let temp = tempfile::tempdir()?; + + let spec: OpenAPI = serde_yaml::from_reader(yaml).unwrap(); + let opts = OutputConfig { + dest_path: temp.path().to_path_buf(), + build_examples: false, + package_name: "keywords".to_string(), + service_name: "Keywords".to_string(), + language: Language::Rust, + config: Default::default(), + github_repo: Some("libninjacom/test".to_string()), + version: None, + derive: vec![], + }; + generate_library(spec, opts) +} diff --git a/libninja/tests/spec/keywords.yaml b/libninja/tests/spec/keywords.yaml new file mode 100644 index 0000000..b55ef6a --- /dev/null +++ b/libninja/tests/spec/keywords.yaml @@ -0,0 +1,48 @@ +openapi: 3.0.1 +servers: + - url: "{scheme}://example.com/keywords" + variables: + scheme: + description: "This OpenAPI schema names fields after Rust keywords." + enum: + - "https" + - "http" + default: "https" +info: + description: >- + This api uses Rust keywords in dangerous places. + version: 1.0.0 + title: Rust keyword test +paths: + /: + get: + operationId: fetch-keywords + summary: Fetch keywords + responses: + "200": + description: Returns a struct with fields named after rust keywords. + content: + application/json: + schema: + $ref: "#/components/schemas/dangerNoodle" +components: + schemas: + dangerNoodle: + type: object + properties: + async: + type: boolean + enum: + type: string + final: + type: boolean + match: + type: integer + mut: + type: boolean + self: + type: string + type: + type: boolean + use: + type: boolean diff --git a/mir_rust/src/lib.rs b/mir_rust/src/lib.rs index 3c01c9a..c98e25d 100644 --- a/mir_rust/src/lib.rs +++ b/mir_rust/src/lib.rs @@ -5,11 +5,11 @@ use regex::{Captures, Regex}; use mir::{Doc, Ident, Literal, ParamKey, Visibility}; -mod file; mod class; -mod import; -mod function; mod r#enum; +mod file; +mod function; +mod import; /// Use this for codegen structs: Function, Class, etc. pub trait ToRustCode { @@ -40,7 +40,6 @@ impl ToRustCode for Visibility { } } - impl ToRustCode for Option { fn to_rust_code(self) -> TokenStream { match self { @@ -72,7 +71,6 @@ impl ToRustCode for ParamKey { } } - pub trait ToRustIdent { fn to_rust_struct(&self) -> Ident; fn to_rust_ident(&self) -> Ident; @@ -155,7 +153,10 @@ fn sanitize_struct(s: impl AsRef) -> Ident { } pub fn is_restricted(s: &str) -> bool { - ["type", "use", "ref", "self", "match", "final"].contains(&s) + [ + "async", "enum", "final", "match", "mut", "ref", "self", "type", "use", + ] + .contains(&s) } fn assert_valid_ident(s: &str, original: &str) { @@ -177,7 +178,10 @@ mod tests { #[test] fn test_filename() { let s = "SdAddress.contractor1099"; - assert_eq!(String::from(s).to_rust_ident().0, "sd_address_contractor1099"); + assert_eq!( + String::from(s).to_rust_ident().0, + "sd_address_contractor1099" + ); assert_eq!(sanitize_filename(s), "sd_address_contractor1099"); } } From a4c48264043a0a6126f7836a0a39eacfb4c7fa6b Mon Sep 17 00:00:00 2001 From: C J Silverio Date: Wed, 3 Apr 2024 18:42:33 -0700 Subject: [PATCH 2/2] Adopt @pscott31's approach in pr #15. This is the right way to do it. --- Cargo.lock | 7 +++++++ mir_rust/Cargo.toml | 1 + mir_rust/src/lib.rs | 12 +++--------- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4c1a6b2..41ca4ac 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -186,6 +186,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "check_keyword" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e4d532198271d4fb82f944bd25527e760a6cfc966c7d592525e092d4c6a4f787" + [[package]] name = "chrono" version = "0.4.31" @@ -731,6 +737,7 @@ dependencies = [ name = "libninja_mir_rust" version = "0.1.0" dependencies = [ + "check_keyword", "convert_case", "libninja_mir", "prettyplease", diff --git a/mir_rust/Cargo.toml b/mir_rust/Cargo.toml index 23cb030..98a7411 100644 --- a/mir_rust/Cargo.toml +++ b/mir_rust/Cargo.toml @@ -17,3 +17,4 @@ syn = "2.0.48" convert_case = "0.6.0" regex = "1.10.3" prettyplease = "0.2.16" +check_keyword = "0.2.0" diff --git a/mir_rust/src/lib.rs b/mir_rust/src/lib.rs index c98e25d..bf49e27 100644 --- a/mir_rust/src/lib.rs +++ b/mir_rust/src/lib.rs @@ -1,3 +1,4 @@ +use check_keyword::CheckKeyword; use convert_case::{Case, Casing}; use proc_macro2::TokenStream; use quote::quote; @@ -130,7 +131,7 @@ fn sanitize(s: impl AsRef) -> String { c }) .into(); - if is_restricted(&s) { + if s.is_keyword() { s += "_" } if s.chars().next().unwrap().is_numeric() { @@ -145,20 +146,13 @@ fn sanitize_struct(s: impl AsRef) -> Ident { let original = s; let s = rewrite_names(s); let mut s = s.to_case(Case::Pascal); - if is_restricted(&s) { + if s.is_keyword() { s += "Struct" } assert_valid_ident(&s, &original); Ident(s) } -pub fn is_restricted(s: &str) -> bool { - [ - "async", "enum", "final", "match", "mut", "ref", "self", "type", "use", - ] - .contains(&s) -} - fn assert_valid_ident(s: &str, original: &str) { if s.chars().next().map(|c| c.is_numeric()).unwrap_or_default() { panic!("Numeric identifier: {}", original)