Forward ported from 2.3-gae: FTL.jj cleanup. Can't close with "]" when using angle bracket tag syntax anymore.
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/0d21cbb7 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/0d21cbb7 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/0d21cbb7 Branch: refs/heads/3 Commit: 0d21cbb77004014654b6e7160ad7791939f0faad Parents: 348e73d Author: ddekany <ddek...@apache.org> Authored: Mon Mar 19 23:42:35 2018 +0100 Committer: ddekany <ddek...@apache.org> Committed: Mon Mar 19 23:42:35 2018 +0100 ---------------------------------------------------------------------- .../core/InterpolationSyntaxTest.java | 27 +++++-- .../core/ParsingErrorMessagesTest.java | 67 ++++++--------- .../freemarker/core/ASTExpStringLiteral.java | 3 +- freemarker-core/src/main/javacc/FTL.jj | 85 ++++++++++++++------ 4 files changed, 109 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0d21cbb7/freemarker-core-test/src/test/java/org/apache/freemarker/core/InterpolationSyntaxTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/InterpolationSyntaxTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/InterpolationSyntaxTest.java index 4d7d0cc..7dc0dd0 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/InterpolationSyntaxTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/InterpolationSyntaxTest.java @@ -48,7 +48,10 @@ public class InterpolationSyntaxTest extends TemplateTest { assertOutput("${'a${1}#{2}b[=3]'}", "a1#{2}b[=3]"); assertOutput("<@r'${1} #{1} [=1]'?interpret />", "1 #{1} [=1]"); - assertOutput("${'\"${1} #{1} [=1]\"'?eval}", "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 @@ -94,7 +97,10 @@ public class InterpolationSyntaxTest extends TemplateTest { assertOutput("[='[\\=1]']", "[=1]"); assertOutput("[='[\\=1][=2]']", "12"); // Usual legacy interpolation escaping glitch... - assertOutput("[=r'[=1]']", "[=1]"); + assertOutput("[=r'[=1]']", "[=1]"); + + assertOutput("<#setting booleanFormat='y,n'>[=2>1]", "y"); + assertOutput("[#ftl][#setting booleanFormat='y,n'][=2>1]", "y"); StringWriter sw = new StringWriter(); new Template(null, "[= 1 + '[= 2 ]' ]", getConfiguration()).dump(sw); @@ -116,19 +122,28 @@ public class InterpolationSyntaxTest extends TemplateTest { } @Test - public void legacyTagSyntaxGlitchStillWorksTest() throws Exception { - String ftl = "<#if [true][0]]t<#else]f</#if]"; + public void legacyTagSyntaxGlitchFixedTest() throws Exception { + String badFtl1 = "<#if [true][0]]OK</#if>"; + String badFtl2 = "<#if true>OK</#if]"; + String badFtl3 = "<#assign x = 'OK'/]${x}"; + String badFtl4 = " <#t/]OK\n"; setConfiguration(new TestConfigurationBuilder() .interpolationSyntax(InterpolationSyntax.DOLLAR) .build()); - assertOutput(ftl, "t"); + assertErrorContains(badFtl1, "\"]\""); + assertErrorContains(badFtl2, "\"]\""); + assertErrorContains(badFtl3, "\"]\""); + assertErrorContains(badFtl4, "\"]\""); // Glitch is not emulated with this: setConfiguration(new TestConfigurationBuilder() .interpolationSyntax(InterpolationSyntax.SQUARE_BRACKET) .build()); - assertErrorContains(ftl, "\"]\""); + assertErrorContains(badFtl1, "\"]\""); + assertErrorContains(badFtl2, "\"]\""); + assertErrorContains(badFtl3, "\"]\""); + assertErrorContains(badFtl4, "\"]\""); } @Test http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0d21cbb7/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java index bf729ea..962e7a9 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ParsingErrorMessagesTest.java @@ -19,19 +19,20 @@ package org.apache.freemarker.core; -import static org.junit.Assert.*; - import java.io.IOException; -import org.apache.freemarker.core.util._StringUtils; +import org.apache.freemarker.test.TemplateTest; import org.apache.freemarker.test.TestConfigurationBuilder; import org.junit.Test; -public class ParsingErrorMessagesTest { +public class ParsingErrorMessagesTest extends TemplateTest { - private Configuration cfg = new TestConfigurationBuilder() - .tagSyntax(TagSyntax.AUTO_DETECT) - .build(); + @Override + protected Configuration createDefaultConfiguration() throws Exception { + return new TestConfigurationBuilder() + .tagSyntax(TagSyntax.AUTO_DETECT) + .build(); + } @Test public void testNeedlessInterpolation() { @@ -122,10 +123,14 @@ 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"); + assertErrorContains("<#ftl>${'x']", "\"]\"", "open"); + super.assertErrorContains("<#ftl>${'x'>", "end of file"); + super.assertErrorContains("[#ftl]${'x'>", "end of file"); + + assertOutput("<#assign x = '${x'>", ""); // TODO [FM3] Legacy glitch... should fail in theory. } @Test @@ -167,43 +172,19 @@ public class ParsingErrorMessagesTest { assertErrorContains("<#function f(a=0, b)></#function>", "with default", "without a default"); assertErrorContains("<#function f(a,)></#function>", "Comma without"); assertErrorContains("<#macro m a{positional}, b{positional},></#macro>", "Comma without"); - assertErrorContains(false, "<#function f(a, b></#function>"); - assertErrorContains(false, "<#function f(></#function>"); - assertErrorContains(false, "[#ftl][#function f(a, b][/#function]", "Missing closing \")\""); - assertErrorContains(false, "[#ftl][#function f(][/#function]", "Missing closing \")\""); + super.assertErrorContains("<#function f(a, b></#function>"); + super.assertErrorContains("<#function f(></#function>"); + super.assertErrorContains("[#ftl][#function f(a, b][/#function]", "Missing closing \")\""); + super.assertErrorContains("[#ftl][#function f(][/#function]", "Missing closing \")\""); assertErrorContains("<#macro m a b)></#macro>", "\")\" without", "opening"); assertErrorContains("<#macro m a b a></#macro>", "\"a\"", "multiple"); } - private void assertErrorContains(String ftl, String... expectedSubstrings) { - assertErrorContains(false, ftl, expectedSubstrings); - assertErrorContains(true, ftl, expectedSubstrings); - } - - private void assertErrorContains(boolean convertToSquare, String ftl, String... expectedSubstrings) { - try { - if (convertToSquare) { - ftl = ftl.replace('<', '[').replace('>', ']'); - } - new Template("adhoc", ftl, cfg); - fail("The template 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 " + _StringUtils.jQuote(netNeedle) + ":\n" + msg); - } - } else if (!msg.contains(needle)) { - fail("The message didn't contain substring " + _StringUtils.jQuote(needle) + ":\n" + msg); - } - } - showError(e); - } catch (IOException e) { - // Won't happen - throw new RuntimeException(e); - } + @Override + protected Throwable assertErrorContains(String ftl, String... expectedSubstrings) { + super.assertErrorContains(ftl, expectedSubstrings); + ftl = ftl.replace('<', '[').replace('>', ']'); + return super.assertErrorContains(ftl, expectedSubstrings); } private void showError(Throwable e) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0d21cbb7/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java index a86c4d5..ebd8ff6 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpStringLiteral.java @@ -49,13 +49,14 @@ final class ASTExpStringLiteral extends ASTExpression implements TemplateStringM * inherit tokenizer-level auto-detected settings. */ void parseValue(FMParserTokenManager parentTkMan, OutputFormat outputFormat) throws ParseException { - // TODO [FM3] (Find related: [interpolation prefixes]) + // TODO [FM3] // The way this works is incorrect (the literal should be parsed without un-escaping), // but we can't fix this backward compatibly. Template parentTemplate = getTemplate(); ParsingConfiguration pCfg = parentTemplate.getParsingConfiguration(); InterpolationSyntax intSyn = pCfg.getInterpolationSyntax(); if (value.length() > 3 && ( + // Find related: [interpolation prefixes] intSyn == InterpolationSyntax.DOLLAR && value.indexOf("${") != -1 || intSyn == InterpolationSyntax.SQUARE_BRACKET && value.indexOf("[=") != -1)) { try { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/0d21cbb7/freemarker-core/src/main/javacc/FTL.jj ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/javacc/FTL.jj b/freemarker-core/src/main/javacc/FTL.jj index 76b32f5..fa29105 100644 --- a/freemarker-core/src/main/javacc/FTL.jj +++ b/freemarker-core/src/main/javacc/FTL.jj @@ -533,7 +533,7 @@ TOKEN_MGR_DECLS: private boolean inFTLHeader; boolean squBracTagSyntax, autodetectTagSyntax, - directiveSyntaxEstablished, + tagSyntaxEstablished, inNamedParameterExpression; InterpolationSyntax interpolationSyntax; int incompatibleImprovements; @@ -550,7 +550,7 @@ TOKEN_MGR_DECLS: final String image = tok.image; char firstChar = image.charAt(0); - if (autodetectTagSyntax && !directiveSyntaxEstablished) { + if (autodetectTagSyntax && !tagSyntaxEstablished) { squBracTagSyntax = (firstChar == '['); } if ((firstChar == '[' && !squBracTagSyntax) || (firstChar == '<' && squBracTagSyntax)) { @@ -558,7 +558,22 @@ TOKEN_MGR_DECLS: return; } - directiveSyntaxEstablished = true; + tagSyntaxEstablished = true; + + // 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 SwitchTo(newLexState); } @@ -581,7 +596,7 @@ TOKEN_MGR_DECLS: private void dynamicTopLevelCall(Token tok) { char firstChar = tok.image.charAt(0); - if (autodetectTagSyntax && !directiveSyntaxEstablished) { + if (autodetectTagSyntax && !tagSyntaxEstablished) { squBracTagSyntax = (firstChar == '['); } if (squBracTagSyntax && firstChar == '<') { @@ -592,7 +607,7 @@ TOKEN_MGR_DECLS: tok.kind = STATIC_TEXT_NON_WS; return; } - directiveSyntaxEstablished = true; + tagSyntaxEstablished = true; SwitchTo(NO_SPACE_EXPRESSION); } @@ -633,9 +648,6 @@ TOKEN_MGR_DECLS: } private void endInterpolation(Token closingTk) { - if (postInterpolationLexState == -1) { - throw newUnexpectedClosingTokenException(closingTk); - } SwitchTo(postInterpolationLexState); postInterpolationLexState = -1; } @@ -674,9 +686,9 @@ TOKEN_MGR_DECLS: } private void ftlHeader(Token matchedToken) { - if (!directiveSyntaxEstablished) { + if (!tagSyntaxEstablished) { squBracTagSyntax = matchedToken.image.charAt(0) == '['; - directiveSyntaxEstablished = true; + tagSyntaxEstablished = true; autodetectTagSyntax = false; } String img = matchedToken.image; @@ -869,9 +881,9 @@ TOKEN: { char firstChar = matchedToken.image.charAt(0); - if (!directiveSyntaxEstablished && autodetectTagSyntax) { + if (!tagSyntaxEstablished && autodetectTagSyntax) { squBracTagSyntax = (firstChar == '['); - directiveSyntaxEstablished = true; + tagSyntaxEstablished = true; } if (firstChar == '<' && squBracTagSyntax) { @@ -1090,18 +1102,18 @@ TOKEN: { if (bracketNesting > 0) { --bracketNesting; - } else if (interpolationSyntax == InterpolationSyntax.SQUARE_BRACKET - && (postInterpolationLexState != -1 || !squBracTagSyntax)) { + } else if (interpolationSyntax == InterpolationSyntax.SQUARE_BRACKET && postInterpolationLexState != -1) { endInterpolation(matchedToken); - } else { - // TODO [FM3] Don't mimic this glitch anymore. - // Glitch where you can close any tag with `]`, like `<#if x]`. We keep it for backward compatibility. + } else if (squBracTagSyntax + && postInterpolationLexState == -1 /* We're in an interpolation => We aren't in a tag */) { matchedToken.kind = DIRECTIVE_END; if (inFTLHeader) { eatNewline(); inFTLHeader = false; } SwitchTo(DEFAULT); + } else { + throw newUnexpectedClosingTokenException(matchedToken); } } | @@ -1128,7 +1140,7 @@ TOKEN: { if (curlyBracketNesting > 0) { --curlyBracketNesting; - } else if (interpolationSyntax != InterpolationSyntax.SQUARE_BRACKET) { + } else if (interpolationSyntax != InterpolationSyntax.SQUARE_BRACKET && postInterpolationLexState != -1) { endInterpolation(matchedToken); } else { throw newUnexpectedClosingTokenException(matchedToken); @@ -1321,19 +1333,46 @@ 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 (tagSyntaxEstablished) { + 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 (inFTLHeader) { + eatNewline(); + inFTLHeader = false; + } + + if (tagSyntaxEstablished) { + 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); + } + } + SwitchTo(DEFAULT); } }
