diff --git a/src/lower/jbuilder_to_library/mod.rs b/src/lower/jbuilder_to_library/mod.rs index d83f866af..0f2abd6ec 100644 --- a/src/lower/jbuilder_to_library/mod.rs +++ b/src/lower/jbuilder_to_library/mod.rs @@ -41,6 +41,9 @@ //! 11. `begin … rescue … end` → the same `begin`; a `rescue` //! first drops a pair the body //! left half-written +//! 12. `x = ` → kept as written, in place; a +//! local the template reads +//! later //! //! (6)-(9) arrived together with campfire's bot API, which is six //! jbuilder templates written in exactly that dialect. @@ -422,6 +425,9 @@ enum JbStmt<'a> { body: &'a Expr, rescues: &'a [RescueClause], }, + /// `x = ` — a template local. Emitted as written; it adds + /// no pair. + Local, /// Unrecognized DSL or non-Send statement. Surfaces as an empty io /// append so the lowered body stays well-formed. Unknown, @@ -476,27 +482,38 @@ fn emit_object(raw_stmts: &[&Expr], ctx: &Ctx) -> Vec { // Whole-template DSL forms (single stmt covers the entire JSON // body) — array! and partial! produce a top-level array or method - // call respectively, no `{}` wrap. - if classified.len() == 1 { + // call respectively, no `{}` wrap. Template locals around that one + // statement stay where they are and do not count. + let mut dsl = classified + .iter() + .enumerate() + .filter(|(_, c)| !matches!(c, JbStmt::Local)); + if let (Some((index, only)), None) = (dsl.next(), dsl.next()) { // Synthesis choke point (whole-template forms): everything // emitted for the single DSL statement attributes back to it. - let src_span = raw_stmts[0].span; - match &classified[0] { + let src_span = raw_stmts[index].span; + let whole = match only { JbStmt::ArrayPartial { collection, partial_path, item_var } => { - let mut out = emit_array_partial(collection, partial_path, item_var, ctx); - for e in &mut out { - e.inherit_span(src_span); - } - return out; + Some(emit_array_partial(collection, partial_path, item_var, ctx)) } JbStmt::Partial { partial_path, arg } => { - let mut out = emit_partial_call(partial_path, arg, ctx); - for e in &mut out { - e.inherit_span(src_span); + Some(emit_partial_call(partial_path, arg, ctx)) + } + _ => None, + }; + if let Some(mut whole) = whole { + for e in &mut whole { + e.inherit_span(src_span); + } + let mut out: Vec = Vec::new(); + for (i, src) in raw_stmts.iter().enumerate() { + if i == index { + out.append(&mut whole); + } else { + out.push(emit_local(src, ctx)); } - return out; } - _ => {} + return out; } } @@ -708,6 +725,9 @@ fn emit_pairs( JbStmt::Guarded { body, rescues } => { sep = emit_guarded(body, rescues, ctx, out, sep); } + JbStmt::Local => { + out.push(emit_local(src, ctx)); + } JbStmt::Unknown => { out.push(io_append_lit(&ctx.accumulator, "")); } @@ -804,6 +824,24 @@ fn emit_guarded( after } +/// A template local as written, its value given the rewrites a pair's +/// value gets (`_url` to `RouteHelpers._path`, `h`): the value is +/// read by pairs later, and the emitted view has no `_url` helpers. +fn emit_local(stmt: &Expr, ctx: &Ctx) -> Expr { + let ExprNode::Assign { target, value } = &*stmt.node else { + return stmt.clone(); + }; + let mut out = Expr::new( + stmt.span, + ExprNode::Assign { + target: target.clone(), + value: rewrite_h_escape(&rewrite_route_helpers(value, ctx)), + }, + ); + out.ty = stmt.ty.clone(); + out +} + fn classify<'a>(stmt: &'a Expr) -> JbStmt<'a> { if let ExprNode::BeginRescue { body, rescues, else_branch: None, ensure: None, .. } = &*stmt.node { if !rescues.is_empty() { @@ -813,6 +851,9 @@ fn classify<'a>(stmt: &'a Expr) -> JbStmt<'a> { if let ExprNode::If { cond, then_branch, else_branch } = &*stmt.node { return JbStmt::Cond { cond, then_branch, else_branch }; } + if let ExprNode::Assign { target: LValue::Var { .. }, .. } = &*stmt.node { + return JbStmt::Local; + } let ExprNode::Send { recv: Some(recv), method, diff --git a/tests/jbuilder_template_locals.rs b/tests/jbuilder_template_locals.rs new file mode 100644 index 000000000..0e33b06d4 --- /dev/null +++ b/tests/jbuilder_template_locals.rs @@ -0,0 +1,226 @@ +//! A template local: `x = ` in a jbuilder template, read by the +//! statements after it. +//! +//! The assignment was an Unknown statement, so it became an empty +//! append. Next to a whole-template `json.array!` or `json.partial!` it +//! also made the template two statements long, so that form went down +//! the object path, where a whole-template form is dropped, and the +//! template rendered `{}`. In an object template the assignment was dropped and +//! the pair that reads it kept: a `NameError` when the template runs. +//! +//! Two layers: the emitted Ruby, and the templates rendered on CRuby +//! (the emitted view modules plus the runtime's `JsonBuilder`, with +//! plain Structs for records) against what Rails 8.1 + jbuilder 2.15 +//! render for the same templates and rows. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::process::Command; + +use roundhouse::emit::ruby; +use roundhouse::ingest::ingest_app_from_tree; + +const SCHEMA: &str = r#"ActiveRecord::Schema.define do + create_table "widgets", force: :cascade do |t| + t.string "name" + t.integer "size" + end +end +"#; + +const ROUTES: &str = r#"Rails.application.routes.draw do + get "widget_listed", to: "widgets#listed", defaults: { format: :json } + get "widget_summary", to: "widgets#summary", defaults: { format: :json } + get "widget_picked", to: "widgets#picked", defaults: { format: :json } + get "widget_linked", to: "widgets#linked", defaults: { format: :json } + resources :widgets, only: :show +end +"#; + +const CONTROLLER: &str = r#"class WidgetsController < ApplicationController + def listed + @widgets = Widget.all + render :listed + end + + def summary + @widgets = Widget.all + render :summary + end + + def picked + @widgets = Widget.all + render :picked + end + + def linked + @widgets = Widget.all + render :linked + end + + def show + @widget = Widget.find(params[:id]) + end +end +"#; + +const PARTIAL: &str = "json.id widget.id\njson.name widget.name\n"; + +/// A local, then a whole-template `array!` over it. +const LISTED: &str = r#"rows = @widgets.to_a +json.array! rows, partial: "widgets/widget", as: :widget +"#; + +/// A local read by two pairs. +const SUMMARY: &str = r#"count = @widgets.size +json.count count +json.empty count.zero? +"#; + +/// A local, then a whole-template `partial!` that passes it. +const PICKED: &str = r#"first = @widgets.first +json.partial! "widgets/widget", widget: first +"#; + +/// A local whose value is a route helper call. +const LINKED: &str = r#"first = @widgets.first +link = widget_url(first) +json.href link +"#; + +fn emitted() -> Vec<(String, String)> { + let files: HashMap> = [ + ("db/schema.rb", SCHEMA), + ("config/routes.rb", ROUTES), + ("app/models/application_record.rb", "class ApplicationRecord < ActiveRecord::Base\n primary_abstract_class\nend\n"), + ("app/models/widget.rb", "class Widget < ApplicationRecord\nend\n"), + ("app/controllers/application_controller.rb", "class ApplicationController < ActionController::Base\nend\n"), + ("app/controllers/widgets_controller.rb", CONTROLLER), + ("app/views/widgets/_widget.json.jbuilder", PARTIAL), + ("app/views/widgets/listed.json.jbuilder", LISTED), + ("app/views/widgets/summary.json.jbuilder", SUMMARY), + ("app/views/widgets/picked.json.jbuilder", PICKED), + ("app/views/widgets/linked.json.jbuilder", LINKED), + ] + .iter() + .map(|(p, c)| (PathBuf::from(p), c.as_bytes().to_vec())) + .collect(); + let mut app = ingest_app_from_tree(files).expect("ingest"); + roundhouse::session::analyze_and_lower(&mut app); + ruby::emit_lowered_jbuilder_views(&app) + .into_iter() + .map(|f| (f.path.to_string_lossy().into_owned(), f.content)) + .collect() +} + +fn view<'a>(files: &'a [(String, String)], suffix: &str) -> &'a str { + files + .iter() + .find(|(p, _)| p.ends_with(suffix)) + .map(|(_, c)| c.as_str()) + .unwrap_or_else(|| { + panic!( + "no emitted view ends with {suffix}; got {:?}", + files.iter().map(|(p, _)| p).collect::>() + ) + }) +} + +#[test] +fn every_emitted_view_parses() { + for (path, source) in emitted().iter().filter(|(p, _)| p.ends_with(".rb")) { + let result = ruby_prism::parse(source.as_bytes()); + let errors: Vec = result.errors().map(|e| e.message().to_string()).collect(); + assert!(errors.is_empty(), "{path} does not parse: {errors:?}\n{source}"); + } +} + +#[test] +fn the_local_is_kept_before_the_statements_that_read_it() { + let files = emitted(); + let src = view(&files, "widgets/listed_json.rb"); + let assign = src.find("rows = widgets.to_a").unwrap_or_else(|| panic!("the local:\n{src}")); + let array = src + .find("rows.map { |widget| Views::Widgets.widget_json(widget) }") + .unwrap_or_else(|| panic!("the whole-template array over it:\n{src}")); + assert!(assign < array, "the local comes first:\n{src}"); + let src = view(&files, "widgets/summary_json.rb"); + let assign = src.find("count = widgets.size").unwrap_or_else(|| panic!("the local:\n{src}")); + let pair = src.find("\\\"count\\\":").unwrap_or_else(|| panic!("the pair:\n{src}")); + assert!(assign < pair, "the local comes first:\n{src}"); + let src = view(&files, "widgets/picked_json.rb"); + let assign = src.find("first = widgets.first").unwrap_or_else(|| panic!("the local:\n{src}")); + let call = src + .find("io << Views::Widgets.widget_json(first)") + .unwrap_or_else(|| panic!("the whole-template partial call with it:\n{src}")); + assert!(assign < call, "the local comes first:\n{src}"); +} + +/// A local's value gets the rewrites a pair's value gets: the emitted +/// view has `RouteHelpers._path`, not `_url`. A value with no +/// helper in it (`summary`'s `widgets.size`) is kept as written. +#[test] +fn a_route_helper_in_a_local_is_rewritten() { + let files = emitted(); + let src = view(&files, "widgets/linked_json.rb"); + assert!( + src.contains("link = RouteHelpers.widget_path(first.id)"), + "the route helper is the runtime's path helper:\n{src}" + ); + assert!(!src.contains("widget_url"), "no `_url` helper is left:\n{src}"); +} + +/// Render the templates on CRuby and compare with what Rails 8.1.4 + +/// jbuilder 2.15.1 answer for the same rows (`b`, `a`, `c`). +#[test] +fn the_templates_render_what_jbuilder_renders() { + let files = emitted(); + let dir = std::env::temp_dir().join(format!( + "roundhouse-jbuilder-template-locals-{}", + std::process::id() + )); + // The emitted tree as it is laid out. `app/views.rb` is the views + // index a partial call requires; it holds nothing these templates + // need. + for (path, source) in files.iter().filter(|(p, _)| p.ends_with(".rb")) { + let file = dir.join(path); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + std::fs::write(&file, source).unwrap(); + } + std::fs::write(dir.join("app/views.rb"), "").unwrap(); + let mut requires = String::new(); + for name in ["_widget_json.rb", "listed_json.rb", "summary_json.rb", "picked_json.rb"] { + let file = dir.join("app/views/widgets").join(name); + requires.push_str(&format!("require {:?}\n", file.display().to_string())); + } + let runtime = Path::new(env!("CARGO_MANIFEST_DIR")).join("runtime/ruby/json_builder.rb"); + let script = format!( + r#"require {runtime:?} +{requires} +require "json" +Widget = Struct.new(:id, :name, :size) +widgets = [Widget.new(1, "b", 5), Widget.new(2, "a", nil), Widget.new(3, "c", nil)] +puts JSON.generate( + "listed" => JSON.parse(Views::Widgets.listed_json(widgets)), + "summary" => JSON.parse(Views::Widgets.summary_json(widgets)), + "picked" => JSON.parse(Views::Widgets.picked_json(widgets)), +) +"#, + runtime = runtime.display().to_string() + ); + let output = Command::new("ruby").args(["-e", &script]).output().unwrap(); + std::fs::remove_dir_all(&dir).unwrap(); + assert!( + output.status.success(), + "{}\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + // Each value is what the Rails app answers for that template. + let expected = concat!( + r#"{"listed":[{"id":1,"name":"b"},{"id":2,"name":"a"},{"id":3,"name":"c"}],"#, + r#""summary":{"count":3,"empty":false},"#, + r#""picked":{"id":1,"name":"b"}}"#, + ); + assert_eq!(String::from_utf8_lossy(&output.stdout).trim(), expected); +}