Add JSONC Support for Project, Meta, and Model JSON files (#1144)

Replaces `serde_json` parsing with `jsonc-parser` throughout the
codebase, enabling support for **comments** and **trailing commas** in
all JSON files including `.project.json`, `.model.json`, and
`.meta.json` files.
MSRV bumps from `1.83.0` to `1.88.0` in order to
use the jsonc_parser dependency.
This commit is contained in:
boatbomber
2025-10-28 17:29:57 -07:00
committed by GitHub
parent aabe6d11b2
commit d0b029f995
15 changed files with 471 additions and 65 deletions

View File

@@ -43,8 +43,8 @@ impl Serialize for Glob {
impl<'de> Deserialize<'de> for Glob {
fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
let glob = <&str as Deserialize>::deserialize(deserializer)?;
let glob = String::deserialize(deserializer)?;
Glob::new(glob).map_err(D::Error::custom)
Glob::new(&glob).map_err(D::Error::custom)
}
}

313
src/json.rs Normal file
View File

@@ -0,0 +1,313 @@
//! Utilities for parsing JSON with comments (JSONC) and deserializing to Rust types.
//!
//! This module provides convenient wrappers around `jsonc_parser` and `serde_json`
//! to reduce boilerplate and improve ergonomics when working with JSONC files.
use anyhow::Context as _;
use serde::de::DeserializeOwned;
/// Parse JSONC text into a `serde_json::Value`.
///
/// This handles the common pattern of calling `jsonc_parser::parse_to_serde_value`
/// and unwrapping the `Option` with a clear error message.
///
/// # Errors
///
/// Returns an error if:
/// - The text is not valid JSONC
/// - The text contains no JSON value
pub fn parse_value(text: &str) -> anyhow::Result<serde_json::Value> {
jsonc_parser::parse_to_serde_value(text, &Default::default())
.context("Failed to parse JSONC")?
.ok_or_else(|| anyhow::anyhow!("File contains no JSON value"))
}
/// Parse JSONC text into a `serde_json::Value` with a custom context message.
///
/// This is useful when you want to provide a specific error message that includes
/// additional information like the file path.
///
/// # Errors
///
/// Returns an error if:
/// - The text is not valid JSONC
/// - The text contains no JSON value
pub fn parse_value_with_context(
text: &str,
context: impl Fn() -> String,
) -> anyhow::Result<serde_json::Value> {
jsonc_parser::parse_to_serde_value(text, &Default::default())
.with_context(|| format!("{}: JSONC parse error", context()))?
.ok_or_else(|| anyhow::anyhow!("{}: File contains no JSON value", context()))
}
/// Parse JSONC text and deserialize it into a specific type.
///
/// This combines parsing JSONC and deserializing into a single operation,
/// eliminating the need to manually chain `parse_to_serde_value` and `from_value`.
///
/// # Errors
///
/// Returns an error if:
/// - The text is not valid JSONC
/// - The text contains no JSON value
/// - The value cannot be deserialized into type `T`
pub fn from_str<T: DeserializeOwned>(text: &str) -> anyhow::Result<T> {
let value = parse_value(text)?;
serde_json::from_value(value).context("Failed to deserialize JSON")
}
/// Parse JSONC text and deserialize it into a specific type with a custom context message.
///
/// This is useful when you want to provide a specific error message that includes
/// additional information like the file path.
///
/// # Errors
///
/// Returns an error if:
/// - The text is not valid JSONC
/// - The text contains no JSON value
/// - The value cannot be deserialized into type `T`
pub fn from_str_with_context<T: DeserializeOwned>(
text: &str,
context: impl Fn() -> String,
) -> anyhow::Result<T> {
let value = parse_value_with_context(text, &context)?;
serde_json::from_value(value).with_context(|| format!("{}: Invalid JSON structure", context()))
}
/// Parse JSONC bytes into a `serde_json::Value` with a custom context message.
///
/// This handles UTF-8 conversion and JSONC parsing in one step.
///
/// # Errors
///
/// Returns an error if:
/// - The bytes are not valid UTF-8
/// - The text is not valid JSONC
/// - The text contains no JSON value
pub fn parse_value_from_slice_with_context(
slice: &[u8],
context: impl Fn() -> String,
) -> anyhow::Result<serde_json::Value> {
let text = std::str::from_utf8(slice)
.with_context(|| format!("{}: File is not valid UTF-8", context()))?;
parse_value_with_context(text, context)
}
/// Parse JSONC bytes and deserialize it into a specific type.
///
/// This handles UTF-8 conversion, JSONC parsing, and deserialization in one step.
///
/// # Errors
///
/// Returns an error if:
/// - The bytes are not valid UTF-8
/// - The text is not valid JSONC
/// - The text contains no JSON value
/// - The value cannot be deserialized into type `T`
pub fn from_slice<T: DeserializeOwned>(slice: &[u8]) -> anyhow::Result<T> {
let text = std::str::from_utf8(slice).context("File is not valid UTF-8")?;
from_str(text)
}
/// Parse JSONC bytes and deserialize it into a specific type with a custom context message.
///
/// This handles UTF-8 conversion, JSONC parsing, and deserialization in one step.
///
/// # Errors
///
/// Returns an error if:
/// - The bytes are not valid UTF-8
/// - The text is not valid JSONC
/// - The text contains no JSON value
/// - The value cannot be deserialized into type `T`
pub fn from_slice_with_context<T: DeserializeOwned>(
slice: &[u8],
context: impl Fn() -> String,
) -> anyhow::Result<T> {
let text = std::str::from_utf8(slice)
.with_context(|| format!("{}: File is not valid UTF-8", context()))?;
from_str_with_context(text, context)
}
#[cfg(test)]
mod tests {
use super::*;
use serde::Deserialize;
#[test]
fn test_parse_value() {
let value = parse_value(r#"{"foo": "bar"}"#).unwrap();
assert_eq!(value["foo"], "bar");
}
#[test]
fn test_parse_value_with_comments() {
let value = parse_value(
r#"{
// This is a comment
"foo": "bar" // Inline comment
}"#,
)
.unwrap();
assert_eq!(value["foo"], "bar");
}
#[test]
fn test_parse_value_with_trailing_comma() {
let value = parse_value(
r#"{
"foo": "bar",
"baz": 123,
}"#,
)
.unwrap();
assert_eq!(value["foo"], "bar");
assert_eq!(value["baz"], 123);
}
#[test]
fn test_parse_value_empty() {
let err = parse_value("").unwrap_err();
assert!(err.to_string().contains("no JSON value"));
}
#[test]
fn test_parse_value_invalid() {
let err = parse_value("{invalid}").unwrap_err();
assert!(err.to_string().contains("parse"));
}
#[test]
fn test_parse_value_with_context() {
let err = parse_value_with_context("{invalid}", || "test.json".to_string()).unwrap_err();
assert!(err.to_string().contains("test.json"));
assert!(err.to_string().contains("parse"));
}
#[derive(Debug, Deserialize, PartialEq)]
struct TestStruct {
foo: String,
bar: i32,
}
#[test]
fn test_from_str() {
let result: TestStruct = from_str(r#"{"foo": "hello", "bar": 42}"#).unwrap();
assert_eq!(
result,
TestStruct {
foo: "hello".to_string(),
bar: 42
}
);
}
#[test]
fn test_from_str_with_comments() {
let result: TestStruct = from_str(
r#"{
// Comment
"foo": "hello",
"bar": 42, // Trailing comma is fine
}"#,
)
.unwrap();
assert_eq!(
result,
TestStruct {
foo: "hello".to_string(),
bar: 42
}
);
}
#[test]
fn test_from_str_invalid_type() {
let err = from_str::<TestStruct>(r#"{"foo": "hello"}"#).unwrap_err();
assert!(err.to_string().contains("deserialize"));
}
#[test]
fn test_from_str_with_context() {
let err = from_str_with_context::<TestStruct>(r#"{"foo": "hello"}"#, || {
"config.json".to_string()
})
.unwrap_err();
assert!(err.to_string().contains("config.json"));
assert!(err.to_string().contains("Invalid JSON structure"));
}
#[test]
fn test_parse_value_from_slice_with_context() {
let err = parse_value_from_slice_with_context(b"{invalid}", || "test.json".to_string())
.unwrap_err();
assert!(err.to_string().contains("test.json"));
assert!(err.to_string().contains("parse"));
}
#[test]
fn test_parse_value_from_slice_with_context_invalid_utf8() {
let err = parse_value_from_slice_with_context(&[0xFF, 0xFF], || "test.json".to_string())
.unwrap_err();
assert!(err.to_string().contains("test.json"));
assert!(err.to_string().contains("UTF-8"));
}
#[test]
fn test_from_slice() {
let result: TestStruct = from_slice(br#"{"foo": "hello", "bar": 42}"#).unwrap();
assert_eq!(
result,
TestStruct {
foo: "hello".to_string(),
bar: 42
}
);
}
#[test]
fn test_from_slice_with_comments() {
let result: TestStruct = from_slice(
br#"{
// Comment
"foo": "hello",
"bar": 42, // Trailing comma is fine
}"#,
)
.unwrap();
assert_eq!(
result,
TestStruct {
foo: "hello".to_string(),
bar: 42
}
);
}
#[test]
fn test_from_slice_invalid_utf8() {
let err = from_slice::<TestStruct>(&[0xFF, 0xFF]).unwrap_err();
assert!(err.to_string().contains("UTF-8"));
}
#[test]
fn test_from_slice_with_context() {
let err = from_slice_with_context::<TestStruct>(br#"{"foo": "hello"}"#, || {
"config.json".to_string()
})
.unwrap_err();
assert!(err.to_string().contains("config.json"));
assert!(err.to_string().contains("Invalid JSON structure"));
}
#[test]
fn test_from_slice_with_context_invalid_utf8() {
let err =
from_slice_with_context::<TestStruct>(&[0xFF, 0xFF], || "config.json".to_string())
.unwrap_err();
assert!(err.to_string().contains("config.json"));
assert!(err.to_string().contains("UTF-8"));
}
}

