fix(cli): create output directories for multi-file --out - #2974
Open
schani wants to merge 6 commits into
Open
Conversation
Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
The previous multi-file rewrite always treated `--out` as a directory and joined each generated filename under it. For languages that pass a representative file path (e.g. Java's `src/main/java/io/quicktype/TopLevel.java` or cjson's `TopLevel.h`) while emitting several files, this nested every file inside a bogus directory named after the file, breaking the java and cjson fixtures (`EEXIST mkdir` / missing `.h`/`.c`). `--out` is now resolved to a directory only when it actually names one — an existing directory, a trailing-separator path, or an extension-less path. Otherwise it is treated as a representative file path and the files are written into its parent directory, matching the long-standing convention. The directory-target behavior that fixes #2491 (writing into an existing/nested `--out` directory instead of scattering into the parent) is preserved. Co-Authored-By: Claude <noreply@anthropic.com>
writeOutput's single-file branch wrote content straight to the literal --out path, ignoring the filename the renderer actually produced. For targets like Java, where a top-level primitive schema emits only a Converter.java file (no TopLevel class), this clobbered the --out path with the wrong content and broke compilation. Restore joining --out's directory with the renderer's filename for every file, matching the pre-existing behavior, while keeping the new directory-creation and multi-file directory resolution from this PR. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
test/unit/cjson-enum-default.test.ts still expected cJSON enum members in alphabetical order, but quicktype now preserves JSON Schema enum declaration order everywhere (see test/unit/enum-order.test.ts). The test was stale against current master (fails there independent of this PR) and blocked this branch's merge-into-master CI after picking up master's latest history. Update the expected block to match declaration order; the "values start at 1" behavior under test is unaffected. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
No generated-output differences✅ Generated outputs are unchanged between the PR base and head revisions. |
No generated-output differences✅ Generated outputs are unchanged between the PR base and head revisions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
--outwith multi-file output (e.g. generating Java) silently scattered generated files into the parent of the given output directory instead of writing them inside it, and using a nested, not-yet-existing output directory crashed withENOENT.Repro:
./dist(an existing directory) ended up empty;Address.java,Person.java,Converter.javawere written into the current working directory instead.Error: ENOENT: no such file or directory, open 'dist/newsub/Converter.java'because the directory didn't exist yet.Root cause
writeOutput()insrc/index.tsalways computed the output path aspath.join(path.dirname(cliOptions.out), filename), treating--outpurely as a file path even when quicktype produced multiple files (e.g. one file per class for Java). It also never created missing directories before writing.The fix
In
writeOutput():--outis now treated as a directory: each file is written topath.join(cliOptions.out, filename).--outkeeps its existing meaning as an exact file path (so--out ./dist/index.tsfor single-file TypeScript output is unaffected).fs.mkdirSync(..., { recursive: true })before writing, so missing nested directories (e.g.--out ./dist/newsub/library) are created automatically instead of crashing.This matches the scope of the maintainer triage note on #2491: only the two confirmed CLI defects (scattering into the parent dir, and missing directories not being created) are fixed here. Unsupported/by-design behavior called out in the same triage (Java
--packagedirectory structure, TypeScript multi-file output) is intentionally left untouched.Test coverage
Added
test/unit/output-directory.test.ts, which driveswriteOutput()directly against real temp directories (viafs.mkdtempSync) and covers:--outpath, with its parent directory created if missing.This is a unit test rather than an end-to-end JSON/JSON-Schema fixture because the bug is about where
writeOutput()places files on disk (a CLI filesystem side effect), not about the content of generated code, which the fixture harness verifies via round-tripped JSON. This matches the repo's documented guidance thattest/unit/is the right place for what the fixture harness can't express.Confirmed the added tests fail on the pre-fix code (
ENOENT/scattered files) and pass after the fix.Verification
npm run buildpasses.npm run test:unit— all 166 unit tests pass (including the 3 new ones).--out ./dist(existing dir) now writes all Java files insidedist/, not the cwd.--out ./dist/newsub/library(missing nested dir) now creates the directory and writes the files, instead of crashing.--out ./dist/index.ts(single-file TypeScript) is unaffected — still writes exactly to that path.FIXTURE=java) locally — this environment has a JRE but nojavac/JDK, so Java driver-program compilation isn't possible here. CI will validate the Java (and other multi-file-capable language) fixtures.Fixes #2491.
🤖 Generated with Claude Code