-
Notifications
You must be signed in to change notification settings - Fork 29
Fix compiled inline JSON rendering for primitive collections #375
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
cae39eb
Encode primitive inline JSON with the shared JSON runtime
dchuk e8c3e5e
Cover conditional primitive collections in inline JSON rendering
dchuk 9e8208c
Document primitive JSON encoder selection
dchuk a6e384d
Merge current upstream for primitive JSON validation
dchuk 559d581
Match Rails HTML escaping for primitive JSON responses
dchuk 5dddfef
Avoid collisions between JSON and view escaping constants
dchuk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| //! Inline primitive JSON must use an encoder present in the compiled tree. | ||
| #[path = "support/emit_and_run.rs"] | ||
| mod emit_and_run; | ||
|
|
||
| /// Build generic controllers covering literal and conditional primitive payloads. | ||
| fn app() -> emit_and_run::Overlay { | ||
| emit_and_run::empty_app() | ||
| .write("app/controllers/application_controller.rb", "class ApplicationController < ActionController::Base\nend\n") | ||
| .write("db/schema.rb", "ActiveRecord::Schema.define do\n create_table \"widgets\", force: :cascade do |t|\n t.string \"name\"\n end\nend\n") | ||
| .write("config/routes.rb", "Rails.application.routes.draw do\n get \"/payload\", to: \"payloads#show\"\n get \"/list\", to: \"payloads#index\"\n get \"/choose\", to: \"payloads#choose\"\nend\n") | ||
| .write("app/controllers/payloads_controller.rb", r#"class PayloadsController < ApplicationController | ||
| def show | ||
| render json: { message: "hello\n\"world\"", html: "<b>&</b>", count: 2, active: true, missing: nil, nested: { tags: ["one", "two"], empty: [], object: {} } }, status: 202 | ||
| end | ||
| def index | ||
| render json: [{ name: "first", count: 1 }, { name: "second", count: 2 }] | ||
| end | ||
| def choose | ||
| render json: (params[:shape] == "array" ? ["one"] : { name: "one" }) | ||
| end | ||
| end | ||
| "#) | ||
| } | ||
|
|
||
| const ASSERTIONS: &str = r#" | ||
| require_relative "app/controllers/payloads_controller" | ||
| controller = PayloadsController.new | ||
| controller.process_action(:show) | ||
| raise "wrong status" unless controller.status == 202 | ||
| raise "wrong content type" unless controller.content_type == "application/json" | ||
| raise controller.body unless controller.body == '{"message":"hello\n\"world\"","html":"\u003cb\u003e\u0026\u003c/b\u003e","count":2,"active":true,"missing":null,"nested":{"tags":["one","two"],"empty":[],"object":{}}}' | ||
| controller = PayloadsController.new | ||
| controller.process_action(:index) | ||
| raise controller.body unless controller.body == '[{"name":"first","count":1},{"name":"second","count":2}]' | ||
| controller = PayloadsController.new | ||
| controller.params = {"shape" => "array"} | ||
| controller.process_action(:choose) | ||
| raise controller.body unless controller.body == '["one"]' | ||
| controller = PayloadsController.new | ||
| controller.params = {"shape" => "hash"} | ||
| controller.process_action(:choose) | ||
| raise controller.body unless controller.body == '{"name":"one"}' | ||
| puts "primitive JSON passed" | ||
| "#; | ||
|
|
||
| /// CRuby preserves primitive payload bytes, status, and content type. | ||
| #[test] | ||
| fn inline_primitive_json_runs() { | ||
| app().run_ruby(ASSERTIONS).assert_passes(); | ||
| } | ||
|
|
||
| /// Temporal values must retain Rails serialization instead of primitive encoding. | ||
| #[test] | ||
| fn a_nested_time_keeps_rails_json_serialization() { | ||
| app() | ||
| .write("app/controllers/payloads_controller.rb", r#"class PayloadsController < ApplicationController | ||
| def show | ||
| render json: { at: Time.utc(2026, 7, 1, 12, 34, 56) } | ||
| end | ||
| def index | ||
| head :no_content | ||
| end | ||
| def choose | ||
| head :no_content | ||
| end | ||
| end | ||
| "#) | ||
| .run_ruby(r#" | ||
| require_relative "app/controllers/payloads_controller" | ||
| controller = PayloadsController.new | ||
| controller.process_action(:show) | ||
| raise controller.body unless controller.body == '{"at":"2026-07-01T12:34:56.000Z"}' | ||
| "#) | ||
| .assert_passes(); | ||
| } | ||
|
|
||
| /// The compiled runtime handles the same primitive and conditional payloads. | ||
| #[test] | ||
| #[ignore = "requires the Spinel toolchain"] | ||
| fn inline_primitive_json_runs_on_spinel() { | ||
| app().run_spinel(ASSERTIONS).assert_passes(); | ||
| } | ||
|
|
||
| /// These targets flatten runtime constants into one namespace. JSON escaping | ||
| /// must coexist with ViewHelpers' HTML escaping when both runtimes are emitted. | ||
| #[test] | ||
| fn json_and_view_html_escape_constants_do_not_collide() { | ||
| use roundhouse::analyze::Analyzer; | ||
| use roundhouse::emit::{crystal, csharp, go, kotlin, swift}; | ||
| use std::collections::BTreeSet; | ||
| use std::path::Path; | ||
|
|
||
| let mut app = roundhouse::ingest::ingest_app(Path::new("fixtures/tiny-blog")) | ||
| .expect("ingest tiny-blog"); | ||
| Analyzer::new(&app).analyze(&mut app); | ||
| let mut collisions = Vec::new(); | ||
| for (target, files, json_path, view_path, declaration) in [ | ||
| ("Go", go::emit(&app), "app/v2/json_builder.go", "app/v2/view_helpers.go", | ||
| "var "), | ||
| ("C#", csharp::emit(&app), "app/runtime/JsonBuilder.cs", "app/runtime/ViewHelpers.cs", | ||
| "public static partial class RuntimeConstants { public static readonly "), | ||
| ("Crystal", crystal::emit(&app), "src/json_builder.cr", "src/view_helpers.cr", | ||
| ""), | ||
| ("Kotlin", kotlin::emit(&app), "src/main/kotlin/JsonBuilder.kt", "src/main/kotlin/ViewHelpers.kt", | ||
| "val "), | ||
| ("Swift", swift::emit(&app), "Sources/App/JsonBuilder.swift", "Sources/App/ViewHelpers.swift", | ||
| "let "), | ||
| ] { | ||
| let names = |path: &str| -> BTreeSet<String> { | ||
| let file = files.iter().find(|file| file.path == Path::new(path)) | ||
| .unwrap_or_else(|| panic!("missing {target} runtime {path}")); | ||
| let names: BTreeSet<_> = file.content.lines() | ||
| .filter_map(|line| line.strip_prefix(declaration)) | ||
| .filter_map(|line| line.split_once(" =")) | ||
| .filter_map(|(left, _)| left.split_whitespace().last()) | ||
| .filter(|name| name.bytes().all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == b'_')) | ||
| .map(str::to_string).collect(); | ||
| assert!(!names.is_empty(), "no {target} runtime constants found in {path}"); | ||
| names | ||
| }; | ||
| let json_names = names(json_path); | ||
| let view_names = names(view_path); | ||
| for name in json_names.intersection(&view_names) { | ||
| collisions.push(format!("{target}: {name} is declared in both {json_path} and {view_path}")); | ||
| } | ||
| } | ||
| assert!(collisions.is_empty(), "{}", collisions.join("\n")); | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: rubys/roundhouse
Length of output: 29637
🏁 Script executed:
Repository: rubys/roundhouse
Length of output: 17560
🏁 Script executed:
Repository: rubys/roundhouse
Length of output: 284
🏁 Script executed:
Repository: rubys/roundhouse
Length of output: 14490
Update the local array type after
map!.When
itemsstarts asArray[String]andmap!replaces its elements withCustomPayloadobjects, the analyzer can keep the local typed asArray[String]. The following inlinerender json: itemscan then useJSON.generate, which serializes the object as an inspected string instead of honoring itsas_jsonresult. Update the binding from themap!block’s return type soJsonRender.encodehandles the array.🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The shared analyzer correction is now in #407 (commit
c351a12e5f04713f9f950754aa7312c48a2f064c), closing #406. The standalone full default gate passed: 3,344 passed, 0 failed, 125 ignored. Combining that exact analyzer module with this PR’s tested treef29abe2655704d140354cbb3a6dfb69d72fd3366produced tree66990d35aaf919e41e71c6219af6ea7eab5ce576: strict emission succeeded and all four actual CRuby response controls passed, including the two previously failing destructive custom-object transformations. The native numeric mutation/identity control passed separately; it does not claim native custom-object JSON support.I have marked #407 as a merge dependency in this PR’s description. This thread should remain open until that fix is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dchukUnderstood. This finding should remain open until#407merges.The reported combined-tree checks cover the two destructive custom-object transformations on CRuby. The native numeric mutation/identity control does not establish native custom-object JSON support.
You are interacting with an AI system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency now includes the reviewed lexical-scope correction: #407 head
f6dccbfa241746469bdff9d509d41067cfd3d281, final tree0ec02eae9f37a29f4f49e341e5ec64dd65ea3fba. Its complete default gate passed with 3,346 passed, 0 failed and 125 ignored. The identical analyzer module combined with this PR’s tested tree produced8b9bddbbc9ab052badeab331b233b5d898dde055; strict emission and all four existing CRuby response contracts passed. General alias analysis and native custom-object JSON remain outside this fix. The description links the current dependency and keeps the merge order explicit; this thread remains open until #407 lands.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dchukUnderstood. This finding remains open until#407lands.The reported combined-tree checks cover strict emission and all four existing CRuby response contracts with the lexical-scope correction. They do not establish general alias analysis or native custom-object JSON support.
You are interacting with an AI system.