Disallow dash in search definition, document, field and function names

7 files changed, 59 insertions, 66 deletions

diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java
index 5ec30f71e7b..640e5156615 100644
--- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java
+++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java
@@ -619,6 +619,7 @@ public class FilesApplicationPackage implements ApplicationPackage {
     @Override
     public Reader getRankingExpression(String name) {
         try {
+            File file = expressionFileNameToFile(name);
             return IOUtils.createReader(expressionFileNameToFile(name), "utf-8");
         }
         catch (IOException e) {
diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj
index e50cbabeb9f..cc365ecb655 100644
--- a/config-model/src/main/javacc/SDParser.jj
+++ b/config-model/src/main/javacc/SDParser.jj
@@ -360,7 +360,8 @@ TOKEN :
 | < CONSTANTS: "constants" >
 | < FILE: "file" >
 | < URI: "uri" >
-| < IDENTIFIER: ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_","-"])* >
+| < IDENTIFIER:           ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_"])* >
+| < IDENTIFIER_WITH_DASH: ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_","-"])* >
 | < QUOTEDSTRING: "\"" ( ~["\""] )* "\"" >
 | < CONTEXT: ["a"-"z","A"-"Z"] (["a"-"z", "A"-"Z", "0"-"9"])* >
 | < DOUBLE: ("-")? (["0"-"9"])+ "." (["0"-"9"])+ >
@@ -672,14 +673,15 @@ void fieldSetItem(String setName, Search search) :
   ( <FIELDS><COLON> field=identifier() { search.fieldSets().addUserFieldSetItem(setName, field); }
     ( <COMMA> field=identifier() { search.fieldSets().addUserFieldSetItem(setName, field); } )* )
   |
-    ( <QUERYCOMMAND> <COLON> (queryCommand = identifier() | queryCommand = quotedString())) { search.fieldSets().userFieldSets().get(setName).queryCommands().add(queryCommand);}
+    ( <QUERYCOMMAND> <COLON> (queryCommand = identifierWithDash() | queryCommand = quotedString())) { search.fieldSets().userFieldSets().get(setName).queryCommands().add(queryCommand);}
   |
     ( match(matchSettings) ) { matchSettings.applyOperations(); search.fieldSets().userFieldSets().get(setName).setMatching(matchSettings.getMatching());}
 }
 
 /**
  * This rule consumes a annotation block from within either a document element or a search element.
- * @param search The search object to add content to.
+
+ * @param search the search object to add content to.
  */
 void annotationOutside(Search search) :
 {
@@ -701,6 +703,7 @@ void annotationOutside(Search search) :
 
 /**
  * This rule consumes a annotation block from within either a document element.
+ *
  * @param document The document object to add content to.
  */
 void annotation(Search search, SDDocumentType document) :
@@ -1186,7 +1189,7 @@ Object sortingSetting(SortingOperation sorting, String attributeName) :
                              | <QUATERNARY> { sorting.setStrength(Sorting.Strength.QUATERNARY); }
                              | <IDENTICAL>  { sorting.setStrength(Sorting.Strength.IDENTICAL); }
                            )
-      | <LOCALE> <COLON> locale = identifier() { sorting.setLocale(locale); }
+      | <LOCALE> <COLON> locale = identifierWithDash() { sorting.setLocale(locale); }
     )
     { return null; }
 }
@@ -1438,7 +1441,7 @@ void summaryProperty(SummaryInFieldLongOperation field) :
     String name, value;
 }
 {
-    name = identifier() <COLON> (value = identifier() | value = quotedString())
+    name = identifierWithDash() <COLON> (value = identifierWithDash() | value = quotedString())
     { field.addProperty(new SummaryField.Property(name, value)); }
 }
 
