Make tokenizer create a Problem token instead of throwing parse errors

This allows the parser to throw the error. The parser can add more
context such as the name of the key whose value is being parsed.

config/src/main/java/com/typesafe/config/impl/Parser.java

@@ -68,12 +68,25 @@ final class Parser {
                 t = buffer.pop();
             }
 
+            if (Tokens.isProblem(t)) {
+                ConfigOrigin origin = Tokens.getProblemOrigin(t);
+                String message = Tokens.getProblemMessage(t);
+                Throwable cause = Tokens.getProblemCause(t);
+                boolean suggestQuotes = Tokens.getProblemSuggestQuotes(t);
+                if (suggestQuotes) {
+                    message = addQuoteSuggestion(t.toString(), message);
+                } else {
+                    message = addKeyName(message);
+                }
+                throw new ConfigException.Parse(origin, message, cause);
+            }
+
             if (flavor == ConfigSyntax.JSON) {
                 if (Tokens.isUnquotedText(t)) {
-                    throw parseError("Token not allowed in valid JSON: '"
+                    throw parseError(addKeyName("Token not allowed in valid JSON: '"
-                        + Tokens.getUnquotedText(t) + "'");
+                            + Tokens.getUnquotedText(t) + "'"));
                 } else if (Tokens.isSubstitution(t)) {
-                    throw parseError("Substitutions (${} syntax) not allowed in JSON");
+                    throw parseError(addKeyName("Substitutions (${} syntax) not allowed in JSON"));
                 }
             }
 
@@ -239,19 +252,35 @@ final class Parser {
         }
 
 
+        private String previousFieldName(Path lastPath) {
+            if (lastPath != null) {
+                return lastPath.render();
+            } else if (pathStack.isEmpty())
+                return null;
+            else
+                return pathStack.peek().render();
+        }
+
+        private String previousFieldName() {
+            return previousFieldName(null);
+        }
+
+        private String addKeyName(String message) {
+            String previousFieldName = previousFieldName();
+            if (previousFieldName != null) {
+                return "in value for key '" + previousFieldName + "': " + message;
+            } else {
+                return message;
+            }
+        }
+
         private String addQuoteSuggestion(String badToken, String message) {
             return addQuoteSuggestion(null, equalsCount > 0, badToken, message);
         }
 
         private String addQuoteSuggestion(Path lastPath, boolean insideEquals, String badToken,
                 String message) {
-            String previousFieldName;
+            String previousFieldName = previousFieldName(lastPath);
-            if (lastPath != null) {
-                previousFieldName = lastPath.render();
-            } else if (pathStack.isEmpty())
-                previousFieldName = null;
-            else
-                previousFieldName = pathStack.peek().render();
 
             String part;
             if (badToken.equals(Tokens.END.toString())) {
@@ -269,8 +298,8 @@ final class Parser {
                             + "try enclosing the value in double quotes";
                 } else {
                     part = message + " (if you intended " + badToken
-                            + " to be part of the value for "
+                            + " to be part of a key or string value, "
-                            + "a setting, try enclosing the value in double quotes";
+                            + "try enclosing the key or value in double quotes";
                 }
             }
 
@@ -331,8 +360,8 @@ final class Parser {
                     String key = (String) Tokens.getValue(token).unwrapped();
                     return Path.newKey(key);
                 } else {
-                    throw parseError("Expecting close brace } or a field name here, got "
+                    throw parseError(addKeyName("Expecting close brace } or a field name here, got "
-                            + token);
+                            + token));
                 }
             } else {
                 List<Token> expression = new ArrayList<Token>();
@@ -343,7 +372,8 @@ final class Parser {
                 }
 
                 if (expression.isEmpty()) {
-                    throw parseError("expecting a close brace or a field name here, got " + t);
+                    throw parseError(addKeyName("expecting a close brace or a field name here, got "
+                            + t));
                 }
 
                 putBack(t); // put back the token we ended with
@@ -561,11 +591,11 @@ final class Parser {
             } else if (t == Tokens.OPEN_SQUARE) {
                 values.add(parseArray());
             } else {
-                throw parseError("List should have ] or a first element after the open [, instead had token: "
+                throw parseError(addKeyName("List should have ] or a first element after the open [, instead had token: "
                         + t
                         + " (if you want "
                         + t
-                        + " to be part of a string value, then double-quote it)");
+                        + " to be part of a string value, then double-quote it)"));
             }
 
             // now remaining elements
@@ -578,11 +608,11 @@ final class Parser {
                     if (t == Tokens.CLOSE_SQUARE) {
                         return new SimpleConfigList(arrayOrigin, values);
                     } else {
-                        throw parseError("List should have ended with ] or had a comma, instead had token: "
+                        throw parseError(addKeyName("List should have ended with ] or had a comma, instead had token: "
                                 + t
                                 + " (if you want "
                                 + t
-                                + " to be part of a string value, then double-quote it)");
+                                + " to be part of a string value, then double-quote it)"));
                     }
                 }
 
@@ -601,11 +631,11 @@ final class Parser {
                     // we allow one trailing comma
                     putBack(t);
                 } else {
-                    throw parseError("List should have had new element after a comma, instead had token: "
+                    throw parseError(addKeyName("List should have had new element after a comma, instead had token: "
                             + t
                             + " (if you want the comma or "
                             + t
-                            + " to be part of a string value, then double-quote it)");
+                            + " to be part of a string value, then double-quote it)"));
                 }
             }
         }

