Cleaned up more lexer/parser logic related to the handling of tag-closer
delimiters ('>' and ']'). This has yielded the following two change log entries
(and some improvements in error message quality, but that's hardly noticeable):
1. When the incompatible_improvements setting is set to 2.3.28 (or greater),
fixed legacy parser glitch where a tag can be closed with an illegal ] (when
it's not part of an expression) despite that the tag syntax is set to angle
brackets. For example <#if x] worked just like <#if x>. Note that it doesn't
affect the legal usage of ], like <#if x[0]> works correctly without this fix
as well.
2. Fixed parser bug that disallowed using > at the top-level inside an
interpolation (${...}). It had the same reason why <#if x > y> doesn't work as
naively expected, but there's no real ambiguity in ${x > y}, so now it's
allowed. Note that ${(x > y)?c} and ${(x > y)?string('y', 'n')}, which are how
booleans are commonly printed, have always worked, as the > operation is not on
the top-level inside the interpolation.
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit:
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/01294537
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/01294537
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/01294537
Branch: refs/heads/2.3
Commit: 01294537b882d06e2ac7df5b5fff590bf45f5227
Parents: f55f9d8
Author: ddekany <[email protected]>
Authored: Mon Mar 19 22:19:37 2018 +0100
Committer: ddekany <[email protected]>
Committed: Mon Mar 19 22:19:37 2018 +0100
----------------------------------------------------------------------
.../java/freemarker/template/Configuration.java | 4 +
src/main/javacc/FTL.jj | 77 ++++++++++++++++----
src/manual/en_US/book.xml | 27 +++++++
.../core/InterpolationSyntaxTest.java | 33 +++++++--
.../core/ParsingErrorMessagesTest.java | 66 ++++++-----------
.../templates/string-builtins3.ftl | 4 +-
6 files changed, 147 insertions(+), 64 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/01294537/src/main/java/freemarker/template/Configuration.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/Configuration.java
b/src/main/java/freemarker/template/Configuration.java
index 3114747..5376dda 100644
--- a/src/main/java/freemarker/template/Configuration.java
+++ b/src/main/java/freemarker/template/Configuration.java
@@ -885,6 +885,10 @@ public class Configuration extends Configurable implements
Cloneable, ParserConf
* (Of course, the parameter default value expression is still
evaluated in the context of the called
* macro or function.) Similarly, {@code
.macro_caller_template_name} (which itself was added in 2.3.28),
* when used in a macro call argument, won't be incorrectly
evaluated in the context of the called macro.
+ * <li><p>Fixed legacy parser glitch where a tag can be closed with
an illegal {@code ]} (when it's not part
+ * of an expression) despite that the tag syntax is set to angle
brackets. For example {@code <#if x]}
+ * worked just like {@code <#if x>}. Note that it doesn't affect
the legal usage of {@code ]}, like
+ * {@code <#if x[0]>} works correctly without this fix as well.
* </ul>
* </li>
* </ul>
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/01294537/src/main/javacc/FTL.jj
----------------------------------------------------------------------
diff --git a/src/main/javacc/FTL.jj b/src/main/javacc/FTL.jj
index 26f3368..630ef39 100644
--- a/src/main/javacc/FTL.jj
+++ b/src/main/javacc/FTL.jj
@@ -667,7 +667,7 @@ TOKEN_MGR_DECLS:
if (!strictSyntaxMode) {
// Legacy feature (or bug?): Tag syntax never gets estabilished in
non-strict mode.
- // We do establilish the naming convention though.
+ // We do establish the naming convention though.
checkNamingConvention(tok, tokenNamingConvention);
SwitchTo(newLexState);
return;
@@ -684,6 +684,24 @@ TOKEN_MGR_DECLS:
// We only get here if this is a strict FTL tag.
directiveSyntaxEstablished = true;
+ if (incompatibleImprovements >= _TemplateAPI.VERSION_INT_2_3_28
+ || interpolationSyntax == SQUARE_BRACKET_INTERPOLATION_SYNTAX)
{
+ // For END_xxx tags, as they can't contain expressions, the
whole tag is a single token. So this is the only
+ // chance to check if we got something inconsistent like
`</#if]`. (We can't do this at the #CLOSE_TAG1 or
+ // such, as at that point it's possible that the tag syntax is
not yet established.)
+ char lastChar = image.charAt(image.length() - 1);
+ // Is it an end tag?
+ if (lastChar == ']' || lastChar == '>') {
+ if (!squBracTagSyntax && lastChar != '>' ||
squBracTagSyntax && lastChar != ']') {
+ throw new TokenMgrError(
+ "The tag shouldn't end with \""+ lastChar +
"\".",
+ TokenMgrError.LEXICAL_ERROR,
+ tok.beginLine, tok.beginColumn,
+ tok.endLine, tok.endColumn);
+ }
+ } // if end-tag
+ }
+
checkNamingConvention(tok, tokenNamingConvention);
SwitchTo(newLexState);
@@ -825,9 +843,6 @@ TOKEN_MGR_DECLS:
}
private void endInterpolation(Token closingTk) {
- if (postInterpolationLexState == -1) {
- throw newUnexpectedClosingTokenException(closingTk);
- }
SwitchTo(postInterpolationLexState);
postInterpolationLexState = -1;
}
@@ -1298,11 +1313,19 @@ TOKEN:
{
if (bracketNesting > 0) {
--bracketNesting;
- } else if (interpolationSyntax == SQUARE_BRACKET_INTERPOLATION_SYNTAX
- && (postInterpolationLexState != -1 || !squBracTagSyntax)) {
+ } else if (interpolationSyntax == SQUARE_BRACKET_INTERPOLATION_SYNTAX
&& postInterpolationLexState != -1) {
endInterpolation(matchedToken);
} else {
- // Glitch where you can close any tag with `]`, like `<#if x]`. We
keep it for backward compatibility.
+ // There's a legacy glitch where you can always close a tag with
`]`, like `<#if x]`. We have to keep that
+ // working for backward compatibility, hence we don't always throw
at !squBracTagSyntax:
+ if (!squBracTagSyntax
+ && (incompatibleImprovements >=
_TemplateAPI.VERSION_INT_2_3_28
+ || interpolationSyntax ==
SQUARE_BRACKET_INTERPOLATION_SYNTAX)
+ || postInterpolationLexState != -1 /* We're in an
interpolation => We aren't in a tag */) {
+ throw newUnexpectedClosingTokenException(matchedToken);
+ }
+
+ // Close tag, either legally or to emulate legacy glitch:
matchedToken.kind = DIRECTIVE_END;
if (inFTLHeader) {
eatNewline();
@@ -1336,7 +1359,7 @@ TOKEN:
{
if (curlyBracketNesting > 0) {
--curlyBracketNesting;
- } else if (interpolationSyntax != SQUARE_BRACKET_INTERPOLATION_SYNTAX)
{
+ } else if (interpolationSyntax != SQUARE_BRACKET_INTERPOLATION_SYNTAX
&& postInterpolationLexState != -1) {
endInterpolation(matchedToken);
} else {
throw newUnexpectedClosingTokenException(matchedToken);
@@ -1529,19 +1552,47 @@ TOKEN:
{
<DIRECTIVE_END : ">">
{
- if (inFTLHeader) eatNewline();
- inFTLHeader = false;
- if (squBracTagSyntax) {
+ if (inFTLHeader) {
+ eatNewline();
+ inFTLHeader = false;
+ }
+ if (squBracTagSyntax || postInterpolationLexState != -1 /* We are in
an interpolation */) {
matchedToken.kind = NATURAL_GT;
} else {
+ if (directiveSyntaxEstablished && ( incompatibleImprovements >=
_TemplateAPI.VERSION_INT_2_3_28
+ || interpolationSyntax ==
SQUARE_BRACKET_INTERPOLATION_SYNTAX)) {
+ if (squBracTagSyntax) {
+ throw new TokenMgrError(
+ "The tag shouldn't end with \"" +
image.charAt(image.length() - 1) + "\".",
+ TokenMgrError.LEXICAL_ERROR,
+ matchedToken.beginLine, matchedToken.beginColumn,
+ matchedToken.endLine, matchedToken.endColumn);
+ }
+ }
+
SwitchTo(DEFAULT);
}
}
|
<EMPTY_DIRECTIVE_END : "/>" | "/]">
{
- if (inFTLHeader) eatNewline();
- inFTLHeader = false;
+ if (directiveSyntaxEstablished && (incompatibleImprovements >=
_TemplateAPI.VERSION_INT_2_3_28
+ || interpolationSyntax ==
SQUARE_BRACKET_INTERPOLATION_SYNTAX)) {
+ String image = matchedToken.image;
+ char lastChar = image.charAt(image.length() - 1);
+ if (!squBracTagSyntax && lastChar != '>' || squBracTagSyntax &&
lastChar != ']') {
+ throw new TokenMgrError(
+ "The tag shouldn't end with \""+ lastChar + "\".",
+ TokenMgrError.LEXICAL_ERROR,
+ matchedToken.beginLine, matchedToken.beginColumn,
+ matchedToken.endLine, matchedToken.endColumn);
+ }
+ }
+
+ if (inFTLHeader) {
+ eatNewline();
+ inFTLHeader = false;
+ }
SwitchTo(DEFAULT);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/01294537/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index ac58a2a..f06b759 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -27788,6 +27788,33 @@ TemplateModel x = env.getVariable("x"); // get
variable x</programlisting>
</listitem>
<listitem>
+ <para>When the <link
+
linkend="pgui_config_incompatible_improvements_how_to_set"><literal>incompatible_improvements</literal>
+ setting</link> is set to 2.3.28 (or greater), fixed legacy
+ parser glitch where a tag can be closed with an illegal
+ <literal>]</literal> (when it's not part of an expression)
+ despite that the tag syntax is set to angle brackets. For
+ example <literal><#if x]</literal> worked just like
+ <literal><#if x></literal>. Note that it doesn't affect
+ the legal usage of <literal>]</literal>, like <literal><#if
+ x[0]></literal> works correctly without this fix as
+ well.</para>
+ </listitem>
+
+ <listitem>
+ <para>Fixed parser bug that disallowed using
+ <literal>></literal> at the top-level inside an interpolation
+ (<literal>${...}</literal>). It had the same reason why
+ <literal><#if x > y></literal> doesn't work as naively
+ expected, but there's no real ambiguity in <literal>${x >
+ y}</literal>, so now it's allowed. Note that <literal>${(x >
+ y)?c}</literal> and <literal>${(x > y)?string('y',
+ 'n')}</literal>, which are how booleans are commonly printed,
+ have always worked, as the <literal>></literal> operation is
+ not on the top-level inside the interpolation.</para>
+ </listitem>
+
+ <listitem>
<para>Fixed incorrect listing of valid
<literal>roundingMode</literal>-s in <link
linkend="topic.extendedJavaDecimalFormat">extended Java decimal
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/01294537/src/test/java/freemarker/core/InterpolationSyntaxTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/core/InterpolationSyntaxTest.java
b/src/test/java/freemarker/core/InterpolationSyntaxTest.java
index 2322d99..9a56f13 100644
--- a/src/test/java/freemarker/core/InterpolationSyntaxTest.java
+++ b/src/test/java/freemarker/core/InterpolationSyntaxTest.java
@@ -7,6 +7,7 @@ import java.io.StringWriter;
import org.junit.Test;
+import freemarker.template.Configuration;
import freemarker.template.Template;
import freemarker.test.TemplateTest;
@@ -27,6 +28,9 @@ public class InterpolationSyntaxTest extends TemplateTest {
assertOutput("<@r'${1} #{1} [=1]'?interpret />", "1 1 [=1]");
assertOutput("${'\"${1} #{1} [=1]\"'?eval}", "1 1 [=1]");
+
+ assertOutput("<#setting booleanFormat='y,n'>${2>1}", "y"); // Not an
error since 2.3.28
+ assertOutput("[#ftl][#setting booleanFormat='y,n']${2>1}", "y"); //
Not an error since 2.3.28
}
@Test
@@ -78,7 +82,9 @@ public class InterpolationSyntaxTest extends TemplateTest {
assertErrorContains("[=", "end of file");
assertErrorContains("[=1", "unclosed \"[\"");
- assertErrorContains("[=1}", "\"}\"", "open");
+
+ assertOutput("<#setting booleanFormat='y,n'>[=2>1]", "y");
+ assertOutput("[#ftl][#setting booleanFormat='y,n'][=2>1]", "y");
assertOutput("[='[\\=1]']", "[=1]");
assertOutput("[='[\\=1][=2]']", "12"); // Usual legacy interpolation
escaping glitch...
@@ -101,18 +107,35 @@ public class InterpolationSyntaxTest extends TemplateTest
{
@Test
public void legacyTagSyntaxGlitchStillWorksTest() throws Exception {
- String ftl = "<#if [true][0]]t<#else]f</#if]";
+ String badFtl1 = "<#if [true][0]]OK</#if>";
+ String badFtl2 = "<#if true>OK</#if]";
+ String badFtl3 = "<#assign x = 'OK'/]${x}";
+ String badFtl4 = " <#t/]OK\n";
+
getConfiguration().setIncompatibleImprovements(Configuration.VERSION_2_3_27);
for (int syntax : new int[] { LEGACY_INTERPOLATION_SYNTAX,
DOLLAR_INTERPOLATION_SYNTAX }) {
getConfiguration().setInterpolationSyntax(syntax);
- assertOutput(ftl, "t");
+ assertOutput(badFtl1, "OK");
+ assertOutput(badFtl2, "OK");
+ assertOutput(badFtl3, "OK");
+ assertOutput(badFtl4, "OK");
}
// Legacy tag closing glitch is not emulated with this:
getConfiguration().setInterpolationSyntax(SQUARE_BRACKET_INTERPOLATION_SYNTAX);
- assertErrorContains(ftl, "\"]\"");
+ assertErrorContains(badFtl1, "\"]\"");
+ assertErrorContains(badFtl2, "\"]\"");
+ assertErrorContains(badFtl3, "\"]\"");
+ assertErrorContains(badFtl4, "\"]\"");
+
+ getConfiguration().setInterpolationSyntax(LEGACY_INTERPOLATION_SYNTAX);
+
getConfiguration().setIncompatibleImprovements(Configuration.VERSION_2_3_28);
+ assertErrorContains(badFtl1, "\"]\"");
+ assertErrorContains(badFtl2, "\"]\"");
+ assertErrorContains(badFtl3, "\"]\"");
+ assertErrorContains(badFtl4, "\"]\"");
}
-
+
@Test
public void errorMessagesAreSquareBracketInterpolationSyntaxAwareTest()
throws Exception {
assertErrorContains("<#if ${x}></#if>", "${...}", "${myExpression}");
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/01294537/src/test/java/freemarker/core/ParsingErrorMessagesTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/core/ParsingErrorMessagesTest.java
b/src/test/java/freemarker/core/ParsingErrorMessagesTest.java
index e471a31..0081662 100644
--- a/src/test/java/freemarker/core/ParsingErrorMessagesTest.java
+++ b/src/test/java/freemarker/core/ParsingErrorMessagesTest.java
@@ -19,23 +19,23 @@
package freemarker.core;
-import static org.junit.Assert.*;
-
-import java.io.IOException;
+import static freemarker.template.Configuration.*;
import org.junit.Test;
import freemarker.template.Configuration;
-import freemarker.template.Template;
-import freemarker.template.utility.StringUtil;
+import freemarker.test.TemplateTest;
-public class ParsingErrorMessagesTest {
+public class ParsingErrorMessagesTest extends TemplateTest {
- private Configuration cfg = new
Configuration(Configuration.VERSION_2_3_21);
- {
+ @Override
+ protected Configuration createConfiguration() throws Exception {
+ Configuration cfg = super.createConfiguration();
+ cfg.setIncompatibleImprovements(Configuration.VERSION_2_3_21);
cfg.setTagSyntax(Configuration.AUTO_DETECT_TAG_SYNTAX);
+ return cfg;
}
-
+
@Test
public void testNeedlessInterpolation() {
assertErrorContains("<#if ${x} == 3></#if>", "instead of ${");
@@ -77,45 +77,23 @@ public class ParsingErrorMessagesTest {
}
@Test
- public void testInterpolatingClosingsErrors() {
- assertErrorContains("${x", "unclosed");
+ public void testInterpolatingClosingsErrors() throws Exception {
+ assertErrorContains("<#ftl>${x", "unclosed");
assertErrorContains("<#assign x = x}>", "\"}\"", "open");
- // TODO assertErrorContains("<#assign x = '${x'>", "unclosed");
- }
-
- private void assertErrorContains(String ftl, String... expectedSubstrings)
{
- assertErrorContains(false, ftl, expectedSubstrings);
- assertErrorContains(true, ftl, expectedSubstrings);
- }
-
- private void assertErrorContains(boolean squareTags, String ftl, String...
expectedSubstrings) {
- try {
- if (squareTags) {
- ftl = ftl.replace('<', '[').replace('>', ']');
- }
- new Template("adhoc", ftl, cfg);
- fail("The tempalte had to fail");
- } catch (ParseException e) {
- String msg = e.getMessage();
- for (String needle: expectedSubstrings) {
- if (needle.startsWith("\\!")) {
- String netNeedle = needle.substring(2);
- if (msg.contains(netNeedle)) {
- fail("The message shouldn't contain substring " +
StringUtil.jQuote(netNeedle) + ":\n" + msg);
- }
- } else if (!msg.contains(needle)) {
- fail("The message didn't contain substring " +
StringUtil.jQuote(needle) + ":\n" + msg);
- }
- }
- showError(e);
- } catch (IOException e) {
- // Won't happen
- throw new RuntimeException(e);
+ assertOutput("<#assign x = '${x'>", ""); // Legacy glitch... should
fail in theory.
+
+ for (int syntax : new int[] { LEGACY_INTERPOLATION_SYNTAX,
DOLLAR_INTERPOLATION_SYNTAX }) {
+ getConfiguration().setInterpolationSyntax(syntax);
+ assertErrorContains("<#ftl>${'x']", "\"]\"", "open");
+ super.assertErrorContains("<#ftl>${'x'>", "end of file");
+ super.assertErrorContains("[#ftl]${'x'>", "end of file");
}
}
- private void showError(Throwable e) {
- //System.out.println(e);
+ protected Throwable assertErrorContains(String ftl, String...
expectedSubstrings) {
+ super.assertErrorContains(ftl, expectedSubstrings);
+ ftl = ftl.replace('<', '[').replace('>', ']');
+ return super.assertErrorContains(ftl, expectedSubstrings);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/01294537/src/test/resources/freemarker/test/templatesuite/templates/string-builtins3.ftl
----------------------------------------------------------------------
diff --git
a/src/test/resources/freemarker/test/templatesuite/templates/string-builtins3.ftl
b/src/test/resources/freemarker/test/templatesuite/templates/string-builtins3.ftl
index 77389fa..06e7dd7 100644
---
a/src/test/resources/freemarker/test/templatesuite/templates/string-builtins3.ftl
+++
b/src/test/resources/freemarker/test/templatesuite/templates/string-builtins3.ftl
@@ -198,7 +198,7 @@
<@assertEquals expected='aa' actual=27?lower_abc />
<@assertEquals expected='ab' actual=28?lower_abc />
<@assertEquals expected='cv' actual=100?lower_abc />
-<@assertFails messageRegexp='0|at least 1']>
+<@assertFails messageRegexp='0|at least 1'>
${0?lower_abc}
</@assertFails>
<@assertFails messageRegexp='0|at least 1'>
@@ -214,7 +214,7 @@
<@assertEquals expected='AA' actual=27?upper_abc />
<@assertEquals expected='AB' actual=28?upper_abc />
<@assertEquals expected='CV' actual=100?upper_abc />
-<@assertFails messageRegexp='0|at least 1']>
+<@assertFails messageRegexp='0|at least 1'>
${0?upper_abc}
</@assertFails>
<@assertFails messageRegexp='0|at least 1'>
