Author: wglass Date: Sat Nov 4 22:02:20 2006 New Revision: 471372 URL: http://svn.apache.org/viewvc?view=rev&rev=471372 Log: Patch for VELOCITY-435. When new property "velocimacro.arguments.strict" is set to true, will throw a ParseErrorException after encountering macro call with invalid number of arguments. Arguably, this should always be the case but we default the property to false for backwards compatibility.
Added: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java (with props) Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/app/VelocityEngine.java jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/defaults/velocity.properties jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTDirective.java jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java jakarta/velocity/engine/trunk/xdocs/docs/developer-guide.xml Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/app/VelocityEngine.java URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/app/VelocityEngine.java?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/app/VelocityEngine.java (original) +++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/app/VelocityEngine.java Sat Nov 4 22:02:20 2006 @@ -38,6 +38,7 @@ import org.apache.velocity.exception.ResourceNotFoundException; import org.apache.velocity.runtime.RuntimeConstants; import org.apache.velocity.runtime.RuntimeInstance; +import org.apache.velocity.runtime.directive.DirectiveInitException; import org.apache.velocity.runtime.log.Log; import org.apache.velocity.runtime.parser.ParseException; import org.apache.velocity.runtime.parser.node.SimpleNode; @@ -333,6 +334,10 @@ catch( RuntimeException e ) { throw e; + } + catch (DirectiveInitException pex) + { + throw new ParseErrorException( pex ); } catch(Exception e) { Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java (original) +++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java Sat Nov 4 22:02:20 2006 @@ -246,6 +246,9 @@ /** switch for local context in VM : default false. */ String VM_CONTEXT_LOCALSCOPE = "velocimacro.context.localscope"; + /** if true, throw an exception for wrong number of arguments **/ + String VM_ARGUMENTS_STRICT = "velocimacro.arguments.strict"; + /* * ---------------------------------------------------------------------- * G E N E R A L R U N T I M E C O N F I G U R A T I O N Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/defaults/velocity.properties URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/defaults/velocity.properties?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/defaults/velocity.properties (original) +++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/defaults/velocity.properties Sat Nov 4 22:02:20 2006 @@ -114,6 +114,17 @@ velocimacro.context.localscope = false # ---------------------------------------------------------------------------- +# VELOCIMACRO STRICT MODE +# ---------------------------------------------------------------------------- +# if true, will throw an exception for incorrect number +# of arguments. false by default (for backwards compatibility) +# but this option will eventually be removed and will always +# act as if true +# ---------------------------------------------------------------------------- +velocimacro.arguments.strict = false + + +# ---------------------------------------------------------------------------- # INTERPOLATION # ---------------------------------------------------------------------------- # turn off and on interpolation of references and directives in string Added: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java?view=auto&rev=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java (added) +++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java Sat Nov 4 22:02:20 2006 @@ -0,0 +1,82 @@ +package org.apache.velocity.runtime.directive; + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import org.apache.velocity.exception.ExtendedParseException; +import org.apache.velocity.runtime.parser.ParseException; + +/** + * Exception generated to indicate parse errors caught during + * directive initialization (e.g. wrong number of arguments) + * + * For internal use in parser - not to be passed to app level. + * Hence this ultimately extends Exception not RuntimeException. + * + * @author <a href="mailto:[EMAIL PROTECTED]">Will Glass-Husain</a> + * @version $Id$ + */ +public class DirectiveInitException extends ParseException + implements ExtendedParseException +{ + private final String templateName; + private final int col; + private final int line; + + /** + * Version Id for serializable + */ + private static final long serialVersionUID = -4985224672336070621L; + + public DirectiveInitException(final String msg, + final String templateName, final int col, final int line) + { + super(msg); + this.templateName = templateName; + this.col = col; + this.line = line; + } + + /** + * Returns the Template name where this exception occured. + * @return the template name + */ + public String getTemplateName() + { + return templateName; + } + + /** + * Returns the line number where this exception occured. + * @return the line number + */ + public int getLineNumber() + { + return line; + } + + /** + * Returns the column number where this exception occured. + * @return the line number + */ + public int getColumnNumber() + { + return col; + } +} Propchange: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java ------------------------------------------------------------------------------ svn:eol-style = native Propchange: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/DirectiveInitException.java ------------------------------------------------------------------------------ svn:keywords = Id Author Date Revision Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java (original) +++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java Sat Nov 4 22:02:20 2006 @@ -29,6 +29,7 @@ import org.apache.velocity.context.InternalContextAdapter; import org.apache.velocity.context.VMContext; import org.apache.velocity.exception.MethodInvocationException; +import org.apache.velocity.runtime.RuntimeConstants; import org.apache.velocity.runtime.RuntimeServices; import org.apache.velocity.runtime.parser.ParserTreeConstants; import org.apache.velocity.runtime.parser.Token; @@ -59,7 +60,8 @@ private int[] callingArgTypes; private HashMap proxyArgHash = new HashMap(); - + private boolean strictArguments; + /** * Return name of this Velocimacro. * @return The name of this Velocimacro. @@ -236,6 +238,11 @@ { super.init( rs, context, node ); + /** + * Throw exception for invalid number of arguments? + */ + strictArguments = rs.getConfiguration().getBoolean(RuntimeConstants.VM_ARGUMENTS_STRICT,false); + /* * how many args did we get? */ @@ -265,11 +272,27 @@ parent = parent.jjtGetParent(); } - rsvc.getLog().error("VM #" + macroName + ": error : too " + - ((getNumArgs() > i) ? "few" : "many") + - " arguments to macro. Wanted " + getNumArgs() + - " got " + i); - return; + String errormsg = "VM #" + macroName + ": error : too " + + ((getNumArgs() > i) ? "few" : "many") + + " arguments to macro. Wanted " + getNumArgs() + + " got " + i; + + if (strictArguments) + { + /** + * indicate col/line assuming it starts at 0 - this will be + * corrected one call up + */ + throw new DirectiveInitException(errormsg, + context.getCurrentTemplateName(), + 0, + 0); + } + else + { + rsvc.getLog().error(errormsg); + return; + } } /* Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTDirective.java URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTDirective.java?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTDirective.java (original) +++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTDirective.java Sat Nov 4 22:02:20 2006 @@ -19,18 +19,18 @@ * under the License. */ -import java.io.Writer; import java.io.IOException; +import java.io.Writer; import org.apache.commons.lang.builder.ToStringBuilder; import org.apache.velocity.context.InternalContextAdapter; -import org.apache.velocity.runtime.directive.Directive; -import org.apache.velocity.runtime.parser.Parser; -import org.apache.velocity.runtime.parser.ParserVisitor; - import org.apache.velocity.exception.MethodInvocationException; import org.apache.velocity.exception.ParseErrorException; import org.apache.velocity.exception.ResourceNotFoundException; +import org.apache.velocity.runtime.directive.Directive; +import org.apache.velocity.runtime.directive.DirectiveInitException; +import org.apache.velocity.runtime.parser.Parser; +import org.apache.velocity.runtime.parser.ParserVisitor; /** * This class is responsible for handling the pluggable @@ -110,7 +110,21 @@ isDirective = true; directive = rsvc.getVelocimacro( directiveName, context.getCurrentTemplateName()); - directive.init( rsvc, context, this ); + try + { + directive.init( rsvc, context, this ); + } + + /** + * correct the line/column number if an exception is caught + */ + catch (DirectiveInitException die) + { + throw new DirectiveInitException(die.getMessage(), + die.getTemplateName(), + die.getColumnNumber() + getColumn(), + die.getLineNumber() + getLine()); + } directive.setLocation( getLine(), getColumn() ); } else Modified: jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java (original) +++ jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/ParseExceptionTestCase.java Sat Nov 4 22:02:20 2006 @@ -47,6 +47,7 @@ /** * Default constructor. + * @param name name of test */ public ParseExceptionTestCase(String name) { @@ -209,4 +210,72 @@ } } + + /** + * Tests that parseException has useful info with macro calls with + * invalid number of arguments + * @throws Exception + */ + public void testParseExceptionMacroInvalidArgumentCount () + throws Exception + { + VelocityEngine ve = new VelocityEngine(); + ve.setProperty("velocimacro.arguments.strict","true"); + ve.init(); + + VelocityContext context = new VelocityContext(); + + Writer writer = new StringWriter(); + + try + { + ve.evaluate(context,writer,"testMacroInvoke", "#macro(foo $a) $a #end #foo('test1' 'test2')"); + fail("Should have thown a ParseErrorException"); + } + catch (ParseErrorException e) + { + assertEquals("testMacroInvoke",e.getTemplateName()); + assertEquals(1,e.getLineNumber()); + assertEquals(24,e.getColumnNumber()); + } + finally + { + if (writer != null) + { + writer.close(); + } + } + } + + + /** + * Tests that parseException has useful info with macro calls with + * invalid number of arguments + * @throws Exception + */ + public void testParseExceptionMacroInvalidArgumentCountNoException () + throws Exception + { + VelocityEngine ve = new VelocityEngine(); + ve.init(); + + VelocityContext context = new VelocityContext(); + + Writer writer = new StringWriter(); + + // will not throw an exception + try + { + ve.evaluate(context,writer,"testMacroInvoke", "#macro(foo $a) $a #end #foo('test1' 'test2')"); + } + finally + { + if (writer != null) + { + writer.close(); + } + } + } + + } Modified: jakarta/velocity/engine/trunk/xdocs/docs/developer-guide.xml URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/xdocs/docs/developer-guide.xml?view=diff&rev=471372&r1=471371&r2=471372 ============================================================================== --- jakarta/velocity/engine/trunk/xdocs/docs/developer-guide.xml (original) +++ jakarta/velocity/engine/trunk/xdocs/docs/developer-guide.xml Sat Nov 4 22:02:20 2006 @@ -1718,6 +1718,14 @@ </p> <p> +<code>velocimacro.arguments.strict = false</code><br/> +When set to true, will throw a <code>ParseErrorException</code> when +parsing a template containing a macro with an invalid number of arguments. +Is set to false by default to maintain backwards compatibility with +templates written before this feature became available. +</p> + + <p> <strong>String Interpolation</strong> </p> --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]