config/src/main/java/com/typesafe/config/impl/TokenType.java

@@ -17,5 +17,5 @@ enum TokenType {
     NEWLINE,
     UNQUOTED_TEXT,
     SUBSTITUTION,
-    RESERVED_CHAR;
+    PROBLEM;
 }

config/src/main/java/com/typesafe/config/impl/Tokenizer.java

@@ -16,6 +16,32 @@ import com.typesafe.config.ConfigOrigin;
 import com.typesafe.config.ConfigSyntax;
 
 final class Tokenizer {
+    // this exception should not leave this file
+    private static class ProblemException extends Exception {
+        private static final long serialVersionUID = 1L;
+
+        final private Token problem;
+
+        ProblemException(Token problem) {
+            this.problem = problem;
+        }
+
+        Token problem() {
+            return problem;
+        }
+    }
+
+    private static String asString(int codepoint) {
+        if (codepoint == '\n')
+            return "newline";
+        else if (codepoint == '\t')
+            return "tab";
+        else if (Character.isISOControl(codepoint))
+            return String.format("control character 0x%x", codepoint);
+        else
+            return String.format("%c", codepoint);
+    }
+
     /**
      * Tokenizes a Reader. Does not close the reader; you have to arrange to do
      * that after you're done with the returned iterator.
@@ -194,23 +220,44 @@ final class Tokenizer {
             }
         }
 
-        private ConfigException parseError(String message) {
+        private ProblemException problem(String message) {
-            return parseError(message, null);
+            return problem("", message, null);
         }
 
-        private ConfigException parseError(String message, Throwable cause) {
+        private ProblemException problem(String what, String message) {
-            return parseError(lineOrigin(), message, cause);
+            return problem(what, message, null);
         }
 
-        private static ConfigException parseError(ConfigOrigin origin,
+        private ProblemException problem(String what, String message, boolean suggestQuotes) {
+            return problem(what, message, suggestQuotes, null);
+        }
+
+        private ProblemException problem(String what, String message, Throwable cause) {
+            return problem(lineOrigin(), what, message, cause);
+        }
+
+        private ProblemException problem(String what, String message, boolean suggestQuotes,
+                Throwable cause) {
+            return problem(lineOrigin(), what, message, suggestQuotes, cause);
+        }
+
+        private static ProblemException problem(ConfigOrigin origin, String what,
                 String message,
                 Throwable cause) {
-            return new ConfigException.Parse(origin, message, cause);
+            return problem(origin, what, message, false, cause);
         }
 
-        private static ConfigException parseError(ConfigOrigin origin,
+        private static ProblemException problem(ConfigOrigin origin, String what, String message,
-                String message) {
+                boolean suggestQuotes, Throwable cause) {
-            return parseError(origin, message, null);
+            if (what == null || message == null)
+                throw new ConfigException.BugOrBroken(
+                        "internal error, creating bad ProblemException");
+            return new ProblemException(Tokens.newProblem(origin, what, message, suggestQuotes,
+                    cause));
+        }
+
+        private static ProblemException problem(ConfigOrigin origin, String message) {
+            return problem(origin, "", message, null);
         }
 
         private ConfigOrigin lineOrigin() {
@@ -273,7 +320,7 @@ final class Tokenizer {
             return Tokens.newUnquotedText(origin, s);
         }
 
-        private Token pullNumber(int firstChar) {
+        private Token pullNumber(int firstChar) throws ProblemException {
             StringBuilder sb = new StringBuilder();
             sb.appendCodePoint(firstChar);
             boolean containedDecimalOrE = false;
@@ -298,16 +345,14 @@ final class Tokenizer {
                     return Tokens.newLong(lineOrigin(), Long.parseLong(s), s);
                 }
             } catch (NumberFormatException e) {
-                throw parseError("Invalid number: '" + s
+                throw problem(s, "Invalid number: '" + s + "'", true /* suggestQuotes */, e);
-                                + "' (if this is in a path, try quoting it with double quotes)",
-                        e);
             }
         }
 
