[iMacTia]

Fixes #42.

pks turns an ERB file into Ruby by joining the contents of its tags, and it treats <%# %> comments like any other tag. Only the first line of a multi-line comment ends up as a Ruby comment, and the rest is parsed as code. Depending on the text, that loses the references before the comment, the ones after it, or the whole file, with no error. A constant named on a later comment line is reported as a reference.

Approach

Comment tags are now dropped before the join, which is what packwerk's ERB parser does. The check is on the whole tag, not the captured contents, because only a tag that opens with exactly <%# is a comment. <%-# and <% # are code tags in Erubi and in packwerk, and their later lines are real Ruby. The default and experimental parsers both go through this one function.

Where to look

• Dropping single-line comments changes one case, which the issue says should behave as today. When a file's first tag is an encoding comment such as <%# encoding: iso-8859-1 %>, it used to become the first line of the Ruby source, where lib-ruby-parser read it as a magic comment. For any encoding but UTF-8 or ASCII-8BIT, pks then reported nothing for the file. Those files are now checked, so they can show new violations. Rails strips that tag before compiling and packwerk ignores it, so this matches both. The CHANGELOG says so and there's a test for it.
• Tags opening with <%%, ERB's escape for a literal <%, are dropped as well. pks used to parse their contents as Ruby, which lost the references after them and sometimes all of them. This follows Erubi, which Rails renders with: it outputs the whole tag as text. packwerk parses the literal's contents as Ruby, so a file with <%%= fails to parse and other literals can report constants from inside them. This is one place pks deliberately doesn't match it.
• hash_after_hyphen_or_space_is_code_not_comment passes on main. It fails if the check moves to the capture, which would treat <%-# and <% # as comments.

Notes

The cache used to reuse an entry whenever the file's content matched, so after an upgrade an ERB file that hadn't changed would keep its old result until someone ran pks delete-cache. Cache entries now include the pks version in their content digest, so the first run after an upgrade is a cold run for every file, not just ERB. That's a separate commit with its own CHANGELOG entry.

Sent with Claude Code

[dduugg]

Thanks, this is a clean fix, and the reasoning in the description checks out against the upstream code. BetterHtml, which packwerk's ERB parser uses, and Erubi both treat a tag as a comment only when # comes straight after <%. capture[0].starts_with("<%#") gives the same answer as both for <%#, <%-#, <% #, <%=#, <%#= and <%#-.

I ran each of the seven texts in the first test against main on its own, and every one fails there. That includes "second (line" and "class foo", which lose the references before the comment. The first three tests fail on main, and the fourth fails if the filter moves to the capture. The encoding explanation holds up too. Without a custom decoder, lib-ruby-parser accepts only UTF-8, ASCII-8BIT and BINARY, pks doesn't set a decoder, and a missing AST becomes an empty result for the whole file. The repro from #42 misses the privacy violation on main and reports it here, with the default parser and with --experimental-parser. clippy, fmt and CI are clean, and it merges cleanly with #65.

A note on the CHANGELOG and a suggestion for the cache.

CHANGELOG

The entry only says "may show new violations" for the encoding case, but the multi-line fix changes results in both directions too. References that used to be missed are now reported. A constant named on a later line of a comment that someone recorded in package_todo.yml is no longer found, so that entry goes stale and pks check fails until pks update removes it. One sentence covering both, with pks update as the remedy, would help anyone upgrading.

Cache

I'd rather not ask everyone to run pks delete-cache. Anyone who keeps tmp/cache between CI runs, which is common because of bootsnap, won't see the note, and the next parser fix would need the same paragraph. Adding the pks version to the stored content digest in EmptyCacheEntry::new (src/packs/caching/mod.rs:32) handles it:

let file_contents_digest = format!(
    "{}-{}",
    file_content_digest(filepath)?,
    env!("CARGO_PKG_VERSION")
);

After an upgrade, every existing entry fails the digest comparison in PerFileCache::get and is rewritten in place. The cache file name is the md5 of the path, so nothing is left orphaned. The cost is one cold run per upgrade across every file type, which seems fine for pks. I tried it on top of this branch. The full suite passes, and on a cache populated by main, the #42 repro reports the violation without a delete-cache.

I'm happy to have it here or in a follow-up that lands before the release. Either way, the cache paragraph in the CHANGELOG can then go.

Test

Nothing guards the single-line half of "nothing inside an ERB comment is reported". <%# Foo %> followed by <%= Bar %> gives [Bar] on main and on this branch, so it hasn't regressed, but one more case in multiline_comment_contents_are_not_references would pin it.

[iMacTia]

Thanks for the careful review! I've added three commits on top. For your three points:

• The CHANGELOG now says the fix changes results both ways, and that a todo entry for a constant named inside a comment goes stale until pks update removes it.
• I took the cache suggestion here rather than in a follow-up. EmptyCacheEntry::new now adds the pks version to the digest, and a new test checks that an entry with the old bare digest or another version's is a miss. I checked that nothing else reads the digest. The delete-cache paragraph is gone, and the cache change has its own CHANGELOG entry.
• Added the single-line case to the comment test, and renamed it to comment_contents_are_not_references since it's not only about multi-line comments anymore.

I also fixed <%% here rather than leaving it for later. It's ERB's escape for a literal <%, and pks parsed the rest of the tag as Ruby, so a file using it lost some or all of its references. It's now treated as text, the way Erubi and Rails do. packwerk parses the literal as Ruby, so it fails on any <%%= and can report constants from inside other literals. pks deliberately differs from it there. That's in the description and has its own CHANGELOG entry.

I've updated the description to match.

Sent with Claude Code