Skip to content

Commit 5041de5

Browse files
lightsofapollolightsofapollo
committed
fix(generator): make 20 real-world specs compile, gate CI on the working set
Running scripts/spec-compile.sh against all 54 OpenAPI 3.x specs in the repo (gitea is Swagger 2.0, skipped) surfaced six classes of generator bugs. Fixed the ones that move the most specs from FAIL → PASS: 1. `r#self` panic `self`, `super`, `crate`, `Self` cannot be raw identifiers in Rust — proc_macro2 panics outright. Spec fields named `self` (datadog-v2, github, microsoft-graph, snyk, …) hit this. Fix: rename to `<keyword>_field` / `<keyword>_param` instead of `r#<keyword>`. 2. operationId collisions reject whole documents T6's strict-error policy was correct per spec but real-world docs (arcade, cal-com, telnyx, val-town, …) often violate it. Fix: auto-disambiguate by suffixing with HTTP method (`opId_post`, `opId_put`), and a counter on further collisions, with a stderr warning. Spec validity is recoverable; whole-document rejection is not. 3. Extensions reject non-`x-*` keys Real specs sprinkle non-`x-` fields in places they don't belong (`produces`/`in`/`type`/`density`/`title`/`description` were observed). Fix: Extensions now accepts any leftover key but exposes `non_extension_keys()` so silent drops remain visible — the CLI can warn instead of erroring. 4. exclusiveMinimum: bool vs number 3.0/Swagger used `bool`; 3.1 (JSON Schema 2020-12) uses `number`. Fix: model as a `bool | f64` enum. 5. `Vec<serde_json::Value>` Ident panic generate_array_item_type split on "::" but produced strings with angle brackets that aren't valid idents. Fix: parse via `syn::parse_str::<syn::Type>` first. 6. enum variant collisions on signed numbers `1` and `-1` both produced `Variant1`. Fix: prefix negatives with `Neg` (e.g. `VariantNeg1`). 7. Twilio-style filter param ident collisions `StartTime`, `StartTime<`, `StartTime>` all snake-cased to `start_time`. Fix: map `<`, `>`, `<=`, `>=` to `_lt`/`_gt`/`_lte`/ `_gte` in sanitize_param_name. Twilio went from CHECK-FAIL to PASS. 8. Version gate didn't run in TOML config flow The `generate` subcommand in src/bin/openapi-to-rust.rs has its own pipeline that bypasses cli::run_generation_cli. Mirrored the version check so Swagger 2.0 specs (gitea) error early with a clear hint instead of failing later inside the deserializer. scripts/spec-compile.sh - Auto-discovers specs/*.{yaml,json}. - Skips Swagger 2.0 with a SKIP marker (gitea). - Optional SPEC_COMPILE_PARSE_ONLY=1 for quick generator-only checks. - Optional SPEC_COMPILE_LIMIT=N / positional whitelist of names. ci(spec-compile) The job now compiles a "gold list" of 20 specs that pass cleanly: anthropic, asana, browserbase, cartesia, cerebras, coda, coingecko, digitalocean, groq, imagekit, launchdarkly, meta-llama, openai, resend, runway, spotify, terminal-shop, twilio, val-town, writer. Local `scripts/spec-compile.sh` (no args) still runs the full corpus. The remaining 34 specs surface other generator bugs (E0308 type mismatches, E0428 name collisions in github, E0117 orphan rule violations in stripe, E0072 recursive type sizing in snyk) — tracked in #14 as follow-ups. All 205 unit tests still pass; clippy + fmt clean. Refs #14
1 parent 22b29d9 commit 5041de5

9 files changed

Lines changed: 261 additions & 89 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,14 +47,23 @@ jobs:
4747
env:
4848
RUSTDOCFLAGS: -D warnings
4949

50-
# Regression guard: generate clients for our reference specs (Anthropic +
51-
# OpenAI) and `cargo check` the result. Catches breakage where a generator
52-
# change still passes unit tests but emits invalid Rust against real-world
53-
# OAS documents. See scripts/spec-compile.sh.
50+
# Regression guard: generate clients for a curated list of real-world specs
51+
# and `cargo check` the result. Catches breakage where a generator change
52+
# still passes unit tests but emits invalid Rust against real-world OAS
53+
# documents. See scripts/spec-compile.sh.
54+
#
55+
# The list is the "gold" subset that currently compiles cleanly. Local
56+
# `scripts/spec-compile.sh` (no args) runs against all of `specs/`; we
57+
# don't gate CI on the full corpus because many of the 50+ specs currently
58+
# surface unfixed generator bugs (tracked in #14).
5459
spec-compile:
5560
runs-on: ubuntu-latest
5661
steps:
5762
- uses: actions/checkout@v4
5863
- uses: dtolnay/rust-toolchain@stable
5964
- uses: Swatinem/rust-cache@v2
60-
- run: scripts/spec-compile.sh
65+
- run: |
66+
scripts/spec-compile.sh \
67+
anthropic asana browserbase cartesia cerebras coda coingecko \
68+
digitalocean groq imagekit launchdarkly meta-llama openai \
69+
resend runway spotify terminal-shop twilio val-town writer

‎scripts/spec-compile.sh‎

Lines changed: 98 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,24 @@
11
#!/usr/bin/env bash
2-
# Smoke-test that generated clients for our reference specs compile cleanly.
3-
# Each spec listed below produces a separate scratch crate; we run the
4-
# `openapi-to-rust` generator into it and then `cargo check`. Any
5-
# regression here means a real-world spec stops compiling.
2+
# Smoke-test that generated clients for every spec under specs/ compile cleanly.
3+
#
4+
# Auto-discovers specs/*.yaml and specs/*.json. Each spec produces a separate
5+
# scratch crate; we run the `openapi-to-rust` generator into it and then
6+
# `cargo check`. Any regression here means a real-world spec stops compiling.
67
#
78
# Usage:
8-
# scripts/spec-compile.sh # run all specs in SPECS
9-
# scripts/spec-compile.sh anthropic openai # run a subset
9+
# scripts/spec-compile.sh # all specs in specs/
10+
# scripts/spec-compile.sh anthropic openai # subset by name
11+
# SPEC_COMPILE_LIMIT=5 scripts/spec-compile.sh # first 5 only (CI smoke)
1012
#
1113
# Env:
12-
# SPEC_COMPILE_KEEP=1 keep the scratch directory under tmp/spec-compile/
13-
# SPEC_COMPILE_OFFLINE=1 pass --offline to cargo invocations
14+
# SPEC_COMPILE_KEEP=1 keep tmp/spec-compile/<name>/ on success
15+
# SPEC_COMPILE_OFFLINE=1 pass --offline to cargo invocations
16+
# SPEC_COMPILE_LIMIT=N process only the first N alphabetically-sorted specs
17+
# SPEC_COMPILE_PARSE_ONLY=1 skip cargo check; only verify the generator
18+
# parses+emits without errors. Faster.
1419
set -euo pipefail
1520
cd "$(dirname "$0")/.."
1621

17-
# (spec_name, spec_path, base_url, auth_type, auth_header)
18-
SPECS=(
19-
"anthropic|specs/anthropic.yaml|https://api.anthropic.com|ApiKey|x-api-key"
20-
"openai|specs/openai.yaml|https://api.openai.com/v1|Bearer|Authorization"
21-
)
22-
23-
# If args are given, treat them as a whitelist of spec names.
24-
WANT=("$@")
25-
2622
OFFLINE=""
2723
if [ "${SPEC_COMPILE_OFFLINE:-}" = "1" ]; then
2824
OFFLINE="--offline"
@@ -32,22 +28,59 @@ echo "[spec-compile] building openapi-to-rust binary..."
3228
cargo build --bin openapi-to-rust $OFFLINE >/dev/null
3329

3430
GEN_BIN="$(pwd)/target/debug/openapi-to-rust"
31+
WORKSPACE="$(pwd)"
3532

36-
ROOT="$(pwd)/tmp/spec-compile"
33+
ROOT="$WORKSPACE/tmp/spec-compile"
3734
rm -rf "$ROOT"
3835
mkdir -p "$ROOT"
3936

40-
failed=()
41-
for entry in "${SPECS[@]}"; do
42-
IFS='|' read -r name spec_path base_url auth_type auth_header <<<"$entry"
37+
# Discover specs. Sort for deterministic output.
38+
mapfile -t ALL_SPECS < <(find specs -maxdepth 1 -type f \( -name "*.yaml" -o -name "*.json" \) | sort)
39+
40+
# Filter by command-line whitelist.
41+
WANT=("$@")
42+
SPECS=()
43+
for spec in "${ALL_SPECS[@]}"; do
44+
name="$(basename "$spec")"
45+
name="${name%.*}"
4346
if [ ${#WANT[@]} -gt 0 ]; then
44-
skip=1
45-
for w in "${WANT[@]}"; do [ "$w" = "$name" ] && skip=0; done
46-
[ $skip -eq 1 ] && continue
47+
keep=0
48+
for w in "${WANT[@]}"; do [ "$w" = "$name" ] && keep=1; done
49+
[ $keep -eq 0 ] && continue
50+
fi
51+
SPECS+=("$name|$spec")
52+
done
53+
54+
if [ -n "${SPEC_COMPILE_LIMIT:-}" ]; then
55+
SPECS=("${SPECS[@]:0:$SPEC_COMPILE_LIMIT}")
56+
fi
57+
58+
if [ ${#SPECS[@]} -eq 0 ]; then
59+
echo "[spec-compile] no specs matched"
60+
exit 0
61+
fi
62+
63+
echo "[spec-compile] running ${#SPECS[@]} spec(s)"
64+
echo
65+
66+
passed=()
67+
failed_gen=()
68+
failed_check=()
69+
skipped=()
70+
for entry in "${SPECS[@]}"; do
71+
IFS='|' read -r name spec_path <<<"$entry"
72+
73+
printf "%-30s " "$name"
74+
75+
# Skip Swagger 2.0 specs — out of scope for this generator. Detect either
76+
# `"swagger": "2.0"` (JSON) or `swagger: "2.0"` / `swagger: 2.0` (YAML).
77+
if grep -qE '("swagger"\s*:|swagger\s*:)\s*"?2\.' "$spec_path" 2>/dev/null \
78+
&& ! grep -qE '("openapi"\s*:|openapi\s*:)' "$spec_path" 2>/dev/null; then
79+
echo "SKIP (Swagger 2.0)"
80+
skipped+=("$name")
81+
continue
4782
fi
4883

49-
echo
50-
echo "==> $name (spec: $spec_path)"
5184
dir="$ROOT/$name"
5285
mkdir -p "$dir/src/generated"
5386

@@ -75,43 +108,59 @@ EOF
75108
pub mod generated;
76109
EOF
77110

111+
# Sanitize module name (replace - with _).
112+
module_name="$(echo "$name" | tr '-' '_')"
113+
78114
cat >"$dir/openapi-to-rust.toml" <<EOF
79115
[generator]
80-
spec_path = "$(pwd)/$spec_path"
116+
spec_path = "$WORKSPACE/$spec_path"
81117
output_dir = "src/generated"
82-
module_name = "$name"
118+
module_name = "$module_name"
83119
84120
[features]
85121
enable_async_client = true
86122
87123
[http_client]
88-
base_url = "$base_url"
124+
base_url = "https://example.invalid"
89125
timeout_seconds = 60
90-
91-
[http_client.auth]
92-
type = "$auth_type"
93-
header_name = "$auth_header"
94126
EOF
95127

96-
(
97-
cd "$dir"
98-
"$GEN_BIN" generate --config openapi-to-rust.toml >/dev/null
99-
if ! cargo check $OFFLINE 2>&1 | tail -200; then
100-
echo "[spec-compile] $name FAILED to compile" >&2
101-
exit 1
102-
fi
103-
) || failed+=("$name")
128+
# Generator step
129+
log="$dir/generate.log"
130+
if ! ( cd "$dir" && "$GEN_BIN" generate --config openapi-to-rust.toml ) >"$log" 2>&1; then
131+
echo "GEN-FAIL"
132+
failed_gen+=("$name")
133+
continue
134+
fi
135+
136+
if [ "${SPEC_COMPILE_PARSE_ONLY:-}" = "1" ]; then
137+
echo "GEN-OK"
138+
passed+=("$name")
139+
[ "${SPEC_COMPILE_KEEP:-}" != "1" ] && rm -rf "$dir"
140+
continue
141+
fi
142+
143+
# Cargo check step
144+
log="$dir/check.log"
145+
if ! ( cd "$dir" && cargo check $OFFLINE ) >"$log" 2>&1; then
146+
err_count=$(grep -cE "^error" "$log" || true)
147+
echo "CHECK-FAIL ($err_count errs)"
148+
failed_check+=("$name")
149+
continue
150+
fi
151+
152+
echo "PASS"
153+
passed+=("$name")
154+
[ "${SPEC_COMPILE_KEEP:-}" != "1" ] && rm -rf "$dir"
104155
done
105156

106-
if [ "${SPEC_COMPILE_KEEP:-}" != "1" ]; then
107-
rm -rf "$ROOT"
108-
fi
157+
echo
158+
echo "[spec-compile] summary: ${#passed[@]} passed, ${#failed_gen[@]} gen-failed, ${#failed_check[@]} check-failed, ${#skipped[@]} skipped"
159+
[ ${#failed_gen[@]} -gt 0 ] && echo " gen-fail: ${failed_gen[*]}"
160+
[ ${#failed_check[@]} -gt 0 ] && echo " check-fail: ${failed_check[*]}"
161+
[ ${#skipped[@]} -gt 0 ] && echo " skipped: ${skipped[*]}"
109162

110-
if [ ${#failed[@]} -gt 0 ]; then
111-
echo
112-
echo "[spec-compile] FAILED: ${failed[*]}" >&2
163+
if [ ${#failed_gen[@]} -gt 0 ] || [ ${#failed_check[@]} -gt 0 ]; then
113164
exit 1
114165
fi
115-
116-
echo
117166
echo "[spec-compile] ✅ all specs compiled cleanly"

‎src/analysis.rs‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3566,12 +3566,33 @@ impl SchemaAnalyzer {
35663566
analysis: &mut SchemaAnalysis,
35673567
) -> Result<()> {
35683568
for (method, operation) in path_item.operations() {
3569-
// Generate operation ID if missing
3570-
let operation_id = operation
3569+
// Generate operation ID if missing.
3570+
let raw_operation_id = operation
35713571
.operation_id
35723572
.clone()
35733573
.unwrap_or_else(|| Self::generate_operation_id(method, path));
35743574

3575+
// T6: detect operationId collisions. Per the OAS spec these MUST
3576+
// be unique, but real-world specs (arcade, cal-com, telnyx,
3577+
// val-town, …) frequently aren't. Auto-disambiguate by suffixing
3578+
// with the method, then a counter, and warn.
3579+
let operation_id = if analysis.operations.contains_key(&raw_operation_id) {
3580+
let method_lower = method.to_lowercase();
3581+
let mut candidate = format!("{}_{}", raw_operation_id, method_lower);
3582+
let mut suffix = 2;
3583+
while analysis.operations.contains_key(&candidate) {
3584+
candidate = format!("{}_{}_{}", raw_operation_id, method_lower, suffix);
3585+
suffix += 1;
3586+
}
3587+
eprintln!(
3588+
"⚠️ duplicate operationId `{}` at `{} {}` — disambiguated to `{}`",
3589+
raw_operation_id, method, path, candidate
3590+
);
3591+
candidate
3592+
} else {
3593+
raw_operation_id
3594+
};
3595+
35753596
let op_info = self.analyze_single_operation(
35763597
&operation_id,
35773598
method,
@@ -3580,14 +3601,6 @@ impl SchemaAnalyzer {
35803601
path_item.parameters.as_ref(),
35813602
analysis,
35823603
)?;
3583-
// T6: detect operationId collisions instead of silently overwriting.
3584-
if let Some(existing) = analysis.operations.get(&operation_id) {
3585-
return Err(GeneratorError::InvalidSchema(format!(
3586-
"duplicate operationId `{}` — first at `{} {}`, then at `{} {}`. \
3587-
OpenAPI requires operationId to be unique across the document.",
3588-
operation_id, existing.method, existing.path, method, path
3589-
)));
3590-
}
35913604
analysis.operations.insert(operation_id, op_info);
35923605
}
35933606
Ok(())

‎src/bin/openapi-to-rust.rs‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,37 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
7979
json_from_str_lossy(&spec_content)?
8080
};
8181

82+
// Version gate: surface unsupported OAS major.minor early.
83+
let oas_version = spec_value
84+
.get("openapi")
85+
.and_then(|v| v.as_str())
86+
.unwrap_or("");
87+
match openapi_to_rust::cli::parse_oas_version(oas_version) {
88+
Some((3, 0)) | Some((3, 1)) => {}
89+
Some((3, 2)) => {
90+
eprintln!("⚠️ OpenAPI {oas_version}: 3.2 is experimentally supported.");
91+
}
92+
Some((major, minor)) => {
93+
eprintln!(
94+
"❌ Unsupported OpenAPI version: {major}.{minor} ({oas_version:?}). \
95+
This generator targets 3.0.x, 3.1.x, and (experimentally) 3.2.x. \
96+
Swagger 2.0 and OAS 1.x are not supported."
97+
);
98+
std::process::exit(1);
99+
}
100+
None => {
101+
let hint = if spec_value.get("swagger").is_some() {
102+
" (looks like Swagger 2.0 — out of scope)"
103+
} else {
104+
""
105+
};
106+
eprintln!(
107+
"❌ Missing or unrecognised `openapi` field{hint}. Expected something like \"3.1.0\", got: {oas_version:?}"
108+
);
109+
std::process::exit(1);
110+
}
111+
}
112+
82113
// Analyze schemas (with extensions if configured)
83114
println!("🔍 Analyzing schemas...");
84115
let mut analyzer = if generator_config.schema_extensions.is_empty() {

‎src/cli.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ async fn load_spec(input: &str, verbose: bool) -> Result<String, Box<dyn std::er
274274

275275
/// Parse the `openapi` version string into (major, minor). Tolerates patch and
276276
/// build-metadata suffixes. Returns None for unrecognised input.
277-
fn parse_oas_version(s: &str) -> Option<(u32, u32)> {
277+
pub fn parse_oas_version(s: &str) -> Option<(u32, u32)> {
278278
let mut parts = s.split('.');
279279
let major = parts.next()?.parse().ok()?;
280280
let minor_raw = parts.next()?;

‎src/client_generator.rs‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,9 +1312,33 @@ impl CodeGenerator {
13121312
}
13131313
}
13141314

1315-
/// Sanitize a parameter name by escaping Rust reserved keywords with raw identifiers
1315+
/// Sanitize a parameter name by escaping Rust reserved keywords with raw
1316+
/// identifiers and disambiguating Twilio-style suffix operators
1317+
/// (`StartTime`, `StartTime<`, `StartTime>` would otherwise all snake-
1318+
/// case to `start_time`).
13161319
fn sanitize_param_name(&self, name: &str) -> String {
1317-
let snake_case = name.to_snake_case();
1320+
// Disambiguate before stripping. `<`, `>`, `<=`, `>=` are common in
1321+
// filter-style query params; map them to `_lt` / `_gt` etc. so the
1322+
// Rust ident is unique while the wire-level param name stays the
1323+
// original string elsewhere in the codegen.
1324+
let suffix = if name.ends_with("<=") {
1325+
"_lte"
1326+
} else if name.ends_with(">=") {
1327+
"_gte"
1328+
} else if name.ends_with('<') {
1329+
"_lt"
1330+
} else if name.ends_with('>') {
1331+
"_gt"
1332+
} else {
1333+
""
1334+
};
1335+
let stripped = name.trim_end_matches(['<', '>', '=']);
1336+
let mut snake_case = stripped.to_snake_case();
1337+
snake_case.push_str(suffix);
1338+
1339+
if matches!(snake_case.as_str(), "self" | "super" | "crate" | "Self") {
1340+
return format!("{snake_case}_param");
1341+
}
13181342
if Self::is_rust_keyword(&snake_case) {
13191343
format!("r#{snake_case}")
13201344
} else {

0 commit comments

Comments
 (0)