From cd0dc387134346062be579ee6f83f2524c49cfa5 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:06:38 +0200 Subject: [PATCH 01/26] Rust: Index templates Rails renders but the linter globs miss --- rust/herb-analysis/src/actionview_cli.rs | 23 +++++++++++++++++++++++ rust/herb-analysis/src/partial_index.rs | 9 ++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 84c9a2516..7975bb584 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -90,6 +90,12 @@ fn header(title: &str) { println!(); } +fn missing_format(file: &str) -> bool { + let name = file.rsplit('/').next().unwrap_or(file); + + name.ends_with(".erb") && name.matches('.').count() == 1 +} + fn plural(count: usize, word: &str) -> String { if count == 1 { word.to_string() @@ -230,6 +236,8 @@ fn check(arguments: &[String]) -> i32 { } } + let formatless: Vec = templates.iter().filter(|file| missing_format(file)).map(|file| relative(file, &root)).collect(); + let partials: Vec = templates .iter() .filter(|file| herb_analysis::partial_resolution::partial_path(file)) @@ -270,6 +278,21 @@ fn check(arguments: &[String]) -> i32 { println!(); + if !formatless.is_empty() { + println!(" {}", "Templates without a format:".bold()); + println!( + " {}", + "Rails reads a template filename as `name.format.handler`. Without a format it matches every one.".dimmed() + ); + println!(); + + for file in &formatless { + println!(" {} {}", "!".yellow().bold(), file.yellow()); + } + + println!(); + } + if !unresolved.is_empty() { println!(" {}", "Unresolved render calls:".bold()); println!(); diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 099ead5f2..151a23ae6 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -40,9 +40,16 @@ impl PartialIndex { let files = config.find_files_for_tool(herb_config::Tool::Linter, Some(project_path)); if !files.is_empty() { - let templates: Vec = files.into_iter().filter(|file| crate::partial_resolution::template_path(file)).collect(); + let mut templates: Vec = files.into_iter().filter(|file| crate::partial_resolution::template_path(file)).collect(); if !templates.is_empty() { + let known: std::collections::BTreeSet<&String> = templates.iter().collect(); + let extra: Vec = index.templates.iter().filter(|file| !known.contains(file)).cloned().collect(); + + templates.extend(extra); + templates.sort(); + templates.dedup(); + index.replace_templates(templates); } } From 3223257673ef92d4047793444079e3aa9eb1abfb Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:09:30 +0200 Subject: [PATCH 02/26] Rust: Resolve partials against Rails' ordered view paths --- rust/herb-analysis/src/partial_index.rs | 33 +++++-- rust/herb-analysis/src/partial_resolution.rs | 19 ++++ rust/herb-analysis/tests/view_roots_test.rs | 91 ++++++++++++++++++++ 3 files changed, 136 insertions(+), 7 deletions(-) create mode 100644 rust/herb-analysis/tests/view_roots_test.rs diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 151a23ae6..68d76f3cf 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -5,10 +5,10 @@ use std::path::{Path, PathBuf}; use herb::herb::{parse_with_options, ParserOptions}; use crate::partial_declaration::PartialDeclaration; -use crate::partial_resolution::{self, by_precedence, partial_name_for, template_path, view_root_for, APPLICATION_DIRECTORY}; +use crate::partial_resolution::{self, by_precedence, partial_name_for_roots, root_index_for, template_path, view_root_for, APPLICATION_DIRECTORY}; pub struct PartialIndex { - view_root: PathBuf, + view_roots: Vec, templates: Vec, by_name: BTreeMap>, declarations: BTreeMap, @@ -73,8 +73,12 @@ impl PartialIndex { } pub fn new(view_root: &Path, templates: Vec) -> Self { + Self::with_view_roots(&[view_root.to_path_buf()], templates) + } + + pub fn with_view_roots(view_roots: &[PathBuf], templates: Vec) -> Self { let mut index = Self { - view_root: view_root.to_path_buf(), + view_roots: view_roots.to_vec(), templates, by_name: BTreeMap::new(), declarations: BTreeMap::new(), @@ -84,6 +88,10 @@ impl PartialIndex { index } + fn root_strings(&self) -> Vec { + self.view_roots.iter().filter_map(|root| root.to_str().map(str::to_string)).collect() + } + fn rebuild(&mut self) { let mut by_name: BTreeMap> = BTreeMap::new(); @@ -95,15 +103,22 @@ impl PartialIndex { by_name.entry(name).or_default().push(file.clone()); } + let roots = self.root_strings(); + for files in by_name.values_mut() { by_precedence(files); + files.sort_by_key(|file| root_index_for(file, &roots)); } self.by_name = by_name; } pub fn view_root(&self) -> &Path { - &self.view_root + self.view_roots.first().map(PathBuf::as_path).unwrap_or_else(|| Path::new(".")) + } + + pub fn view_roots(&self) -> &[PathBuf] { + &self.view_roots } pub fn templates(&self) -> &[String] { @@ -138,7 +153,7 @@ impl PartialIndex { } pub fn partial_name_for(&self, file: &str) -> Option { - partial_name_for(file, self.view_root.to_str()?) + partial_name_for_roots(file, &self.root_strings()) } pub fn files_for(&self, partial_name: &str) -> &[String] { @@ -147,9 +162,13 @@ impl PartialIndex { fn source_directory_for(&self, source_file: &str) -> Option { let directory = Path::new(source_file).parent()?; - let relative = directory.strip_prefix(&self.view_root).ok()?; - Some(relative.to_str()?.to_string()) + self + .view_roots + .iter() + .find_map(|root| directory.strip_prefix(root).ok()) + .and_then(|relative| relative.to_str()) + .map(str::to_string) } pub fn resolve(&self, partial_name: &str, source_file: Option<&str>) -> &[String] { diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index 448e35d6a..d98cf0391 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -70,6 +70,13 @@ pub fn partial_path(file: &str) -> bool { name.starts_with(PARTIAL_PREFIX) && EXTENSIONS.iter().any(|extension| name.ends_with(extension)) } +pub fn relative_to_view_roots(path: &str, view_roots: &[String]) -> Option<(usize, String)> { + view_roots + .iter() + .enumerate() + .find_map(|(index, root)| relative_to_view_root(path, root).map(|relative| (index, relative))) +} + fn relative_to_view_root(path: &str, view_root: &str) -> Option { let normalized_path = normalize(path); let normalized_root = normalize(view_root); @@ -94,6 +101,18 @@ fn without_extension(name: &str) -> &str { } } +pub fn partial_name_for_roots(file: &str, view_roots: &[String]) -> Option { + view_roots.iter().find_map(|root| partial_name_for(file, root)) +} + +pub fn template_name_for_roots(file: &str, view_roots: &[String]) -> Option { + view_roots.iter().find_map(|root| template_name_for(file, root)) +} + +pub fn root_index_for(file: &str, view_roots: &[String]) -> usize { + relative_to_view_roots(file, view_roots).map(|(index, _)| index).unwrap_or(view_roots.len()) +} + pub fn partial_name_for(file: &str, view_root: &str) -> Option { if !partial_path(file) { return None; diff --git a/rust/herb-analysis/tests/view_roots_test.rs b/rust/herb-analysis/tests/view_roots_test.rs new file mode 100644 index 000000000..83f7ea599 --- /dev/null +++ b/rust/herb-analysis/tests/view_roots_test.rs @@ -0,0 +1,91 @@ +use std::fs; +use std::path::PathBuf; + +use herb_analysis::partial_index::PartialIndex; + +fn scratch(name: &str) -> PathBuf { + let root = std::env::temp_dir().join(format!("herb-view-roots-{name}")); + + let _ = fs::remove_dir_all(&root); + + root +} + +fn write(path: &PathBuf, body: &str) { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, body).unwrap(); +} + +#[test] +fn names_a_partial_from_a_secondary_view_root() { + let root = scratch("secondary"); + let app = root.join("app/views"); + let engine = root.join("engines/billing/app/views"); + + write(&app.join("home/index.html.erb"), "
\n"); + write(&engine.join("billing/_invoice.html.erb"), "
\n"); + + let templates = vec![ + app.join("home/index.html.erb").to_str().unwrap().to_string(), + engine.join("billing/_invoice.html.erb").to_str().unwrap().to_string(), + ]; + + let index = PartialIndex::with_view_roots(&[app.clone(), engine.clone()], templates); + + assert_eq!(vec!["billing/invoice"], index.names()); +} + +#[test] +fn an_earlier_view_root_shadows_a_later_one() { + let root = scratch("shadow"); + let app = root.join("app/views"); + let engine = root.join("engines/billing/app/views"); + + write(&app.join("billing/_invoice.html.erb"), "
app
\n"); + write(&engine.join("billing/_invoice.html.erb"), "
engine
\n"); + + let templates = vec![ + engine.join("billing/_invoice.html.erb").to_str().unwrap().to_string(), + app.join("billing/_invoice.html.erb").to_str().unwrap().to_string(), + ]; + + let index = PartialIndex::with_view_roots(&[app.clone(), engine.clone()], templates); + let resolved = index.resolve("billing/invoice", None); + + assert_eq!(2, resolved.len()); + assert!(resolved[0].starts_with(app.to_str().unwrap()), "app view path should win, got {}", resolved[0]); +} + +#[test] +fn resolves_a_sibling_within_the_root_that_owns_the_caller() { + let root = scratch("sibling"); + let app = root.join("app/views"); + let engine = root.join("engines/billing/app/views"); + + write(&engine.join("billing/index.html.erb"), "
\n"); + write(&engine.join("billing/_row.html.erb"), "
\n"); + + let templates = vec![ + engine.join("billing/index.html.erb").to_str().unwrap().to_string(), + engine.join("billing/_row.html.erb").to_str().unwrap().to_string(), + ]; + + let index = PartialIndex::with_view_roots(&[app, engine.clone()], templates); + let caller = engine.join("billing/index.html.erb").to_str().unwrap().to_string(); + + assert_eq!(1, index.resolve("row", Some(&caller)).len()); +} + +#[test] +fn a_single_root_behaves_as_before() { + let root = scratch("single"); + let app = root.join("app/views"); + + write(&app.join("shared/_header.html.erb"), "
\n"); + + let templates = vec![app.join("shared/_header.html.erb").to_str().unwrap().to_string()]; + let index = PartialIndex::new(&app, templates); + + assert_eq!(vec!["shared/header"], index.names()); + assert_eq!(1, index.resolve("shared/header", None).len()); +} From cf9bd6c694ce651fe1a3b26ced111ca9a90bce7a Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:13:57 +0200 Subject: [PATCH 03/26] Analysis: Resolve partials against Rails' ordered view paths --- lib/herb/analysis/partial_index.rb | 27 +++++++-- lib/herb/analysis/partial_resolution.rb | 28 +++++++++ sig/herb/analysis/partial_index.rbs | 7 ++- sig/herb/analysis/partial_resolution.rbs | 12 ++++ test/analysis/view_roots_test.rb | 75 ++++++++++++++++++++++++ 5 files changed, 141 insertions(+), 8 deletions(-) create mode 100644 test/analysis/view_roots_test.rb diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index cc4ad92c8..0c47853a7 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -28,14 +28,20 @@ def self.resolve_view_root(project_path) PartialResolution.view_root_for(project_path) end - #: (String | Pathname, Array[String]) -> void + #: (String | Pathname | Array[String | Pathname], Array[String]) -> void def initialize(view_root, templates) - @view_root = Pathname.new(view_root) + roots = view_root.is_a?(Array) ? view_root : [view_root] + + @view_roots = roots.map { |root| Pathname.new(root) } #: Array[Pathname] + @view_root = @view_roots.first || Pathname.new(".") @templates = templates @by_name = build_index(templates) @declarations = {} #: Hash[String, PartialDeclaration?] end + #: () -> Array[Pathname] + attr_reader :view_roots + #: (String?) -> Array[String] def files_for(partial_name) return [] unless partial_name @@ -73,7 +79,7 @@ def self.partial_name_for(file, view_root) #: (String) -> String? def partial_name_for(file) - self.class.partial_name_for(file, @view_root) + PartialResolution.partial_name_for_roots(file, @view_roots) end #: () -> Array[String] @@ -105,7 +111,9 @@ def update(file) @templates = (@templates | [file]).sort files = (@by_name[name] || []) | [file] - @by_name[name] = PartialResolution.by_precedence(files) + ordered = PartialResolution.by_precedence(files) + + @by_name[name] = ordered.sort_by { |candidate| PartialResolution.root_index_for(candidate, @view_roots) } name end @@ -152,7 +160,10 @@ def to_h #: (String) -> String? def source_directory_for(source_file) - Pathname.new(File.dirname(source_file)).relative_path_from(@view_root).to_s + directory = Pathname.new(File.dirname(source_file)) + root = @view_roots.find { |candidate| directory.to_s.start_with?(candidate.to_s) } || @view_root + + directory.relative_path_from(root).to_s rescue ArgumentError nil end @@ -181,7 +192,11 @@ def build_index(files) (map[name] ||= []) << file end - map.each_value { |files| files.replace(PartialResolution.by_precedence(files)) } + map.each_value do |candidates| + ordered = PartialResolution.by_precedence(candidates) + candidates.replace(ordered.sort_by { |file| PartialResolution.root_index_for(file, @view_roots) }) + end + map end end diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index 071c671d2..ff764a21a 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -142,6 +142,34 @@ def partial_name_for(file, view_root) directory == "." ? name : "#{directory}/#{name}" end + #: (String, Array[String | Pathname]) -> [Integer, String]? + def relative_to_view_roots(file, view_roots) + view_roots.each_with_index do |root, index| + relative = relative_to_view_root(file, root) + + return [index, relative] if relative + end + + nil + end + + #: (String, Array[String | Pathname]) -> String? + def partial_name_for_roots(file, view_roots) + view_roots.filter_map { |root| partial_name_for(file, root) }.first + end + + #: (String, Array[String | Pathname]) -> String? + def template_name_for_roots(file, view_roots) + view_roots.filter_map { |root| template_name_for(file, root) }.first + end + + #: (String, Array[String | Pathname]) -> Integer + def root_index_for(file, view_roots) + found = relative_to_view_roots(file, view_roots) + + found ? found[0] : view_roots.size + end + private #: (String, String | Pathname) -> String? diff --git a/sig/herb/analysis/partial_index.rbs b/sig/herb/analysis/partial_index.rbs index 23fdc9d3c..b18304517 100644 --- a/sig/herb/analysis/partial_index.rbs +++ b/sig/herb/analysis/partial_index.rbs @@ -15,8 +15,11 @@ module Herb # : (String | Pathname) -> Pathname def self.resolve_view_root: (String | Pathname) -> Pathname - # : (String | Pathname, Array[String]) -> void - def initialize: (String | Pathname, Array[String]) -> void + # : (String | Pathname | Array[String | Pathname], Array[String]) -> void + def initialize: (String | Pathname | Array[String | Pathname], Array[String]) -> void + + # : () -> Array[Pathname] + attr_reader view_roots: untyped # : (String?) -> Array[String] def files_for: (String?) -> Array[String] diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index 44444f5c3..fe9eb11ee 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -50,6 +50,18 @@ module Herb # : (String, String | Pathname) -> String? def self.partial_name_for: (String, String | Pathname) -> String? + # : (String, Array[String | Pathname]) -> [Integer, String]? + def self.relative_to_view_roots: (String, Array[String | Pathname]) -> [ Integer, String ]? + + # : (String, Array[String | Pathname]) -> String? + def self.partial_name_for_roots: (String, Array[String | Pathname]) -> String? + + # : (String, Array[String | Pathname]) -> String? + def self.template_name_for_roots: (String, Array[String | Pathname]) -> String? + + # : (String, Array[String | Pathname]) -> Integer + def self.root_index_for: (String, Array[String | Pathname]) -> Integer + # : (String, String | Pathname) -> String? private def self.relative_to_view_root: (String, String | Pathname) -> String? end diff --git a/test/analysis/view_roots_test.rb b/test/analysis/view_roots_test.rb new file mode 100644 index 000000000..753830215 --- /dev/null +++ b/test/analysis/view_roots_test.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require_relative "../test_helper" +require_relative "../../lib/herb/analysis/partial_index" + +require "tmpdir" + +module Analysis + class ViewRootsTest < Minitest::Spec + def write(root, relative, body = "
\n") + path = File.join(root, relative) + + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, body) + + path + end + + test "names a partial from a secondary view root" do + Dir.mktmpdir do |dir| + app = File.join(dir, "app", "views") + engine = File.join(dir, "engines", "billing", "app", "views") + + entry = write(app, "home/index.html.erb") + invoice = write(engine, "billing/_invoice.html.erb") + + index = Herb::Analysis::PartialIndex.new([app, engine], [entry, invoice]) + + assert_equal ["billing/invoice"], index.names + end + end + + test "an earlier view root shadows a later one" do + Dir.mktmpdir do |dir| + app = File.join(dir, "app", "views") + engine = File.join(dir, "engines", "billing", "app", "views") + + engine_invoice = write(engine, "billing/_invoice.html.erb", "
engine
\n") + app_invoice = write(app, "billing/_invoice.html.erb", "
app
\n") + + index = Herb::Analysis::PartialIndex.new([app, engine], [engine_invoice, app_invoice]) + resolved = index.resolve("billing/invoice", nil) + + assert_equal 2, resolved.size + assert_equal app_invoice, resolved.first + end + end + + test "resolves a sibling within the root that owns the caller" do + Dir.mktmpdir do |dir| + app = File.join(dir, "app", "views") + engine = File.join(dir, "engines", "billing", "app", "views") + + entry = write(engine, "billing/index.html.erb") + row = write(engine, "billing/_row.html.erb") + + index = Herb::Analysis::PartialIndex.new([app, engine], [entry, row]) + + assert_equal [row], index.resolve("row", entry) + end + end + + test "a single root behaves as before" do + Dir.mktmpdir do |dir| + app = File.join(dir, "app", "views") + header = write(app, "shared/_header.html.erb") + + index = Herb::Analysis::PartialIndex.new(app, [header]) + + assert_equal ["shared/header"], index.names + assert_equal [header], index.resolve("shared/header", nil) + end + end + end +end From 4eb5e6ab01d00b615b7e049c117347e61c59d847 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:22:04 +0200 Subject: [PATCH 04/26] Analysis: Resolve partials against Rails' ordered view paths in TypeScript --- .../packages/analysis/src/partial-index.ts | 14 ++-- .../analysis/src/partial-resolution.ts | 44 +++++++++++- .../packages/analysis/test/view-roots.test.ts | 72 +++++++++++++++++++ 3 files changed, 122 insertions(+), 8 deletions(-) create mode 100644 javascript/packages/analysis/test/view-roots.test.ts diff --git a/javascript/packages/analysis/src/partial-index.ts b/javascript/packages/analysis/src/partial-index.ts index b06e7d961..bb9b25bac 100644 --- a/javascript/packages/analysis/src/partial-index.ts +++ b/javascript/packages/analysis/src/partial-index.ts @@ -1,5 +1,5 @@ import { isERBStrictLocalsNode, isRubyParameterNode } from "@herb-tools/core" -import { PARTIAL_EXTENSIONS, partialNameForFile, resolvePartial } from "./partial-resolution" +import { PARTIAL_EXTENSIONS, partialNameForRoots, resolvePartial } from "./partial-resolution" import type { DocumentNode } from "@herb-tools/core" import type { PartialPaths } from "./partial-resolution" @@ -91,6 +91,7 @@ export function declarationFromDocument(document: DocumentNode, file: string): P export class PartialIndex { readonly viewRoot: string + readonly viewRoots: string[] private readonly declarations: Map private readonly files: PartialPaths @@ -100,8 +101,9 @@ export class PartialIndex { return new PartialIndex(data.viewRoot, new Map(Object.entries(data.partials))) } - constructor(viewRoot: string, declarations: Map) { - this.viewRoot = viewRoot + constructor(viewRoot: string | string[], declarations: Map) { + this.viewRoots = Array.isArray(viewRoot) ? viewRoot : [viewRoot] + this.viewRoot = this.viewRoots[0] ?? "." this.declarations = declarations this.files = new Map() this.byFile = new Map() @@ -117,7 +119,7 @@ export class PartialIndex { } lookup(partialName: string, sourceFile: string | undefined): PartialDeclaration | null { - const file = resolvePartial(partialName, sourceFile ?? "", this.files, this.viewRoot) + const file = resolvePartial(partialName, sourceFile ?? "", this.files, this.viewRoots) if (file === null) return null @@ -125,7 +127,7 @@ export class PartialIndex { } update(declaration: PartialDeclaration): string | null { - const name = partialNameForFile(declaration.file, this.viewRoot) + const name = partialNameForRoots(declaration.file, this.viewRoots) if (name === null) return null const existing = this.declarations.get(name) @@ -144,7 +146,7 @@ export class PartialIndex { } remove(file: string): string | null { - const name = partialNameForFile(file, this.viewRoot) + const name = partialNameForRoots(file, this.viewRoots) if (name === null) return null const existing = this.declarations.get(name) diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index d20a08ad1..9943278e2 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -50,6 +50,40 @@ function relativeToViewRoot(path: string, viewRoot: string): string | null { return normalizedPath.slice(normalizedRoot.length + 1) } +export function relativeToViewRoots(path: string, viewRoots: string[]): [number, string] | null { + for (const [index, root] of viewRoots.entries()) { + const relative = relativeToViewRoot(path, root) + + if (relative !== null) return [index, relative] + } + + return null +} + +export function partialNameForRoots(filePath: string, viewRoots: string[]): string | null { + for (const root of viewRoots) { + const name = partialNameForFile(filePath, root) + + if (name !== null) return name + } + + return null +} + +export function templateNameForRoots(filePath: string, viewRoots: string[]): string | null { + for (const root of viewRoots) { + const name = templateNameForFile(filePath, root) + + if (name !== null) return name + } + + return null +} + +export function rootIndexFor(filePath: string, viewRoots: string[]): number { + return relativeToViewRoots(filePath, viewRoots)?.[0] ?? viewRoots.length +} + export function projectRelativePath(filePath: string, projectPath: string | undefined): string { if (!projectPath) return normalize(filePath) @@ -136,12 +170,18 @@ export function layoutCandidatesFor(templateFile: string, viewRoot: string): str return candidates } -export function resolvePartial(partialName: string, sourceFile: string, index: PartialPaths, viewRoot: string): string | null { +export function resolvePartial( + partialName: string, + sourceFile: string, + index: PartialPaths, + viewRoot: string | string[] +): string | null { + const viewRoots = Array.isArray(viewRoot) ? viewRoot : [viewRoot] const exact = index.get(partialName) if (exact !== undefined) return exact - const sourceDirectory = relativeToViewRoot(dirname(normalize(sourceFile)), viewRoot) + const sourceDirectory = relativeToViewRoots(dirname(normalize(sourceFile)), viewRoots)?.[1] ?? null if (sourceDirectory !== null && sourceDirectory !== ".") { const relative = index.get(`${sourceDirectory}/${partialName}`) diff --git a/javascript/packages/analysis/test/view-roots.test.ts b/javascript/packages/analysis/test/view-roots.test.ts new file mode 100644 index 000000000..b710eabe3 --- /dev/null +++ b/javascript/packages/analysis/test/view-roots.test.ts @@ -0,0 +1,72 @@ +import { describe, test, expect } from "vitest" + +import { + partialNameForRoots, + relativeToViewRoots, + resolvePartial, + rootIndexFor, + templateNameForRoots, +} from "../src/partial-resolution" + +import type { PartialPaths } from "../src/partial-resolution" + +const APP = "app/views" +const ENGINE = "engines/billing/app/views" +const ROOTS = [APP, ENGINE] + +describe("relativeToViewRoots", () => { + test("returns the first root that contains the file", () => { + expect(relativeToViewRoots(`${APP}/home/index.html.erb`, ROOTS)).toEqual([0, "home/index.html.erb"]) + expect(relativeToViewRoots(`${ENGINE}/billing/_invoice.html.erb`, ROOTS)).toEqual([1, "billing/_invoice.html.erb"]) + }) + + test("returns null when no root contains the file", () => { + expect(relativeToViewRoots("lib/elsewhere/_thing.html.erb", ROOTS)).toBeNull() + }) +}) + +describe("partialNameForRoots", () => { + test("names a partial from a secondary view root", () => { + expect(partialNameForRoots(`${ENGINE}/billing/_invoice.html.erb`, ROOTS)).toBe("billing/invoice") + }) + + test("names a partial from the primary view root", () => { + expect(partialNameForRoots(`${APP}/shared/_header.html.erb`, ROOTS)).toBe("shared/header") + }) + + test("returns null for a file outside every root", () => { + expect(partialNameForRoots("lib/_thing.html.erb", ROOTS)).toBeNull() + }) +}) + +describe("templateNameForRoots", () => { + test("names a template from a secondary view root", () => { + expect(templateNameForRoots(`${ENGINE}/billing/index.html.erb`, ROOTS)).toBe("billing/index") + }) +}) + +describe("rootIndexFor", () => { + test("orders an earlier view root ahead of a later one", () => { + expect(rootIndexFor(`${APP}/billing/_invoice.html.erb`, ROOTS)).toBe(0) + expect(rootIndexFor(`${ENGINE}/billing/_invoice.html.erb`, ROOTS)).toBe(1) + }) + + test("sorts an unknown file last", () => { + expect(rootIndexFor("lib/_thing.html.erb", ROOTS)).toBe(ROOTS.length) + }) +}) + +describe("resolvePartial", () => { + test("resolves a sibling within the root that owns the caller", () => { + const index: PartialPaths = new Map([["billing/row", `${ENGINE}/billing/_row.html.erb`]]) + const caller = `${ENGINE}/billing/index.html.erb` + + expect(resolvePartial("row", caller, index, ROOTS)).toBe(`${ENGINE}/billing/_row.html.erb`) + }) + + test("accepts a single root", () => { + const index: PartialPaths = new Map([["shared/header", `${APP}/shared/_header.html.erb`]]) + + expect(resolvePartial("shared/header", "", index, APP)).toBe(`${APP}/shared/_header.html.erb`) + }) +}) From 9d645d58a8c0cea3deb48c5425b843255be8ee38 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:29:52 +0200 Subject: [PATCH 05/26] Analysis: Take view roots as a list in every binding --- .../analysis/src/partial-index-builder.ts | 2 +- .../packages/analysis/src/partial-index.ts | 6 +++--- .../analysis/src/partial-resolution.ts | 3 +-- .../analysis/test/partial-index.test.ts | 8 ++++---- .../analysis/test/partial-resolution.test.ts | 20 +++++++++---------- .../packages/analysis/test/view-roots.test.ts | 4 ++-- lib/herb/analysis/partial_index.rb | 10 ++++------ lib/herb/analysis/template_dependencies.rb | 2 +- rust/herb-analysis/src/partial_index.rs | 8 ++------ rust/herb-analysis/tests/view_roots_test.rs | 10 +++++----- sig/herb/analysis/partial_index.rbs | 4 ++-- test/analysis/view_roots_test.rb | 4 ++-- 12 files changed, 37 insertions(+), 44 deletions(-) diff --git a/javascript/packages/analysis/src/partial-index-builder.ts b/javascript/packages/analysis/src/partial-index-builder.ts index 0039fb330..2a2bfd84d 100644 --- a/javascript/packages/analysis/src/partial-index-builder.ts +++ b/javascript/packages/analysis/src/partial-index-builder.ts @@ -58,7 +58,7 @@ export async function buildPartialIndex(herb: HerbBackend, projectPath: string): if (declaration) declarations.set(name, declaration) } - return new PartialIndex(viewRoot, declarations) + return new PartialIndex([viewRoot], declarations) } export function partialIndexFrom(data: SerializedPartialIndex | undefined): PartialIndex | undefined { diff --git a/javascript/packages/analysis/src/partial-index.ts b/javascript/packages/analysis/src/partial-index.ts index bb9b25bac..23ecfc133 100644 --- a/javascript/packages/analysis/src/partial-index.ts +++ b/javascript/packages/analysis/src/partial-index.ts @@ -98,11 +98,11 @@ export class PartialIndex { private readonly byFile: Map static from(data: SerializedPartialIndex): PartialIndex { - return new PartialIndex(data.viewRoot, new Map(Object.entries(data.partials))) + return new PartialIndex([data.viewRoot], new Map(Object.entries(data.partials))) } - constructor(viewRoot: string | string[], declarations: Map) { - this.viewRoots = Array.isArray(viewRoot) ? viewRoot : [viewRoot] + constructor(viewRoots: string[], declarations: Map) { + this.viewRoots = viewRoots this.viewRoot = this.viewRoots[0] ?? "." this.declarations = declarations this.files = new Map() diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 9943278e2..db8cd4909 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -174,9 +174,8 @@ export function resolvePartial( partialName: string, sourceFile: string, index: PartialPaths, - viewRoot: string | string[] + viewRoots: string[] ): string | null { - const viewRoots = Array.isArray(viewRoot) ? viewRoot : [viewRoot] const exact = index.get(partialName) if (exact !== undefined) return exact diff --git a/javascript/packages/analysis/test/partial-index.test.ts b/javascript/packages/analysis/test/partial-index.test.ts index b708507ac..d5d55eb19 100644 --- a/javascript/packages/analysis/test/partial-index.test.ts +++ b/javascript/packages/analysis/test/partial-index.test.ts @@ -15,7 +15,7 @@ beforeAll(async () => { }) describe("PartialIndex", () => { - const index = new PartialIndex("app/views", new Map([ + const index = new PartialIndex(["app/views"], new Map([ ["users/card", declaration("app/views/users/_card.html.erb", [{ name: "user", required: true }])], ["application/flash", declaration("app/views/application/_flash.html.erb", [{ name: "message", required: true }])], ])) @@ -57,7 +57,7 @@ describe("PartialIndex", () => { describe("PartialIndex updates", () => { function index(): PartialIndex { - return new PartialIndex("app/views", new Map([ + return new PartialIndex(["app/views"], new Map([ ["users/card", declaration("app/views/users/_card.html.erb", [{ name: "user", required: true }])], ])) } @@ -120,7 +120,7 @@ describe("PartialIndex updates", () => { }) test("lets the base template take over from a variant", () => { - const partials = new PartialIndex("app/views", new Map([ + const partials = new PartialIndex(["app/views"], new Map([ ["users/card", declaration("app/views/users/_card.en.html.erb", [{ name: "user", required: true }])], ])) @@ -130,7 +130,7 @@ describe("PartialIndex updates", () => { }) test("forgets the displaced variant when the base template takes over", () => { - const partials = new PartialIndex("app/views", new Map([ + const partials = new PartialIndex(["app/views"], new Map([ ["users/card", declaration("app/views/users/_card.html+phone.erb", [])], ])) diff --git a/javascript/packages/analysis/test/partial-resolution.test.ts b/javascript/packages/analysis/test/partial-resolution.test.ts index 1e5f9ff12..286df28b8 100644 --- a/javascript/packages/analysis/test/partial-resolution.test.ts +++ b/javascript/packages/analysis/test/partial-resolution.test.ts @@ -126,45 +126,45 @@ describe("@herb-tools/core", () => { ]) test("resolves a fully qualified name", () => { - expect(resolvePartial("users/card", "app/views/posts/index.html.erb", index, VIEW_ROOT)).toBe("app/views/users/_card.html.erb") + expect(resolvePartial("users/card", "app/views/posts/index.html.erb", index, [VIEW_ROOT])).toBe("app/views/users/_card.html.erb") }) test("resolves a bare name against the rendering template's directory", () => { - expect(resolvePartial("avatar", "app/views/users/show.html.erb", index, VIEW_ROOT)).toBe("app/views/users/_avatar.html.erb") + expect(resolvePartial("avatar", "app/views/users/show.html.erb", index, [VIEW_ROOT])).toBe("app/views/users/_avatar.html.erb") }) test("falls back to the application directory for a bare name", () => { - expect(resolvePartial("flash", "app/views/posts/index.html.erb", index, VIEW_ROOT)).toBe("app/views/application/_flash.html.erb") + expect(resolvePartial("flash", "app/views/posts/index.html.erb", index, [VIEW_ROOT])).toBe("app/views/application/_flash.html.erb") }) test("prefers the exact name over the relative one", () => { - expect(resolvePartial("users/card", "app/views/admin/index.html.erb", index, VIEW_ROOT)).toBe("app/views/users/_card.html.erb") + expect(resolvePartial("users/card", "app/views/admin/index.html.erb", index, [VIEW_ROOT])).toBe("app/views/users/_card.html.erb") }) test("resolves a qualified name relative to the rendering template's directory", () => { const nested = paths(["app/views/admin/users/_card.html.erb"]) - expect(resolvePartial("users/card", "app/views/admin/index.html.erb", nested, VIEW_ROOT)).toBe("app/views/admin/users/_card.html.erb") + expect(resolvePartial("users/card", "app/views/admin/index.html.erb", nested, [VIEW_ROOT])).toBe("app/views/admin/users/_card.html.erb") }) test("does not fall back to the application directory for a qualified name", () => { - expect(resolvePartial("users/flash", "app/views/posts/index.html.erb", index, VIEW_ROOT)).toBeNull() + expect(resolvePartial("users/flash", "app/views/posts/index.html.erb", index, [VIEW_ROOT])).toBeNull() }) test("returns null for an unknown partial", () => { - expect(resolvePartial("users/missing", "app/views/posts/index.html.erb", index, VIEW_ROOT)).toBeNull() + expect(resolvePartial("users/missing", "app/views/posts/index.html.erb", index, [VIEW_ROOT])).toBeNull() }) test("resolves from a template at the view root", () => { - expect(resolvePartial("flash", "app/views/index.html.erb", index, VIEW_ROOT)).toBe("app/views/application/_flash.html.erb") + expect(resolvePartial("flash", "app/views/index.html.erb", index, [VIEW_ROOT])).toBe("app/views/application/_flash.html.erb") }) test("resolves from a source file outside the view root", () => { - expect(resolvePartial("users/card", "app/components/card_component.html.erb", index, VIEW_ROOT)).toBe("app/views/users/_card.html.erb") + expect(resolvePartial("users/card", "app/components/card_component.html.erb", index, [VIEW_ROOT])).toBe("app/views/users/_card.html.erb") }) test("resolves without a known source file", () => { - expect(resolvePartial("users/card", "", index, VIEW_ROOT)).toBe("app/views/users/_card.html.erb") + expect(resolvePartial("users/card", "", index, [VIEW_ROOT])).toBe("app/views/users/_card.html.erb") }) }) }) diff --git a/javascript/packages/analysis/test/view-roots.test.ts b/javascript/packages/analysis/test/view-roots.test.ts index b710eabe3..bd598905a 100644 --- a/javascript/packages/analysis/test/view-roots.test.ts +++ b/javascript/packages/analysis/test/view-roots.test.ts @@ -64,9 +64,9 @@ describe("resolvePartial", () => { expect(resolvePartial("row", caller, index, ROOTS)).toBe(`${ENGINE}/billing/_row.html.erb`) }) - test("accepts a single root", () => { + test("resolves with a single root", () => { const index: PartialPaths = new Map([["shared/header", `${APP}/shared/_header.html.erb`]]) - expect(resolvePartial("shared/header", "", index, APP)).toBe(`${APP}/shared/_header.html.erb`) + expect(resolvePartial("shared/header", "", index, [APP])).toBe(`${APP}/shared/_header.html.erb`) }) }) diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index 0c47853a7..5ee59935f 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -20,7 +20,7 @@ def self.build(project_path, templates: nil) view_root = resolve_view_root(root) files = templates || Dir[view_root.join("**", PartialResolution::TEMPLATE_GLOB_PATTERN)].sort - new(view_root, files) + new([view_root], files) end #: (String | Pathname) -> Pathname @@ -28,11 +28,9 @@ def self.resolve_view_root(project_path) PartialResolution.view_root_for(project_path) end - #: (String | Pathname | Array[String | Pathname], Array[String]) -> void - def initialize(view_root, templates) - roots = view_root.is_a?(Array) ? view_root : [view_root] - - @view_roots = roots.map { |root| Pathname.new(root) } #: Array[Pathname] + #: (Array[String | Pathname], Array[String]) -> void + def initialize(view_roots, templates) + @view_roots = view_roots.map { |root| Pathname.new(root) } #: Array[Pathname] @view_root = @view_roots.first || Pathname.new(".") @templates = templates @by_name = build_index(templates) diff --git a/lib/herb/analysis/template_dependencies.rb b/lib/herb/analysis/template_dependencies.rb index 597c82b0c..20f3a1bd9 100644 --- a/lib/herb/analysis/template_dependencies.rb +++ b/lib/herb/analysis/template_dependencies.rb @@ -376,7 +376,7 @@ def trace_state(entry_point, state) return nil unless entry_result return nil unless entry_result.instance_variables.include?(state) || entry_result.constants.include?(state) - index = PartialIndex.new(@view_root, reachable) + index = PartialIndex.new([@view_root], reachable) affected = Set.new([entry_point]) #: Set[String] state_locals = {} #: Hash[String, Set[String]] diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 68d76f3cf..77898ce2d 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -65,18 +65,14 @@ impl PartialIndex { collect_templates(&view_root, &mut templates); templates.sort(); - Self::new(&view_root, templates) + Self::new(&[view_root], templates) } pub fn resolve_view_root(project_path: &Path) -> PathBuf { view_root_for(project_path) } - pub fn new(view_root: &Path, templates: Vec) -> Self { - Self::with_view_roots(&[view_root.to_path_buf()], templates) - } - - pub fn with_view_roots(view_roots: &[PathBuf], templates: Vec) -> Self { + pub fn new(view_roots: &[PathBuf], templates: Vec) -> Self { let mut index = Self { view_roots: view_roots.to_vec(), templates, diff --git a/rust/herb-analysis/tests/view_roots_test.rs b/rust/herb-analysis/tests/view_roots_test.rs index 83f7ea599..223083c56 100644 --- a/rust/herb-analysis/tests/view_roots_test.rs +++ b/rust/herb-analysis/tests/view_roots_test.rs @@ -30,7 +30,7 @@ fn names_a_partial_from_a_secondary_view_root() { engine.join("billing/_invoice.html.erb").to_str().unwrap().to_string(), ]; - let index = PartialIndex::with_view_roots(&[app.clone(), engine.clone()], templates); + let index = PartialIndex::new(&[app.clone(), engine.clone()], templates); assert_eq!(vec!["billing/invoice"], index.names()); } @@ -49,7 +49,7 @@ fn an_earlier_view_root_shadows_a_later_one() { app.join("billing/_invoice.html.erb").to_str().unwrap().to_string(), ]; - let index = PartialIndex::with_view_roots(&[app.clone(), engine.clone()], templates); + let index = PartialIndex::new(&[app.clone(), engine.clone()], templates); let resolved = index.resolve("billing/invoice", None); assert_eq!(2, resolved.len()); @@ -70,21 +70,21 @@ fn resolves_a_sibling_within_the_root_that_owns_the_caller() { engine.join("billing/_row.html.erb").to_str().unwrap().to_string(), ]; - let index = PartialIndex::with_view_roots(&[app, engine.clone()], templates); + let index = PartialIndex::new(&[app, engine.clone()], templates); let caller = engine.join("billing/index.html.erb").to_str().unwrap().to_string(); assert_eq!(1, index.resolve("row", Some(&caller)).len()); } #[test] -fn a_single_root_behaves_as_before() { +fn a_single_root_still_resolves() { let root = scratch("single"); let app = root.join("app/views"); write(&app.join("shared/_header.html.erb"), "
\n"); let templates = vec![app.join("shared/_header.html.erb").to_str().unwrap().to_string()]; - let index = PartialIndex::new(&app, templates); + let index = PartialIndex::new(&[app.clone()], templates); assert_eq!(vec!["shared/header"], index.names()); assert_eq!(1, index.resolve("shared/header", None).len()); diff --git a/sig/herb/analysis/partial_index.rbs b/sig/herb/analysis/partial_index.rbs index b18304517..a8edb36a0 100644 --- a/sig/herb/analysis/partial_index.rbs +++ b/sig/herb/analysis/partial_index.rbs @@ -15,8 +15,8 @@ module Herb # : (String | Pathname) -> Pathname def self.resolve_view_root: (String | Pathname) -> Pathname - # : (String | Pathname | Array[String | Pathname], Array[String]) -> void - def initialize: (String | Pathname | Array[String | Pathname], Array[String]) -> void + # : (Array[String | Pathname], Array[String]) -> void + def initialize: (Array[String | Pathname], Array[String]) -> void # : () -> Array[Pathname] attr_reader view_roots: untyped diff --git a/test/analysis/view_roots_test.rb b/test/analysis/view_roots_test.rb index 753830215..e0bb61827 100644 --- a/test/analysis/view_roots_test.rb +++ b/test/analysis/view_roots_test.rb @@ -60,12 +60,12 @@ def write(root, relative, body = "
\n") end end - test "a single root behaves as before" do + test "a single root still resolves" do Dir.mktmpdir do |dir| app = File.join(dir, "app", "views") header = write(app, "shared/_header.html.erb") - index = Herb::Analysis::PartialIndex.new(app, [header]) + index = Herb::Analysis::PartialIndex.new([app], [header]) assert_equal ["shared/header"], index.names assert_equal [header], index.resolve("shared/header", nil) From 8c4affd821dedf0655014e49999c649a3ebf65ac Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:44:29 +0200 Subject: [PATCH 06/26] Analysis: Drop the singular view root --- javascript/packages/analysis/src/partial-index.ts | 8 +++----- .../packages/analysis/src/partial-resolution.ts | 10 ++++++++++ .../packages/analysis/src/render-graph-builder.ts | 12 ++++++------ javascript/packages/analysis/test/view-roots.test.ts | 11 +++++++++++ lib/herb/analysis/partial_index.rb | 2 -- lib/herb/analysis/partial_resolution.rb | 5 +++++ lib/herb/analysis/project_index.rb | 4 ++-- lib/herb/analysis/render_graph/builder.rb | 6 +++--- rust/herb-analysis/src/actionview_cli.rs | 12 ++++++++---- rust/herb-analysis/src/partial_index.rs | 4 ---- rust/herb-analysis/src/partial_resolution.rs | 8 ++++++++ rust/herb-analysis/src/project_index.rs | 4 ++-- rust/herb-analysis/src/render_graph_builder.rs | 10 ++++------ rust/herb-analysis/tests/partial_index_test.rs | 6 +++--- rust/herb-analysis/tests/project_index_test.rs | 2 +- rust/herb-analysis/tests/view_roots_test.rs | 2 +- sig/herb/analysis/partial_index.rbs | 2 -- sig/herb/analysis/partial_resolution.rbs | 3 +++ sig/herb/analysis/project_index.rbs | 2 +- test/analysis/partial_index_test.rb | 4 ++-- test/analysis/project_index_test.rb | 2 +- 21 files changed, 74 insertions(+), 45 deletions(-) diff --git a/javascript/packages/analysis/src/partial-index.ts b/javascript/packages/analysis/src/partial-index.ts index 23ecfc133..2cf2dfee6 100644 --- a/javascript/packages/analysis/src/partial-index.ts +++ b/javascript/packages/analysis/src/partial-index.ts @@ -23,7 +23,7 @@ export interface PartialDeclaration { } export interface SerializedPartialIndex { - viewRoot: string + viewRoots: string[] partials: Record } @@ -90,7 +90,6 @@ export function declarationFromDocument(document: DocumentNode, file: string): P } export class PartialIndex { - readonly viewRoot: string readonly viewRoots: string[] private readonly declarations: Map @@ -98,12 +97,11 @@ export class PartialIndex { private readonly byFile: Map static from(data: SerializedPartialIndex): PartialIndex { - return new PartialIndex([data.viewRoot], new Map(Object.entries(data.partials))) + return new PartialIndex(data.viewRoots, new Map(Object.entries(data.partials))) } constructor(viewRoots: string[], declarations: Map) { this.viewRoots = viewRoots - this.viewRoot = this.viewRoots[0] ?? "." this.declarations = declarations this.files = new Map() this.byFile = new Map() @@ -164,6 +162,6 @@ export class PartialIndex { } toJSON(): SerializedPartialIndex { - return { viewRoot: this.viewRoot, partials: Object.fromEntries(this.declarations) } + return { viewRoots: this.viewRoots, partials: Object.fromEntries(this.declarations) } } } diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index db8cd4909..3b45e5b5c 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -145,6 +145,16 @@ export function templateNameForFile(filePath: string, viewRoot: string): string return directory === "." ? withoutExtension : `${directory}/${withoutExtension}` } +export function layoutCandidatesForRoots(templateFile: string, viewRoots: string[]): string[] { + for (const root of viewRoots) { + const candidates = layoutCandidatesFor(templateFile, root) + + if (candidates.length > 0) return candidates + } + + return [] +} + export function layoutCandidatesFor(templateFile: string, viewRoot: string): string[] { const relative = relativeToViewRoot(normalize(templateFile), viewRoot) diff --git a/javascript/packages/analysis/src/render-graph-builder.ts b/javascript/packages/analysis/src/render-graph-builder.ts index bbc6431f5..1c36ad3c9 100644 --- a/javascript/packages/analysis/src/render-graph-builder.ts +++ b/javascript/packages/analysis/src/render-graph-builder.ts @@ -6,7 +6,7 @@ import { readFileSync } from "node:fs" import { getTagLocalName, isERBCaseNode, isERBIfNode, isERBOutputNode, isERBRenderNode, isERBUnlessNode, isHTMLElementNode, isPrismNodeType, isRubyRenderLocalNode } from "@herb-tools/core" import { outranksTemplate } from "./partial-index" -import { layoutCandidatesFor, templateNameForFile, isPartialPath } from "./partial-resolution" +import { layoutCandidatesForRoots, templateNameForRoots, isPartialPath } from "./partial-resolution" import { renderPartialExpression } from "./render-expression" import { staticAncestorAttributes } from "./ancestor-attributes" @@ -227,11 +227,11 @@ export function collectCallSites(herb: HerbBackend, partials: PartialIndex, file return { unresolved, isDocumentRoot, yields, roots: { ...roots, renders: rootRenders, resolved: rootsResolved } } } -function addLayoutCallSites(files: string[], layoutYields: Map, viewRoot: string, callSites: Map): void { +function addLayoutCallSites(files: string[], layoutYields: Map, viewRoots: string[], callSites: Map): void { const layouts = new Map() for (const file of files) { - const name = templateNameForFile(file, viewRoot) + const name = templateNameForRoots(file, viewRoots) if (name === null || !layoutYields.has(file)) { continue @@ -247,7 +247,7 @@ function addLayoutCallSites(files: string[], layoutYields: Map { - const files = await templatesIn(projectPath, partials.viewRoot, options.include ?? []) + const files = await templatesIn(projectPath, partials.viewRoots[0] ?? ".", options.include ?? []) const excluded = options.exclude?.length ? picomatch(options.exclude) : null const callSites = new Map() const documentRoots = new Set() @@ -318,7 +318,7 @@ export async function buildRenderGraph(herb: HerbBackend, projectPath: string, p } if (options.resolveLayouts !== false) { - addLayoutCallSites(scanned, layoutYields, partials.viewRoot, callSites) + addLayoutCallSites(scanned, layoutYields, partials.viewRoots, callSites) } return new RenderGraph(callSites, roots, documentRoots, unresolvedRenders, skippedFiles) diff --git a/javascript/packages/analysis/test/view-roots.test.ts b/javascript/packages/analysis/test/view-roots.test.ts index bd598905a..bbfe77ec8 100644 --- a/javascript/packages/analysis/test/view-roots.test.ts +++ b/javascript/packages/analysis/test/view-roots.test.ts @@ -1,5 +1,7 @@ import { describe, test, expect } from "vitest" +import { PartialIndex } from "../src/partial-index" + import { partialNameForRoots, relativeToViewRoots, @@ -70,3 +72,12 @@ describe("resolvePartial", () => { expect(resolvePartial("shared/header", "", index, [APP])).toBe(`${APP}/shared/_header.html.erb`) }) }) + +describe("serialization", () => { + test("round-trips every view root", () => { + const index = new PartialIndex(ROOTS, new Map()) + const restored = PartialIndex.from(index.toJSON()) + + expect(restored.viewRoots).toEqual(ROOTS) + }) +}) diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index 5ee59935f..c14802404 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -10,7 +10,6 @@ module Analysis class PartialIndex APPLICATION_DIRECTORY = "application" #: String - attr_reader :view_root #: Pathname attr_reader :templates #: Array[String] @@ -31,7 +30,6 @@ def self.resolve_view_root(project_path) #: (Array[String | Pathname], Array[String]) -> void def initialize(view_roots, templates) @view_roots = view_roots.map { |root| Pathname.new(root) } #: Array[Pathname] - @view_root = @view_roots.first || Pathname.new(".") @templates = templates @by_name = build_index(templates) @declarations = {} #: Hash[String, PartialDeclaration?] diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index ff764a21a..d34aeb4d4 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -100,6 +100,11 @@ def template_name_for(file, view_root) directory == "." ? name : "#{directory}/#{name}" end + #: (String, Array[String | Pathname]) -> Array[String] + def layout_candidates_for_roots(template_file, view_roots) + view_roots.lazy.map { |root| layout_candidates_for(template_file, root) }.find { |candidates| candidates.any? } || [] + end + #: (String, String | Pathname) -> Array[String] def layout_candidates_for(template_file, view_root) relative = relative_to_view_root(template_file, view_root) diff --git a/lib/herb/analysis/project_index.rb b/lib/herb/analysis/project_index.rb index 998d9a2aa..d5449fb27 100644 --- a/lib/herb/analysis/project_index.rb +++ b/lib/herb/analysis/project_index.rb @@ -45,8 +45,8 @@ def index_call_sites end #: () -> Pathname? - def view_root - @partials&.view_root + def view_roots + @partials&.view_roots end #: (String, ?String?) -> bool diff --git a/lib/herb/analysis/render_graph/builder.rb b/lib/herb/analysis/render_graph/builder.rb index 45ccb83ce..99a19b1da 100644 --- a/lib/herb/analysis/render_graph/builder.rb +++ b/lib/herb/analysis/render_graph/builder.rb @@ -156,11 +156,11 @@ def build(templates) #: (Array[String], Hash[String, Array[YieldSite]], Hash[String, Array[PartialCallSite]]) -> void def add_layout_call_sites(files, layout_yields, call_sites) - view_root = @partials.view_root + view_roots = @partials.view_roots layouts = {} #: Hash[String, String] files.each do |file| - name = PartialResolution.template_name_for(file, view_root) + name = PartialResolution.template_name_for_roots(file, view_roots) next unless name && layout_yields.key?(file) @@ -172,7 +172,7 @@ def add_layout_call_sites(files, layout_yields, call_sites) end files.each do |file| - PartialResolution.layout_candidates_for(file, view_root).each do |candidate| + PartialResolution.layout_candidates_for_roots(file, view_roots).each do |candidate| layout = layouts[candidate] next if layout.nil? || layout == file diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 7975bb584..db9cc7f8c 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -602,10 +602,14 @@ fn graph(arguments: &[String]) -> i32 { } fn view_relative(file: &str, index: &PartialIndex) -> String { - Path::new(file) - .strip_prefix(index.view_root()) + let path = Path::new(file); + + index + .view_roots() + .iter() + .find_map(|root| path.strip_prefix(root).ok()) .map(|rest| rest.display().to_string()) - .unwrap_or_else(|_| file.to_string()) + .unwrap_or_else(|| file.to_string()) } fn reverse_graph(renders: &BTreeMap>, index: &PartialIndex) -> BTreeMap> { @@ -702,7 +706,7 @@ fn collect_renders(index: &mut PartialIndex, templates: &[String]) -> (BTreeMap< let mut renders: BTreeMap> = BTreeMap::new(); let mut prefixes: BTreeSet = BTreeSet::new(); let mut layouts: BTreeSet = BTreeSet::new(); - let flow = StateFlow::new(index.view_root()); + let flow = StateFlow::new(index.view_roots().first().map(PathBuf::as_path).unwrap_or_else(|| Path::new("."))); for file in templates { let result = flow.analyze(file); diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 77898ce2d..62ebd88ce 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -109,10 +109,6 @@ impl PartialIndex { self.by_name = by_name; } - pub fn view_root(&self) -> &Path { - self.view_roots.first().map(PathBuf::as_path).unwrap_or_else(|| Path::new(".")) - } - pub fn view_roots(&self) -> &[PathBuf] { &self.view_roots } diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index d98cf0391..876cd62de 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -167,6 +167,14 @@ pub fn template_name_for(file: &str, view_root: &str) -> Option { } } +pub fn layout_candidates_for_roots(template_file: &str, view_roots: &[String]) -> Vec { + view_roots + .iter() + .map(|root| layout_candidates_for(template_file, root)) + .find(|candidates| !candidates.is_empty()) + .unwrap_or_default() +} + pub fn layout_candidates_for(template_file: &str, view_root: &str) -> Vec { let Some(relative) = relative_to_view_root(template_file, view_root) else { return Vec::new(); diff --git a/rust/herb-analysis/src/project_index.rs b/rust/herb-analysis/src/project_index.rs index ccece30d9..a41d30867 100644 --- a/rust/herb-analysis/src/project_index.rs +++ b/rust/herb-analysis/src/project_index.rs @@ -66,8 +66,8 @@ impl ProjectIndex { self.graph.as_ref() } - pub fn view_root(&self) -> Option<&Path> { - self.partials.as_ref().map(|partials| partials.view_root()) + pub fn view_roots(&self) -> Option<&[PathBuf]> { + self.partials.as_ref().map(|partials| partials.view_roots()) } pub fn handle_change(&mut self, path: &str, source: Option<&str>) -> bool { diff --git a/rust/herb-analysis/src/render_graph_builder.rs b/rust/herb-analysis/src/render_graph_builder.rs index 0ffa130c4..3cc4ef4fc 100644 --- a/rust/herb-analysis/src/render_graph_builder.rs +++ b/rust/herb-analysis/src/render_graph_builder.rs @@ -6,7 +6,7 @@ use herb::nodes::{AnyNode, ERBCaseNode, ERBIfNode, ERBRenderNode, ERBUnlessNode, use herb::visitor::Visitor; use crate::partial_index::PartialIndex; -use crate::partial_resolution::{layout_candidates_for, outranks_template, partial_path, template_name_for, LAYOUTS_DIRECTORY}; +use crate::partial_resolution::{layout_candidates_for_roots, outranks_template, partial_path, template_name_for_roots, LAYOUTS_DIRECTORY}; use crate::render_graph::{CallSiteLocation, PartialCallSite, RenderGraph, StaticAttributeMap, TemplateRoots}; const RENDER_MARKER: &str = "render"; @@ -179,14 +179,12 @@ impl<'a> Builder<'a> { } fn add_layout_call_sites(&self, files: &[String], layout_yields: &BTreeMap>, graph: &mut RenderGraph) { - let Some(view_root) = self.partials.view_root().to_str() else { - return; - }; + let view_roots: Vec = self.partials.view_roots().iter().filter_map(|root| root.to_str().map(str::to_string)).collect(); let mut layouts: BTreeMap = BTreeMap::new(); for file in files { - let Some(name) = template_name_for(file, view_root) else { + let Some(name) = template_name_for_roots(file, &view_roots) else { continue; }; @@ -203,7 +201,7 @@ impl<'a> Builder<'a> { } for file in files { - for candidate in layout_candidates_for(file, view_root) { + for candidate in layout_candidates_for_roots(file, &view_roots) { let Some(layout) = layouts.get(&candidate) else { continue; }; diff --git a/rust/herb-analysis/tests/partial_index_test.rs b/rust/herb-analysis/tests/partial_index_test.rs index 58815b729..6e156678e 100644 --- a/rust/herb-analysis/tests/partial_index_test.rs +++ b/rust/herb-analysis/tests/partial_index_test.rs @@ -1,5 +1,5 @@ use std::fs; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use herb_analysis::partial_index::PartialIndex; @@ -51,7 +51,7 @@ fn resolves_the_view_root_to_app_views_when_it_is_there() { let project = Project::new("view_root"); project.write("app/views/posts/index.html.erb"); - assert_eq!(project.index().view_root(), project.root.join("app/views")); + assert_eq!(project.index().view_roots(), [project.root.join("app/views")]); } #[test] @@ -59,7 +59,7 @@ fn falls_back_to_the_project_root_when_there_is_no_app_views() { let project = Project::new("flat_root"); project.write("posts/index.html.erb"); - assert_eq!(project.index().view_root(), Path::new(&project.root)); + assert_eq!(project.index().view_roots(), [project.root.as_path()].map(PathBuf::from)); } #[test] diff --git a/rust/herb-analysis/tests/project_index_test.rs b/rust/herb-analysis/tests/project_index_test.rs index 42cb672ed..0ed433992 100644 --- a/rust/herb-analysis/tests/project_index_test.rs +++ b/rust/herb-analysis/tests/project_index_test.rs @@ -151,5 +151,5 @@ fn exposes_the_view_root_it_resolved() { let project = Project::new("view_root"); project.write("app/views/posts/index.html.erb", "
"); - assert_eq!(project.indexed().view_root().expect("view root"), project.root.join("app/views")); + assert_eq!(project.indexed().view_roots().expect("view roots"), [project.root.join("app/views")]); } diff --git a/rust/herb-analysis/tests/view_roots_test.rs b/rust/herb-analysis/tests/view_roots_test.rs index 223083c56..f7c8b2ed5 100644 --- a/rust/herb-analysis/tests/view_roots_test.rs +++ b/rust/herb-analysis/tests/view_roots_test.rs @@ -84,7 +84,7 @@ fn a_single_root_still_resolves() { write(&app.join("shared/_header.html.erb"), "
\n"); let templates = vec![app.join("shared/_header.html.erb").to_str().unwrap().to_string()]; - let index = PartialIndex::new(&[app.clone()], templates); + let index = PartialIndex::new(std::slice::from_ref(&app), templates); assert_eq!(vec!["shared/header"], index.names()); assert_eq!(1, index.resolve("shared/header", None).len()); diff --git a/sig/herb/analysis/partial_index.rbs b/sig/herb/analysis/partial_index.rbs index a8edb36a0..8e3eac097 100644 --- a/sig/herb/analysis/partial_index.rbs +++ b/sig/herb/analysis/partial_index.rbs @@ -5,8 +5,6 @@ module Herb class PartialIndex APPLICATION_DIRECTORY: String - attr_reader view_root: Pathname - attr_reader templates: Array[String] # : (String | Pathname, ?templates: Array[String]?) -> PartialIndex diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index fe9eb11ee..9596b620b 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -44,6 +44,9 @@ module Herb # : (String, String | Pathname) -> String? def self.template_name_for: (String, String | Pathname) -> String? + # : (String, Array[String | Pathname]) -> Array[String] + def self.layout_candidates_for_roots: (String, Array[String | Pathname]) -> Array[String] + # : (String, String | Pathname) -> Array[String] def self.layout_candidates_for: (String, String | Pathname) -> Array[String] diff --git a/sig/herb/analysis/project_index.rbs b/sig/herb/analysis/project_index.rbs index aca0c65cb..cc806e127 100644 --- a/sig/herb/analysis/project_index.rbs +++ b/sig/herb/analysis/project_index.rbs @@ -22,7 +22,7 @@ module Herb def index_call_sites: () -> void # : () -> Pathname? - def view_root: () -> Pathname? + def view_roots: () -> Pathname? # : (String, ?String?) -> bool def handle_change: (String, ?String?) -> bool diff --git a/test/analysis/partial_index_test.rb b/test/analysis/partial_index_test.rb index 362d9d7cd..c3ab12b52 100644 --- a/test/analysis/partial_index_test.rb +++ b/test/analysis/partial_index_test.rb @@ -25,13 +25,13 @@ def write(path, content = "
\n") test "resolves the view root to app/views when it is there" do write("app/views/posts/index.html.erb") - assert_equal File.join(@project_path, "app", "views"), Herb::Analysis::PartialIndex.build(@project_path).view_root.to_s + assert_equal [File.join(@project_path, "app", "views")], Herb::Analysis::PartialIndex.build(@project_path).view_roots.map(&:to_s) end test "falls back to the project root when there is no app/views" do write("posts/index.html.erb") - assert_equal @project_path, Herb::Analysis::PartialIndex.build(@project_path).view_root.to_s + assert_equal [@project_path], Herb::Analysis::PartialIndex.build(@project_path).view_roots.map(&:to_s) end test "maps a qualified partial name to its file" do diff --git a/test/analysis/project_index_test.rb b/test/analysis/project_index_test.rb index df4bb2963..af41ca475 100644 --- a/test/analysis/project_index_test.rb +++ b/test/analysis/project_index_test.rb @@ -135,6 +135,6 @@ def reindexed test "exposes the view root it resolved" do write("index.html.erb", "
") - assert_equal File.join(@project_path, "app", "views"), indexed.view_root.to_s + assert_equal [File.join(@project_path, "app", "views")], indexed.view_roots.map(&:to_s) end end From 4fdd1a4f16ec7b1a59cbc9bb7b807ee7be450c8f Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:47:53 +0200 Subject: [PATCH 07/26] RBS --- lib/herb/analysis/project_index.rb | 2 +- sig/herb/analysis/project_index.rbs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/herb/analysis/project_index.rb b/lib/herb/analysis/project_index.rb index d5449fb27..1f88053de 100644 --- a/lib/herb/analysis/project_index.rb +++ b/lib/herb/analysis/project_index.rb @@ -44,7 +44,7 @@ def index_call_sites @graph = graph_builder.build(partials.templates) end - #: () -> Pathname? + #: () -> Array[Pathname]? def view_roots @partials&.view_roots end diff --git a/sig/herb/analysis/project_index.rbs b/sig/herb/analysis/project_index.rbs index cc806e127..0a03533ab 100644 --- a/sig/herb/analysis/project_index.rbs +++ b/sig/herb/analysis/project_index.rbs @@ -21,8 +21,8 @@ module Herb # : () -> void def index_call_sites: () -> void - # : () -> Pathname? - def view_roots: () -> Pathname? + # : () -> Array[Pathname]? + def view_roots: () -> Array[Pathname]? # : (String, ?String?) -> bool def handle_change: (String, ?String?) -> bool From afb98e9edbdf5ff887c71aeb1143e97d69f7f887 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 05:54:32 +0200 Subject: [PATCH 08/26] Analysis: Resolve partial names that spell out the extension --- .../analysis/src/partial-resolution.ts | 10 +++++++ lib/herb/analysis/partial_index.rb | 1 + lib/herb/analysis/partial_resolution.rb | 7 +++++ lib/herb/analysis/render_analyzer.rb | 16 ++++++++++- lib/herb/analysis/ruby_locals_index.rb | 1 - .../ruby_locals_index/named_reference.rb | 2 -- .../ruby_locals_index/offset_table.rb | 3 -- lib/herb/analysis/template_dependencies.rb | 2 -- rust/herb-analysis/src/actionview_cli.rs | 28 ++++++++++++++++++- rust/herb-analysis/src/partial_index.rs | 1 + rust/herb-analysis/src/partial_resolution.rs | 7 +++++ sig/herb/analysis/partial_resolution.rbs | 3 ++ sig/herb/analysis/render_analyzer.rbs | 3 ++ 13 files changed, 74 insertions(+), 10 deletions(-) diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 3b45e5b5c..0f9d4af99 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -90,6 +90,14 @@ export function projectRelativePath(filePath: string, projectPath: string | unde return relativeToViewRoot(filePath, projectPath) ?? normalize(filePath) } +export function withoutTemplateExtension(partialName: string): string { + for (const extension of PARTIAL_EXTENSIONS) { + if (partialName.endsWith(extension)) return partialName.slice(0, -extension.length) + } + + return partialName +} + export function isTemplatePath(filePath: string): boolean { const name = basename(normalize(filePath)) @@ -186,6 +194,8 @@ export function resolvePartial( index: PartialPaths, viewRoots: string[] ): string | null { + partialName = withoutTemplateExtension(partialName) + const exact = index.get(partialName) if (exact !== undefined) return exact diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index c14802404..2d89cfb7c 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -49,6 +49,7 @@ def files_for(partial_name) def resolve(partial_name, source_file) return [] unless partial_name + partial_name = PartialResolution.without_template_extension(partial_name) exact = files_for(partial_name) return exact if exact.any? diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index d34aeb4d4..7d94cfff9 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -41,6 +41,13 @@ def view_root_for(project_path) candidates.find(&:directory?) || root end + #: (String) -> String + def without_template_extension(partial_name) + extension = EXTENSIONS.find { |candidate| partial_name.end_with?(candidate) } + + extension ? partial_name.delete_suffix(extension) : partial_name + end + #: (String) -> bool def template_path?(file) name = File.basename(file) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index 3f07ce9b6..a588884c1 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -456,7 +456,10 @@ def print_file_lists(result) calls.each do |call| location = call[:location] ? dimmed("at #{call[:location]}") : nil expected = expected_file_path(call[:partial], result.view_root) - puts " #{bold(red("\u2717"))} #{bold(call[:partial])} #{location} #{dimmed("-")} #{dimmed(expected)}" + kind = render_name_kind(call[:partial]) + label = kind ? " #{dimmed("(#{kind})")}" : "" + + puts " #{bold(red("\u2717"))} #{bold(call[:partial])}#{label} #{location} #{dimmed("-")} #{dimmed(expected)}" end end end @@ -1177,6 +1180,17 @@ def resolve_partial(partial_name, source_file, _partial_files, view_root) partial_index(view_root).resolve(partial_name, source_file).first end + #: (String) -> String? + def render_name_kind(name) + return "instance variable" if name.start_with?("@") + return "interpolated" if name.include?("\#{") + return "conditional" if name.include?("?") && name.include?(":") + return "method call" if name.include?("(") || name.include?(".") + return "expression" if name.include?(" ") + + nil + end + def expected_file_path(partial_name, view_root) parts = partial_name.split("/") parts[-1] = "_#{parts[-1]}" diff --git a/lib/herb/analysis/ruby_locals_index.rb b/lib/herb/analysis/ruby_locals_index.rb index cebb92c7b..da5749e54 100644 --- a/lib/herb/analysis/ruby_locals_index.rb +++ b/lib/herb/analysis/ruby_locals_index.rb @@ -59,7 +59,6 @@ def find(name) @locals.find { |local| local.name == name } end - # Every name the template binds, regardless of where. #: () -> Set[String] def names @locals.to_set(&:name) diff --git a/lib/herb/analysis/ruby_locals_index/named_reference.rb b/lib/herb/analysis/ruby_locals_index/named_reference.rb index 9bed92e89..5468e0567 100644 --- a/lib/herb/analysis/ruby_locals_index/named_reference.rb +++ b/lib/herb/analysis/ruby_locals_index/named_reference.rb @@ -3,8 +3,6 @@ module Herb module Analysis class RubyLocalsIndex - # A name, with where it appears in the source as a byte offset and length, - # which is how Prism reports it. class NamedReference attr_reader :name #: String attr_reader :start_offset #: Integer diff --git a/lib/herb/analysis/ruby_locals_index/offset_table.rb b/lib/herb/analysis/ruby_locals_index/offset_table.rb index fdab6e800..e9a0560de 100644 --- a/lib/herb/analysis/ruby_locals_index/offset_table.rb +++ b/lib/herb/analysis/ruby_locals_index/offset_table.rb @@ -3,11 +3,8 @@ module Herb module Analysis class RubyLocalsIndex - # Prism reports byte offsets into the whole template while the Herb AST - # reports lines and columns, so one of them has to be translated. class OffsetTable # @rbs! - # @line_starts: Array[Integer] #: (String) -> void def initialize(source) diff --git a/lib/herb/analysis/template_dependencies.rb b/lib/herb/analysis/template_dependencies.rb index 20f3a1bd9..af74fd65f 100644 --- a/lib/herb/analysis/template_dependencies.rb +++ b/lib/herb/analysis/template_dependencies.rb @@ -100,7 +100,6 @@ def dependency_index(file_path) end # @rbs! - # KERNEL_METHODS: Array[String] KERNEL_METHODS = [ "rand", "srand", "format", "sprintf", "raise", "loop", "sleep", "catch", "throw", "block_given?", "caller", "binding", "frozen?", "freeze", "dup", "clone", "tap", "then", @@ -340,7 +339,6 @@ def symbol_after(line, keyword) end # @rbs! - # UNCOUNTABLE: Array[String] UNCOUNTABLE = ["series", "species", "news", "information", "equipment", "money"].freeze #: (String, String) -> String? diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index db9cc7f8c..ae5809371 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -96,6 +96,30 @@ fn missing_format(file: &str) -> bool { name.ends_with(".erb") && name.matches('.').count() == 1 } +fn render_name_kind(name: &str) -> Option<&'static str> { + if name.starts_with('@') { + return Some("instance variable"); + } + + if name.contains("#{") { + return Some("interpolated"); + } + + if name.contains('?') && name.contains(':') { + return Some("conditional"); + } + + if name.contains('(') || name.contains('.') { + return Some("method call"); + } + + if name.contains(' ') { + return Some("expression"); + } + + None +} + fn plural(count: usize, word: &str) -> String { if count == 1 { word.to_string() @@ -298,7 +322,9 @@ fn check(arguments: &[String]) -> i32 { println!(); for (file, name) in &unresolved { - println!(" {} {} {}", "\u{2717}".red().bold(), name, format!("in {file}").dimmed()); + let kind = render_name_kind(name).map(|kind| format!(" ({kind})").dimmed().to_string()).unwrap_or_default(); + + println!(" {} {}{} {}", "\u{2717}".red().bold(), name, kind, format!("in {file}").dimmed()); } println!(); diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 62ebd88ce..e20fcc667 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -164,6 +164,7 @@ impl PartialIndex { } pub fn resolve(&self, partial_name: &str, source_file: Option<&str>) -> &[String] { + let partial_name = partial_resolution::without_template_extension(partial_name); let exact = self.files_for(partial_name); if !exact.is_empty() { diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index 876cd62de..d90b0488f 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -56,6 +56,13 @@ fn normalize(path: &str) -> String { } } +pub fn without_template_extension(partial_name: &str) -> &str { + EXTENSIONS + .iter() + .find_map(|extension| partial_name.strip_suffix(extension)) + .unwrap_or(partial_name) +} + pub fn template_path(file: &str) -> bool { let normalized = normalize(file); let name = basename(&normalized); diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index 9596b620b..b4c7f01ae 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -26,6 +26,9 @@ module Herb # : (String | Pathname) -> Pathname def self.view_root_for: (String | Pathname) -> Pathname + # : (String) -> String + def self.without_template_extension: (String) -> String + # : (String) -> bool def self.template_path?: (String) -> bool diff --git a/sig/herb/analysis/render_analyzer.rbs b/sig/herb/analysis/render_analyzer.rbs index ffa8c17b0..f224d630b 100644 --- a/sig/herb/analysis/render_analyzer.rbs +++ b/sig/herb/analysis/render_analyzer.rbs @@ -117,6 +117,9 @@ module Herb def resolve_partial: (untyped partial_name, untyped source_file, untyped _partial_files, untyped view_root) -> untyped + # : (String) -> String? + def render_name_kind: (String) -> String? + def expected_file_path: (untyped partial_name, untyped view_root) -> untyped def label: (untyped text, ?untyped width) -> untyped From 48da114a1e342f9e23fdd2104cb5ea3117e6a725 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 06:13:10 +0200 Subject: [PATCH 09/26] Analysis: Resolve conditional renders whose branches are literals --- rust/herb-analysis/src/actionview_cli.rs | 60 +++++++++++++++++++++++- 1 file changed, 59 insertions(+), 1 deletion(-) diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index ae5809371..81ebb80fa 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -96,6 +96,30 @@ fn missing_format(file: &str) -> bool { name.ends_with(".erb") && name.matches('.').count() == 1 } +fn static_branches(expression: &str) -> Vec { + let mut found = Vec::new(); + let mut rest = expression; + + while let Some(start) = rest.find(['"', '\'']) { + let quote = rest.as_bytes()[start] as char; + let after = &rest[start + 1..]; + + let Some(end) = after.find(quote) else { + break; + }; + + let literal = &after[..end]; + + if !literal.is_empty() && !literal.contains("#{") { + found.push(literal.to_string()); + } + + rest = &after[end + 1..]; + } + + found +} + fn render_name_kind(name: &str) -> Option<&'static str> { if name.starts_with('@') { return Some("instance variable"); @@ -186,6 +210,7 @@ fn check(arguments: &[String]) -> i32 { let mut files_with_renders: BTreeSet = BTreeSet::new(); let mut dynamic_renders = 0usize; let mut dynamic_sites: Vec<(String, String)> = Vec::new(); + let mut branching_sites: Vec<(String, String, Vec)> = Vec::new(); let mut other_renders = 0usize; let mut with_partial_count = 0usize; @@ -255,7 +280,24 @@ fn check(arguments: &[String]) -> i32 { match index.resolve(name, Some(file)).first() { Some(target) => rendered.push(target.clone()), - None => unresolved.push((relative(file, &root), name.clone())), + None => { + let branches = static_branches(name); + let targets: Vec = branches + .iter() + .filter_map(|branch| index.resolve(branch, Some(file)).first().cloned()) + .collect(); + + if branches.len() > 1 && targets.len() == branches.len() { + rendered.extend(targets.iter().cloned()); + branching_sites.push(( + relative(file, &root), + name.clone(), + targets.iter().map(|target| relative(target, &root)).collect(), + )); + } else { + unresolved.push((relative(file, &root), name.clone())); + } + } } } } @@ -317,6 +359,22 @@ fn check(arguments: &[String]) -> i32 { println!(); } + if !branching_sites.is_empty() { + println!(" {}", "Conditional render calls:".bold()); + println!(" {}", "The partial name is chosen at runtime, but every branch is a literal.".dimmed()); + println!(); + + for (file, expression, targets) in &branching_sites { + println!(" {} {} {}", "?".yellow().bold(), expression, format!("in {file}").dimmed()); + + for target in targets { + println!(" {} {}", "\u{2192}".dimmed(), target.green()); + } + } + + println!(); + } + if !unresolved.is_empty() { println!(" {}", "Unresolved render calls:".bold()); println!(); From 7ef3b11066b1346a2f819a5934ee659482634096 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 06:35:02 +0200 Subject: [PATCH 10/26] Analysis: Resolve partials against the caller's format --- .../analysis/src/partial-resolution.ts | 20 +++++ .../analysis/test/partial-resolution.test.ts | 14 +++ lib/herb/analysis/partial_index.rb | 22 ++++- lib/herb/analysis/partial_resolution.rb | 18 ++++ rust/herb-analysis/src/actionview_cli.rs | 3 +- rust/herb-analysis/src/partial_index.rs | 18 +++- rust/herb-analysis/src/partial_resolution.rs | 12 +++ rust/herb-analysis/tests/formats_test.rs | 88 +++++++++++++++++++ sig/herb/analysis/partial_index.rbs | 8 ++ sig/herb/analysis/partial_resolution.rbs | 5 ++ test/analysis/formats_test.rb | 83 +++++++++++++++++ 11 files changed, 288 insertions(+), 3 deletions(-) create mode 100644 rust/herb-analysis/tests/formats_test.rs create mode 100644 test/analysis/formats_test.rb diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 0f9d4af99..7c647a4ab 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -90,6 +90,26 @@ export function projectRelativePath(filePath: string, projectPath: string | unde return relativeToViewRoot(filePath, projectPath) ?? normalize(filePath) } +export function formatOf(filePath: string): string | null { + const name = basename(normalize(filePath)) + const dot = name.indexOf(".") + + if (dot === -1) return null + + const extension = name.slice(dot) + const stripped = extension.endsWith(".erb") + ? extension.slice(0, -".erb".length) + : extension.endsWith(".herb") + ? extension.slice(0, -".herb".length) + : null + + if (stripped === null) return null + + const format = stripped.startsWith(".") ? stripped.slice(1) : stripped + + return format === "" ? null : format +} + export function withoutTemplateExtension(partialName: string): string { for (const extension of PARTIAL_EXTENSIONS) { if (partialName.endsWith(extension)) return partialName.slice(0, -extension.length) diff --git a/javascript/packages/analysis/test/partial-resolution.test.ts b/javascript/packages/analysis/test/partial-resolution.test.ts index 286df28b8..93ad6b539 100644 --- a/javascript/packages/analysis/test/partial-resolution.test.ts +++ b/javascript/packages/analysis/test/partial-resolution.test.ts @@ -1,6 +1,7 @@ import { describe, test, expect } from "vitest" import { + formatOf, isPartialPath, partialNameForFile, resolvePartial, @@ -168,3 +169,16 @@ describe("@herb-tools/core", () => { }) }) }) + +describe("formatOf", () => { + test("reads the format out of a filename", () => { + expect(formatOf("app/views/posts/_row.html.erb")).toBe("html") + expect(formatOf("app/views/posts/_row.turbo_stream.erb")).toBe("turbo_stream") + expect(formatOf("app/views/posts/_row.html.herb")).toBe("html") + }) + + test("returns null when the filename carries no format", () => { + expect(formatOf("app/views/posts/_row.erb")).toBeNull() + expect(formatOf("app/views/posts/_row.herb")).toBeNull() + }) +}) diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index 2d89cfb7c..a70c21210 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -10,7 +10,6 @@ module Analysis class PartialIndex APPLICATION_DIRECTORY = "application" #: String - attr_reader :templates #: Array[String] #: (String | Pathname, ?templates: Array[String]?) -> PartialIndex @@ -45,8 +44,29 @@ def files_for(partial_name) @by_name[partial_name] || [] end + #: (String?, String?) -> Array[String] #: (String?, String?) -> Array[String] def resolve(partial_name, source_file) + candidates = candidates_for(partial_name, source_file) + format = source_file ? PartialResolution.format_of(source_file) : nil + + return candidates unless format + + candidates.sort_by do |file| + candidate = PartialResolution.format_of(file) + + if candidate == format + 0 + elsif candidate.nil? + 1 + else + 2 + end + end + end + + #: (String?, String?) -> Array[String] + def candidates_for(partial_name, source_file) return [] unless partial_name partial_name = PartialResolution.without_template_extension(partial_name) diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index 7d94cfff9..7ed81152d 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -41,6 +41,24 @@ def view_root_for(project_path) candidates.find(&:directory?) || root end + #: (String) -> String? + def format_of(file) + base = File.basename(file) + dot = base.index(".") + + return nil unless dot + + extension = base[dot..].to_s + stripped = extension.delete_suffix(".erb") + stripped = stripped.delete_suffix(".herb") if stripped == extension + + return nil if stripped == extension + + format = stripped.delete_prefix(".") + + format.empty? ? nil : format + end + #: (String) -> String def without_template_extension(partial_name) extension = EXTENSIONS.find { |candidate| partial_name.end_with?(candidate) } diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 81ebb80fa..98631fb05 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -874,7 +874,8 @@ fn reachable_partials( continue; } - let Some(file) = index.resolve(&name, None).first() else { + let resolved = index.resolve(&name, None); + let Some(file) = resolved.first() else { continue; }; diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index e20fcc667..4e03ae6e2 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -163,7 +163,23 @@ impl PartialIndex { .map(str::to_string) } - pub fn resolve(&self, partial_name: &str, source_file: Option<&str>) -> &[String] { + pub fn resolve(&self, partial_name: &str, source_file: Option<&str>) -> Vec { + let candidates = self.candidates(partial_name, source_file); + let Some(format) = source_file.and_then(partial_resolution::format_of) else { + return candidates.to_vec(); + }; + + let mut ordered = candidates.to_vec(); + ordered.sort_by_key(|file| match partial_resolution::format_of(file) { + Some(candidate) if candidate == format => 0, + None => 1, + Some(_) => 2, + }); + + ordered + } + + fn candidates(&self, partial_name: &str, source_file: Option<&str>) -> &[String] { let partial_name = partial_resolution::without_template_extension(partial_name); let exact = self.files_for(partial_name); diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index d90b0488f..d0515047f 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -56,6 +56,18 @@ fn normalize(path: &str) -> String { } } +pub fn format_of(file: &str) -> Option { + let normalized = normalize(file); + let base = basename(&normalized); + let dot = base.find('.')?; + let extension = &base[dot..]; + + let stripped = extension.strip_suffix(".erb").or_else(|| extension.strip_suffix(".herb"))?; + let format = stripped.strip_prefix('.')?; + + (!format.is_empty()).then(|| format.to_string()) +} + pub fn without_template_extension(partial_name: &str) -> &str { EXTENSIONS .iter() diff --git a/rust/herb-analysis/tests/formats_test.rs b/rust/herb-analysis/tests/formats_test.rs new file mode 100644 index 000000000..71d373e4a --- /dev/null +++ b/rust/herb-analysis/tests/formats_test.rs @@ -0,0 +1,88 @@ +use std::fs; +use std::path::PathBuf; + +use herb_analysis::partial_index::PartialIndex; +use herb_analysis::partial_resolution::format_of; + +fn scratch(name: &str) -> PathBuf { + let root = std::env::temp_dir().join(format!("herb-formats-{name}")); + + let _ = fs::remove_dir_all(&root); + + root +} + +fn write(path: &PathBuf) -> String { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, "
\n").unwrap(); + + path.to_str().unwrap().to_string() +} + +#[test] +fn reads_the_format_out_of_a_filename() { + assert_eq!(Some("html".to_string()), format_of("app/views/posts/_row.html.erb")); + assert_eq!(Some("turbo_stream".to_string()), format_of("app/views/posts/_row.turbo_stream.erb")); + assert_eq!(Some("html".to_string()), format_of("app/views/posts/_row.html.herb")); + assert_eq!(None, format_of("app/views/posts/_row.erb")); + assert_eq!(None, format_of("app/views/posts/_row.herb")); +} + +#[test] +fn a_caller_reaches_the_partial_matching_its_own_format() { + let root = scratch("matching"); + let views = root.join("app/views"); + + let html_caller = write(&views.join("posts/index.html.erb")); + let turbo_caller = write(&views.join("posts/index.turbo_stream.erb")); + let html_partial = write(&views.join("posts/_row.html.erb")); + let turbo_partial = write(&views.join("posts/_row.turbo_stream.erb")); + + let index = PartialIndex::new( + &[views], + vec![html_caller.clone(), turbo_caller.clone(), html_partial.clone(), turbo_partial.clone()], + ); + + assert_eq!(html_partial, index.resolve("posts/row", Some(&html_caller))[0]); + assert_eq!(turbo_partial, index.resolve("posts/row", Some(&turbo_caller))[0]); +} + +#[test] +fn a_formatless_partial_serves_any_caller() { + let root = scratch("formatless"); + let views = root.join("app/views"); + + let turbo_caller = write(&views.join("posts/index.turbo_stream.erb")); + let partial = write(&views.join("posts/_row.erb")); + + let index = PartialIndex::new(&[views], vec![turbo_caller.clone(), partial.clone()]); + + assert_eq!(partial, index.resolve("posts/row", Some(&turbo_caller))[0]); +} + +#[test] +fn a_formatless_partial_loses_to_an_exact_format_match() { + let root = scratch("exact-wins"); + let views = root.join("app/views"); + + let turbo_caller = write(&views.join("posts/index.turbo_stream.erb")); + let formatless = write(&views.join("posts/_row.erb")); + let turbo_partial = write(&views.join("posts/_row.turbo_stream.erb")); + + let index = PartialIndex::new(&[views], vec![turbo_caller.clone(), formatless, turbo_partial.clone()]); + + assert_eq!(turbo_partial, index.resolve("posts/row", Some(&turbo_caller))[0]); +} + +#[test] +fn extension_precedence_still_decides_when_no_format_matches() { + let root = scratch("fallback"); + let views = root.join("app/views"); + + let turbo_caller = write(&views.join("posts/index.turbo_stream.erb")); + let html_partial = write(&views.join("posts/_row.html.erb")); + + let index = PartialIndex::new(&[views], vec![turbo_caller.clone(), html_partial.clone()]); + + assert_eq!(html_partial, index.resolve("posts/row", Some(&turbo_caller))[0]); +} diff --git a/sig/herb/analysis/partial_index.rbs b/sig/herb/analysis/partial_index.rbs index 8e3eac097..bb413775a 100644 --- a/sig/herb/analysis/partial_index.rbs +++ b/sig/herb/analysis/partial_index.rbs @@ -22,8 +22,16 @@ module Herb # : (String?) -> Array[String] def files_for: (String?) -> Array[String] + # : (String?, String?) -> Array[String] + # Rails picks the candidate whose format matches the template doing the rendering, so a + # `.turbo_stream.erb` caller reaches the turbo_stream partial even though `.html.erb` outranks + # it everywhere else. A partial with no format of its own matches any caller. # : (String?, String?) -> Array[String] def resolve: (String?, String?) -> Array[String] + | (String?, String?) -> Array[String] + + # : (String?, String?) -> Array[String] + def candidates_for: (String?, String?) -> Array[String] # : (String, String | Pathname) -> String? def self.partial_name_for: (String, String | Pathname) -> String? diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index b4c7f01ae..dc87524c7 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -26,6 +26,11 @@ module Herb # : (String | Pathname) -> Pathname def self.view_root_for: (String | Pathname) -> Pathname + # The format segment of a template filename, if it carries one. `_row.html.erb` is `html`, + # `_row.erb` is none and therefore matches any format. + # : (String) -> String? + def self.format_of: (String) -> String? + # : (String) -> String def self.without_template_extension: (String) -> String diff --git a/test/analysis/formats_test.rb b/test/analysis/formats_test.rb new file mode 100644 index 000000000..bc2ee9da5 --- /dev/null +++ b/test/analysis/formats_test.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require_relative "../test_helper" +require_relative "../../lib/herb/analysis/partial_index" + +require "tmpdir" + +module Analysis + class FormatsTest < Minitest::Spec + def write(root, relative) + path = File.join(root, relative) + + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, "
\n") + + path + end + + test "reads the format out of a filename" do + assert_equal "html", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.html.erb") + assert_equal "turbo_stream", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.turbo_stream.erb") + assert_equal "html", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.html.herb") + assert_nil Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.erb") + assert_nil Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.herb") + end + + test "a caller reaches the partial matching its own format" do + Dir.mktmpdir do |dir| + views = File.join(dir, "app", "views") + + html_caller = write(views, "posts/index.html.erb") + turbo_caller = write(views, "posts/index.turbo_stream.erb") + html_partial = write(views, "posts/_row.html.erb") + turbo_partial = write(views, "posts/_row.turbo_stream.erb") + + index = Herb::Analysis::PartialIndex.new([views], [html_caller, turbo_caller, html_partial, turbo_partial]) + + assert_equal html_partial, index.resolve("posts/row", html_caller).first + assert_equal turbo_partial, index.resolve("posts/row", turbo_caller).first + end + end + + test "a formatless partial serves any caller" do + Dir.mktmpdir do |dir| + views = File.join(dir, "app", "views") + + turbo_caller = write(views, "posts/index.turbo_stream.erb") + partial = write(views, "posts/_row.erb") + + index = Herb::Analysis::PartialIndex.new([views], [turbo_caller, partial]) + + assert_equal partial, index.resolve("posts/row", turbo_caller).first + end + end + + test "a formatless partial loses to an exact format match" do + Dir.mktmpdir do |dir| + views = File.join(dir, "app", "views") + + turbo_caller = write(views, "posts/index.turbo_stream.erb") + formatless = write(views, "posts/_row.erb") + turbo_partial = write(views, "posts/_row.turbo_stream.erb") + + index = Herb::Analysis::PartialIndex.new([views], [turbo_caller, formatless, turbo_partial]) + + assert_equal turbo_partial, index.resolve("posts/row", turbo_caller).first + end + end + + test "extension precedence still decides when no format matches" do + Dir.mktmpdir do |dir| + views = File.join(dir, "app", "views") + + turbo_caller = write(views, "posts/index.turbo_stream.erb") + html_partial = write(views, "posts/_row.html.erb") + + index = Herb::Analysis::PartialIndex.new([views], [turbo_caller, html_partial]) + + assert_equal html_partial, index.resolve("posts/row", turbo_caller).first + end + end + end +end From eab40eec8ba8b70d4ef9a79b17d9f164a1575d4e Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 06:38:02 +0200 Subject: [PATCH 11/26] Analysis: Don't report a guessed object partial as unresolved --- rust/herb-analysis/src/actionview_cli.rs | 4 +- .../herb-analysis/tests/object_render_test.rs | 62 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 rust/herb-analysis/tests/object_render_test.rs diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 98631fb05..090333002 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -242,6 +242,8 @@ fn check(arguments: &[String]) -> i32 { for call in &result.render_calls { files_with_renders.insert(file.clone()); + let guessed = call.partial.is_none() && call.layout.is_none(); + let target = call .partial .clone() @@ -294,7 +296,7 @@ fn check(arguments: &[String]) -> i32 { name.clone(), targets.iter().map(|target| relative(target, &root)).collect(), )); - } else { + } else if !guessed { unresolved.push((relative(file, &root), name.clone())); } } diff --git a/rust/herb-analysis/tests/object_render_test.rs b/rust/herb-analysis/tests/object_render_test.rs new file mode 100644 index 000000000..e39d72b71 --- /dev/null +++ b/rust/herb-analysis/tests/object_render_test.rs @@ -0,0 +1,62 @@ +use std::fs; +use std::path::PathBuf; +use std::process::Command; + +fn binary() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_herb-analysis")) +} + +fn scratch(name: &str) -> PathBuf { + let root = std::env::temp_dir().join(format!("herb-object-render-{name}")); + + let _ = fs::remove_dir_all(&root); + + root +} + +fn write(root: &PathBuf, relative: &str, body: &str) { + let path = root.join(relative); + + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, body).unwrap(); +} + +fn check(root: &PathBuf) -> String { + let output = Command::new(binary()).args(["actionview", "check", root.to_str().unwrap()]).output().unwrap(); + + String::from_utf8_lossy(&output.stdout).to_string() +} + +#[test] +fn a_guessed_object_partial_that_does_not_exist_is_not_an_error() { + let root = scratch("miss"); + + write(&root, "app/views/components/index.html.erb", "<%= render body do %><% end %>\n"); + + let output = check(&root); + + assert!(!output.contains("bodys/body"), "a guessed name should not be reported as unresolved:\n{output}"); +} + +#[test] +fn a_named_partial_that_does_not_exist_is_still_an_error() { + let root = scratch("named"); + + write(&root, "app/views/posts/index.html.erb", "<%= render \"posts/missing\" %>\n"); + + let output = check(&root); + + assert!(output.contains("posts/missing"), "an explicit name should still be reported:\n{output}"); +} + +#[test] +fn a_guessed_object_partial_that_exists_still_resolves() { + let root = scratch("hit"); + + write(&root, "app/views/posts/index.html.erb", "<%= render post %>\n"); + write(&root, "app/views/posts/_post.html.erb", "
\n"); + + let output = check(&root); + + assert!(!output.contains("posts/post"), "an existing guessed target should resolve:\n{output}"); +} From e0254f331904ab1f1972565c5dc14b2e8c4f7d22 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 06:41:45 +0200 Subject: [PATCH 12/26] Analysis: Resolve template variants --- .../analysis/src/partial-resolution.ts | 27 +++++++++++++++- .../analysis/test/partial-resolution.test.ts | 18 +++++++++++ lib/herb/analysis/partial_index.rb | 16 ++++++---- lib/herb/analysis/partial_resolution.rb | 20 +++++++++++- rust/herb-analysis/src/actionview_cli.rs | 16 ++++++++-- rust/herb-analysis/src/partial_index.rs | 12 ++++--- rust/herb-analysis/src/partial_resolution.rs | 14 ++++++++ rust/herb-analysis/tests/formats_test.rs | 32 ++++++++++++++++++- sig/herb/analysis/partial_resolution.rbs | 5 +++ test/analysis/formats_test.rb | 28 ++++++++++++++++ 10 files changed, 172 insertions(+), 16 deletions(-) diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 7c647a4ab..0fcf909de 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -105,11 +105,36 @@ export function formatOf(filePath: string): string | null { if (stripped === null) return null - const format = stripped.startsWith(".") ? stripped.slice(1) : stripped + const withVariant = stripped.startsWith(".") ? stripped.slice(1) : stripped + const format = withVariant.split("+")[0] ?? withVariant return format === "" ? null : format } +export function variantOf(filePath: string): string | null { + const name = basename(normalize(filePath)) + const dot = name.indexOf(".") + + if (dot === -1) return null + + const extension = name.slice(dot) + const stripped = extension.endsWith(".erb") + ? extension.slice(0, -".erb".length) + : extension.endsWith(".herb") + ? extension.slice(0, -".herb".length) + : null + + if (stripped === null) return null + + const plus = stripped.indexOf("+") + + if (plus === -1) return null + + const variant = stripped.slice(plus + 1) + + return variant === "" ? null : variant +} + export function withoutTemplateExtension(partialName: string): string { for (const extension of PARTIAL_EXTENSIONS) { if (partialName.endsWith(extension)) return partialName.slice(0, -extension.length) diff --git a/javascript/packages/analysis/test/partial-resolution.test.ts b/javascript/packages/analysis/test/partial-resolution.test.ts index 93ad6b539..37fce52de 100644 --- a/javascript/packages/analysis/test/partial-resolution.test.ts +++ b/javascript/packages/analysis/test/partial-resolution.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect } from "vitest" import { formatOf, + variantOf, isPartialPath, partialNameForFile, resolvePartial, @@ -182,3 +183,20 @@ describe("formatOf", () => { expect(formatOf("app/views/posts/_row.herb")).toBeNull() }) }) + +describe("variantOf", () => { + test("reads the variant out of a filename", () => { + expect(variantOf("app/views/posts/_row.html+mobile.erb")).toBe("mobile") + expect(variantOf("app/views/posts/_row.html+tablet.herb")).toBe("tablet") + }) + + test("returns null when the filename carries no variant", () => { + expect(variantOf("app/views/posts/_row.html.erb")).toBeNull() + expect(variantOf("app/views/posts/_row.erb")).toBeNull() + }) + + test("a variant keeps the format of its base template", () => { + expect(formatOf("app/views/posts/_row.html+mobile.erb")).toBe("html") + expect(formatOf("app/views/posts/_row.turbo_stream+mobile.erb")).toBe("turbo_stream") + }) +}) diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index a70c21210..891177807 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -55,13 +55,15 @@ def resolve(partial_name, source_file) candidates.sort_by do |file| candidate = PartialResolution.format_of(file) - if candidate == format - 0 - elsif candidate.nil? - 1 - else - 2 - end + matches = if candidate == format + 0 + elsif candidate.nil? + 1 + else + 2 + end + + [matches, PartialResolution.variant_of(file) ? 1 : 0] end end diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index 7ed81152d..0ba1c55e4 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -54,11 +54,29 @@ def format_of(file) return nil if stripped == extension - format = stripped.delete_prefix(".") + format = stripped.delete_prefix(".").split("+").first.to_s format.empty? ? nil : format end + #: (String) -> String? + def variant_of(file) + base = File.basename(file) + dot = base.index(".") + + return nil unless dot + + extension = base[dot..].to_s + stripped = extension.delete_suffix(".erb") + stripped = stripped.delete_suffix(".herb") if stripped == extension + + return nil if stripped == extension + + _, variant = stripped.split("+", 2) + + variant.to_s.empty? ? nil : variant + end + #: (String) -> String def without_template_extension(partial_name) extension = EXTENSIONS.find { |candidate| partial_name.end_with?(candidate) } diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 090333002..b8bf00099 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -280,8 +280,20 @@ fn check(arguments: &[String]) -> i32 { eprintln!("{file}\t{name}"); } - match index.resolve(name, Some(file)).first() { - Some(target) => rendered.push(target.clone()), + let resolved = index.resolve(name, Some(file)); + + match resolved.first() { + Some(target) => { + rendered.push(target.clone()); + + let format = herb_analysis::partial_resolution::format_of(target); + + for candidate in resolved.iter().skip(1) { + if herb_analysis::partial_resolution::variant_of(candidate).is_some() && herb_analysis::partial_resolution::format_of(candidate) == format { + rendered.push(candidate.clone()); + } + } + } None => { let branches = static_branches(name); let targets: Vec = branches diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 4e03ae6e2..750b4e890 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -170,10 +170,14 @@ impl PartialIndex { }; let mut ordered = candidates.to_vec(); - ordered.sort_by_key(|file| match partial_resolution::format_of(file) { - Some(candidate) if candidate == format => 0, - None => 1, - Some(_) => 2, + ordered.sort_by_key(|file| { + let matches = match partial_resolution::format_of(file) { + Some(candidate) if candidate == format => 0, + None => 1, + Some(_) => 2, + }; + + (matches, usize::from(partial_resolution::variant_of(file).is_some())) }); ordered diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index d0515047f..41660e39d 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -65,9 +65,23 @@ pub fn format_of(file: &str) -> Option { let stripped = extension.strip_suffix(".erb").or_else(|| extension.strip_suffix(".herb"))?; let format = stripped.strip_prefix('.')?; + let format = format.split('+').next().unwrap_or(format); + (!format.is_empty()).then(|| format.to_string()) } +pub fn variant_of(file: &str) -> Option { + let normalized = normalize(file); + let base = basename(&normalized); + let dot = base.find('.')?; + let extension = &base[dot..]; + + let stripped = extension.strip_suffix(".erb").or_else(|| extension.strip_suffix(".herb"))?; + let (_, variant) = stripped.split_once('+')?; + + (!variant.is_empty()).then(|| variant.to_string()) +} + pub fn without_template_extension(partial_name: &str) -> &str { EXTENSIONS .iter() diff --git a/rust/herb-analysis/tests/formats_test.rs b/rust/herb-analysis/tests/formats_test.rs index 71d373e4a..081399ac5 100644 --- a/rust/herb-analysis/tests/formats_test.rs +++ b/rust/herb-analysis/tests/formats_test.rs @@ -2,7 +2,7 @@ use std::fs; use std::path::PathBuf; use herb_analysis::partial_index::PartialIndex; -use herb_analysis::partial_resolution::format_of; +use herb_analysis::partial_resolution::{format_of, variant_of}; fn scratch(name: &str) -> PathBuf { let root = std::env::temp_dir().join(format!("herb-formats-{name}")); @@ -86,3 +86,33 @@ fn extension_precedence_still_decides_when_no_format_matches() { assert_eq!(html_partial, index.resolve("posts/row", Some(&turbo_caller))[0]); } + +#[test] +fn reads_the_variant_out_of_a_filename() { + assert_eq!(Some("mobile".to_string()), variant_of("app/views/posts/_row.html+mobile.erb")); + assert_eq!(Some("tablet".to_string()), variant_of("app/views/posts/_row.html+tablet.herb")); + assert_eq!(None, variant_of("app/views/posts/_row.html.erb")); + assert_eq!(None, variant_of("app/views/posts/_row.erb")); +} + +#[test] +fn a_variant_keeps_the_format_of_its_base_template() { + assert_eq!(Some("html".to_string()), format_of("app/views/posts/_row.html+mobile.erb")); + assert_eq!(Some("turbo_stream".to_string()), format_of("app/views/posts/_row.turbo_stream+mobile.erb")); +} + +#[test] +fn the_plain_template_is_preferred_over_a_variant() { + let root = scratch("variant"); + let views = root.join("app/views"); + + let caller = write(&views.join("posts/index.html.erb")); + let variant = write(&views.join("posts/_row.html+mobile.erb")); + let plain = write(&views.join("posts/_row.html.erb")); + + let index = PartialIndex::new(&[views], vec![caller.clone(), variant.clone(), plain.clone()]); + let resolved = index.resolve("posts/row", Some(&caller)); + + assert_eq!(plain, resolved[0]); + assert!(resolved.contains(&variant), "the variant is still reachable: {resolved:?}"); +} diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index dc87524c7..1cbb970a3 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -31,6 +31,11 @@ module Herb # : (String) -> String? def self.format_of: (String) -> String? + # The variant a template is restricted to, if any. Rails renders `_row.html+mobile.erb` only + # when that variant is requested, and falls back to the plain template otherwise. + # : (String) -> String? + def self.variant_of: (String) -> String? + # : (String) -> String def self.without_template_extension: (String) -> String diff --git a/test/analysis/formats_test.rb b/test/analysis/formats_test.rb index bc2ee9da5..7b3b4092b 100644 --- a/test/analysis/formats_test.rb +++ b/test/analysis/formats_test.rb @@ -24,6 +24,34 @@ def write(root, relative) assert_nil Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.herb") end + test "reads the variant out of a filename" do + assert_equal "mobile", Herb::Analysis::PartialResolution.variant_of("app/views/posts/_row.html+mobile.erb") + assert_equal "tablet", Herb::Analysis::PartialResolution.variant_of("app/views/posts/_row.html+tablet.herb") + assert_nil Herb::Analysis::PartialResolution.variant_of("app/views/posts/_row.html.erb") + assert_nil Herb::Analysis::PartialResolution.variant_of("app/views/posts/_row.erb") + end + + test "a variant keeps the format of its base template" do + assert_equal "html", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.html+mobile.erb") + assert_equal "turbo_stream", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.turbo_stream+mobile.erb") + end + + test "the plain template is preferred over a variant" do + Dir.mktmpdir do |dir| + views = File.join(dir, "app", "views") + + caller_file = write(views, "posts/index.html.erb") + variant = write(views, "posts/_row.html+mobile.erb") + plain = write(views, "posts/_row.html.erb") + + index = Herb::Analysis::PartialIndex.new([views], [caller_file, variant, plain]) + resolved = index.resolve("posts/row", caller_file) + + assert_equal plain, resolved.first + assert_includes resolved, variant + end + end + test "a caller reaches the partial matching its own format" do Dir.mktmpdir do |dir| views = File.join(dir, "app", "views") From 3c1d5054ec22a818dd4d73753904a7642497f791 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 06:50:57 +0200 Subject: [PATCH 13/26] Analysis: Resolve partials against the caller's format in TypeScript --- .../analysis/src/partial-index-builder.ts | 5 ++- .../packages/analysis/src/partial-index.ts | 4 +- .../analysis/src/partial-resolution.ts | 28 +++++++++++-- .../analysis/test/partial-resolution.test.ts | 42 +++++++++++++++++++ 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/javascript/packages/analysis/src/partial-index-builder.ts b/javascript/packages/analysis/src/partial-index-builder.ts index 2a2bfd84d..661217a35 100644 --- a/javascript/packages/analysis/src/partial-index-builder.ts +++ b/javascript/packages/analysis/src/partial-index-builder.ts @@ -45,11 +45,14 @@ export async function buildPartialIndex(herb: HerbBackend, projectPath: string): const viewRoot = await findViewRoot(projectPath) const files = await partialsIn(projectPath, viewRoot) const declarations = new Map() + const filesByName = new Map() for (const file of files.sort()) { const name = partialNameForFile(file, viewRoot) if (name === null) continue + filesByName.set(name, [...(filesByName.get(name) ?? []), file]) + const existing = declarations.get(name) if (existing && !outranksTemplate(file, existing.file)) continue @@ -58,7 +61,7 @@ export async function buildPartialIndex(herb: HerbBackend, projectPath: string): if (declaration) declarations.set(name, declaration) } - return new PartialIndex([viewRoot], declarations) + return new PartialIndex([viewRoot], declarations, filesByName) } export function partialIndexFrom(data: SerializedPartialIndex | undefined): PartialIndex | undefined { diff --git a/javascript/packages/analysis/src/partial-index.ts b/javascript/packages/analysis/src/partial-index.ts index 2cf2dfee6..14d6bb9a6 100644 --- a/javascript/packages/analysis/src/partial-index.ts +++ b/javascript/packages/analysis/src/partial-index.ts @@ -100,14 +100,14 @@ export class PartialIndex { return new PartialIndex(data.viewRoots, new Map(Object.entries(data.partials))) } - constructor(viewRoots: string[], declarations: Map) { + constructor(viewRoots: string[], declarations: Map, filesByName?: Map) { this.viewRoots = viewRoots this.declarations = declarations this.files = new Map() this.byFile = new Map() for (const [name, declaration] of declarations) { - this.files.set(name, declaration.file) + this.files.set(name, filesByName?.get(name) ?? declaration.file) this.byFile.set(declaration.file, declaration) } } diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 0fcf909de..71d1f37c1 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -15,7 +15,7 @@ export const PARTIAL_GLOB_PATTERN = `_${TEMPLATE_GLOB_PATTERN}` const PARTIAL_PREFIX = "_" const APPLICATION_DIRECTORY = "application" -export type PartialPaths = Map +export type PartialPaths = Map function normalize(path: string): string { const separated = path.replace(/\\/g, "/") @@ -233,6 +233,26 @@ export function layoutCandidatesFor(templateFile: string, viewRoot: string): str return candidates } +function pickForCaller(candidates: string | string[], sourceFile: string): string | null { + if (!Array.isArray(candidates)) return candidates + if (candidates.length === 0) return null + + const format = formatOf(sourceFile) + + if (format === null) return candidates[0] ?? null + + const ranked = [...candidates].sort((a, b) => rankForFormat(a, format) - rankForFormat(b, format)) + + return ranked[0] ?? null +} + +function rankForFormat(file: string, format: string): number { + const candidate = formatOf(file) + const matches = candidate === format ? 0 : candidate === null ? 1 : 2 + + return matches * 2 + (variantOf(file) === null ? 0 : 1) +} + export function resolvePartial( partialName: string, sourceFile: string, @@ -243,20 +263,20 @@ export function resolvePartial( const exact = index.get(partialName) - if (exact !== undefined) return exact + if (exact !== undefined) return pickForCaller(exact, sourceFile) const sourceDirectory = relativeToViewRoots(dirname(normalize(sourceFile)), viewRoots)?.[1] ?? null if (sourceDirectory !== null && sourceDirectory !== ".") { const relative = index.get(`${sourceDirectory}/${partialName}`) - if (relative !== undefined) return relative + if (relative !== undefined) return pickForCaller(relative, sourceFile) } if (!partialName.includes("/")) { const application = index.get(`${APPLICATION_DIRECTORY}/${partialName}`) - if (application !== undefined) return application + if (application !== undefined) return pickForCaller(application, sourceFile) } return null diff --git a/javascript/packages/analysis/test/partial-resolution.test.ts b/javascript/packages/analysis/test/partial-resolution.test.ts index 37fce52de..804b68223 100644 --- a/javascript/packages/analysis/test/partial-resolution.test.ts +++ b/javascript/packages/analysis/test/partial-resolution.test.ts @@ -200,3 +200,45 @@ describe("variantOf", () => { expect(formatOf("app/views/posts/_row.turbo_stream+mobile.erb")).toBe("turbo_stream") }) }) + +describe("format-aware resolution", () => { + const HTML_CALLER = "app/views/posts/index.html.erb" + const TURBO_CALLER = "app/views/posts/index.turbo_stream.erb" + + test("a caller reaches the partial matching its own format", () => { + const index: PartialPaths = new Map([ + ["posts/row", ["app/views/posts/_row.html.erb", "app/views/posts/_row.turbo_stream.erb"]], + ]) + + expect(resolvePartial("posts/row", HTML_CALLER, index, [VIEW_ROOT])).toBe("app/views/posts/_row.html.erb") + expect(resolvePartial("posts/row", TURBO_CALLER, index, [VIEW_ROOT])).toBe("app/views/posts/_row.turbo_stream.erb") + }) + + test("a formatless partial serves any caller", () => { + const index: PartialPaths = new Map([["posts/row", ["app/views/posts/_row.erb"]]]) + + expect(resolvePartial("posts/row", TURBO_CALLER, index, [VIEW_ROOT])).toBe("app/views/posts/_row.erb") + }) + + test("a formatless partial loses to an exact format match", () => { + const index: PartialPaths = new Map([ + ["posts/row", ["app/views/posts/_row.erb", "app/views/posts/_row.turbo_stream.erb"]], + ]) + + expect(resolvePartial("posts/row", TURBO_CALLER, index, [VIEW_ROOT])).toBe("app/views/posts/_row.turbo_stream.erb") + }) + + test("the plain template is preferred over a variant", () => { + const index: PartialPaths = new Map([ + ["posts/row", ["app/views/posts/_row.html+mobile.erb", "app/views/posts/_row.html.erb"]], + ]) + + expect(resolvePartial("posts/row", HTML_CALLER, index, [VIEW_ROOT])).toBe("app/views/posts/_row.html.erb") + }) + + test("a single file still resolves", () => { + const index: PartialPaths = new Map([["posts/row", "app/views/posts/_row.html.erb"]]) + + expect(resolvePartial("posts/row", HTML_CALLER, index, [VIEW_ROOT])).toBe("app/views/posts/_row.html.erb") + }) +}) From 3cc85b3f06f1bc5a9c8717ab237e0182a64ef53c Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 06:57:01 +0200 Subject: [PATCH 14/26] Analysis: Update viewRoot consumers outside the analysis package --- javascript/packages/analysis/src/project-index.ts | 6 +++--- .../packages/analysis/test/partial-index-builder.test.ts | 4 ++-- javascript/packages/analysis/test/partial-index.test.ts | 2 +- javascript/packages/analysis/test/project-index.test.ts | 4 ++-- javascript/packages/language-server/src/session.ts | 2 +- .../packages/language-service/src/completion_provider.ts | 2 +- .../src/rules/actionview-prefer-qualified-partial-path.ts | 4 ++-- rust/herb-analysis/tests/object_render_test.rs | 6 +++--- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/javascript/packages/analysis/src/project-index.ts b/javascript/packages/analysis/src/project-index.ts index baf60e444..d491e0e9b 100644 --- a/javascript/packages/analysis/src/project-index.ts +++ b/javascript/packages/analysis/src/project-index.ts @@ -58,8 +58,8 @@ export class ProjectIndex { return this.callerIndex } - get viewRoot(): string | undefined { - return this.partialIndex?.viewRoot + get viewRoots(): string[] | undefined { + return this.partialIndex?.viewRoots } async indexAll(): Promise { @@ -71,7 +71,7 @@ export class ProjectIndex { try { this.partialIndex = await buildPartialIndex(this.backend, this.root) - this.logger?.log(`[Partials] Indexed ${this.partialIndex.size} partials under ${this.partialIndex.viewRoot}`) + this.logger?.log(`[Partials] Indexed ${this.partialIndex.size} partials under ${this.partialIndex.viewRoots.join(", ")}`) } catch (error) { this.logger?.warn(`[Partials] Failed to index partials: ${this.messageFor(error)}`) } diff --git a/javascript/packages/analysis/test/partial-index-builder.test.ts b/javascript/packages/analysis/test/partial-index-builder.test.ts index afed4e34d..74f0f38e9 100644 --- a/javascript/packages/analysis/test/partial-index-builder.test.ts +++ b/javascript/packages/analysis/test/partial-index-builder.test.ts @@ -84,7 +84,7 @@ describe("buildPartialIndex", () => { const index = await buildPartialIndex(Herb, root) - expect(index.viewRoot).toBe("app/views") + expect(index.viewRoots).toEqual(["app/views"]) expect(index.size).toBe(2) expect(index.lookup("users/card", "app/views/posts/index.html.erb")).toEqual({ @@ -126,7 +126,7 @@ describe("buildPartialIndex", () => { const index = await buildPartialIndex(Herb, root) - expect(index.viewRoot).toBe(".") + expect(index.viewRoots).toEqual(["."]) expect(index.lookup("views/card", "index.html.erb")?.file).toBe("views/_card.html.erb") }) diff --git a/javascript/packages/analysis/test/partial-index.test.ts b/javascript/packages/analysis/test/partial-index.test.ts index d5d55eb19..cc2b0ad86 100644 --- a/javascript/packages/analysis/test/partial-index.test.ts +++ b/javascript/packages/analysis/test/partial-index.test.ts @@ -43,7 +43,7 @@ describe("PartialIndex", () => { test("round trips through its serialized form", () => { const restored = PartialIndex.from(index.toJSON()) - expect(restored.viewRoot).toBe("app/views") + expect(restored.viewRoots).toEqual(["app/views"]) expect(restored.size).toBe(2) expect(restored.lookup("users/card", "app/views/posts/index.html.erb")?.locals).toEqual([{ name: "user", required: true }]) }) diff --git a/javascript/packages/analysis/test/project-index.test.ts b/javascript/packages/analysis/test/project-index.test.ts index 910dcb46f..a18cdfe1e 100644 --- a/javascript/packages/analysis/test/project-index.test.ts +++ b/javascript/packages/analysis/test/project-index.test.ts @@ -64,13 +64,13 @@ describe("ProjectIndex", () => { test("reports the view root it found", async () => { const index = await analyzerFor() - expect(index.viewRoot).toBe("app/views") + expect(index.viewRoots).toEqual(["app/views"]) }) test("falls back to the project root when there is no app/views", async () => { const index = await analyzerFor({ "templates/_card.html.erb": `
\n` }) - expect(index.viewRoot).toBe(".") + expect(index.viewRoots).toEqual(["."]) }) }) diff --git a/javascript/packages/language-server/src/session.ts b/javascript/packages/language-server/src/session.ts index dd238bd9c..5a31f6ca0 100644 --- a/javascript/packages/language-server/src/session.ts +++ b/javascript/packages/language-server/src/session.ts @@ -105,7 +105,7 @@ export class Session { private viewRootFor(documentPath: string): string | null { const project = this.projects.containing(documentPath) - const viewRoot = project?.index.viewRoot + const viewRoot = project?.index.viewRoots?.[0] if (!project || viewRoot === undefined) return null diff --git a/javascript/packages/language-service/src/completion_provider.ts b/javascript/packages/language-service/src/completion_provider.ts index bf28a3f96..246346dcd 100644 --- a/javascript/packages/language-service/src/completion_provider.ts +++ b/javascript/packages/language-service/src/completion_provider.ts @@ -464,7 +464,7 @@ export class CompletionProvider { if (!partials) return null const file = this.relativePathFor(document.uri) - const directory = file === null ? null : this.directoryOf(file, partials.viewRoot) + const directory = file === null ? null : this.directoryOf(file, partials.viewRoots[0] ?? ".") const lowercasePrefix = prefix.toLowerCase() const nameRange = Range.create(document.positionAt(document.offsetAt(position) - prefix.length), position) diff --git a/javascript/packages/linter/src/rules/actionview-prefer-qualified-partial-path.ts b/javascript/packages/linter/src/rules/actionview-prefer-qualified-partial-path.ts index 8f1b73cb6..9867b7414 100644 --- a/javascript/packages/linter/src/rules/actionview-prefer-qualified-partial-path.ts +++ b/javascript/packages/linter/src/rules/actionview-prefer-qualified-partial-path.ts @@ -3,7 +3,7 @@ import { ParserRule } from "../types.js" import { renderPartialExpression } from "@herb-tools/analysis" import { isERBOutputNode, isPrismNodeType, locationFromByteOffset, substringFromByteOffset } from "@herb-tools/core" -import { partialNameForFile } from "@herb-tools/analysis" +import { partialNameForRoots } from "@herb-tools/analysis" import type { ERBRenderNode, ParseResult, ParserOptions, PrismNode } from "@herb-tools/core" import type { BaseAutofixContext, FullRuleConfig, LintContext, LintOffense, Mutable, UnboundLintOffense } from "../types.js" @@ -106,7 +106,7 @@ class ActionViewPreferQualifiedPartialPathVisitor extends BaseRuleVisitor PathBuf { @@ -14,14 +14,14 @@ fn scratch(name: &str) -> PathBuf { root } -fn write(root: &PathBuf, relative: &str, body: &str) { +fn write(root: &Path, relative: &str, body: &str) { let path = root.join(relative); fs::create_dir_all(path.parent().unwrap()).unwrap(); fs::write(path, body).unwrap(); } -fn check(root: &PathBuf) -> String { +fn check(root: &Path) -> String { let output = Command::new(binary()).args(["actionview", "check", root.to_str().unwrap()]).output().unwrap(); String::from_utf8_lossy(&output.stdout).to_string() From d6fbf3d387a191a8c4760f58c91f06ed5118cf30 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 07:01:27 +0200 Subject: [PATCH 15/26] Analysis: List the partials a dynamic render could reach --- rust/herb-analysis/src/actionview_cli.rs | 21 ++++++++++++--- rust/herb-analysis/src/partial_index.rs | 6 +++++ .../herb-analysis/tests/object_render_test.rs | 26 +++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index b8bf00099..1eda34a52 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -209,7 +209,7 @@ fn check(arguments: &[String]) -> i32 { let mut rendered: Vec = Vec::new(); let mut files_with_renders: BTreeSet = BTreeSet::new(); let mut dynamic_renders = 0usize; - let mut dynamic_sites: Vec<(String, String)> = Vec::new(); + let mut dynamic_sites: Vec<(String, String, Vec)> = Vec::new(); let mut branching_sites: Vec<(String, String, Vec)> = Vec::new(); let mut other_renders = 0usize; let mut with_partial_count = 0usize; @@ -262,7 +262,13 @@ fn check(arguments: &[String]) -> i32 { .map(|prefix| format!("{prefix}/#{{...}}")) .unwrap_or_else(|| "#{...}".to_string()); - dynamic_sites.push((relative(file, &root), shown)); + let candidates = call + .dynamic_prefix + .as_ref() + .map(|prefix| index.names_under(prefix).iter().map(|name| (*name).to_string()).collect()) + .unwrap_or_default(); + + dynamic_sites.push((relative(file, &root), shown, candidates)); } continue; @@ -404,11 +410,18 @@ fn check(arguments: &[String]) -> i32 { if !dynamic_sites.is_empty() { println!(" {}", "Dynamic render calls:".bold()); - println!(" {}", "The partial name is built at runtime, so it cannot be resolved statically.".dimmed()); + println!( + " {}", + "The partial name is built at runtime. Where the directory is known, every partial under it is listed.".dimmed() + ); println!(); - for (file, shown) in &dynamic_sites { + for (file, shown, candidates) in &dynamic_sites { println!(" {} {} {}", "\u{2717}".red().bold(), shown.red().bold(), format!("in {file}").dimmed()); + + for candidate in candidates { + println!(" {} {}", "\u{2192}".dimmed(), candidate.dimmed()); + } } println!(); diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 750b4e890..5543d2528 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -121,6 +121,12 @@ impl PartialIndex { self.by_name.keys().map(|name| name.as_str()).collect() } + pub fn names_under(&self, prefix: &str) -> Vec<&str> { + let prefix = format!("{}/", prefix.trim_end_matches('/')); + + self.by_name.keys().filter(|name| name.starts_with(&prefix)).map(|name| name.as_str()).collect() + } + pub fn to_h(&mut self) -> BTreeMap { let names: Vec = self.names().iter().map(|name| name.to_string()).collect(); let mut partials = BTreeMap::new(); diff --git a/rust/herb-analysis/tests/object_render_test.rs b/rust/herb-analysis/tests/object_render_test.rs index c407a5b6d..4caf38c67 100644 --- a/rust/herb-analysis/tests/object_render_test.rs +++ b/rust/herb-analysis/tests/object_render_test.rs @@ -60,3 +60,29 @@ fn a_guessed_object_partial_that_exists_still_resolves() { assert!(!output.contains("posts/post"), "an existing guessed target should resolve:\n{output}"); } + +#[test] +fn a_dynamic_render_lists_the_partials_under_its_prefix() { + let root = scratch("dynamic-prefix"); + + write(&root, "app/views/admin/show.html.erb", "<%= render \"admin/parts/#{name}\" %>\n"); + write(&root, "app/views/admin/parts/_alpha.html.erb", "
\n"); + write(&root, "app/views/admin/parts/_beta.html.erb", "
\n"); + + let output = check(&root); + + assert!(output.contains("admin/parts/alpha"), "candidates should be listed:\n{output}"); + assert!(output.contains("admin/parts/beta"), "candidates should be listed:\n{output}"); +} + +#[test] +fn a_dynamic_render_with_no_known_directory_lists_nothing() { + let root = scratch("dynamic-bare"); + + write(&root, "app/views/admin/show.html.erb", "<%= render \"#{name}\" %>\n"); + write(&root, "app/views/admin/parts/_alpha.html.erb", "
\n"); + + let output = check(&root); + + assert!(!output.contains("admin/parts/alpha"), "nothing should be claimed:\n{output}"); +} From cbb6e3b3a1ef04a6202d50cd7adedb592227bbf6 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 07:03:18 +0200 Subject: [PATCH 16/26] Analysis: Ignore component templates at any depth --- lib/herb/analysis/render_analyzer.rb | 4 ++-- rust/herb-analysis/src/actionview_cli.rs | 8 ++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index a588884c1..14deec877 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -529,7 +529,7 @@ def print_summary_line(result) #: (String) -> bool def component_template?(relative) - relative.start_with?("app/components/") + relative.start_with?("app/components/") || relative.include?("/app/components/") end #: (Array[Hash[Symbol, untyped]]) -> void @@ -558,7 +558,7 @@ def print_warning_summary_line(warnings) return unless ignored.positive? - puts " #{label("Ignored")} #{dimmed("#{ignored} component #{pluralize(ignored, "template")} in app/components/")}" + puts " #{label("Ignored")} #{dimmed("#{ignored} component #{pluralize(ignored, "template")} under app/components/")}" end private diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 1eda34a52..970a4c761 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -537,7 +537,11 @@ fn check(arguments: &[String]) -> i32 { println!( " {} {}", label("Ignored"), - format!("{ignored_components} component {} in app/components/", plural(ignored_components, "template")).dimmed() + format!( + "{ignored_components} component {} under app/components/", + plural(ignored_components, "template") + ) + .dimmed() ); } @@ -1454,7 +1458,7 @@ fn print_dependency_warnings( } if !rest.is_empty() { - if relative.starts_with("app/components/") { + if relative.starts_with("app/components/") || relative.contains("/app/components/") { ignored_components += 1; } else if partial && !declared && candidates.is_none_or(BTreeSet::is_empty) { uninferable.push((relative, rest)); From 49285bd29000e6cf9ac8c5f9cfcc0e16791bac49 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 07:09:34 +0200 Subject: [PATCH 17/26] Analysis: Only treat path-like names as dynamic renders --- rust/herb-analysis/src/actionview_cli.rs | 10 +++++++--- rust/herb-analysis/src/template_dependencies.rs | 8 +++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 970a4c761..b5d8accb4 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -97,8 +97,12 @@ fn missing_format(file: &str) -> bool { } fn static_branches(expression: &str) -> Vec { - let mut found = Vec::new(); - let mut rest = expression; + let mut found: Vec = Vec::new(); + + let mut rest = match expression.split_once('?') { + Some((_, branches)) => branches, + None => expression, + }; while let Some(start) = rest.find(['"', '\'']) { let quote = rest.as_bytes()[start] as char; @@ -110,7 +114,7 @@ fn static_branches(expression: &str) -> Vec { let literal = &after[..end]; - if !literal.is_empty() && !literal.contains("#{") { + if !literal.is_empty() && !literal.contains("#{") && !found.iter().any(|seen| seen == literal) { found.push(literal.to_string()); } diff --git a/rust/herb-analysis/src/template_dependencies.rs b/rust/herb-analysis/src/template_dependencies.rs index c325a02a1..358dd5df1 100644 --- a/rust/herb-analysis/src/template_dependencies.rs +++ b/rust/herb-analysis/src/template_dependencies.rs @@ -361,13 +361,19 @@ fn dynamic_prefix_of(value: &str) -> Option { let value = value.trim_start_matches(['"', '\'']); let head = value.split("#{").next()?.trim_end_matches('/'); - if head.is_empty() { + if head.is_empty() || !partial_path_segment(head) { None } else { Some(head.to_string()) } } +fn partial_path_segment(value: &str) -> bool { + value + .chars() + .all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '_' || c == '/' || c == '-') +} + fn interpolated_render_prefix(node: &herb::prism::PrismNode) -> Option { if node.is("InterpolatedStringNode") { let first = node.children.first()?; From 49e525fed36f3f76b6edcb38b62c30a571362842 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 07:18:35 +0200 Subject: [PATCH 18/26] Analysis: Resolve conditional render branches in Ruby --- lib/herb/analysis/render_analyzer.rb | 26 +++++++++++++++++-- sig/herb/analysis/partial_index.rbs | 3 --- sig/herb/analysis/partial_resolution.rbs | 4 --- sig/herb/analysis/render_analyzer.rbs | 3 +++ sig/herb/analysis/ruby_locals_index.rbs | 1 - .../ruby_locals_index/named_reference.rbs | 2 -- sig/herb/analysis/template_dependencies.rbs | 2 -- 7 files changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index 14deec877..34c949a54 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -1056,11 +1056,22 @@ def build_render_graph(render_calls_by_file, partial_files, view_root) next unless partial_reference resolved = resolve_partial(partial_reference, file, partial_files, view_root) + if resolved resolved_name = partial_name_for_file(resolved, view_root) resolved_names << resolved_name if resolved_name else - resolved_names << partial_reference + branches = static_branches(partial_reference) + targets = branches.filter_map { |branch| resolve_partial(branch, file, partial_files, view_root) } + + if branches.size > 1 && targets.size == branches.size + targets.each do |target| + name = partial_name_for_file(target, view_root) + resolved_names << name if name + end + else + resolved_names << partial_reference + end end end @@ -1171,8 +1182,12 @@ def collect_all_dynamic_prefixes(dynamic_calls, ruby_references) def find_unresolved(render_calls, partial_files, view_root) render_calls.select do |call| next false unless call[:partial] + next false if resolve_partial(call[:partial], call[:file], partial_files, view_root) + + branches = static_branches(call[:partial]) + targets = branches.filter_map { |branch| resolve_partial(branch, call[:file], partial_files, view_root) } - !resolve_partial(call[:partial], call[:file], partial_files, view_root) + !(branches.size > 1 && targets.size == branches.size) end end @@ -1180,6 +1195,13 @@ def resolve_partial(partial_name, source_file, _partial_files, view_root) partial_index(view_root).resolve(partial_name, source_file).first end + #: (String) -> Array[String] + def static_branches(expression) + branches = expression.split("?", 2).last.to_s + + branches.scan(/["']([^"'\#]+)["']/).flatten.uniq + end + #: (String) -> String? def render_name_kind(name) return "instance variable" if name.start_with?("@") diff --git a/sig/herb/analysis/partial_index.rbs b/sig/herb/analysis/partial_index.rbs index bb413775a..9d5498d7f 100644 --- a/sig/herb/analysis/partial_index.rbs +++ b/sig/herb/analysis/partial_index.rbs @@ -23,9 +23,6 @@ module Herb def files_for: (String?) -> Array[String] # : (String?, String?) -> Array[String] - # Rails picks the candidate whose format matches the template doing the rendering, so a - # `.turbo_stream.erb` caller reaches the turbo_stream partial even though `.html.erb` outranks - # it everywhere else. A partial with no format of its own matches any caller. # : (String?, String?) -> Array[String] def resolve: (String?, String?) -> Array[String] | (String?, String?) -> Array[String] diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index 1cbb970a3..28d964a65 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -26,13 +26,9 @@ module Herb # : (String | Pathname) -> Pathname def self.view_root_for: (String | Pathname) -> Pathname - # The format segment of a template filename, if it carries one. `_row.html.erb` is `html`, - # `_row.erb` is none and therefore matches any format. # : (String) -> String? def self.format_of: (String) -> String? - # The variant a template is restricted to, if any. Rails renders `_row.html+mobile.erb` only - # when that variant is requested, and falls back to the plain template otherwise. # : (String) -> String? def self.variant_of: (String) -> String? diff --git a/sig/herb/analysis/render_analyzer.rbs b/sig/herb/analysis/render_analyzer.rbs index f224d630b..5ba348492 100644 --- a/sig/herb/analysis/render_analyzer.rbs +++ b/sig/herb/analysis/render_analyzer.rbs @@ -117,6 +117,9 @@ module Herb def resolve_partial: (untyped partial_name, untyped source_file, untyped _partial_files, untyped view_root) -> untyped + # : (String) -> Array[String] + def static_branches: (String) -> Array[String] + # : (String) -> String? def render_name_kind: (String) -> String? diff --git a/sig/herb/analysis/ruby_locals_index.rbs b/sig/herb/analysis/ruby_locals_index.rbs index 4b5a36a02..24c5cbc1a 100644 --- a/sig/herb/analysis/ruby_locals_index.rbs +++ b/sig/herb/analysis/ruby_locals_index.rbs @@ -24,7 +24,6 @@ module Herb # : (String) -> Local? def find: (String) -> Local? - # Every name the template binds, regardless of where. # : () -> Set[String] def names: () -> Set[String] diff --git a/sig/herb/analysis/ruby_locals_index/named_reference.rbs b/sig/herb/analysis/ruby_locals_index/named_reference.rbs index e63910b7c..053eb2546 100644 --- a/sig/herb/analysis/ruby_locals_index/named_reference.rbs +++ b/sig/herb/analysis/ruby_locals_index/named_reference.rbs @@ -3,8 +3,6 @@ module Herb module Analysis class RubyLocalsIndex - # A name, with where it appears in the source as a byte offset and length, - # which is how Prism reports it. class NamedReference attr_reader name: String diff --git a/sig/herb/analysis/template_dependencies.rbs b/sig/herb/analysis/template_dependencies.rbs index 0153504af..ffefc8560 100644 --- a/sig/herb/analysis/template_dependencies.rbs +++ b/sig/herb/analysis/template_dependencies.rbs @@ -60,7 +60,6 @@ module Herb def dependency_index: (untyped file_path) -> untyped # @rbs! - # KERNEL_METHODS: Array[String] KERNEL_METHODS: untyped def scan_helpers!: () -> untyped @@ -83,7 +82,6 @@ module Herb def symbol_after: (String, String) -> String? # @rbs! - # UNCOUNTABLE: Array[String] UNCOUNTABLE: untyped # : (String, String) -> String? From 2845ba4d96ea0336ea102e020f7942e474bca8e4 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 07:21:18 +0200 Subject: [PATCH 19/26] Analysis: Bring the Ruby check output back to parity --- lib/herb/analysis/render_analyzer.rb | 27 ++++++++++++++++++++++++++- sig/herb/analysis/render_analyzer.rbs | 6 ++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index 34c949a54..46328e330 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -438,6 +438,19 @@ def analyze_from_collected(render_calls_by_file:, dynamic_prefixes_from_erb: [], end def print_file_lists(result) + formatless = result.partial_files.values.select { |file| missing_format?(file) }.sort + + if formatless.any? + puts "\n" + puts " #{bold("Templates without a format:")}" + puts " #{dimmed("Rails reads a template filename as `name.format.handler`. Without a format it matches every one.")}" + puts "" + + formatless.each do |file| + puts " #{bold(yellow("!"))} #{yellow(relative_path(file))}" + end + end + return unless result.issues? if result.unresolved.any? @@ -1168,7 +1181,7 @@ def collect_all_dynamic_prefixes(dynamic_calls, ruby_references) prefix = call[:partial].gsub(/\A["']|["']\z/, "") prefix = prefix.split("\#{").first&.chomp("/") - prefix unless prefix.nil? || prefix.empty? + prefix if prefix && !prefix.empty? && partial_path_segment?(prefix) } ruby_references.each do |reference| @@ -1195,6 +1208,18 @@ def resolve_partial(partial_name, source_file, _partial_files, view_root) partial_index(view_root).resolve(partial_name, source_file).first end + #: (String) -> bool + def missing_format?(file) + name = File.basename(file) + + name.end_with?(".erb") && name.count(".") == 1 + end + + #: (String) -> bool + def partial_path_segment?(value) + value.match?(%r{\A[a-z0-9_/-]+\z}) + end + #: (String) -> Array[String] def static_branches(expression) branches = expression.split("?", 2).last.to_s diff --git a/sig/herb/analysis/render_analyzer.rbs b/sig/herb/analysis/render_analyzer.rbs index 5ba348492..fc337d9f2 100644 --- a/sig/herb/analysis/render_analyzer.rbs +++ b/sig/herb/analysis/render_analyzer.rbs @@ -117,6 +117,12 @@ module Herb def resolve_partial: (untyped partial_name, untyped source_file, untyped _partial_files, untyped view_root) -> untyped + # : (String) -> bool + def missing_format?: (String) -> bool + + # : (String) -> bool + def partial_path_segment?: (String) -> bool + # : (String) -> Array[String] def static_branches: (String) -> Array[String] From c05373223bd3a636eac5d942ac9a8c4d9d0776ef Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 08:08:43 +0200 Subject: [PATCH 20/26] Analysis: List the partials a dynamic render could reach in Ruby --- lib/herb/analysis/render_analyzer.rb | 20 +++++++++++++++++++- sig/herb/analysis/render_analyzer.rbs | 3 +++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index 46328e330..e0cdb48d1 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -481,13 +481,17 @@ def print_file_lists(result) puts "\n #{separator}" if result.unresolved.any? puts "\n" puts " #{bold("Dynamic render calls:")}" - puts " #{dimmed("The partial name is built at runtime, so it cannot be resolved statically.")}" + puts " #{dimmed("The partial name is built at runtime. Where the directory is known, every partial under it is listed.")}" puts "" result.dynamic_calls.each do |call| shown = dynamic_call_display(call) puts " #{bold(red("\u2717"))} #{bold(red(shown))} #{dimmed("in #{relative_path(call[:file])}")}" + + names_under(call, result.partial_files).each do |name| + puts " #{dimmed("\u2192")} #{dimmed(name)}" + end end end @@ -1208,6 +1212,20 @@ def resolve_partial(partial_name, source_file, _partial_files, view_root) partial_index(view_root).resolve(partial_name, source_file).first end + #: (Hash[Symbol, untyped], Hash[String, String]) -> Array[String] + def names_under(call, partial_files) + prefix = call[:dynamic_prefix] + + unless prefix + raw = call[:partial].to_s.sub(/\A["']/, "") + prefix = raw.split("\#{").first.to_s.chomp("/") + end + + return [] if prefix.to_s.empty? || !partial_path_segment?(prefix) + + partial_files.keys.select { |name| name.start_with?("#{prefix}/") }.sort + end + #: (String) -> bool def missing_format?(file) name = File.basename(file) diff --git a/sig/herb/analysis/render_analyzer.rbs b/sig/herb/analysis/render_analyzer.rbs index fc337d9f2..1df999a30 100644 --- a/sig/herb/analysis/render_analyzer.rbs +++ b/sig/herb/analysis/render_analyzer.rbs @@ -117,6 +117,9 @@ module Herb def resolve_partial: (untyped partial_name, untyped source_file, untyped _partial_files, untyped view_root) -> untyped + # : (Hash[Symbol, untyped], Hash[String, String]) -> Array[String] + def names_under: (Hash[Symbol, untyped], Hash[String, String]) -> Array[String] + # : (String) -> bool def missing_format?: (String) -> bool From 692f7b38900ee7bc0fd2e2b62d75477ad33301d3 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 08:15:54 +0200 Subject: [PATCH 21/26] Analysis: Follow renders out of prefix-matched partials when finding unused --- lib/herb/analysis/render_analyzer.rb | 37 +++++++++++++++++++++++++++ sig/herb/analysis/render_analyzer.rbs | 3 +++ 2 files changed, 40 insertions(+) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index e0cdb48d1..6f65a6198 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -451,6 +451,23 @@ def print_file_lists(result) end end + conditional = conditional_calls(result) + + if conditional.any? + puts "\n" + puts " #{bold("Conditional render calls:")}" + puts " #{dimmed("The partial name is chosen at runtime, but every branch is a literal.")}" + puts "" + + conditional.each do |call, targets| + puts " #{bold(yellow("?"))} #{call[:partial]} #{dimmed("in #{relative_path(call[:file])}")}" + + targets.each do |target| + puts " #{dimmed("\u2192")} #{green(relative_path(target))}" + end + end + end + return unless result.issues? if result.unresolved.any? @@ -1131,6 +1148,13 @@ def find_unused_by_reachability(render_graph, partial_files, ruby_references, dy queue << resolved_file if resolved_file end + partial_files.each do |name, file| + next unless dynamic_prefixes.any? { |prefix| name.start_with?("#{prefix}/") } + + reachable << name + queue << file if file + end + visited_files = Set.new until queue.empty? @@ -1212,6 +1236,19 @@ def resolve_partial(partial_name, source_file, _partial_files, view_root) partial_index(view_root).resolve(partial_name, source_file).first end + #: (untyped) -> Array[[Hash[Symbol, untyped], Array[String]]] + def conditional_calls(result) + result.render_calls.filter_map do |call| + next unless call[:partial] + next if resolve_partial(call[:partial], call[:file], result.partial_files, result.view_root) + + branches = static_branches(call[:partial]) + targets = branches.filter_map { |branch| resolve_partial(branch, call[:file], result.partial_files, result.view_root) } + + [call, targets] if branches.size > 1 && targets.size == branches.size + end + end + #: (Hash[Symbol, untyped], Hash[String, String]) -> Array[String] def names_under(call, partial_files) prefix = call[:dynamic_prefix] diff --git a/sig/herb/analysis/render_analyzer.rbs b/sig/herb/analysis/render_analyzer.rbs index 1df999a30..b6e6bbcce 100644 --- a/sig/herb/analysis/render_analyzer.rbs +++ b/sig/herb/analysis/render_analyzer.rbs @@ -117,6 +117,9 @@ module Herb def resolve_partial: (untyped partial_name, untyped source_file, untyped _partial_files, untyped view_root) -> untyped + # : (untyped) -> Array[[Hash[Symbol, untyped], Array[String]]] + def conditional_calls: (untyped) -> Array[[ Hash[Symbol, untyped], Array[String] ]] + # : (Hash[Symbol, untyped], Hash[String, String]) -> Array[String] def names_under: (Hash[Symbol, untyped], Hash[String, String]) -> Array[String] From ee1bd56bf2ba66603f3b1e93fb00c84a4eb594e5 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 08:22:30 +0200 Subject: [PATCH 22/26] Analysis: Scan every statement in a Ruby file for render references --- rust/herb-analysis/src/ruby_render_references.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rust/herb-analysis/src/ruby_render_references.rs b/rust/herb-analysis/src/ruby_render_references.rs index 1f2b301fa..01c4a13b6 100644 --- a/rust/herb-analysis/src/ruby_render_references.rs +++ b/rust/herb-analysis/src/ruby_render_references.rs @@ -74,6 +74,7 @@ pub fn collect_from_source(source: &str, references: &mut RubyRenderReferences) let wrapped = format!("<% {} %>", source); let options = ParserOptions { prism_nodes: true, + prism_program: true, ..Default::default() }; @@ -81,6 +82,12 @@ pub fn collect_from_source(source: &str, references: &mut RubyRenderReferences) return; }; + if let Some(program) = result.value.prism() { + walk(program, references); + + return; + } + for child in &result.value.children { if let herb::nodes::AnyNode::ERBContentNode(node) = child { if let Some(prism) = node.prism() { From 17891136504c74054a5818df5d713d6818ec661e Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Fri, 14 Aug 2026 11:33:35 +0200 Subject: [PATCH 23/26] Analysis: Fix CI and bring the Ruby check output closer to parity --- javascript/packages/analysis/src/partial-resolution.ts | 5 +++-- .../packages/analysis/test/partial-resolution.test.ts | 1 + lib/herb/analysis/partial_resolution.rb | 2 +- lib/herb/analysis/ruby_locals_index/offset_table.rb | 1 + rust/herb-analysis/src/partial_resolution.rs | 6 +++--- rust/herb-analysis/tests/formats_test.rs | 1 + rust/herb-analysis/tests/object_render_test.rs | 2 ++ sig/herb/analysis/ruby_locals_index/offset_table.rbs | 2 -- test/analysis/formats_test.rb | 1 + 9 files changed, 13 insertions(+), 8 deletions(-) diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 71d1f37c1..8b8e25406 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -105,8 +105,9 @@ export function formatOf(filePath: string): string | null { if (stripped === null) return null - const withVariant = stripped.startsWith(".") ? stripped.slice(1) : stripped - const format = withVariant.split("+")[0] ?? withVariant + const segments = stripped.startsWith(".") ? stripped.slice(1) : stripped + const last = segments.split(".").pop() ?? segments + const format = last.split("+")[0] ?? last return format === "" ? null : format } diff --git a/javascript/packages/analysis/test/partial-resolution.test.ts b/javascript/packages/analysis/test/partial-resolution.test.ts index 804b68223..89ccb0bcc 100644 --- a/javascript/packages/analysis/test/partial-resolution.test.ts +++ b/javascript/packages/analysis/test/partial-resolution.test.ts @@ -176,6 +176,7 @@ describe("formatOf", () => { expect(formatOf("app/views/posts/_row.html.erb")).toBe("html") expect(formatOf("app/views/posts/_row.turbo_stream.erb")).toBe("turbo_stream") expect(formatOf("app/views/posts/_row.html.herb")).toBe("html") + expect(formatOf("app/views/posts/_row.en.html.erb")).toBe("html") }) test("returns null when the filename carries no format", () => { diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index 0ba1c55e4..fcf0b6e25 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -54,7 +54,7 @@ def format_of(file) return nil if stripped == extension - format = stripped.delete_prefix(".").split("+").first.to_s + format = stripped.delete_prefix(".").split(".").last.to_s.split("+").first.to_s format.empty? ? nil : format end diff --git a/lib/herb/analysis/ruby_locals_index/offset_table.rb b/lib/herb/analysis/ruby_locals_index/offset_table.rb index e9a0560de..4a3178bf7 100644 --- a/lib/herb/analysis/ruby_locals_index/offset_table.rb +++ b/lib/herb/analysis/ruby_locals_index/offset_table.rb @@ -5,6 +5,7 @@ module Analysis class RubyLocalsIndex class OffsetTable # @rbs! + # @line_starts: Array[Integer] #: (String) -> void def initialize(source) diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index 41660e39d..230dbae55 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -63,8 +63,8 @@ pub fn format_of(file: &str) -> Option { let extension = &base[dot..]; let stripped = extension.strip_suffix(".erb").or_else(|| extension.strip_suffix(".herb"))?; - let format = stripped.strip_prefix('.')?; - + let segments = stripped.strip_prefix('.')?; + let format = segments.rsplit('.').next().unwrap_or(segments); let format = format.split('+').next().unwrap_or(format); (!format.is_empty()).then(|| format.to_string()) @@ -77,7 +77,7 @@ pub fn variant_of(file: &str) -> Option { let extension = &base[dot..]; let stripped = extension.strip_suffix(".erb").or_else(|| extension.strip_suffix(".herb"))?; - let (_, variant) = stripped.split_once('+')?; + let (_, variant) = stripped.rsplit_once('+')?; (!variant.is_empty()).then(|| variant.to_string()) } diff --git a/rust/herb-analysis/tests/formats_test.rs b/rust/herb-analysis/tests/formats_test.rs index 081399ac5..28c33bd48 100644 --- a/rust/herb-analysis/tests/formats_test.rs +++ b/rust/herb-analysis/tests/formats_test.rs @@ -26,6 +26,7 @@ fn reads_the_format_out_of_a_filename() { assert_eq!(Some("html".to_string()), format_of("app/views/posts/_row.html.herb")); assert_eq!(None, format_of("app/views/posts/_row.erb")); assert_eq!(None, format_of("app/views/posts/_row.herb")); + assert_eq!(Some("html".to_string()), format_of("app/views/posts/_row.en.html.erb")); } #[test] diff --git a/rust/herb-analysis/tests/object_render_test.rs b/rust/herb-analysis/tests/object_render_test.rs index 4caf38c67..b2def58d8 100644 --- a/rust/herb-analysis/tests/object_render_test.rs +++ b/rust/herb-analysis/tests/object_render_test.rs @@ -1,3 +1,5 @@ +#![cfg(feature = "cli")] + use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; diff --git a/sig/herb/analysis/ruby_locals_index/offset_table.rbs b/sig/herb/analysis/ruby_locals_index/offset_table.rbs index 4e2fbe794..9fc574412 100644 --- a/sig/herb/analysis/ruby_locals_index/offset_table.rbs +++ b/sig/herb/analysis/ruby_locals_index/offset_table.rbs @@ -3,8 +3,6 @@ module Herb module Analysis class RubyLocalsIndex - # Prism reports byte offsets into the whole template while the Herb AST - # reports lines and columns, so one of them has to be translated. class OffsetTable @line_starts: Array[Integer] diff --git a/test/analysis/formats_test.rb b/test/analysis/formats_test.rb index 7b3b4092b..ce4a6563c 100644 --- a/test/analysis/formats_test.rb +++ b/test/analysis/formats_test.rb @@ -22,6 +22,7 @@ def write(root, relative) assert_equal "html", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.html.herb") assert_nil Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.erb") assert_nil Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.herb") + assert_equal "html", Herb::Analysis::PartialResolution.format_of("app/views/posts/_row.en.html.erb") end test "reads the variant out of a filename" do From cf34d01d4ae3b44a7669d210ca1a7a60953acf71 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sat, 15 Aug 2026 15:43:47 +0200 Subject: [PATCH 24/26] Analysis: Prefer the plainest template when several formats match --- .../analysis/src/partial-resolution.ts | 22 ++++++++++++++++++- ...fer-qualified-partial-path.autofix.test.ts | 2 +- .../actionview-no-strict-locals-error.test.ts | 4 ++-- ...view-prefer-qualified-partial-path.test.ts | 4 ++-- lib/herb/analysis/partial_index.rb | 2 +- lib/herb/analysis/partial_resolution.rb | 16 ++++++++++++++ rust/herb-analysis/src/partial_index.rs | 6 ++++- rust/herb-analysis/src/partial_resolution.rs | 17 ++++++++++++++ sig/herb/analysis/partial_resolution.rbs | 3 +++ 9 files changed, 68 insertions(+), 8 deletions(-) diff --git a/javascript/packages/analysis/src/partial-resolution.ts b/javascript/packages/analysis/src/partial-resolution.ts index 8b8e25406..1e05eafbf 100644 --- a/javascript/packages/analysis/src/partial-resolution.ts +++ b/javascript/packages/analysis/src/partial-resolution.ts @@ -112,6 +112,26 @@ export function formatOf(filePath: string): string | null { return format === "" ? null : format } +export function hasLocale(filePath: string): boolean { + const name = basename(normalize(filePath)) + const dot = name.indexOf(".") + + if (dot === -1) return false + + const extension = name.slice(dot) + const stripped = extension.endsWith(".erb") + ? extension.slice(0, -".erb".length) + : extension.endsWith(".herb") + ? extension.slice(0, -".herb".length) + : null + + if (stripped === null) return false + + const segments = (stripped.startsWith(".") ? stripped.slice(1) : stripped).split(".") + + return segments.length > 1 +} + export function variantOf(filePath: string): string | null { const name = basename(normalize(filePath)) const dot = name.indexOf(".") @@ -251,7 +271,7 @@ function rankForFormat(file: string, format: string): number { const candidate = formatOf(file) const matches = candidate === format ? 0 : candidate === null ? 1 : 2 - return matches * 2 + (variantOf(file) === null ? 0 : 1) + return matches * 4 + (variantOf(file) === null ? 0 : 2) + (hasLocale(file) ? 1 : 0) } export function resolvePartial( diff --git a/javascript/packages/linter/test/autofix/actionview-prefer-qualified-partial-path.autofix.test.ts b/javascript/packages/linter/test/autofix/actionview-prefer-qualified-partial-path.autofix.test.ts index e1cbf4e88..c6a9bc25a 100644 --- a/javascript/packages/linter/test/autofix/actionview-prefer-qualified-partial-path.autofix.test.ts +++ b/javascript/packages/linter/test/autofix/actionview-prefer-qualified-partial-path.autofix.test.ts @@ -11,7 +11,7 @@ function declaration(file: string): PartialDeclaration { return { file, hasDeclaration: false, hasKeywordRest: false, locals: [] } } -const partials = new PartialIndex("app/views", new Map([ +const partials = new PartialIndex(["app/views"], new Map([ ["posts/card", declaration("app/views/posts/_card.html.erb")], ["posts/row", declaration("app/views/posts/_row.html.erb")], ["application/flash", declaration("app/views/application/_flash.html.erb")], diff --git a/javascript/packages/linter/test/rules/actionview-no-strict-locals-error.test.ts b/javascript/packages/linter/test/rules/actionview-no-strict-locals-error.test.ts index af3a105e4..bdd3df895 100644 --- a/javascript/packages/linter/test/rules/actionview-no-strict-locals-error.test.ts +++ b/javascript/packages/linter/test/rules/actionview-no-strict-locals-error.test.ts @@ -15,7 +15,7 @@ function declaration(file: string, locals: PartialDeclaration["locals"], overrid return { file, hasDeclaration: true, hasKeywordRest: false, locals, ...overrides } } -const partials = new PartialIndex("app/views", new Map([ +const partials = new PartialIndex(["app/views"], new Map([ ["users/card", declaration("app/views/users/_card.html.erb", [ { name: "user", required: true }, { name: "size", required: false }, @@ -230,7 +230,7 @@ describe("actionview-no-strict-locals-error", () => { }) describe("declaration frames", () => { - const located = new PartialIndex("app/views", new Map([ + const located = new PartialIndex(["app/views"], new Map([ ["users/card", { ...declaration("app/views/users/_card.html.erb", [{ name: "user", required: true }]), location: { line: 1, column: 0 } }], ["users/plain", declaration("app/views/users/_plain.html.erb", [{ name: "user", required: true }])], ])) diff --git a/javascript/packages/linter/test/rules/actionview-prefer-qualified-partial-path.test.ts b/javascript/packages/linter/test/rules/actionview-prefer-qualified-partial-path.test.ts index b10f17d61..6c917b5f6 100644 --- a/javascript/packages/linter/test/rules/actionview-prefer-qualified-partial-path.test.ts +++ b/javascript/packages/linter/test/rules/actionview-prefer-qualified-partial-path.test.ts @@ -13,7 +13,7 @@ function declaration(file: string): PartialDeclaration { return { file, hasDeclaration: false, hasKeywordRest: false, locals: [] } } -const partials = new PartialIndex("app/views", new Map([ +const partials = new PartialIndex(["app/views"], new Map([ ["posts/card", declaration("app/views/posts/_card.html.erb")], ["application/flash", declaration("app/views/application/_flash.html.erb")], ])) @@ -70,7 +70,7 @@ describe("actionview-prefer-qualified-partial-path", () => { test("falls back to the generic advice when the partial does not resolve", () => { expectInfo(GENERIC) - assertOffenses(`<%= render "card" %>`, { fileName: "app/views/nowhere/index.html.erb", partials: new PartialIndex("app/views", new Map()) }) + assertOffenses(`<%= render "card" %>`, { fileName: "app/views/nowhere/index.html.erb", partials: new PartialIndex(["app/views"], new Map()) }) }) test("does not flag a qualified path in the shorthand form", () => { diff --git a/lib/herb/analysis/partial_index.rb b/lib/herb/analysis/partial_index.rb index 891177807..3f88a98a4 100644 --- a/lib/herb/analysis/partial_index.rb +++ b/lib/herb/analysis/partial_index.rb @@ -63,7 +63,7 @@ def resolve(partial_name, source_file) 2 end - [matches, PartialResolution.variant_of(file) ? 1 : 0] + [matches, PartialResolution.variant_of(file) ? 1 : 0, PartialResolution.has_locale?(file) ? 1 : 0] end end diff --git a/lib/herb/analysis/partial_resolution.rb b/lib/herb/analysis/partial_resolution.rb index fcf0b6e25..eca7af85c 100644 --- a/lib/herb/analysis/partial_resolution.rb +++ b/lib/herb/analysis/partial_resolution.rb @@ -59,6 +59,22 @@ def format_of(file) format.empty? ? nil : format end + #: (String) -> bool + def has_locale?(file) + base = File.basename(file) + dot = base.index(".") + + return false unless dot + + extension = base[dot..].to_s + stripped = extension.delete_suffix(".erb") + stripped = stripped.delete_suffix(".herb") if stripped == extension + + return false if stripped == extension + + stripped.delete_prefix(".").split(".").size > 1 + end + #: (String) -> String? def variant_of(file) base = File.basename(file) diff --git a/rust/herb-analysis/src/partial_index.rs b/rust/herb-analysis/src/partial_index.rs index 5543d2528..ba3a2f25f 100644 --- a/rust/herb-analysis/src/partial_index.rs +++ b/rust/herb-analysis/src/partial_index.rs @@ -183,7 +183,11 @@ impl PartialIndex { Some(_) => 2, }; - (matches, usize::from(partial_resolution::variant_of(file).is_some())) + ( + matches, + usize::from(partial_resolution::variant_of(file).is_some()), + usize::from(partial_resolution::has_locale(file)), + ) }); ordered diff --git a/rust/herb-analysis/src/partial_resolution.rs b/rust/herb-analysis/src/partial_resolution.rs index 230dbae55..96d905746 100644 --- a/rust/herb-analysis/src/partial_resolution.rs +++ b/rust/herb-analysis/src/partial_resolution.rs @@ -70,6 +70,23 @@ pub fn format_of(file: &str) -> Option { (!format.is_empty()).then(|| format.to_string()) } +pub fn has_locale(file: &str) -> bool { + let normalized = normalize(file); + let base = basename(&normalized); + + let Some(dot) = base.find('.') else { + return false; + }; + + let extension = &base[dot..]; + + let Some(stripped) = extension.strip_suffix(".erb").or_else(|| extension.strip_suffix(".herb")) else { + return false; + }; + + stripped.strip_prefix('.').is_some_and(|segments| segments.split('.').count() > 1) +} + pub fn variant_of(file: &str) -> Option { let normalized = normalize(file); let base = basename(&normalized); diff --git a/sig/herb/analysis/partial_resolution.rbs b/sig/herb/analysis/partial_resolution.rbs index 28d964a65..9f120a1aa 100644 --- a/sig/herb/analysis/partial_resolution.rbs +++ b/sig/herb/analysis/partial_resolution.rbs @@ -29,6 +29,9 @@ module Herb # : (String) -> String? def self.format_of: (String) -> String? + # : (String) -> bool + def self.has_locale?: (String) -> bool + # : (String) -> String? def self.variant_of: (String) -> String? From 542cd03cb41727f7fc35669041478cfa9a8e2875 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sat, 15 Aug 2026 15:50:23 +0200 Subject: [PATCH 25/26] Analysis: Require the actionview framework and show project-relative paths --- lib/herb/analysis/render_analyzer.rb | 4 ++ rust/herb-analysis/src/actionview_cli.rs | 45 +++++++++++++++++++ .../tests/actionview_cli_test.rs | 1 + .../herb-analysis/tests/object_render_test.rs | 1 + 4 files changed, 51 insertions(+) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index 6f65a6198..1a12f609a 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -1339,6 +1339,10 @@ def format_duration(seconds) end def relative_path(path) + relative = Pathname.new(path).relative_path_from(@project_path).to_s + + return relative unless relative.start_with?("..") + Pathname.new(path).relative_path_from(Pathname.pwd).to_s rescue ArgumentError path.to_s diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index b5d8accb4..997ea491c 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -9,7 +9,52 @@ use herb_analysis::render_graph::Verdict; use herb_analysis::ruby_render_references; use herb_analysis::state_flow::{FlowNode, StateFlow}; +fn actionview_configured(project_path: &Path) -> Result<(), String> { + let Ok(config) = herb_config::Config::load(project_path, None) else { + return Ok(()); + }; + + match config.config.framework { + Some(herb_config::Framework::ActionView) => Ok(()), + Some(other) => Err(format!("{other:?}").to_lowercase()), + None => Err("ruby".to_string()), + } +} + +fn report_missing_framework(project_path: &Path, framework: &str) -> i32 { + println!(); + println!( + " {}", + "Herb also works outside of ActionView, but the `herb actionview` commands require the project to be explicitly configured for ActionView.".dimmed() + ); + println!(); + println!( + " The project at '{}' is not configured to use ActionView (current framework: '{framework}').", + project_path.display() + ); + println!(); + println!(" To enable ActionView support, add the following to your `.herb.yml`:"); + println!(); + println!(" {}", "framework: actionview".bold()); + println!(); + + 1 +} + pub fn run(command: &str, arguments: &[String]) -> i32 { + if !matches!(command, "check" | "graph" | "dependencies" | "flow" | "context" | "signature") { + eprintln!("{}", format!("Unknown actionview subcommand: {command}").red()); + print_usage(); + + return 1; + } + + let root = project_root(arguments); + + if let Err(framework) = actionview_configured(&root) { + return report_missing_framework(&root, &framework); + } + match command { "check" => check(arguments), "graph" => graph(arguments), diff --git a/rust/herb-analysis/tests/actionview_cli_test.rs b/rust/herb-analysis/tests/actionview_cli_test.rs index 5b7b4e769..d895e7258 100644 --- a/rust/herb-analysis/tests/actionview_cli_test.rs +++ b/rust/herb-analysis/tests/actionview_cli_test.rs @@ -15,6 +15,7 @@ impl Project { let _ = fs::remove_dir_all(&root); fs::create_dir_all(root.join("app/views/posts")).expect("create project"); fs::create_dir_all(root.join("app/views/layouts")).expect("create layouts"); + fs::write(root.join(".herb.yml"), "framework: actionview\n").expect("configure project"); Self { root } } diff --git a/rust/herb-analysis/tests/object_render_test.rs b/rust/herb-analysis/tests/object_render_test.rs index b2def58d8..1dc76fe74 100644 --- a/rust/herb-analysis/tests/object_render_test.rs +++ b/rust/herb-analysis/tests/object_render_test.rs @@ -21,6 +21,7 @@ fn write(root: &Path, relative: &str, body: &str) { fs::create_dir_all(path.parent().unwrap()).unwrap(); fs::write(path, body).unwrap(); + fs::write(root.join(".herb.yml"), "framework: actionview\n").unwrap(); } fn check(root: &Path) -> String { From 5d91a6307a98e7b3028d8c24cad11a72c58894a2 Mon Sep 17 00:00:00 2001 From: Marco Roth Date: Sat, 15 Aug 2026 15:53:56 +0200 Subject: [PATCH 26/26] Analysis: Count every skipped component template --- lib/herb/analysis/render_analyzer.rb | 2 +- rust/herb-analysis/src/actionview_cli.rs | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/herb/analysis/render_analyzer.rb b/lib/herb/analysis/render_analyzer.rb index 1a12f609a..cf72d68f4 100644 --- a/lib/herb/analysis/render_analyzer.rb +++ b/lib/herb/analysis/render_analyzer.rb @@ -578,7 +578,7 @@ def print_warning_summary_line(warnings) locals = files_for.call(:undeclared_local) uninferable = files_for.call(:uninferable_local) ivars = files_for.call(:ivar_in_partial) - ignored = files_for.call(:ignored_component) + ignored = find_erb_files.map { |file| relative_path(file) }.count { |file| component_template?(file) } parts = [] #: Array[String] parts << stat(locals, "undeclared #{pluralize(locals, "local")}", :yellow) if locals.positive? diff --git a/rust/herb-analysis/src/actionview_cli.rs b/rust/herb-analysis/src/actionview_cli.rs index 997ea491c..91a774d62 100644 --- a/rust/herb-analysis/src/actionview_cli.rs +++ b/rust/herb-analysis/src/actionview_cli.rs @@ -135,6 +135,10 @@ fn header(title: &str) { println!(); } +fn component_template(relative: &str) -> bool { + relative.starts_with("app/components/") || relative.contains("/app/components/") +} + fn missing_format(file: &str) -> bool { let name = file.rsplit('/').next().unwrap_or(file); @@ -1481,7 +1485,11 @@ fn print_dependency_warnings( let mut unknown: Vec<(String, Vec)> = Vec::new(); let mut likely_locals: Vec<(String, Vec)> = Vec::new(); let mut uninferable: Vec<(String, Vec)> = Vec::new(); - let mut ignored_components = 0usize; + let ignored_components = templates + .iter() + .map(|file| relative(file, root)) + .filter(|file| component_template(file)) + .count(); for file in templates { let result = flow.analyze(file); @@ -1507,8 +1515,9 @@ fn print_dependency_warnings( } if !rest.is_empty() { - if relative.starts_with("app/components/") || relative.contains("/app/components/") { - ignored_components += 1; + if component_template(&relative) { + // Counted from the template list instead, so the total does not depend on how many + // helpers each binding happens to resolve. } else if partial && !declared && candidates.is_none_or(BTreeSet::is_empty) { uninferable.push((relative, rest)); } else {