Skip to content

A bare literal in a template is not a dropped statement - #383

Merged
rubys merged 1 commit into
rubys:mainfrom
alextakitani:fix/view-walker-literal-statement
Oct 4, 2026
Merged

rubys merged 1 commit into
rubys:mainfrom
alextakitani:fix/view-walker-literal-statement

Conversation

@alextakitani

@alextakitani alextakitani commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Probed on main (26365fa).

The view walker's catch-all files a lower_residue line ("template statement dropped (unknown stmt)") so dropped template statements are visible. Every <% unless %> trips it with a false positive, and without a location:

<% unless @articles.empty? %>
  <p>listed</p>
<% end %>

unless reaches the walker as if cond then nil else … end. The then arm is a synthesized nil literal (synthetic span), so the catch-all reports it as a dropped statement with no file or line. The rendered output is already correct; only the ledger is wrong. In a real app (a Rails time tracker) these were the only "template statement dropped" lines left, and with no span there was nothing to follow.

A literal in statement position renders nothing and runs no side effect, so there is nothing to drop. It now lowers to the same io << "" without a ledger line (noop_io_append, split out of todo_io_append). Genuine drops still report, pinned by the existing an_unlowerable_statement_files_residue.

Tests (tests/view_walker_residue.rs): an_unless_block_files_no_residue_and_keeps_its_body and a_literal_statement_files_no_residue; both fail on main.

Ran: view_walker_residue (5), emit_and_run (107), lowered_ruby_emit (108), real_blog (6).

Summary by CodeRabbit

  • Bug Fixes
    • Literal statements and unless block bodies no longer produce unexpected residue in generated output.

`<% unless cond %>…<% end %>` reaches the view walker as
`if cond then nil else … end`. The `then` arm is a synthesized `nil`
with no span; it fell to the catch-all, which filed a span-less
"template statement dropped (unknown stmt)" for every `unless` in a
template. A literal in statement position renders nothing and runs
nothing, so nothing is lost: it now lowers to the same empty append
without a ledger line. The catch-all keeps reporting real drops.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
Generated by CodeRabbit — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fa90a0db-0e8d-4daa-a56e-83013adbe402
📥 Commits

Reviewing files that changed from the base of the PR and between 26365fa and 179addd.

📒 Files selected for processing (3)
  • src/lower/view_to_library/mod.rs
  • src/lower/view_to_library/walker.rs
  • tests/view_walker_residue.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Bare literal statements now lower to no-op appends instead of unknown-statement TODO appends. Regression tests check that unless bodies remain in output and that literal nil statements produce no residue.

Changes

Literal statement lowering

Layer / File(s) Summary
No-op append handling
src/lower/view_to_library/mod.rs, src/lower/view_to_library/walker.rs, tests/view_walker_residue.rs
todo_io_append returns a no-op append after its diagnostic call. The walker uses a no-op append for bare literal statements. Tests check unless output and confirm that unless and nil statements produce no residue.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: rubys

Merge Risk: ⚪ Minimal · up to 179ad

The change addresses false residue reports for bare literals while retaining diagnostics for unsupported statements. No material merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: treating bare literals in templates as statements rather than reporting them as dropped.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@rubys
rubys merged commit ef3e5d2 into rubys:main Oct 4, 2026
1 check passed
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.

2 participants