Cleanup

6 files changed, 77 insertions, 77 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/FeatureNames.java b/config-model/src/main/java/com/yahoo/searchdefinition/FeatureNames.java
index 649d7bddcc2..3db818c4a85 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/FeatureNames.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/FeatureNames.java
@@ -92,34 +92,15 @@ public class FeatureNames {
     }
 
     public static ReferenceNode.Reference asConstantFeature(String constantName) {
-        return ReferenceNode.Reference.simple("constant", constantName);
+        return ReferenceNode.Reference.simple("constant", quoteIfNecessary(constantName));
     }
 
     public static ReferenceNode.Reference asAttributeFeature(String attributeName) {
-        return ReferenceNode.Reference.simple("attribute", attributeName);
+        return ReferenceNode.Reference.simple("attribute", quoteIfNecessary(attributeName));
     }
 
     public static ReferenceNode.Reference asQueryFeature(String propertyName) {
-        return ReferenceNode.Reference.simple("query", propertyName);
-    }
-
-    /** Returns true if this is a constant, attribute, or query feature */
-    public static boolean isSimpleFeature(String feature) {
-        return FeatureNames.isConstantFeature(feature) ||
-               FeatureNames.isAttributeFeature(feature) ||
-               FeatureNames.isQueryFeature(feature);
-    }
-
-    public static boolean isConstantFeature(String feature) {
-        return feature.startsWith("constant(");
-    }
-
-    public static boolean isAttributeFeature(String feature) {
-        return feature.startsWith("attribute(");
-    }
-
-    public static boolean isQueryFeature(String feature) {
-        return feature.startsWith("query(");
+        return ReferenceNode.Reference.simple("query", quoteIfNecessary(propertyName));
     }
 
     /**
@@ -138,4 +119,11 @@ public class FeatureNames {
         });
     }
 
+    private static String quoteIfNecessary(String s) {
+        if (identifierRegexp.matcher(s).matches())
+            return s;
+        else
+            return "\"" + s + "\"";
+    }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/MapEvaluationTypeContext.java b/config-model/src/main/java/com/yahoo/searchdefinition/MapEvaluationTypeContext.java
index d7ab30f5451..43e8c7db357 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/MapEvaluationTypeContext.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/MapEvaluationTypeContext.java
@@ -55,12 +55,11 @@ public class MapEvaluationTypeContext extends FunctionReferenceContext implement
             throw new IllegalArgumentException("Not expecting unstructured names here");
         ReferenceNode.Reference reference = (ReferenceNode.Reference)name;
 
-        System.out.println("Returning type of " + name);
-
         Optional<String> binding = boundIdentifier(reference);
         if (binding.isPresent()) {
-            System.out.println("  Is bound identifier: " + binding.get());
             try {
+                // This is not pretty, but changing to bind expressions rather
+                // than their string values requires deeper changes
                 return new RankingExpression(binding.get()).type(this);
             }
             catch (ParseException e) {
@@ -72,20 +71,17 @@ public class MapEvaluationTypeContext extends FunctionReferenceContext implement
             // The argument may be a local identifier bound to the actual value
             String argument = simpleArgument(reference.arguments()).get();
             reference = ReferenceNode.Reference.simple(reference.name(), bindings.getOrDefault(argument, argument));
-            System.out.println("  Is simple feature reference: " + reference);
             return featureTypes.get(reference);
         }
 
         Optional<ExpressionFunction> function = functionInvocation(reference);
         if (function.isPresent()) {
-            System.out.println("  Is function reference: " + function.get());
             return function.get().getBody().type(this.withBindings(bind(function.get().arguments(), reference.arguments())));
         }
 
         // We do not know what this is - since we do not have complete knowledge abut the match features
         // in Java we must assume this is a match feature and return the double type - which is the type of all
         // all match features
-        System.out.println("  Is something else");
         return TensorType.empty;
     }
 
@@ -123,7 +119,7 @@ public class MapEvaluationTypeContext extends FunctionReferenceContext implement
         if ( ! (argument instanceof ReferenceNode)) return Optional.empty();
         ReferenceNode refArgument = (ReferenceNode)argument;
 
-        if ( ! refArgument.isBindableName()) return Optional.empty();
+        if ( ! refArgument.reference().isIdentifier()) return Optional.empty();
 
         return Optional.of(refArgument.getName());
     }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Context.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Context.java
index 861f9565d66..9779e01bf51 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Context.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/Context.java
@@ -46,6 +46,7 @@ public abstract class Context implements EvaluationContext {
      *                  calculation to output several), or null to output the
      *                  "main" (or only) value.
      */
+    // TODO: Remove/change to use reference?
     public Value get(String name, Arguments arguments, String output) {
         if (arguments != null && arguments.expressions().size() > 0)
             name = name + "(" + arguments.expressions().stream().map(ExpressionNode::toString).collect(Collectors.joining(",")) + ")";
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapTypeContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapTypeContext.java
index 2ddc8213d94..4ce7dd561e2 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapTypeContext.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/MapTypeContext.java
@@ -17,8 +17,8 @@ public class MapTypeContext implements TypeContext {
 
     private final Map<Name, TensorType> featureTypes = new HashMap<>();
 
-    public void setType(String name, TensorType type) {
-        featureTypes.put(new Name(name), type);
+    public void setType(Name name, TensorType type) {
+        featureTypes.put(name, type);
     }
 
     @Override
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();
+        }
+
     }
 
 }
diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java
index 18444c7f8b6..da546dd30d0 100644
--- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java
+++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/evaluation/TypeResolutionTestCase.java
@@ -4,6 +4,7 @@ package com.yahoo.searchlib.rankingexpression.evaluation;
 
 import com.yahoo.searchlib.rankingexpression.RankingExpression;
 import com.yahoo.searchlib.rankingexpression.parser.ParseException;
+import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode;
 import com.yahoo.tensor.TensorType;
 import com.yahoo.tensor.evaluation.TypeContext;
 import org.junit.Test;
@@ -19,11 +20,16 @@ public class TypeResolutionTestCase {
     @Test
     public void testTypeResolution() {
         MapTypeContext context = new MapTypeContext();
-        context.setType("query(x1)", TensorType.fromSpec("tensor(x[])"));
-        context.setType("query(x2)", TensorType.fromSpec("tensor(x[10])"));
-        context.setType("query(y1)", TensorType.fromSpec("tensor(y[])"));
-        context.setType("query(xy1)", TensorType.fromSpec("tensor(x[10],y[])"));
-        context.setType("query(xy2)", TensorType.fromSpec("tensor(x[],y[10])"));
+        context.setType(ReferenceNode.Reference.simple("query", "x1"),
+                        TensorType.fromSpec("tensor(x[])"));
+        context.setType(ReferenceNode.Reference.simple("query", "x2"),
+                        TensorType.fromSpec("tensor(x[10])"));
+        context.setType(ReferenceNode.Reference.simple("query", "y1"),
+                        TensorType.fromSpec("tensor(y[])"));
+        context.setType(ReferenceNode.Reference.simple("query", "xy1"),
+                        TensorType.fromSpec("tensor(x[10],y[])"));
+        context.setType(ReferenceNode.Reference.simple("query", "xy2"),
+                        TensorType.fromSpec("tensor(x[],y[10])"));
 
         assertType("tensor(x[])", "query(x1)", context);
         assertType("tensor(x[])", "if (1>0, query(x1), query(x2))", context);