Revert "Bratseth/typecheck all"

8 files changed, 38 insertions, 121 deletions

diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
index 67724058e64..eb0c6067fca 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/DeployState.java
@@ -333,7 +333,7 @@ public class DeployState implements ConfigDefinitionStore {
                     closeIgnoreException(reader.getReader());
                 }
             }
-            builder.build(logger);
+            builder.build(logger, queryProfiles);
             return SearchDocumentModel.fromBuilderAndNames(builder, names);
         }
 
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 c01b009e93b..dd03cb8b2a7 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/FeatureNames.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/FeatureNames.java
@@ -101,18 +101,6 @@ public class FeatureNames {
         return canonicalize("query(\"" + propertyName + "\")");
     }
 
-    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(");
-    }
-
     /**
      * Returns the single argument of the given feature name, without any quotes,
      * or empty if it is not a valid query, attribute or constant feature name
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
index d1a29271014..bcbc7cc99e2 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/RankProfile.java
@@ -2,9 +2,15 @@
 package com.yahoo.searchdefinition;
 
 import com.yahoo.config.application.api.ApplicationPackage;
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.io.reader.NamedReader;
+import com.yahoo.processing.request.CompoundName;
+import com.yahoo.search.query.profile.QueryProfile;
 import com.yahoo.search.query.profile.QueryProfileRegistry;
+import com.yahoo.search.query.profile.config.QueryProfileXMLReader;
 import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.QueryProfileType;
+import com.yahoo.search.query.profile.types.TensorFieldType;
 import com.yahoo.search.query.ranking.Diversity;
 import com.yahoo.searchdefinition.document.SDField;
 import com.yahoo.searchdefinition.expressiontransforms.RankProfileTransformContext;
@@ -13,6 +19,7 @@ import com.yahoo.searchlib.rankingexpression.ExpressionFunction;
 import com.yahoo.searchlib.rankingexpression.FeatureList;
 import com.yahoo.searchlib.rankingexpression.RankingExpression;
 import com.yahoo.searchlib.rankingexpression.evaluation.TensorValue;
+import com.yahoo.searchlib.rankingexpression.evaluation.TypeMapContext;
 import com.yahoo.searchlib.rankingexpression.evaluation.Value;
 import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode;
 import com.yahoo.tensor.TensorType;
@@ -578,11 +585,8 @@ public class RankProfile implements Serializable, Cloneable {
     }
 
     /**
-     * Will take the parser-set textual ranking expressions and turn into ranking expression objects,
-     * if not already done
+     * Will take the parser-set textual ranking expressions and turn into objects
      */
