JavaTemplate: add failing tests for literal #{ in template source - #8470
Closed
martinfrancois wants to merge 1 commit into
Closed
JavaTemplate: add failing tests for literal #{ in template source#8470martinfrancois wants to merge 1 commit into
martinfrancois wants to merge 1 commit into
Conversation
`hashBraceInStringLiteral` pins that a raw `#{` spliced into template
source throws "The parameter foo must be defined before it is referenced".
`escapedHashBraceInStringLiteral` pins that the `\#{` escape supported by
PropertyPlaceholderHelper is undone by the fixed-point loop in
Substitutions. Both are marked @ExpectedToFail until an escape survives.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's changed?
Adds 2 known-failing tests to
JavaTemplateSubstitutionsTestthat reproduce two shapes of the same problem: a raw literal#{in template source makesJavaTemplatethrow before any parsing (hashBraceInStringLiteral), and the backslash escape\#{thatPropertyPlaceholderHelperdocuments is consumed but then re-parsed as a placeholder (escapedHashBraceInStringLiteral). No framework code changes. The tests are marked@ExpectedToFailso the suite stays green; removing the annotation shows the failure.What's your motivation?
There is no way to splice the two characters
#{into code through aJavaTemplate. A visitor replaces the string literal"placeholder"usingJavaTemplate.builder("\"#{foo}\"")with zero template parameters, on this source:The run fails with:
After the recipe runs, the code should read
String s = "#{foo}";, but no output is produced at all: the#{foo}inside the Java string literal is treated as a template placeholder before template parsing even starts.The second test uses
JavaTemplate.builder("\"\\#{foo}\""), the backslash escape thatPropertyPlaceholderHelperdocuments in its javadoc since #6817. It throws the same exception. The mechanism, measured with a standalone program against the compiled rewrite-core classes: pass 1 ofreplacePlaceholdersconsumes the escape, calls the resolver zero times, and returns text containing a bare#{foo}. Because that output differs from the input, the fixed-point loop inSubstitutions.substitute()runs a second pass, which re-parses the now unescaped#{foo}and resolves keyfoo, one iteration later. So the documented helper-level escape does not survive the loop above it. It was never a documentedJavaTemplate-level feature.Found while preparing openrewrite/rewrite-static-analysis#976, which works around this defect in a recipe.
Anything in particular you'd like reviewers to focus on?
The first test pins literal pass-through, which is one of at least two reasonable designs: the exception could be intentional validation, and a documented escape syntax would be the other option. Direction welcome. Also, a fix cannot live in
Substitutionsalone:JavaTemplateParserbuilds its ownPropertyPlaceholderHelper("#{", "}", null), which must stay consistent with any escape handling.We think the second shape is a genuine bug: the escape is documented on the helper, and the loop silently undoes it. If you agree this should change, I would gladly prepare the fix. If this behavior is intended, feel free to close this and we know it is settled.
Have you considered any alternatives or workarounds?
openrewrite/rewrite-static-analysis#976 emits
\u0023instead of#, so tokens containing#{survive substitution. That works, but every affected recipe would need the same trick.Any additional context
On main,
JavaTemplateSubstitutionsTesthas 15 tests; with these additions 17 run. The 2 new ones abort as expected under@ExpectedToFail, and all 15 pre-existing tests pass.This reproduction was prepared with AI assistance (Claude Code). I reviewed the tests and this description.
Checklist
This draft adds reproduction tests only, so the first box stays unticked on purpose; I will complete it together with the fix if you want one. The formatter run was calibrated per file; I declined reindentation of untouched lines.