-        private void pullEscapeSequence(StringBuilder sb) {
+        private void pullEscapeSequence(StringBuilder sb) throws ProblemException {
             int escaped = nextCharRaw();
             if (escaped == -1)
-                throw parseError("End of input but backslash in string had nothing after it");
+                throw problem("End of input but backslash in string had nothing after it");
 
             switch (escaped) {
             case '"':
@@ -340,54 +385,43 @@ final class Tokenizer {
                 for (int i = 0; i < 4; ++i) {
                     int c = nextCharSkippingComments();
                     if (c == -1)
-                        throw parseError("End of input but expecting 4 hex digits for \\uXXXX escape");
+                        throw problem("End of input but expecting 4 hex digits for \\uXXXX escape");
                     a[i] = (char) c;
                 }
                 String digits = new String(a);
                 try {
                     sb.appendCodePoint(Integer.parseInt(digits, 16));
                 } catch (NumberFormatException e) {
-                    throw parseError(
+                    throw problem(digits, String.format(
-                            String.format(
+                            "Malformed hex digits after \\u escape in string: '%s'", digits), e);
-                                    "Malformed hex digits after \\u escape in string: '%s'",
-                                    digits), e);
                 }
             }
                 break;
             default:
-                throw parseError(String
+                throw problem(
-                        .format("backslash followed by '%c', this is not a valid escape sequence",
+                        asString(escaped),
-                                escaped));
+                        String.format(
+                                "backslash followed by '%s', this is not a valid escape sequence (quoted strings use JSON escaping, so use double-backslash \\\\ for literal backslash)",
+                                asString(escaped)));
             }
         }
 
-        private ConfigException controlCharacterError(int c) {
+        private Token pullQuotedString() throws ProblemException {
-            String asString;
-            if (c == '\n')
-                asString = "newline";
-            else if (c == '\t')
-                asString = "tab";
-            else
-                asString = String.format("control character 0x%x", c);
-            return parseError("JSON does not allow unescaped " + asString
-                    + " in quoted strings, use a backslash escape");
-        }
-
-        private Token pullQuotedString() {
             // the open quote has already been consumed
             StringBuilder sb = new StringBuilder();
             int c = '\0'; // value doesn't get used
             do {
                 c = nextCharRaw();
                 if (c == -1)
-                    throw parseError("End of input but string quote was still open");
+                    throw problem("End of input but string quote was still open");
 
                 if (c == '\\') {
                     pullEscapeSequence(sb);
                 } else if (c == '"') {
                     // end the loop, done!
                 } else if (Character.isISOControl(c)) {
-                    throw controlCharacterError(c);
+                    throw problem(asString(c), "JSON does not allow unescaped " + asString(c)
+                            + " in quoted strings, use a backslash escape");
                 } else {
                     sb.appendCodePoint(c);
                 }
@@ -395,12 +429,13 @@ final class Tokenizer {
             return Tokens.newString(lineOrigin(), sb.toString());
         }
 
-        private Token pullSubstitution() {
+        private Token pullSubstitution() throws ProblemException {
             // the initial '$' has already been consumed
             ConfigOrigin origin = lineOrigin();
             int c = nextCharSkippingComments();
             if (c != '{') {
-                throw parseError("'$' not followed by {");
+                throw problem(asString(c), "'$' not followed by {, '" + asString(c)
+                        + "' not allowed after '$'", true /* suggestQuotes */);
             }
 
             boolean optional = false;
@@ -425,7 +460,7 @@ final class Tokenizer {
                     // end the loop, done!
                     break;
                 } else if (t == Tokens.END) {
-                    throw parseError(origin,
+                    throw problem(origin,
                             "Substitution ${ was not closed with a }");
                 } else {
                     Token whitespace = saver.check(t, origin, lineNumber);
@@ -438,7 +473,7 @@ final class Tokenizer {
             return Tokens.newSubstitution(origin, optional, expression);
         }
 
-        private Token pullNextToken(WhitespaceSaver saver) {
+        private Token pullNextToken(WhitespaceSaver saver) throws ProblemException {
             int c = nextCharAfterWhitespace(saver);
             if (c == -1) {
                 return Tokens.END;
@@ -482,7 +517,8 @@ final class Tokenizer {
                     if (firstNumberChars.indexOf(c) >= 0) {
                         t = pullNumber(c);
                     } else if (notInUnquotedText.indexOf(c) >= 0) {
-                        t = Tokens.newReservedChar(lineOrigin(), c);
+                        throw problem(asString(c), "Reserved character '" + asString(c)
+                                + "' is not allowed outside quotes", true /* suggestQuotes */);
                     } else {
                         putBack(c);
                         t = pullUnquotedText();
@@ -506,7 +542,7 @@ final class Tokenizer {
             }
         }
 
-        private void queueNextToken() {
+        private void queueNextToken() throws ProblemException {
             Token t = pullNextToken(whitespaceSaver);
             Token whitespace = whitespaceSaver.check(t, origin, lineNumber);
             if (whitespace != null)
@@ -523,7 +559,11 @@ final class Tokenizer {
         public Token next() {
             Token t = tokens.remove();
             if (tokens.isEmpty() && t != Tokens.END) {
-                queueNextToken();
+                try {
+                    queueNextToken();
+                } catch (ProblemException e) {
+                    tokens.add(e.problem());
+                }
                 if (tokens.isEmpty())
                     throw new ConfigException.BugOrBroken(
                             "bug: tokens queue should not be empty here");

config/src/main/java/com/typesafe/config/impl/Tokens.java

@@ -9,6 +9,7 @@ import com.typesafe.config.ConfigException;
 import com.typesafe.config.ConfigOrigin;
 import com.typesafe.config.ConfigValueType;
 
+/* FIXME the way the subclasses of Token are private with static isFoo and accessors is kind of ridiculous. */
 final class Tokens {
     static private class Value extends Token {
 
@@ -119,42 +120,70 @@ final class Tokens {
         }
     }
 
-    static private class ReservedChar extends Token {
+    static private class Problem extends Token {
         final private ConfigOrigin origin;
-        final private int value;
+        final private String what;
+        final private String message;
+        final private boolean suggestQuotes;
+        final private Throwable cause;
 
-        ReservedChar(ConfigOrigin origin, int c) {
+        Problem(ConfigOrigin origin, String what, String message, boolean suggestQuotes,
-            super(TokenType.RESERVED_CHAR);
+                Throwable cause) {
+            super(TokenType.PROBLEM);
             this.origin = origin;
-            this.value = c;
+            this.what = what;
+            this.message = message;
+            this.suggestQuotes = suggestQuotes;
+            this.cause = cause;
         }
 
         ConfigOrigin origin() {
             return origin;
         }
 
+        String message() {
+            return message;
+        }
+
+        boolean suggestQuotes() {
+            return suggestQuotes;
+        }
+
+        Throwable cause() {
+            return cause;
+        }
+
         @Override
         public String toString() {
             StringBuilder sb = new StringBuilder();
             sb.append('\'');
-            sb.appendCodePoint(value);
+            sb.append(what);
             sb.append('\'');
             return sb.toString();
         }
 
         @Override
         protected boolean canEqual(Object other) {
-            return other instanceof ReservedChar;
+            return other instanceof Problem;
         }
 
         @Override
         public boolean equals(Object other) {
-            return super.equals(other) && ((ReservedChar) other).value == value;
+            return super.equals(other) && ((Problem) other).what.equals(what)
+                    && ((Problem) other).message.equals(message)
+                    && ((Problem) other).suggestQuotes == suggestQuotes
+                    && ConfigUtil.equalsHandlingNull(((Problem) other).cause, cause);
         }
 
         @Override
         public int hashCode() {
-            return 41 * (41 + super.hashCode()) + value;
+            int h = 41 * (41 + super.hashCode());
+            h = 41 * (h + what.hashCode());
+            h = 41 * (h + message.hashCode());
+            h = 41 * (h + Boolean.valueOf(suggestQuotes).hashCode());
+            if (cause != null)
+                h = 41 * (h + cause.hashCode());
+            return h;
         }
     }
 
@@ -239,15 +268,40 @@ final class Tokens {
         }
     }
 
-    static boolean isReservedChar(Token token) {
+    static boolean isProblem(Token token) {
-        return token instanceof ReservedChar;
+        return token instanceof Problem;
     }
 
-    static ConfigOrigin getReservedCharOrigin(Token token) {
+    static ConfigOrigin getProblemOrigin(Token token) {
-        if (token instanceof ReservedChar) {
+        if (token instanceof Problem) {
-            return ((ReservedChar) token).origin();
+            return ((Problem) token).origin();
         } else {
-            throw new ConfigException.BugOrBroken("tried to get reserved char origin from " + token);
+            throw new ConfigException.BugOrBroken("tried to get problem origin from " + token);
+        }
+    }
+
+    static String getProblemMessage(Token token) {
+        if (token instanceof Problem) {
+            return ((Problem) token).message();
+        } else {
+            throw new ConfigException.BugOrBroken("tried to get problem message from " + token);
+        }
+    }
+
+    static boolean getProblemSuggestQuotes(Token token) {
+        if (token instanceof Problem) {
+            return ((Problem) token).suggestQuotes();
+        } else {
+            throw new ConfigException.BugOrBroken("tried to get problem suggestQuotes from "
+                    + token);
+        }
+    }
+
+    static Throwable getProblemCause(Token token) {
+        if (token instanceof Problem) {
+            return ((Problem) token).cause();
+        } else {
+            throw new ConfigException.BugOrBroken("tried to get problem cause from " + token);
         }
     }
 
@@ -318,8 +372,9 @@ final class Tokens {
         return new Line(lineNumberJustEnded);
     }
 
-    static Token newReservedChar(ConfigOrigin origin, int codepoint) {
+    static Token newProblem(ConfigOrigin origin, String what, String message,
-        return new ReservedChar(origin, codepoint);
+            boolean suggestQuotes, Throwable cause) {
+        return new Problem(origin, what, message, suggestQuotes, cause);
     }
 
     static Token newUnquotedText(ConfigOrigin origin, String s) {

config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala

@@ -3,7 +3,7 @@
  */
 package com.typesafe.config.impl
 
-import org.junit.Assert.assertEquals
+import org.junit.Assert._
 import org.junit.Test
 import com.typesafe.config.ConfigException
 
@@ -158,7 +158,7 @@ class TokenizerTest extends TestUtils {
     }
 
     @Test
-    def tokenizerThrowsOnInvalidStrings(): Unit = {
+    def tokenizerReturnsProblemOnInvalidStrings(): Unit = {
         val invalidTests = List(""" "\" """, // nothing after a backslash
             """ "\q" """, // there is no \q escape sequence
             "\"\\u123\"", // too short
@@ -166,15 +166,14 @@ class TokenizerTest extends TestUtils {
             "\"\\u1\"", // too short
             "\"\\u\"", // too short
             "\"", // just a single quote
-            """ "abcdefg""" // no end quote
+            """ "abcdefg""", // no end quote
+            """\"\""" // file ends with a backslash
             )
 
         for (t <- invalidTests) {
-            describeFailure(t) {
+            val tokenized = tokenizeAsList(t)
-                intercept[ConfigException] {
+            val maybeProblem = tokenized.find(Tokens.isProblem(_))
-                    tokenizeAsList(t)
+            assertTrue(maybeProblem.isDefined)
-                }
-            }
         }
     }
 
@@ -217,10 +216,14 @@ class TokenizerTest extends TestUtils {
 
     @Test
     def tokenizeReservedChars() {
-        val tokenized = tokenizeAsList("+`^?!@*&\\")
+        for (invalid <- "+`^?!@*&\\") {
-        assertEquals(Seq(Tokens.START, tokenReserved('+'), tokenReserved('`'),
+            val tokenized = tokenizeAsList(invalid.toString)
-            tokenReserved('^'), tokenReserved('?'), tokenReserved('!'), tokenReserved('@'),
+            assertEquals(3, tokenized.size)
-            tokenReserved('*'), tokenReserved('&'), tokenReserved('\\'),
+            assertEquals(Tokens.START, tokenized(0))
-            Tokens.END), tokenized)
+            assertEquals(Tokens.END, tokenized(2))
+            val problem = tokenized(1)
+            assertTrue("reserved char is a problem", Tokens.isProblem(problem))
+            assertEquals("'" + invalid + "'", problem.toString())
+        }
     }
 }