Skip to content

Report malformed schema JSON as a compile error instead of panicking#1030

Open
danieleades wants to merge 1 commit into
oxidecomputer:mainfrom
danieleades:fix/macro-json-error
Open

Report malformed schema JSON as a compile error instead of panicking#1030
danieleades wants to merge 1 commit into
oxidecomputer:mainfrom
danieleades:fix/macro-json-error

Conversation

@danieleades

Copy link
Copy Markdown
Contributor

Problem

import_types! unwraps the result of serde_json::from_reader, so a JSON syntax error in the schema file (stray comma, truncated file, ...) panics the proc macro. Users see an opaque error with no indication of what went wrong or where:

error: proc macro panicked
 --> src/main.rs:1:1
  |
1 | typify::import_types!(schema = "bad.json");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: message: called `Result::unwrap()` on an `Err` value: Error("key must be a string", line: 4, column: 34)

Fix

Map the parse error to a spanned syn::Error, matching the existing handling of file-open failures. serde_json's line/column information is preserved in the message:

error: couldn't parse file bad.json: key must be a string at line 4 column 34
 --> src/main.rs:1:32
  |
1 | typify::import_types!(schema = "bad.json");
  |                                ^^^^^^^^^^

This addresses the malformed-JSON half of the diagnostics concerns raised in #516.

Trade-offs

None.

Testing

  • cargo test --workspace --locked passes.
  • The repo's trybuild usage only covers pass cases (no compile_fail harness), so the before/after output above was verified manually with a scratch crate depending on typify by path with a malformed bad.json.

Previously, import_types! unwrapped the serde_json parse result, so a
syntax error in the schema file surfaced as an opaque "proc macro
panicked" error. Map the parse error to a spanned syn::Error, matching
the existing handling of file-open failures and preserving serde_json's
line/column information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant