1 files changed, 50 insertions, 41 deletions
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
index e121fa12b5f..8c6bc6a6291 100755
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/ReferenceNode.java
@@ -9,7 +9,6 @@ import com.yahoo.tensor.TensorType;
 import com.yahoo.tensor.evaluation.TypeContext;
 
 import java.util.ArrayDeque;
-import java.util.Arrays;
 import java.util.Deque;
 import java.util.List;
 import java.util.Objects;
@@ -27,44 +26,46 @@ import java.util.stream.Collectors;
 //       At least the first two should be split into separate classes.
 public final class ReferenceNode extends CompositeNode {
 
-    private final String name, output;
-
-    private final Arguments arguments;
+    private final Reference reference;
 
+    /* Creates a node with a simple identifier reference */
     public ReferenceNode(String name) {
         this(name, null, null);
     }
 
     public ReferenceNode(String name, List<? extends ExpressionNode> arguments, String output) {
-        Objects.requireNonNull(name, "name cannot be null");
-        this.name = name;
-        this.arguments = arguments != null ? new Arguments(arguments) : new Arguments();
-        this.output = output;
+        this.reference = new Reference(name,
+                                       arguments != null ? new Arguments(arguments) : new Arguments(),
+                                       output);
+    }
+
+    public ReferenceNode(Reference reference) {
+        this.reference = reference;
     }
 
     public String getName() {
-        return name;
+        return reference.name();
     }
 
     /** Returns the arguments, never null */
-    public Arguments getArguments() { return arguments; }
+    public Arguments getArguments() { return reference.arguments(); }
 
     /** Returns a copy of this where the arguments are replaced by the given arguments */
     public ReferenceNode setArguments(List<ExpressionNode> arguments) {
-        return new ReferenceNode(name, arguments, output);
+        return new ReferenceNode(reference.withArguments(new Arguments(arguments)));
     }
 
     /** Returns the specific output this references, or null if none specified */
-    public String getOutput() { return output; }
+    public String getOutput() { return reference.output(); }
 
     /** Returns a copy of this node with a modified output */
     public ReferenceNode setOutput(String output) {
-        return new ReferenceNode(name, arguments.expressions(), output);
+        return new ReferenceNode(reference.withOutput(output));
     }
 
     /** Returns an empty list as this has no children */
     @Override
-    public List<ExpressionNode> children() { return arguments.expressions(); }
+    public List<ExpressionNode> children() { return reference.arguments().expressions(); }
 
     @Override
     public String toString(SerializationContext context, Deque<String> path, CompositeNode parent) {
@@ -74,12 +75,12 @@ public final class ReferenceNode extends CompositeNode {
     private String toString(SerializationContext context, Deque<String> path, boolean includeOutput) {
         if (path == null)
             path = new ArrayDeque<>();
-        String myName = this.name;
-        String myOutput = this.output;
-        List<ExpressionNode> myArguments = this.arguments.expressions();
+        String myName = getName();
+        String myOutput = getOutput();
+        List<ExpressionNode> myArguments = getArguments().expressions();
 
         String resolvedArgument = context.getBinding(myName);
-        if (resolvedArgument != null && isBindableName()) {
+        if (resolvedArgument != null && reference.isIdentifier()) {
             // Replace this whole node with the value of the argument value that it maps to
             myName = resolvedArgument;
             myArguments = null;
@@ -88,7 +89,7 @@ public final class ReferenceNode extends CompositeNode {
             // Replace by the referenced expression
             ExpressionFunction function = context.getFunction(myName);
             if (function != null && myArguments != null && function.arguments().size() == myArguments.size() && myOutput == null) {
-                String myPath = name + this.arguments.expressions();
+                String myPath = getName() + getArguments().expressions();
                 if (path.contains(myPath)) {
                     throw new IllegalStateException("Cycle in ranking expression function: " + path);
                 }
@@ -101,6 +102,7 @@ public final class ReferenceNode extends CompositeNode {
                 myOutput = null;
             }
         }
+
         // Always print the same way, the magic is already done.
         StringBuilder ret = new StringBuilder(myName);
         if (myArguments != null && myArguments.size() > 0) {
@@ -118,15 +120,12 @@ public final class ReferenceNode extends CompositeNode {
         return ret.toString();
     }
 
-    /** Returns whether this is a name that can be bound to a value (during argument passing) */
-    public boolean isBindableName() {
-        return this.arguments.expressions().size() == 0 && output == null;
-    }
+    /** Returns the reference of this node */
+    public Reference reference() { return reference; }
 
     @Override
     public TensorType type(TypeContext context) {
-        TensorType type = context.getType(new Reference(name, arguments, output,
-                                                        toString(new SerializationContext(), null, true)));
+        TensorType type = context.getType(reference);
         if (type == null)
             throw new IllegalArgumentException("Unknown feature '" + toString() + "'");
         return type;
@@ -134,33 +133,28 @@ public final class ReferenceNode extends CompositeNode {
 
     @Override
     public Value evaluate(Context context) {
-        if (arguments.expressions().isEmpty() && output == null)
-            return context.get(name);
-        return context.get(name, arguments, output);
+        return context.get(reference.toString());
     }
 
     @Override
     public CompositeNode setChildren(List<ExpressionNode> newChildren) {
-        return new ReferenceNode(name, newChildren, output);
+        return setArguments(newChildren);
     }
 
     /** Wraps the content of this in a form which can be passed to a type context */
     // TODO: Extract to top level?
     public static class Reference extends TypeContext.Name {
 
-        private static final Pattern identifierRegexp = Pattern.compile("[A-Za-z0-9_][A-Za-z0-9_-]*");
-
         private final String name;
         private final Arguments arguments;
 
         /** The output, or null if none */
         private final String output;
 
-        public Reference(String name, Arguments arguments, String output, String stringForm) {
-            super(stringForm);
+        public Reference(String name, Arguments arguments, String output) {
+            super(name);
             Objects.requireNonNull(name, "name cannot be null");
             Objects.requireNonNull(arguments, "arguments cannot be null");
-            Objects.requireNonNull(stringForm, "stringForm cannot be null");
             this.name = name;
             this.arguments = arguments;
             this.output = output;
@@ -174,8 +168,7 @@ public final class ReferenceNode extends CompositeNode {
         public static Reference simple(String name, String argumentValue) {
             return new Reference(name,
                                  new Arguments(new ReferenceNode(argumentValue)),
-                                 null,
-                                 name + "(" + quoteIfNecessary(argumentValue) + ")");
+                                 null);
         }
 
         /**
@@ -197,11 +190,17 @@ public final class ReferenceNode extends CompositeNode {
             return Optional.of(simple(featureName, argument));
         }
 
-        private static String quoteIfNecessary(String s) {
-            if (identifierRegexp.matcher(s).matches())
-                return s;
-            else
-                return "\"" + s + "\"";
+        /** Returns whether this is a simple identifier - no arguments or output */
+        public boolean isIdentifier() {
+            return this.arguments.expressions().size() == 0 && output == null;
+        }
+
+        public Reference withArguments(Arguments arguments) {
+            return new Reference(name, arguments, output);
+        }
+
+        public Reference withOutput(String output) {
+            return new Reference(name, arguments, output);
         }
 
         @Override
@@ -220,6 +219,16 @@ public final class ReferenceNode extends CompositeNode {
             return Objects.hash(name, arguments, output);
         }
 
+        @Override
+        public String toString() {
+            StringBuilder b = new StringBuilder(name);
+            if (arguments != null && arguments.expressions().size() > 0)
+                b.append("(").append(arguments.expressions().stream().map(ExpressionNode::toString).collect(Collectors.joining(","))).append(")");
+            if (output !=null)
+                b.append(".").append(output);
+            return b.toString();
+        }
+
     }
 
 }