You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2012/07/11 14:20:43 UTC

svn commit: r1360141 - in /commons/proper/jexl/trunk/src: main/java/org/apache/commons/jexl3/internal/ main/java/org/apache/commons/jexl3/parser/ test/java/org/apache/commons/jexl3/ test/java/org/apache/commons/jexl3/internal/

Author: henrib
Date: Wed Jul 11 12:20:43 2012
New Revision: 1360141

URL: http://svn.apache.org/viewvc?rev=1360141&view=rev
Log:
JEXL-137:
Added new inner class Engine.VarCollector to properly collect variables;
Updated TemplateEngine to use new Engine.VarCollector;
Updated test;
Added Dumper class to ease characterizing AST related issues;

Added:
    commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/Dumper.java   (with props)
Modified:
    commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Engine.java
    commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
    commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java
    commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ParserVisitor.java
    commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/VarTest.java

Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Engine.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Engine.java?rev=1360141&r1=1360140&r2=1360141&view=diff
==============================================================================
--- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Engine.java (original)
+++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/Engine.java Wed Jul 11 12:20:43 2012
@@ -34,7 +34,6 @@ import org.apache.commons.jexl3.parser.A
 import org.apache.commons.jexl3.parser.ASTIdentifierAccess;
 import org.apache.commons.jexl3.parser.ASTJexlScript;
 import org.apache.commons.jexl3.parser.ASTMethodNode;
-import org.apache.commons.jexl3.parser.ASTReference;
 import org.apache.commons.jexl3.parser.JexlNode;
 import org.apache.commons.jexl3.parser.Parser;
 