View File

@@ -10,6 +10,7 @@ mod tree_view;
mod auth_cookie;
mod change_processor;
mod glob;
mod json;
mod lua_ast;
mod message_queue;
mod multimap;

View File

@@ -11,7 +11,7 @@ use rbx_dom_weak::{Ustr, UstrMap};
use serde::{Deserialize, Serialize};
use thiserror::Error;
use crate::{glob::Glob, resolution::UnresolvedValue, snapshot::SyncRule};
use crate::{glob::Glob, json, resolution::UnresolvedValue, snapshot::SyncRule};
static PROJECT_FILENAME: &str = "default.project.json";
@@ -214,8 +214,11 @@ impl Project {
project_file_location: PathBuf,
fallback_name: Option<&str>,
) -> Result<Self, Error> {
let mut project: Self = serde_json::from_slice(contents).map_err(|source| Error::Json {
source,
let mut project: Self = json::from_slice(contents).map_err(|e| Error::Json {
source: serde_json::Error::io(std::io::Error::new(
std::io::ErrorKind::InvalidData,
e.to_string(),
)),
path: project_file_location.clone(),
})?;
project.file_location = project_file_location;
@@ -399,13 +402,13 @@ mod test {
#[test]
fn path_node_required() {
let path_node: PathNode = serde_json::from_str(r#""src""#).unwrap();
let path_node: PathNode = json::from_str(r#""src""#).unwrap();
assert_eq!(path_node, PathNode::Required(PathBuf::from("src")));
}
#[test]
fn path_node_optional() {
let path_node: PathNode = serde_json::from_str(r#"{ "optional": "src" }"#).unwrap();
let path_node: PathNode = json::from_str(r#"{ "optional": "src" }"#).unwrap();
assert_eq!(
path_node,
PathNode::Optional(OptionalPathNode::new(PathBuf::from("src")))
@@ -414,7 +417,7 @@ mod test {
#[test]
fn project_node_required() {
let project_node: ProjectNode = serde_json::from_str(
let project_node: ProjectNode = json::from_str(
r#"{
"$path": "src"
}"#,
@@ -429,7 +432,7 @@ mod test {
#[test]
fn project_node_optional() {
let project_node: ProjectNode = serde_json::from_str(
let project_node: ProjectNode = json::from_str(
r#"{
"$path": { "optional": "src" }
}"#,
@@ -446,7 +449,7 @@ mod test {
#[test]
fn project_node_none() {
let project_node: ProjectNode = serde_json::from_str(
let project_node: ProjectNode = json::from_str(
r#"{
"$className": "Folder"
}"#,
@@ -458,7 +461,7 @@ mod test {
#[test]
fn project_node_optional_serialize_absolute() {
let project_node: ProjectNode = serde_json::from_str(
let project_node: ProjectNode = json::from_str(
r#"{
"$path": { "optional": "..\\src" }
}"#,
@@ -471,7 +474,7 @@ mod test {
#[test]
fn project_node_optional_serialize_absolute_no_change() {
let project_node: ProjectNode = serde_json::from_str(
let project_node: ProjectNode = json::from_str(
r#"{
"$path": { "optional": "../src" }
}"#,
@@ -484,7 +487,7 @@ mod test {
#[test]
fn project_node_optional_serialize_optional() {
let project_node: ProjectNode = serde_json::from_str(
let project_node: ProjectNode = json::from_str(
r#"{
"$path": "..\\src"
}"#,
@@ -494,4 +497,57 @@ mod test {
let serialized = serde_json::to_string(&project_node).unwrap();
assert_eq!(serialized, r#"{"$path":"../src"}"#);
}
#[test]
fn project_with_jsonc_features() {
// Test that JSONC features (comments and trailing commas) are properly handled
let project_json = r#"{
// This is a single-line comment
"name": "TestProject",
/* This is a
multi-line comment */
"tree": {
"$path": "src", // Comment after value
},
"servePort": 34567,
"emitLegacyScripts": false,
// Test glob parsing with comments
"globIgnorePaths": [
"**/*.spec.lua", // Ignore test files
"**/*.test.lua",
],
"syncRules": [
{
"pattern": "*.data.json",
"use": "json", // Trailing comma in object
},
{
"pattern": "*.module.lua",
"use": "moduleScript",
}, // Trailing comma in array
], // Another trailing comma
}"#;
let project = Project::load_from_slice(
project_json.as_bytes(),
PathBuf::from("/test/default.project.json"),
None,
)
.expect("Failed to parse project with JSONC features");
// Verify the parsed values
assert_eq!(project.name, Some("TestProject".to_string()));
assert_eq!(project.serve_port, Some(34567));
assert_eq!(project.emit_legacy_scripts, Some(false));
// Verify glob_ignore_paths were parsed correctly
assert_eq!(project.glob_ignore_paths.len(), 2);
assert!(project.glob_ignore_paths[0].is_match("test/foo.spec.lua"));
assert!(project.glob_ignore_paths[1].is_match("test/bar.test.lua"));
// Verify sync_rules were parsed correctly
assert_eq!(project.sync_rules.len(), 2);
assert!(project.sync_rules[0].include.is_match("data.data.json"));
assert!(project.sync_rules[1].include.is_match("init.module.lua"));
}
}

View File

@@ -248,14 +248,15 @@ fn nonexhaustive_list(values: &[&str]) -> String {
#[cfg(test)]
mod test {
use super::*;
use crate::json;
fn resolve(class: &str, prop: &str, json_value: &str) -> Variant {
let unresolved: UnresolvedValue = serde_json::from_str(json_value).unwrap();
let unresolved: UnresolvedValue = json::from_str(json_value).unwrap();
unresolved.resolve(class, prop).unwrap()
}
fn resolve_unambiguous(json_value: &str) -> Variant {
let unresolved: UnresolvedValue = serde_json::from_str(json_value).unwrap();
let unresolved: UnresolvedValue = json::from_str(json_value).unwrap();
unresolved.resolve_unambiguous().unwrap()
}

View File

@@ -1,10 +1,10 @@
use std::path::Path;
use anyhow::Context;
use memofs::{IoResultExt, Vfs};
use rbx_dom_weak::ustr;
use crate::{
json,
lua_ast::{Expression, Statement},
snapshot::{InstanceContext, InstanceMetadata, InstanceSnapshot},
};
@@ -19,8 +19,9 @@ pub fn snapshot_json(
) -> anyhow::Result<Option<InstanceSnapshot>> {
let contents = vfs.read(path)?;
let value: serde_json::Value = serde_json::from_slice(&contents)
.with_context(|| format!("File contains malformed JSON: {}", path.display()))?;
let value = json::parse_value_from_slice_with_context(&contents, || {
format!("File contains malformed JSON: {}", path.display())
})?;
let as_lua = json_to_lua(value).to_string();

View File

@@ -9,6 +9,7 @@ use rbx_dom_weak::{
use serde::Deserialize;
use crate::{
json,
resolution::UnresolvedValue,
snapshot::{InstanceContext, InstanceSnapshot},
RojoRef,
@@ -28,8 +29,9 @@ pub fn snapshot_json_model(
return Ok(None);
}
let mut instance: JsonModel = serde_json::from_str(contents_str)
.with_context(|| format!("File is not a valid JSON model: {}", path.display()))?;
let mut instance: JsonModel = json::from_str_with_context(contents_str, || {
format!("File is not a valid JSON model: {}", path.display())
})?;
if let Some(top_level_name) = &instance.name {
let new_name = format!("{}.model.json", top_level_name);

View File

@@ -4,7 +4,7 @@ use anyhow::{format_err, Context};
use rbx_dom_weak::{types::Attributes, Ustr, UstrMap};
use serde::{Deserialize, Serialize};
use crate::{resolution::UnresolvedValue, snapshot::InstanceSnapshot, RojoRef};
use crate::{json, resolution::UnresolvedValue, snapshot::InstanceSnapshot, RojoRef};
/// Represents metadata in a sibling file with the same basename.
///
@@ -34,7 +34,7 @@ pub struct AdjacentMetadata {
impl AdjacentMetadata {
pub fn from_slice(slice: &[u8], path: PathBuf) -> anyhow::Result<Self> {
let mut meta: Self = serde_json::from_slice(slice).with_context(|| {
let mut meta: Self = json::from_slice_with_context(slice, || {
format!(
"File contained malformed .meta.json data: {}",
path.display()
@@ -131,7 +131,7 @@ pub struct DirectoryMetadata {
impl DirectoryMetadata {
pub fn from_slice(slice: &[u8], path: PathBuf) -> anyhow::Result<Self> {
let mut meta: Self = serde_json::from_slice(slice).with_context(|| {
let mut meta: Self = json::from_slice_with_context(slice, || {
format!(
"File contained malformed init.meta.json data: {}",
path.display()

View File

@@ -17,6 +17,7 @@ use rbx_dom_weak::{
};
use crate::{
json,
serve_session::ServeSession,
snapshot::{InstanceWithMeta, PatchSet, PatchUpdate},
web::{
@@ -139,7 +140,7 @@ impl ApiService {
let body = body::to_bytes(request.into_body()).await.unwrap();
let request: WriteRequest = match serde_json::from_slice(&body) {
let request: WriteRequest = match json::from_slice(&body) {
Ok(request) => request,
Err(err) => {
return json(