Detect loops in unbound arguments

4 files changed, 58 insertions, 8 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentReferenceResolver.java b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentReferenceResolver.java
index b0bc376de81..14f8a0a9d37 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/DocumentReferenceResolver.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/DocumentReferenceResolver.java
@@ -52,8 +52,8 @@ public class DocumentReferenceResolver {
         Search search = searchMapping.get(targetDocumentName);
         if (search == null) {
             throw new IllegalArgumentException(
-                    String.format("The field '%s' is an invalid document reference. " +
-                                  "Could not find document with '%s' in any search definitions", field.getName(), targetDocumentName));
+                    String.format("Invalid document reference '%s': " +
+                                  "Could not find document type '%s'", field.getName(), targetDocumentName));
         }
         return new DocumentReference(field, search);
     }
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 2be3022ce6e..7f690b3e43c 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/MapEvaluationTypeContext.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/MapEvaluationTypeContext.java
@@ -89,12 +89,14 @@ public class MapEvaluationTypeContext extends FunctionReferenceContext implement
                                                currentResolutionCallStack.stream().map(Reference::toString).collect(Collectors.joining(" -> ")) +
                                                " -> " + reference);
 
-        // A reference to a function argument?
+
+        // Bound toi a function argument, and not to a same-named identifier (which would lead to a loop)?
         Optional<String> binding = boundIdentifier(reference);
-        if (binding.isPresent()) {
+        if (binding.isPresent() && ! binding.get().equals(reference.toString())) {
             try {
                 // This is not pretty, but changing to bind expressions rather
                 // than their string values requires deeper changes
+                System.out.println("Resolving type of " + reference + " bound to " + binding);
                 return new RankingExpression(binding.get()).type(this);
             } catch (ParseException e) {
                 throw new IllegalArgumentException(e);
@@ -125,8 +127,8 @@ public class MapEvaluationTypeContext extends FunctionReferenceContext implement
                 return featureTensorType.get();
             }
 
-            // 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
+            // We do not know what this is - since we do not have complete knowledge about the match features
+            // in Java we must assume this is a match feature and return the double type - which is the type of
             // all match features
             return TensorType.empty;
         }
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java b/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java
index 9803bd2f55d..8378ec811a5 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/DocumentReferenceResolverTest.java
@@ -69,8 +69,7 @@ public class DocumentReferenceResolverTest {
 
         exceptionRule.expect(IllegalArgumentException.class);
         exceptionRule.expectMessage(
-                "The field 'bar_ref' is an invalid document reference. " +
-                        "Could not find document with 'bar' in any search definitions");
+                "Invalid document reference 'bar_ref': Could not find document type 'bar'");
         resolver.resolveReferences(fooDocument);
     }
 
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionLoopDetectionTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionLoopDetectionTestCase.java
index 9a0dcc7dd07..78484d0c889 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionLoopDetectionTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/RankingExpressionLoopDetectionTestCase.java
@@ -194,4 +194,53 @@ public class RankingExpressionLoopDetectionTestCase {
         builder.build();
     }
 
+    @Test
+    public void testNoLoopWithTheSameNestedIdentifierWhichIsUnbound() throws ParseException {
+        RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
+        SearchBuilder builder = new SearchBuilder(rankProfileRegistry);
+        builder.importString(
+                "search test {\n" +
+                "    document test { \n" +
+                "    }\n" +
+                "    rank-profile test {\n" +
+                "        first-phase {\n" +
+                "            expression: foo()\n" +
+                "        }\n" +
+                "        function foo() {\n" +
+                "            expression: bar(x)\n" +
+                "        }\n" +
+                "        function bar(x) {\n" +
+                "            expression: x + x\n" +
+                "        }\n" +
+                "    }\n" +
+                "}\n");
+        builder.build();
+    }
+
+    @Test
+    public void testNoLoopWithTheSameAlternatingNestedIdentifierWhichIsUnbound() throws ParseException {
+        RankProfileRegistry rankProfileRegistry = new RankProfileRegistry();
+        SearchBuilder builder = new SearchBuilder(rankProfileRegistry);
+        builder.importString(
+                "search test {\n" +
+                "    document test { \n" +
+                "    }\n" +
+                "    rank-profile test {\n" +
+                "        first-phase {\n" +
+                "            expression: foo()\n" +
+                "        }\n" +
+                "        function foo() {\n" +
+                "            expression: bar(x)\n" +
+                "        }\n" +
+                "        function bar(y) {\n" +
+                "            expression: baz(y)\n" +
+                "        }\n" +
+                "        function baz(x) {\n" +
+                "            expression: x + x\n" +
+                "        }\n" +
+                "    }\n" +
+                "}\n");
+        builder.build();
+    }
+
 }