-    // TODO: There doesn't appear to be any good reason to defer parsing of ranking expressions
-    //       until this is called. Simplify by parsing them right away.
     public void parseExpressions() {
         try {
             parseRankingExpressions();
@@ -600,23 +604,20 @@ public class RankProfile implements Serializable, Cloneable {
         for (Map.Entry<String, Macro> e : getMacros().entrySet()) {
             String macroName = e.getKey();
             Macro macro = e.getValue();
-            if (macro.getRankingExpression() == null) {
-                RankingExpression expr = parseRankingExpression(macroName, macro.getTextualExpression());
-                macro.setRankingExpression(expr);
-                macro.setTextualExpression(expr.getRoot().toString());
-            }
+            RankingExpression expr = parseRankingExpression(macroName, macro.getTextualExpression());
+            macro.setRankingExpression(expr);
+            macro.setTextualExpression(expr.getRoot().toString());
         }
     }
 
     /**
      * Passes ranking expressions on to parser
-     *
      * @throws ParseException if either of the ranking expressions could not be parsed
      */
     private void parseRankingExpressions() throws ParseException {
-        if (getFirstPhaseRankingString() != null && firstPhaseRanking == null)
+        if (getFirstPhaseRankingString() != null)
             setFirstPhaseRanking(parseRankingExpression("firstphase", getFirstPhaseRankingString()));
-        if (getSecondPhaseRankingString() != null && secondPhaseRanking == null)
+        if (getSecondPhaseRankingString() != null)
             setSecondPhaseRanking(parseRankingExpression("secondphase", getSecondPhaseRankingString()));
     }
 
@@ -747,7 +748,7 @@ public class RankProfile implements Serializable, Cloneable {
      * referable from this rank profile.
      */
     public TypeContext typeContext(QueryProfileRegistry queryProfiles) {
-        MapTypeContext context = new MapTypeContext();
+        TypeMapContext context = new TypeMapContext();
 
         // Add small constants
         getConstants().forEach((k, v) -> context.setType(FeatureNames.asConstantFeature(k), v.type()));
@@ -763,8 +764,7 @@ public class RankProfile implements Serializable, Cloneable {
         for (QueryProfileType queryProfileType : queryProfiles.getTypeRegistry().allComponents()) {
             for (FieldDescription field : queryProfileType.declaredFields().values()) {
                 TensorType type = field.getType().asTensorType();
-                if ( ! FeatureNames.isQueryFeature(field.getName())) continue;
-                String feature = FeatureNames.canonicalize(field.getName());
+                String feature = FeatureNames.asQueryFeature(field.getName());
                 TensorType existingType = context.getType(feature);
                 if (existingType != null)
                     type = existingType.dimensionwiseGeneralizationWith(type).orElseThrow( () ->
@@ -910,7 +910,7 @@ public class RankProfile implements Serializable, Cloneable {
      */
     public static class Macro implements Serializable, Cloneable {
 
-        private final String name;
+        private String name=null;
         private String textualExpression=null;
         private RankingExpression expression=null;
         private List<String> formalParams = new ArrayList<>();
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
index 469f29098ad..762c0fec838 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java
@@ -34,6 +34,7 @@ import java.util.List;
  * expressions, using the setRankXXX() methods, 3) invoke the {@link #build()} method, and 4) retrieve the built
  * search objects using the {@link #getSearch(String)} method.
  */
+// TODO: This should be cleaned up and more or maybe completely taken over by MockApplicationPackage
 public class SearchBuilder {
 
     private final DocumentTypeManager docTypeMgr = new DocumentTypeManager();
@@ -195,7 +196,11 @@ public class SearchBuilder {
      * @throws IllegalStateException Thrown if this method has already been called.
      */
     public void build() {
-        build(new BaseDeployLogger());
+        build(new BaseDeployLogger(), new QueryProfiles());
+    }
+
+    public void build(DeployLogger logger) {
+        build(logger, new QueryProfiles());
     }
 
     /**
@@ -204,10 +209,12 @@ public class SearchBuilder {
      *
      * @throws IllegalStateException Thrown if this method has already been called.
      * @param deployLogger The logger to use during build
+     * @param queryProfiles The query profiles contained in the application this search is part of.
      */
-    public void build(DeployLogger deployLogger) {
-        if (isBuilt) throw new IllegalStateException("Model already built");
-
+    public void build(DeployLogger deployLogger, QueryProfiles queryProfiles) {
+        if (isBuilt) {
+            throw new IllegalStateException("Searches already built.");
+        }
         List<Search> built = new ArrayList<>();
         List<SDDocumentType> sdocs = new ArrayList<>();
         sdocs.add(SDDocumentType.VESPA_DOCUMENT);
@@ -233,7 +240,7 @@ public class SearchBuilder {
         for (Search search : new SearchOrderer().order(searchList)) {
             new FieldOperationApplierForSearch().process(search);
             // These two needed for a couple of old unit tests, ideally these are just read from app
-            process(search, deployLogger, new QueryProfiles(queryProfileRegistry));
+            process(search, deployLogger, queryProfiles);
             built.add(search);
         }
         builder.addToModel(searchList);
@@ -247,6 +254,8 @@ public class SearchBuilder {
     /**
      * Processes and returns the given {@link Search} object. This method has been factored out of the {@link
      * #build()} method so that subclasses can choose not to build anything.
+     *
+     * @param search The object to build.
      */
     protected void process(Search search, DeployLogger deployLogger, QueryProfiles queryProfiles) {
         Processing.process(search, deployLogger, rankProfileRegistry, queryProfiles);
@@ -343,7 +352,7 @@ public class SearchBuilder {
                                                   rankProfileRegistry,
                                                   queryprofileRegistry);
         builder.importFile(fileName);
-        builder.build(deployLogger);
+        builder.build(deployLogger, new QueryProfiles());
         return builder;
     }
 
@@ -359,7 +368,7 @@ public class SearchBuilder {
         for (Iterator<Path> i = Files.list(new File(dir).toPath()).filter(p -> p.getFileName().toString().endsWith(".sd")).iterator(); i.hasNext(); ) {
             builder.importFile(i.next());
         }
-        builder.build(new BaseDeployLogger());
+        builder.build(new BaseDeployLogger(), new QueryProfiles());
         return builder;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/MapTypeContext.java b/config-model/src/main/java/com/yahoo/searchdefinition/TypeMapContext.java
index e0dc7a2f33c..40e9db1413f 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/MapTypeContext.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/TypeMapContext.java
@@ -10,13 +10,10 @@ import java.util.Map;
 
 /**
  * A context which only contains type information.
- * This returns empty tensor types (double) for unknown features which are not
- * query, attribute or constant features, as we do not have information about which such
- * features exist (but we know those that exist are doubles).
  *
  * @author bratseth
  */
-public class MapTypeContext implements TypeContext {
+public class TypeMapContext implements TypeContext {
 
     private final Map<String, TensorType> featureTypes = new HashMap<>();
 
@@ -26,14 +23,10 @@ public class MapTypeContext implements TypeContext {
 
     @Override
     public TensorType getType(String name) {
-        if (FeatureNames.isConstantFeature(name) ||
-            FeatureNames.isAttributeFeature(name) ||
-            FeatureNames.isQueryFeature(name))
-            return featureTypes.get(FeatureNames.canonicalize(name));
-        else
-            return TensorType.empty; // we do not have type information for these. Correct would be either empty or null
+        return featureTypes.get(FeatureNames.canonicalize(name));
     }
 
+    /** Returns an unmodifiable map of the bindings in this */
     public Map<String, TensorType> bindings() { return Collections.unmodifiableMap(featureTypes); }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/IndexingValues.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/IndexingValues.java
index cc634abef01..ee65c9bec02 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/IndexingValues.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/IndexingValues.java
@@ -13,7 +13,7 @@ import com.yahoo.vespa.indexinglanguage.expressions.OutputExpression;
 import com.yahoo.vespa.model.container.search.QueryProfiles;
 
 /**
- * @author Simon Thoresen Hult
+ * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a>
  */
 public class IndexingValues extends Processor {
 
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java
index 061a803cb48..90183848094 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java
@@ -76,9 +76,8 @@ public class Processing {
                 ImportedFieldsInSummayValidator::new,
                 FastAccessValidator::new,
                 ReservedMacroNames::new,
-                RankingExpressionTypeValidator::new,
 
-                // These should be last.
+                // These two should be last.
                 IndexingValidation::new,
                 IndexingValues::new);
     }
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java
deleted file mode 100644
index a7a5ad58430..00000000000
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/RankingExpressionTypeValidator.java
+++ /dev/null
@@ -1,72 +0,0 @@
-// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-package com.yahoo.searchdefinition.processing;
-
-import com.yahoo.config.application.api.DeployLogger;
-import com.yahoo.search.query.profile.QueryProfileRegistry;
-import com.yahoo.searchdefinition.MapTypeContext;
-import com.yahoo.searchdefinition.RankProfile;
-import com.yahoo.searchdefinition.RankProfileRegistry;
-import com.yahoo.searchdefinition.Search;
-import com.yahoo.searchlib.rankingexpression.RankingExpression;
-import com.yahoo.tensor.TensorType;
-import com.yahoo.tensor.evaluation.TypeContext;
-import com.yahoo.vespa.model.container.search.QueryProfiles;
-
-public class RankingExpressionTypeValidator extends Processor {
-
-    private final QueryProfileRegistry queryProfiles;
-
-    public RankingExpressionTypeValidator(Search search,
-                                          DeployLogger deployLogger,
-                                          RankProfileRegistry rankProfileRegistry,
-                                          QueryProfiles queryProfiles) {
-        super(search, deployLogger, rankProfileRegistry, queryProfiles);
-        this.queryProfiles = queryProfiles.getRegistry();
-    }
-
-    @Override
-    public void process() {
-        for (RankProfile profile : rankProfileRegistry.allRankProfiles()) {
-            try {
-                validate(profile);
-            }
-            catch (IllegalArgumentException e) {
-                throw new IllegalArgumentException("In " + search + ", " + profile, e);
-            }
-        }
-    }
-
-    /** Throws an IllegalArgumentException if the given rank profile does not produce valid type */
-    private void validate(RankProfile profile) {
-        profile.parseExpressions();
-        TypeContext context = profile.typeContext(queryProfiles);
-        for (RankProfile.Macro macro : profile.getMacros().values())
-            ensureValid(macro.getRankingExpression(), "macro '" + macro.getName() + "'", context);
-        ensureValidDouble(profile.getFirstPhaseRanking(), "first-phase expression", context);
-        ensureValidDouble(profile.getSecondPhaseRanking(), "second-phase expression", context);
-    }
-
-    private TensorType ensureValid(RankingExpression expression, String expressionDescription, TypeContext context) {
-        if (expression == null) return null;
-
-        TensorType type;
-        try {
-            type = expression.type(context);
-        }
-        catch (IllegalArgumentException e) {
-            throw new IllegalArgumentException("The " + expressionDescription + " is invalid", e);
-        }
-        if (type == null) // Not expected to happen
-            throw new IllegalStateException("Could not determine the type produced by " + expressionDescription);
-        return type;
-    }
-
-    private void ensureValidDouble(RankingExpression expression, String expressionDescription, TypeContext context) {
-        if (expression == null) return;
-        TensorType type = ensureValid(expression, expressionDescription, context);
-        if ( ! type.equals(TensorType.empty))
-            throw new IllegalArgumentException("The " + expressionDescription + " must produce a double " +
-                                               "(a tensor with no dimensions), but produces " + type);
-    }
-
-}