@@ -120,7 +119,6 @@ public class Engine extends JexlEngine {
      * The default charset.
      */
     protected final Charset charset;
-
     /**
      * The default cache load factor.
      */
@@ -503,85 +501,116 @@ public class Engine extends JexlEngine {
      * are written in 'dot' or 'bracketed' notation. (a.b is equivalent to a['b']).</p>
      * @param script the script
      * @return the set of variables, each as a list of strings (ant-ish variables use more than 1 string)
-     * or the empty set if no variables are used
+     *         or the empty set if no variables are used
      */
     protected Set<List<String>> getVariables(JexlNode script) {
-        Set<List<String>> refs = new LinkedHashSet<List<String>>();
-        getVariables(script, refs, null);
-        return refs;
+        VarCollector collector = new VarCollector();
+        getVariables(script, collector);
+        return collector.collected();
+    }
+
+    /**
+     * Utility class to collect variables.
+     */
+    protected static class VarCollector {
+        /**
+         * The collected variables represented as a set of list of strings.
+         */
+        private Set<List<String>> refs = new LinkedHashSet<List<String>>();
+        /**
+         * The current variable being collected.
+         */
+        private List<String> ref = new ArrayList<String>();
+        /**
+         * The node that started the collect.
+         */
+        private JexlNode root = null;
+
+        /**
+         * Starts/stops a variable collect.
+         * @param node starts if not null, stop if null
+         */
+        public void collect(JexlNode node) {
+            if (!ref.isEmpty()) {
+                refs.add(ref);
+                ref = new ArrayList<String>();
+            }
+            root = node;
+        }
+
+        /**
+         * @return true if currently collecting a variable, false otherwise
+         */
+        public boolean isCollecting() {
+            return root instanceof ASTIdentifier;
+        }
+
+        /**
+         * Adds a 'segment' to the variable being collected.
+         * @param name the name
+         */
+        public void add(String name) {
+            ref.add(name);
+        }
+
+        /**
+         *@return the collected variables
+         */
+        public Set<List<String>> collected() {
+            return refs;
+        }
     }
 
     /**
      * Fills up the list of variables accessed by a node.
      * @param node the node
-     * @param refs the set of variable being filled
-     * @param ref  the current variable being filled
+     * @param collector the variable collector
      */
-    protected void getVariables(JexlNode node, Set<List<String>> refs, List<String> ref) {
+    protected void getVariables(JexlNode node, VarCollector collector) {
         if (node instanceof ASTIdentifier) {
             JexlNode parent = node.jjtGetParent();
             if (parent instanceof ASTMethodNode || parent instanceof ASTFunctionNode) {
                 // skip identifiers for methods and functions
+                collector.collect(null);
                 return;
             }
             ASTIdentifier identifier = (ASTIdentifier) node;
             if (identifier.getSymbol() < 0) {
-                if (ref == null) {
-                    ref = new ArrayList<String>();
-                    refs.add(ref);
-                }
-                ref.add(identifier.getName());
+                // start collecting from identifier
+                collector.collect(identifier);
+                collector.add(identifier.getName());
+            } else {
+                collector.collect(null);
             }
-        } else {
+        } else if (node instanceof ASTIdentifierAccess) {
+            // belt and suspender since an identifier should have been seen first
+            if (collector.isCollecting()) {
+                collector.add(((ASTIdentifierAccess) node).getName());
+            }
+        } else if (node instanceof ASTArrayAccess) {
             int num = node.jjtGetNumChildren();
-            boolean array = node instanceof ASTArrayAccess;
-            boolean reference = node instanceof ASTReference;
-            if (array || reference) {
-                List<String> var = ref != null ? ref : new ArrayList<String>();
-                boolean varf = true;
-                for (int i = 0; i < num; ++i) {
-                    JexlNode child = node.jjtGetChild(i);
-                    if (array) {
-                        if (varf && child.isConstant()) {
-                            String image = child.toString();
-                            if (image == null) {
-                                var.add(new Debugger().data(child));
-                            } else {
-                                var.add(image);
-                            }
-                        } else if (child instanceof ASTIdentifier) {
-                            ASTIdentifier ichild = (ASTIdentifier) child;
-                            if (ichild.getSymbol() < 0) {
-                                List<String> di = new ArrayList<String>(1);
-                                di.add(ichild.getName());
-                                refs.add(di);
-                            }
-                            var = new ArrayList<String>();
-                            varf = false;
-                        }
-                        continue;
-                    } else if (child instanceof ASTIdentifier) {
-                        ASTIdentifier ichild = (ASTIdentifier) child;
-                        if (i == 0 && ichild.getSymbol() < 0) {
-                            var.add(ichild.getName());
-                        }
-                        varf = false;
-                        continue;
-                    } else if (child instanceof ASTIdentifierAccess) {
-                        var.add(((ASTIdentifierAccess) child).getName());
-                        varf = false;
-                        continue;
+            // collect only if array access is const and follows an identifier
+            boolean collecting = collector.isCollecting();
+            for (int i = 0; i < num; ++i) {
+                JexlNode child = node.jjtGetChild(i);
+                if (collecting && child.isConstant()) {
+                    String image = child.toString();
+                    if (image == null) {
+                        image = new Debugger().data(child);
                     }
-                    getVariables(child, refs, var);
-                }
-                if (!var.isEmpty() && var != ref) {
-                    refs.add(var);
-                }
-            } else {
-                for (int i = 0; i < num; ++i) {
-                    getVariables(node.jjtGetChild(i), refs, null);
+                    collector.add(image);
+                } else {
+                    collecting = false;
+                    collector.collect(null);
+                    getVariables(child, collector);
                 }
             }
+        } else {
+            int num = node.jjtGetNumChildren();
+            for (int i = 0; i < num; ++i) {
+                getVariables(node.jjtGetChild(i), collector);
+            }
+            collector.collect(null);
         }
     }
 
@@ -608,15 +637,15 @@ public class Engine extends JexlEngine {
     /**
      * Parses an expression.
      *
-     * @param info       information structure
-     * @param expr       the expression to parse
-     * @param scope      the script frame
-     * @param registers  whether the parser should allow the unnamed '#number' syntax for 'registers'
+     * @param info      information structure
+     * @param expr      the expression to parse
+     * @param scope     the script frame
+     * @param registers whether the parser should allow the unnamed '#number' syntax for 'registers'
      * @return the parsed tree
      * @throws JexlException if any error occured during parsing
      */
     protected ASTJexlScript parse(JexlInfo info, String expr, Scope scope, boolean registers) {
-        final boolean cached = expr.length() < cacheThreshold && cache != null ;
+        final boolean cached = expr.length() < cacheThreshold && cache != null;
         ASTJexlScript script;
         synchronized (parser) {
             if (cached) {

Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java?rev=1360141&r1=1360140&r2=1360141&view=diff
==============================================================================
--- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java (original)
+++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/internal/TemplateEngine.java Wed Jul 11 12:20:43 2012
@@ -34,7 +34,6 @@ import java.io.Writer;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -78,7 +77,8 @@ public final class TemplateEngine extend
         IMMEDIATE(1),
         /** Deferred TemplateExpression, count index 2. */
         DEFERRED(2),
-        /** Nested (which are deferred) expressions, count index 2. */
+        /** Nested (which are deferred) expressions, count
+         * index 2. */
         NESTED(2),
         /** Composite expressions are not counted, index -1. */
         COMPOSITE(-1);
@@ -229,9 +229,9 @@ public final class TemplateEngine extend
 
         /**
          * Fills up the list of variables accessed by this unified expression.
-         * @param refs the set of variable being filled
+         * @param collector the variable collector
          */
-        protected void getVariables(Set<List<String>> refs) {
+        protected void getVariables(Engine.VarCollector collector) {
             // nothing to do
         }
 
@@ -374,14 +374,14 @@ public final class TemplateEngine extend
 
         @Override
         public Set<List<String>> getVariables() {
-            Set<List<String>> refs = new LinkedHashSet<List<String>>();
-            getVariables(refs);
-            return refs;
+            Engine.VarCollector collector = new Engine.VarCollector();
+            getVariables(collector);
+            return collector.collected();
         }
 
         @Override
-        protected void getVariables(Set<List<String>> refs) {
-            jexl.getVariables(node, refs, null);
+        protected void getVariables(Engine.VarCollector collector) {
+            jexl.getVariables(node, collector);
         }
     }
 
@@ -438,7 +438,7 @@ public final class TemplateEngine extend
         }
 
         @Override
-        protected void getVariables(Set<List<String>> refs) {
+        protected void getVariables(Engine.VarCollector collector) {
             // noop
         }
     }
@@ -532,11 +532,11 @@ public final class TemplateEngine extend
 
         @Override
         public Set<List<String>> getVariables() {
-            Set<List<String>> refs = new LinkedHashSet<List<String>>();
+            Engine.VarCollector collector = new Engine.VarCollector();
             for (TemplateExpression expr : exprs) {
-                expr.getVariables(refs);
+                expr.getVariables(collector);
             }
-            return refs;
+            return collector.collected();
         }
 
         @Override
@@ -857,7 +857,7 @@ public final class TemplateEngine extend
         /**
          * Creates a new template from an character input.
          * @param directive the prefix for lines of code; can not be "$", "${", "#" or "#{"
-         * since this would preclude being able to differentiate directives and template expressions
+         *                  since this would preclude being able to differentiate directives and template expressions
          * @param reader    the input reader
          * @param parms     the parameter names
          * @throws NullPointerException     if either the directive prefix or input is null

Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java?rev=1360141&r1=1360140&r2=1360141&view=diff
==============================================================================
--- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java (original)
+++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java Wed Jul 11 12:20:43 2012
@@ -52,4 +52,9 @@ public final class ASTIdentifierAccess e
     public Object jjtAccept(ParserVisitor visitor, Object data) {
         return visitor.visit(this, data);
     }
+
+    @Override
+    public String toString() {
+        return name;
+    }
 }

Modified: commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ParserVisitor.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ParserVisitor.java?rev=1360141&r1=1360140&r2=1360141&view=diff
==============================================================================
--- commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ParserVisitor.java (original)
+++ commons/proper/jexl/trunk/src/main/java/org/apache/commons/jexl3/parser/ParserVisitor.java Wed Jul 11 12:20:43 2012
@@ -1,9 +1,10 @@
 /*
- * Copyright 2011 The Apache Software Foundation.
- *
- * Licensed 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
+ * 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
  *

Modified: commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/VarTest.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/VarTest.java?rev=1360141&r1=1360140&r2=1360141&view=diff
==============================================================================
--- commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/VarTest.java (original)
+++ commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/VarTest.java Wed Jul 11 12:20:43 2012
@@ -230,10 +230,29 @@ public class VarTest extends JexlTestCas
         assertTrue(eq(expect, vars));
 
         e = JEXL.createScript("a + b.c + b.c.d + e['f']");
-        //LOGGER.info(flattenedStr(e));
         vars = e.getVariables();
         expect = mkref(new String[][]{{"a"}, {"b", "c"}, {"b", "c", "d"}, {"e", "f"}});
         assertTrue(eq(expect, vars));
+
+        e = JEXL.createScript("D[E[F]]");
+        vars = e.getVariables();
+        expect = mkref(new String[][]{{"D"}, {"E"}, {"F"}});
+        assertTrue(eq(expect, vars));
+
+        e = JEXL.createScript("D[E[F[G[H]]]]");
+        vars = e.getVariables();
+        expect = mkref(new String[][]{{"D"}, {"E"}, {"F"}, {"G"}, {"H"}});
+        assertTrue(eq(expect, vars));
+
+        e = JEXL.createScript(" A + B[C] + D[E[F]] + x[y[z]] ");
+        vars = e.getVariables();
+        expect = mkref(new String[][]{{"A"}, {"B"}, {"C"}, {"D"}, {"E"}, {"F"}, {"x"} , {"y"}, {"z"}});
+        assertTrue(eq(expect, vars));
+
+        e = JEXL.createScript(" A + B[C] + D.E['F'] + x[y.z] ");
+        vars = e.getVariables();
+        expect = mkref(new String[][]{{"A"}, {"B"}, {"C"}, {"D", "E", "F"}, {"x"} , {"y", "z"}});
+        assertTrue(eq(expect, vars));
     }
 
     public void testMix() throws Exception {

Added: commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/Dumper.java
URL: http://svn.apache.org/viewvc/commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/Dumper.java?rev=1360141&view=auto
==============================================================================
--- commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/Dumper.java (added)
+++ commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/Dumper.java Wed Jul 11 12:20:43 2012
@@ -0,0 +1,78 @@
+/*
+ * 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.
+ */
+package org.apache.commons.jexl3.internal;
+
+import org.apache.commons.jexl3.JexlScript;
+import org.apache.commons.jexl3.parser.ASTIdentifier;
+import org.apache.commons.jexl3.parser.ASTIdentifierAccess;
+import org.apache.commons.jexl3.parser.JexlNode;
+
+/**
+ * Utility to dump AST, useful in debug sessions.
+ */
+public class Dumper {
+    private StringBuilder strb = new StringBuilder();
+    private int indent = 0;
+
+    private void indent() {
+        for (int i = 0; i < indent; ++i) {
+            strb.append("  ");
+        }
+    }
+
+    private void dump(JexlNode node, Object data) {
+        final int num = node.jjtGetNumChildren();
+        indent();
+        strb.append(node.getClass().getSimpleName());
+        if (node instanceof ASTIdentifier) {
+            strb.append("@");
+            strb.append(node.toString());
+        } else if (node instanceof ASTIdentifierAccess) {
+            strb.append("@");
+            strb.append(node.toString());
+        }
+        strb.append('(');
+        indent += 1;
+        for (int c = 0; c < num; ++c) {
+            JexlNode child = node.jjtGetChild(c);
+            if (c > 0) {
+                strb.append(',');
+            }
+            strb.append('\n');
+            dump(child, data);
+        }
+        indent -= 1;
+        if (num > 0) {
+            strb.append('\n');
+            indent();
+        }
+        strb.append(')');
+    }
+
+    private Dumper(JexlScript script) {
+        dump(((Script) script).script, null);
+    }
+
+    @Override
+    public String toString() {
+        return strb.toString();
+    }
+
+    public static String toString(JexlScript script) {
+        return new Dumper(script).toString();
+    }
+}

Propchange: commons/proper/jexl/trunk/src/test/java/org/apache/commons/jexl3/internal/Dumper.java
------------------------------------------------------------------------------
    svn:eol-style = native