Improve template parse error messages (#796)

* Add hints to template parse errors

* Improve other parse errors

* Update CHANGELOG

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - Update to whelk 1.0.4
 - Run [`query`] on existing TDB dataset (instead of ontology input) in [#792]
+- Improved error messages for [`template`] parse errors in [#796]
 
 ### Fixed
 - Fix blank node subjects in [`report`] in [#767]
@@ -221,6 +222,7 @@ First official release of ROBOT!
 [`template`]: http://robot.obolibrary.org/template
 [`validate`]: http://robot.obolibrary.org/validate
 
+[#796]: https://github.com/ontodev/robot/pull/796
 [#792]: https://github.com/ontodev/robot/pull/792
 [#783]: https://github.com/ontodev/robot/pull/783
 [#767]: https://github.com/ontodev/robot/pull/767

docs/template.md

@@ -310,6 +310,10 @@ AL rdfs:label@en
 
 The provided value cannot be parsed and may not be in proper Manchester syntax. See [Manchester Syntax](http://www.w3.org/2007/OWL/wiki/ManchesterSyntax) for more details. If you are using labels, make sure the labels are defined in the `--input` ontology or using the `LABEL` column. Also ensure that all properties use a label instead of a CURIE or IRI.
 
+When using a restriction (`some`, `only`, `min`, `max`, `exactly`, or `value`) the term that preceeds the restriction must be a property. 
+
+Terms joined using `and` or `or` must be of the same entity type, e.g., you cannot join an object property and a class in an expression.
+
 ### Merge Error
 
 `--merge-before` and `--merge-after` cannot be used simultaneously.

robot-command/src/main/java/org/obolibrary/robot/QueryCommand.java

@@ -247,7 +247,7 @@ private static void executeInMemory(
    * @throws IOException on problem running queries
    */
   private static void executeOnDisk(CommandLine line, List<List<String>> queries)
-    throws IOException, MissingArgumentException {
+      throws IOException, MissingArgumentException {
     Dataset dataset = createTDBDataset(line);
     boolean keepMappings = CommandLineHelper.getBooleanValue(line, "keep-tdb-mappings", false);
     String tdbDir = CommandLineHelper.getDefaultValue(line, "tdb-directory", ".tdb");

robot-core/src/main/java/org/obolibrary/robot/TemplateHelper.java

@@ -1,5 +1,6 @@
 package org.obolibrary.robot;
 
+import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.opencsv.CSVParserBuilder;
 import com.opencsv.CSVReader;
@@ -19,6 +20,7 @@
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 import org.apache.commons.io.FilenameUtils;
 import org.obolibrary.robot.exceptions.ColumnException;
 import org.obolibrary.robot.exceptions.RowParseException;
@@ -60,7 +62,7 @@ public class TemplateHelper {
   /** Error message when a given class expression is not able to be parsed. */
   protected static final String manchesterParseError =
       NS
-          + "MANCHESTER PARSE ERROR the expression '%s' at row %d, column %d in table \"%s\" cannot be parsed: %s";
+          + "MANCHESTER PARSE ERROR the expression (%s) at row %d, column %d in table \"%s\" cannot be parsed: %s";
 
   /** Error message when the CLASS_TYPE is not subclass or equivalent. */
   private static final String classTypeError =
@@ -932,6 +934,7 @@ protected static OWLClassExpression tryParse(
       expr = parser.parse(content);
     } catch (OWLParserException e) {
       String cause = getManchesterErrorCause(e);
+
       throw new RowParseException(
           String.format(manchesterParseError, content, rowNum, column + 1, tableName, cause),
           rowNum,
@@ -943,22 +946,51 @@ protected static OWLClassExpression tryParse(
 
   /**
    * Given an OWLParserException, determine if we can identify the offending term. Return that as
-   * the cause.
+   * the cause along with some explanation of what is expected.
    *
    * @param e exception to get cause of
    * @return String cause of exception
    */
   private static String getManchesterErrorCause(OWLParserException e) {
     String cause = e.getMessage();
-    String pattern = ".*Encountered ([^ ]*|'.*') at line.*";
-    Pattern p = Pattern.compile(pattern);
+    String pattern = ".*Encountered ([^ ]*|'.*') at .+. Expected one of:\n([\\s\\S]+)";
+    Pattern p = Pattern.compile(pattern, Pattern.MULTILINE);
     Matcher m = p.matcher(e.getMessage());
     if (m.find()) {
-      if (m.group(1).startsWith("'")) {
-        return "encountered unknown " + m.group(1);
-      } else {
-        return String.format("encountered unknown '%s'", m.group(1));
+      String keyword = m.group(1);
+      if (keyword.startsWith("'")) {
+        keyword = keyword.substring(1, keyword.length() - 1);
+      }
+
+      // Override expected message for restrictions (we want to reference the term before the
+      // restriction)
+      if (Arrays.asList("some", "only", "min", "max", "exactly", "value").contains(keyword)) {
+        return String.format(
+            "encountered unexpected '%1$s'\n\thint: the term before '%1$s' must be a property",
+            keyword);
       }
+
+      // Otherwise print what was expected by the parser in that place
+      // Some of these have hard to understand "expected" values, so we rewrite them the best we can
+      String expected = m.group(2);
+      List<String> expectedSplit =
+          Lists.newArrayList(expected.split("\n"))
+              .stream()
+              .map(String::trim)
+              .filter(x -> !x.equals(""))
+              .collect(Collectors.toList());
+      if (expectedSplit.contains("{")) {
+        expectedSplit.add(expectedSplit.indexOf("{"), "{ ... }");
+        expectedSplit.remove("{");
+      }
+      if (expectedSplit.contains("(")) {
+        expectedSplit.add(expectedSplit.indexOf("("), "( ... )");
+        expectedSplit.remove("(");
+      }
+      expectedSplit.remove("|EOF|");
+
+      return String.format("encountered '%s', but expected one of:\n\t", keyword)
+          + String.join("\n\t", expectedSplit);
     }
     return cause;
   }