@@ -1453,7 +1456,7 @@ void fieldStemming(FieldOperationContainer field) :
     StemmingOperation op = new StemmingOperation();
 }
 {
-    <STEMMING> <COLON> setting = identifier()
+    <STEMMING> <COLON> setting = identifierWithDash()
     {
         op.setSetting(setting);
         field.addOperation(op);
@@ -1470,7 +1473,7 @@ void searchStemming(Search search) :
     String setting;
 }
 {
-    <STEMMING> <COLON> setting = identifier()
+    <STEMMING> <COLON> setting = identifierWithDash()
     { search.setStemming(Stemming.get(setting)); }
 }
 
@@ -1485,7 +1488,7 @@ void normalizing(FieldOperationContainer field) :
     String setting;
 }
 {
-    <NORMALIZING> <COLON> setting = identifier()
+    <NORMALIZING> <COLON> setting = identifierWithDash()
     {
         field.addOperation(new NormalizingOperation(setting));
     }
@@ -1543,7 +1546,7 @@ void queryCommand(FieldOperationContainer container) :
     QueryCommandOperation field = new QueryCommandOperation();
 }
 {
-    <QUERYCOMMAND> <COLON> command = identifier()
+    <QUERYCOMMAND> <COLON> command = identifierWithDash()
     {
         field.addQueryCommand(command);
         container.addOperation(field);
@@ -1556,7 +1559,7 @@ void alias(FieldOperationContainer container) :
     String alias;
 }
 {
-    <ALIAS> [aliasedName = identifier()] <COLON> alias = identifier()
+    <ALIAS> [aliasedName = identifier()] <COLON> alias = identifierWithDash()
     {
        AliasOperation op = new AliasOperation(aliasedName, alias);
        container.addOperation(op);
@@ -1790,9 +1793,9 @@ Object indexBody(IndexOperation index) :
     double threshold;
 }
 {
-    ( <PREFIX>                                   { index.setPrefix(true); }
-      | <ALIAS> <COLON> str = identifier()       { index.addAlias(str); }
-      | <STEMMING> <COLON> str = identifier()    { index.setStemming(str); }
+    ( <PREFIX>                                           { index.setPrefix(true); }
+      | <ALIAS> <COLON> str = identifierWithDash()       { index.addAlias(str); }
+      | <STEMMING> <COLON> str = identifierWithDash()    { index.setStemming(str); }
       | <RISE> {
               if (true) throw new ParseException("'index:rise' is no longer an option. Use 'indexing:attribute' instead. " +
                                        "If it is a weighted set field you should also add 'attribute:fast-search'." +
@@ -1877,7 +1880,7 @@ void rankProfile(Search search) :
     RankProfile profile;
 }
 {
-    ( <RANKPROFILE> name = identifier()
+    ( <RANKPROFILE> name = identifierWithDash()
         {
             if (documentsOnly) {
                 profile = new DocumentsOnlyRankProfile(name, search, rankProfileRegistry);
@@ -1934,7 +1937,7 @@ void inheritsRankProfile(RankProfile profile) :
     String str;
 }
 {
-    <INHERITS> str = identifier()
+    <INHERITS> str = identifierWithDash()
     { profile.setInherited(str); }
 }
 
@@ -2233,9 +2236,9 @@ String rankPropertyItem() :
     String image, ret = "";
 }
 {
-    ( ( image = identifier()              { ret += image; }
-        | image = quotedString()          { ret += image; }
-        | ( "(" | ")" | <DOT> | <COMMA> ) { ret += token.image; } )+ )
+    ( ( image = identifierWithDash()              { ret += image; }
+        | image = quotedString()                  { ret += image; }
+        | ( "(" | ")" | <DOT> | <COMMA> )         { ret += token.image; } )+ )
     { return ret; }
 }
 
@@ -2476,6 +2479,16 @@ String expression() :
     { return exp; }
 }
 
+String identifierWithDash() :
+{
+    String identifier;
+}
+{
+    ( identifier = identifier() { return identifier; } )
+    |
+    ( <IDENTIFIER_WITH_DASH> { return token.image; } )
+}
+
 /**
  * This rule consumes an identifier. This must be kept in sync with all word tokens that should be parseable as
  * identifiers.
diff --git a/config-model/src/test/examples/invalid-name.sd b/config-model/src/test/examples/invalid-name.sd
new file mode 100644
index 00000000000..f26fcc723f4
--- /dev/null
+++ b/config-model/src/test/examples/invalid-name.sd
@@ -0,0 +1,12 @@
+# Dashes in names are not allowed
+search invalid-name {
+
+  document invalid-name {
+
+      field title type string {
+
+      }
+
+  }
+
+}
\ No newline at end of file
diff --git a/config-model/src/test/examples/simple-with-weird-name.sd b/config-model/src/test/examples/simple-with-weird-name.sd
deleted file mode 100644
index 109f4f7bba7..00000000000
--- a/config-model/src/test/examples/simple-with-weird-name.sd
+++ /dev/null
@@ -1,13 +0,0 @@
-# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-# A minimal doc with name which is incompatible with YQL+
-search simple-with-weird-name {
-
-  document simple-with-weird-name {
-
-    field title type string {
-      indexing: summary | index
-    }
-
-  }
-
-}
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java
index 278471cb37a..a26154fc8da 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java
@@ -73,35 +73,16 @@ public class SearchDefinitionsParsingTestCase extends SearchDefinitionTestCase {
         }
     }
 
-    private static class WarningCatcher extends Handler {
-        volatile boolean gotYqlWarning = false;
-
-        @Override
-        public void publish(LogRecord record) {
-            if (record.getLevel() == Level.WARNING && record.getMessage().indexOf("YQL") >= 0) {
-                gotYqlWarning = true;
+    @Test
+    public void illegalSearchDefinitionName() throws IOException, ParseException {
+        try {
+            SearchBuilder.buildFromFile("src/test/examples/invalid-name.sd");
+            fail("Name with dash passed");
+        } catch (ParseException e) {
+            if ( ! e.getMessage().contains("invalid-name")) {
+                throw e;
             }
         }
-
-        @Override
-        public void flush() {
-            // intentionally left blank
-        }
-
-        @Override
-        public void close() throws SecurityException {
-            // intentionally left blank
-        }
     }
 
-
-    @Test
-    public void requireYqlCompatibilityIsTested() throws Exception {
-        Logger log = Logger.getLogger("DeployLogger");
-        WarningCatcher w = new WarningCatcher();
-        log.addHandler(w);
-        assertNotNull(SearchBuilder.buildFromFile("src/test/examples/simple-with-weird-name.sd"));
-        log.removeHandler(w);
-        assertTrue(w.gotYqlWarning);
-    }
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java
index 30f1df6a394..4180f9f6de4 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java
@@ -26,7 +26,6 @@ import static org.junit.Assert.*;
 
 /**
  * @author gjoranv
- * @since 5.1.10
  */
 public class SearchBuilderTest extends ContainerModelBuilderTestBase {
 
@@ -35,7 +34,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase {
     }
 
     @Test
-    public void gui_search_handler_is_always_included_when_search_is_specified() throws Exception{
+    public void gui_search_handler_is_always_included_when_search_is_specified() {
         Element clusterElem = DomBuilderTest.parse(
                 "<jdisc id='default' version='1.0'>",
                 "  <search />",
@@ -61,7 +60,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase {
 
 
     @Test
-    public void search_handler_bindings_can_be_overridden() throws Exception {
+    public void search_handler_bindings_can_be_overridden() {
         Element clusterElem = DomBuilderTest.parse(
                 "<jdisc id='default' version='1.0'>",
                 "  <search>",
@@ -80,7 +79,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase {
     }
 
     @Test
-    public void search_handler_bindings_can_be_disabled() throws Exception {
+    public void search_handler_bindings_can_be_disabled() {
         Element clusterElem = DomBuilderTest.parse(
                 "<jdisc id='default' version='1.0'>",
                 "  <search>",
@@ -111,7 +110,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase {
         assertThat(chainsConfig().chains(), hasItemWithMethod("vespa", "id"));
     }
 
-    private void createClusterWithOnlyDefaultChains() throws SAXException, IOException {
+    private void createClusterWithOnlyDefaultChains() {
         Element containerElem = DomBuilderTest.parse(
                 "<jdisc id='default' version='1.0'>",
                 "  <search/>",
@@ -124,7 +123,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase {
     }
 
     @Test
-    public void manually_setting_up_search_handler_is_forbidden() throws IOException, SAXException {
+    public void manually_setting_up_search_handler_is_forbidden() {
         try {
             Element clusterElem = DomBuilderTest.parse(
                     "<jdisc id='default' version='1.0'>",
@@ -193,7 +192,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase {
     }
 
 
-    private VespaModel getVespaModelWithMusic(String hosts, String services) throws ParseException {
+    private VespaModel getVespaModelWithMusic(String hosts, String services) {
         return new VespaModelCreatorWithMockPkg(hosts, services, ApplicationPackageUtils.generateSearchDefinitions("music")).create();
     }
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java
index 0ad5fb3b0bd..66f59717407 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java
@@ -47,7 +47,7 @@ public class ReservedDocumentTypeNameValidatorTest {
     public void validation_is_case_insensitive() {
         expectedException.expect(IllegalArgumentException.class);
         expectedException.expectMessage("The following document types conflict with reserved keyword names: " +
-                "'NULL', 'True', 'anD'.");
+                                        "'NULL', 'True', 'anD'.");
 
         ReservedDocumentTypeNameValidator validator = new ReservedDocumentTypeNameValidator();
         Map<String, NewDocumentType> orderedDocTypes = new TreeMap<>(asDocTypeMapping(Arrays.asList("NULL", "True", "anD")));