This is an automated email from the ASF dual-hosted git repository. ddekany pushed a commit to branch 2.3-gae in repository https://gitbox.apache.org/repos/asf/freemarker.git
The following commit(s) were added to refs/heads/2.3-gae by this push: new e88d9df More helpful parser error messages for nesting problems (caused by missed or malformed end-tags usually). e88d9df is described below commit e88d9df9ed8ea5fb7cf085d7aa87ff2db012dd4c Author: ddekany <ddek...@apache.org> AuthorDate: Sun Oct 25 22:35:22 2020 +0100 More helpful parser error messages for nesting problems (caused by missed or malformed end-tags usually). --- src/main/java/freemarker/core/ParseException.java | 316 +++++++++++++-------- src/manual/en_US/book.xml | 5 + .../freemarker/core/ParsingErrorMessagesTest.java | 24 +- .../freemarker/test/templatesuite/templates/if.ftl | 8 +- 4 files changed, 230 insertions(+), 123 deletions(-) diff --git a/src/main/java/freemarker/core/ParseException.java b/src/main/java/freemarker/core/ParseException.java index 4307508..6ab3ed6 100644 --- a/src/main/java/freemarker/core/ParseException.java +++ b/src/main/java/freemarker/core/ParseException.java @@ -20,8 +20,9 @@ package freemarker.core; import java.io.IOException; -import java.util.HashSet; -import java.util.Iterator; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.Set; import freemarker.template.Template; @@ -44,6 +45,9 @@ import freemarker.template.utility.StringUtil; */ public class ParseException extends IOException implements FMParserConstants { + private static final String END_TAG_SYNTAX_HINT + = "(Note that FreeMarker end-tags must have # or @ after the / character.)"; + /** * This is the last token that has been consumed successfully. If * this object has been created due to a parse error, the token @@ -371,138 +375,214 @@ public class ParseException extends IOException implements FMParserConstants { if (description != null) return description; // When we already have it from the constructor } - String tokenErrDesc; - if (currentToken != null) { - tokenErrDesc = getCustomTokenErrorDescription(); - if (tokenErrDesc == null) { - // The default JavaCC message generation stuff follows. - StringBuilder expected = new StringBuilder(); - int maxSize = 0; - for (int i = 0; i < expectedTokenSequences.length; i++) { - if (i != 0) { - expected.append(eol); - } - expected.append(" "); - if (maxSize < expectedTokenSequences[i].length) { - maxSize = expectedTokenSequences[i].length; - } - for (int j = 0; j < expectedTokenSequences[i].length; j++) { - if (j != 0) expected.append(' '); - expected.append(tokenImage[expectedTokenSequences[i][j]]); - } + if (currentToken == null) { + return null; + } + + Token unexpectedTok = currentToken.next; + + if (unexpectedTok.kind == EOF) { + Set<String> endTokenDescs = getExpectedEndTokenDescs(); + return "Unexpected end of file reached." + + (endTokenDescs.size() == 0 + ? "" + : " You have an unclosed " + joinWithAnds(endTokenDescs) + + ". Check if the FreeMarker end-tags are present, and aren't malformed. " + + END_TAG_SYNTAX_HINT); + } + + int maxExpectedTokenSequenceLength = 0; + for (int i = 0; i < expectedTokenSequences.length; i++) { + int[] expectedTokenSequence = expectedTokenSequences[i]; + if (maxExpectedTokenSequenceLength < expectedTokenSequence.length) { + maxExpectedTokenSequenceLength = expectedTokenSequence.length; + } + } + + StringBuilder tokenErrDesc = new StringBuilder(); + tokenErrDesc.append("Encountered "); + boolean encounteredEndTag = false; + for (int i = 0; i < maxExpectedTokenSequenceLength; i++) { + if (i != 0) { + tokenErrDesc.append(" "); + } + if (unexpectedTok.kind == 0) { + tokenErrDesc.append(tokenImage[0]); + break; + } + + String image = unexpectedTok.image; + if (i == 0) { + if (image.startsWith("</") || image.startsWith("[/")) { + encounteredEndTag = true; } - tokenErrDesc = "Encountered \""; - Token tok = currentToken.next; - for (int i = 0; i < maxSize; i++) { - if (i != 0) tokenErrDesc += " "; - if (tok.kind == 0) { - tokenErrDesc += tokenImage[0]; - break; - } - tokenErrDesc += add_escapes(tok.image); - tok = tok.next; + } + tokenErrDesc.append(StringUtil.jQuote(image)); + unexpectedTok = unexpectedTok.next; + } + Set<String> expectedEndTokenDescs; + int unexpTokKind = currentToken.next.kind; + if (getIsEndToken(unexpTokKind) || unexpTokKind == ELSE || unexpTokKind == ELSE_IF) { + expectedEndTokenDescs = new LinkedHashSet<>(getExpectedEndTokenDescs()); + if (unexpTokKind == ELSE || unexpTokKind == ELSE_IF) { + // If <\#if> was expected, yet #else or #elseif wasn't, then this isn't nesting related problem. + expectedEndTokenDescs.remove(getEndTokenDescIfIsEndToken(END_IF)); + } else { + expectedEndTokenDescs.remove(getEndTokenDescIfIsEndToken(unexpTokKind)); + } + } else { + expectedEndTokenDescs = Collections.emptySet(); + } + // Generate more helpful error message if this was a nesting related problem: + if (!expectedEndTokenDescs.isEmpty()) { + if (unexpTokKind == ELSE || unexpTokKind == ELSE_IF) { + tokenErrDesc.append(", which can only be used where an #if"); + if (unexpTokKind == ELSE) { + tokenErrDesc.append(" or #list"); } - tokenErrDesc += "\", but "; - - if (expectedTokenSequences.length == 1) { - tokenErrDesc += "was expecting:" + eol; + tokenErrDesc.append(" could be closed"); + } + tokenErrDesc.append(", but at this place only "); + tokenErrDesc.append(expectedEndTokenDescs.size() > 1 ? "these" : "this"); + tokenErrDesc.append(" can be closed: "); + boolean first = true; + for (String expectedEndTokenDesc : expectedEndTokenDescs) { + if (!first) { + tokenErrDesc.append(", "); } else { - tokenErrDesc += "was expecting one of:" + eol; + first = false; } - tokenErrDesc += expected; + tokenErrDesc.append( + !expectedEndTokenDesc.startsWith("\"") + ? StringUtil.jQuote(expectedEndTokenDesc) + : expectedEndTokenDesc); + } + tokenErrDesc.append("."); + if (encounteredEndTag) { + tokenErrDesc.append(" This usually because of wrong nesting of FreeMarker directives, like a " + + "missed or malformed end-tag somewhere. " + END_TAG_SYNTAX_HINT); } + tokenErrDesc.append(eol); + tokenErrDesc.append("Was "); + } else { + tokenErrDesc.append(", but was "); + } + + if (expectedTokenSequences.length == 1) { + tokenErrDesc.append("expecting pattern:"); } else { - tokenErrDesc = null; + tokenErrDesc.append("expecting one of these patterns:"); + } + tokenErrDesc.append(eol); + + for (int i = 0; i < expectedTokenSequences.length; i++) { + if (i != 0) { + tokenErrDesc.append(eol); + } + tokenErrDesc.append(" "); + int[] expectedTokenSequence = expectedTokenSequences[i]; + for (int j = 0; j < expectedTokenSequence.length; j++) { + if (j != 0) { + tokenErrDesc.append(' '); + } + tokenErrDesc.append(tokenImage[expectedTokenSequence[j]]); + } } - return tokenErrDesc; + + return tokenErrDesc.toString(); } - private String getCustomTokenErrorDescription() { - final Token nextToken = currentToken.next; - final int kind = nextToken.kind; - if (kind == EOF) { - Set/*<String>*/ endNames = new HashSet(); - for (int i = 0; i < expectedTokenSequences.length; i++) { - int[] sequence = expectedTokenSequences[i]; - for (int j = 0; j < sequence.length; j++) { - switch (sequence[j]) { - case END_FOREACH: - endNames.add( "#foreach"); - break; - case END_LIST: - endNames.add( "#list"); - break; - case END_SWITCH: - endNames.add( "#switch"); - break; - case END_IF: - endNames.add( "#if"); - break; - case END_COMPRESS: - endNames.add( "#compress"); - break; - case END_MACRO: - endNames.add( "#macro"); - case END_FUNCTION: - endNames.add( "#function"); - break; - case END_TRANSFORM: - endNames.add( "#transform"); - break; - case END_ESCAPE: - endNames.add( "#escape"); - break; - case END_NOESCAPE: - endNames.add( "#noescape"); - break; - case END_ASSIGN: - endNames.add( "#assign"); - break; - case END_LOCAL: - endNames.add( "#local"); - break; - case END_GLOBAL: - endNames.add( "#global"); - break; - case END_ATTEMPT: - endNames.add( "#attempt"); - break; - case CLOSING_CURLY_BRACKET: - endNames.add( "\"{\""); - break; - case CLOSE_BRACKET: - endNames.add( "\"[\""); - break; - case CLOSE_PAREN: - endNames.add( "\"(\""); - break; - case UNIFIED_CALL_END: - endNames.add( "@..."); - break; - } + /** + * Returns the descriptions end-tags (or expression closing tokens) that we could have at this point. + * This is for generating error messages. + */ + private Set<String> getExpectedEndTokenDescs() { + Set<String> endTokenDescs = new LinkedHashSet<>(); + for (int i = 0; i < expectedTokenSequences.length; i++) { + int[] sequence = expectedTokenSequences[i]; + for (int j = 0; j < sequence.length; j++) { + int token = sequence[j]; + String endTokenDesc = getEndTokenDescIfIsEndToken(token); + if (endTokenDesc != null) { + endTokenDescs.add(endTokenDesc); } } - return "Unexpected end of file reached." - + (endNames.size() == 0 ? "" : " You have an unclosed " + concatWithOrs(endNames) + "."); - } else if (kind == ELSE) { - return "Unexpected directive, \"#else\". " - + "Check if you have a valid #if-#elseif-#else or #list-#else structure."; - } else if (kind == END_IF || kind == ELSE_IF) { - return "Unexpected directive, " - + StringUtil.jQuote(nextToken) - + ". Check if you have a valid #if-#elseif-#else structure."; } - return null; + return endTokenDescs; + } + + private boolean getIsEndToken(int token) { + return getEndTokenDescIfIsEndToken(token) != null; + } + + private String getEndTokenDescIfIsEndToken(int token) { + String endTokenDesc = null; + switch (token) { + case END_FOREACH: + endTokenDesc = "#foreach"; + break; + case END_LIST: + endTokenDesc = "#list"; + break; + case END_SEP: + endTokenDesc = "#sep"; + break; + case END_ITEMS: + endTokenDesc = "#items"; + break; + case END_SWITCH: + endTokenDesc = "#switch"; + break; + case END_IF: + endTokenDesc = "#if"; + break; + case END_COMPRESS: + endTokenDesc = "#compress"; + break; + case END_MACRO: + case END_FUNCTION: + endTokenDesc = "#macro or #function"; + break; + case END_TRANSFORM: + endTokenDesc = "#transform"; + break; + case END_ESCAPE: + endTokenDesc = "#escape"; + break; + case END_NOESCAPE: + endTokenDesc = "#noescape"; + break; + case END_ASSIGN: + case END_GLOBAL: + case END_LOCAL: + endTokenDesc = "#assign or #local or #global"; + break; + case END_ATTEMPT: + endTokenDesc = "#attempt"; + break; + case CLOSING_CURLY_BRACKET: + endTokenDesc = "\"{\""; + break; + case CLOSE_BRACKET: + endTokenDesc = "\"[\""; + break; + case CLOSE_PAREN: + endTokenDesc = "\"(\""; + break; + case UNIFIED_CALL_END: + endTokenDesc = "@..."; + break; + } + return endTokenDesc; } - private String concatWithOrs(Set/*<String>*/ endNames) { - StringBuilder sb = new StringBuilder(); - for (Iterator/*<String>*/ it = endNames.iterator(); it.hasNext(); ) { - String endName = (String) it.next(); + private String joinWithAnds(Collection<String> strings) { + StringBuilder sb = new StringBuilder(); + for (String s : strings) { if (sb.length() != 0) { - sb.append(" or "); + sb.append(" and "); } - sb.append(endName); + sb.append(s); } return sb.toString(); } diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index 9d9123e..3ba63f6 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -29419,6 +29419,11 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> <itemizedlist> <listitem> + <para>More helpful parser error messages for nesting problems + (caused by missed or malformed end-tags usually).</para> + </listitem> + + <listitem> <para>Added <literal>DOMNodeSupport</literal> and <literal>JythonSupport</literal> <literal>boolean</literal> properties to <literal>DefaultObjectWrapper</literal>. This diff --git a/src/test/java/freemarker/core/ParsingErrorMessagesTest.java b/src/test/java/freemarker/core/ParsingErrorMessagesTest.java index 73797ba..09af694 100644 --- a/src/test/java/freemarker/core/ParsingErrorMessagesTest.java +++ b/src/test/java/freemarker/core/ParsingErrorMessagesTest.java @@ -91,7 +91,29 @@ public class ParsingErrorMessagesTest extends TemplateTest { assertErrorContains("[#ftl]${'x'>", "end of file"); } } - + + @Test + public void testNestingErrors() throws Exception { + assertErrorContains( + "<#if true><#list xs as x></list></#if>", + "</#if>", "#list", "end-tag"); + assertErrorContains( + "<#if true><#assign x><#else></#assign></#if>", + "<#else>", "#if", "#list", "#assign"); + assertErrorContains( + "<#list xs><#items as x></#list>", + "</#list>", "#items", "end-tag"); + assertErrorContains( + "<#list xs as x><#sep></#if></#list>", + "</#if>", "#list", "#sep", "end-tag"); + assertErrorContains( + "<#list xs as x>", + "end of file", "#list", "end-tag"); + assertErrorContains( + "<#if true>text<#list xs as x></#list>", + "end of file", "#if", "end-tag"); + } + /** * "assertErrorContains" with both angle bracket and square bracket tag syntax, by converting the input tag syntax. * Beware, it uses primitive search-and-replace. diff --git a/src/test/resources/freemarker/test/templatesuite/templates/if.ftl b/src/test/resources/freemarker/test/templatesuite/templates/if.ftl index 97c3f4b..71013ae 100644 --- a/src/test/resources/freemarker/test/templatesuite/templates/if.ftl +++ b/src/test/resources/freemarker/test/templatesuite/templates/if.ftl @@ -103,7 +103,7 @@ </#list></#list> <#-- parsing errors --> -<@assertFails message="valid #if-#elseif-#else"><@"<#if t><#else><#elseif t2></#if>"?interpret /></@> -<@assertFails message="valid #if-#elseif-#else"><@"<#if t><#else><#else></#if>"?interpret /></@> -<@assertFails message="valid #if-#elseif-#else"><@"<#else></#else>"?interpret /></@> -<@assertFails message="valid #if-#elseif-#else"><@"<#elseif t></#elseif>"?interpret /></@> +<@assertFails message="<#elseif"><@"<#if t><#else><#elseif t2></#if>"?interpret /></@> +<@assertFails message="<#else>"><@"<#if t><#else><#else></#if>"?interpret /></@> +<@assertFails message="<#else>"><@"<#else></#else>"?interpret /></@> +<@assertFails message="<#elseif"><@"<#elseif t></#elseif>"?interpret /></@>