chore: prepare qjson public naming#42
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR renames the crate and public C ABI from quickdecode/qjd_* to qjson/qjson_*, adds include/qjson.h, updates Cargo metadata and Makefile/docs, rewires Rust FFI exports and Lua FFI bindings, updates benchmarks, and converts all tests to the new API names. Changesqjson crate rename from lua-quick-decode
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
|
There was a problem hiding this comment.
Pull request overview
This PR renames the public Rust crate, C FFI, and Lua module/API surface from quickdecode/qjd to qjson, while adding Apache-2.0 licensing metadata and refreshing documentation, tests, CI comments, and benchmarks to match the new public naming.
Changes:
- Renames Rust crate/lib, FFI symbols/types/constants, Lua module paths, and tests to
qjson. - Replaces the old public C header with
include/qjson.hand adds Apache-2.0 license metadata. - Updates README, benchmark docs/scripts, conformance docs, CI comments, and project guidance to reflect current naming and behavior.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yml |
Updates Lua FFI loading comment for qjson. |
Cargo.toml |
Renames package/lib and adds license/repository metadata. |
CLAUDE.md |
Updates project guidance and invariants to qjson. |
LICENSE |
Adds Apache-2.0 license text. |
Makefile |
Updates build artifact description. |
README.md |
Updates setup, usage, benchmark, and conformance docs. |
benches/lua_bench.lua |
Renames benchmark module/API references. |
benches/perf_probe.lua |
Renames perf probe module/API references. |
docs/benchmarks.md |
Updates benchmark documentation naming. |
docs/rfc8259-conformance.md |
Updates conformance doc naming/error constants. |
include/lua_quick_decode.h |
Removes old C header. |
include/qjson.h |
Adds renamed public C header. |
lua/qjson.lua |
Renames LuaJIT FFI declarations and wrapper API. |
lua/qjson/table.lua |
Renames lazy table API internals and messages. |
src/cursor.rs |
Renames internal error type references. |
src/decode/number.rs |
Renames number decode error references. |
src/decode/string.rs |
Renames string decode error references. |
src/doc.rs |
Renames document error/type references. |
src/error.rs |
Renames public Rust error/type enums and variants. |
src/ffi.rs |
Renames exported C ABI symbols/types/constants. |
src/lib.rs |
Updates crate docs while preserving test API exports. |
src/options.rs |
Renames public parse option constants. |
src/path.rs |
Renames path parser error references. |
src/validate/mod.rs |
Renames validation error references and docs. |
src/validate/number.rs |
Renames number validation errors/docs. |
src/validate/strings/avx2.rs |
Renames AVX2 string validator error type. |
src/validate/strings/mod.rs |
Renames string validator error references/tests. |
src/validate/strings/neon.rs |
Renames NEON string validator error type. |
src/validate/strings/scalar.rs |
Renames scalar string validator error references. |
tests/ffi_cursor.rs |
Updates FFI cursor tests to qjson. |
tests/ffi_cursor_bytes.rs |
Updates FFI cursor byte-span tests. |
tests/ffi_numbers.rs |
Updates numeric FFI tests. |
tests/ffi_object_iter.rs |
Updates object iteration FFI tests. |
tests/ffi_options_smoke.rs |
Updates parse-options FFI smoke tests. |
tests/ffi_panic_safety.rs |
Updates panic barrier test symbol. |
tests/ffi_smoke.rs |
Updates basic FFI smoke tests. |
tests/ffi_strings.rs |
Updates string FFI tests. |
tests/ffi_typeof.rs |
Updates type/len FFI tests. |
tests/ffi_wide_object.rs |
Updates wide-object FFI test. |
tests/json_test_suite.rs |
Updates JSONTestSuite imports/options. |
tests/lua/basic_spec.lua |
Updates basic Lua tests. |
tests/lua/cjson_compat_spec.lua |
Updates cjson compatibility tests. |
tests/lua/escape_spec.lua |
Updates string escape Lua tests. |
tests/lua/gc_spec.lua |
Updates GC Lua tests. |
tests/lua/lazy_table_spec.lua |
Updates lazy table Lua tests. |
tests/lua/options_spec.lua |
Updates parse option Lua tests. |
tests/rfc8259_compliance.rs |
Updates RFC 8259 test imports/error constants. |
tests/scanner_crosscheck.rs |
Updates scanner crosscheck imports. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Around line 14-17: The CLAUDE.md currently gives conflicting descriptions of
the "make lint" target (one place lists "cargo clippy -D warnings + cargo fmt
--check" while another says only clippy); reconcile them by updating all
occurrences so "make lint" consistently documents both checks: "cargo clippy -D
warnings" and "cargo fmt --check" (or explicitly state the chosen single
behavior if you prefer only one), and remove or replace the outdated entry so
every mention of "make lint" uses the same command list.
In `@src/ffi.rs`:
- Around line 153-157: Wrap the body of pub unsafe extern "C" fn qjson_free(doc:
*mut qjson_doc) in std::panic::catch_unwind so any panic from Box::from_raw (or
Document Drop) is caught; inside the closure perform the null check and
Box::from_raw(doc) as before, and on Err(...) translate the panic into the FFI
error token by calling the existing error conversion (e.g.,
qjson_set_last_error(QJSON_OOM) or the project's equivalent) so panics do not
unwind across the FFI boundary and the function remains void-returning.
In `@tests/ffi_cursor.rs`:
- Around line 42-43: The unsafe calls call qjson_open(...) and then
unconditionally call c.assume_init(), which is UB if qjson_open failed; update
each site (the calls in tests like the one at the top and the similar ones at
the other occurrences) to check qjson_open's return value before calling
assume_init(): capture the return (e.g., rc = unsafe { qjson_open(...) }),
assert/propagate an error or panic/test-fail if rc indicates failure, and only
then call let c = unsafe { c.assume_init() }; apply the same pattern to the
other occurrences you noted (around lines 65-66, 92-93, 116-117) so qjson_open
is validated before assume_init() is used.
In `@tests/ffi_numbers.rs`:
- Around line 27-28: The tests call unsafe qjson_get_* functions and ignore
their return codes; update each success-path test (get_i64_negative,
get_f64_basic, get_bool, get_f64_negative_zero_and_exponent) to capture the
return value into a variable (e.g., rc) from the unsafe
qjson_get_i64/qjson_get_f64/qjson_get_bool call and assert the call succeeded
(assert_eq!(rc, 0)) before asserting the extracted value, so failures in the C
API surface as test failures rather than silently leaving initialized values
unchanged.
In `@tests/ffi_object_iter.rs`:
- Around line 13-14: The helper open_root currently ignores the qjson_open
return code; capture the return value from qjson_open when calling it in
open_root (the call that sets doc and cur), and assert that it indicates success
before returning (e.g., assert_eq!(rc, QJSON_OK) or assert!(rc == 0) depending
on the project's error constant). This ensures tests using open_root (and the
doc/cur values) do not proceed with an invalid cursor if qjson_open failed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5185154-bd33-48f8-ad6e-842f1b48ee0f
📒 Files selected for processing (48)
.github/workflows/ci.ymlCLAUDE.mdCargo.tomlLICENSEMakefileREADME.mdbenches/lua_bench.luabenches/perf_probe.luadocs/benchmarks.mddocs/rfc8259-conformance.mdinclude/lua_quick_decode.hinclude/qjson.hlua/qjson.lualua/qjson/table.luasrc/cursor.rssrc/decode/number.rssrc/decode/string.rssrc/doc.rssrc/error.rssrc/ffi.rssrc/lib.rssrc/options.rssrc/path.rssrc/validate/mod.rssrc/validate/number.rssrc/validate/strings/avx2.rssrc/validate/strings/mod.rssrc/validate/strings/neon.rssrc/validate/strings/scalar.rstests/ffi_cursor.rstests/ffi_cursor_bytes.rstests/ffi_numbers.rstests/ffi_object_iter.rstests/ffi_options_smoke.rstests/ffi_panic_safety.rstests/ffi_smoke.rstests/ffi_strings.rstests/ffi_typeof.rstests/ffi_wide_object.rstests/json_test_suite.rstests/lua/basic_spec.luatests/lua/cjson_compat_spec.luatests/lua/escape_spec.luatests/lua/gc_spec.luatests/lua/lazy_table_spec.luatests/lua/options_spec.luatests/rfc8259_compliance.rstests/scanner_crosscheck.rs
💤 Files with no reviewable changes (1)
- include/lua_quick_decode.h
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.
Rename the public package/module/API surface to qjson and add Apache-2.0 licensing metadata.
This also updates README/benchmark/conformance docs so the setup and current RFC 8259 status match the code. Rockspec work is intentionally left for a separate change.
Review feedback addressed after opening:
Checked locally:
Note: cargo fmt --check still wants to reflow the existing hand-aligned Rust style; I left that alone because the repo docs say rustfmt is not part of the lint gate.
Summary by CodeRabbit
Breaking Changes
Documentation
Tests & Benchmarks