diff --git a/config/src/main/java/com/typesafe/config/impl/Parser.java b/config/src/main/java/com/typesafe/config/impl/Parser.java index ca6939b4..ad26a4e0 100644 --- a/config/src/main/java/com/typesafe/config/impl/Parser.java +++ b/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: '" - + Tokens.getUnquotedText(t) + "'"); + throw parseError(addKeyName("Token not allowed in valid JSON: '" + + 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; - if (lastPath != null) { - previousFieldName = lastPath.render(); - } else if (pathStack.isEmpty()) - previousFieldName = null; - else - previousFieldName = pathStack.peek().render(); + String previousFieldName = previousFieldName(lastPath); 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 " - + "a setting, try enclosing the value in double quotes"; + + " to be part of a key or string value, " + + "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 " - + token); + throw parseError(addKeyName("Expecting close brace } or a field name here, got " + + token)); } } else { List expression = new ArrayList(); @@ -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)")); } } } diff --git a/config/src/main/java/com/typesafe/config/impl/TokenType.java b/config/src/main/java/com/typesafe/config/impl/TokenType.java index 9ee552ed..ace12fa7 100644 --- a/config/src/main/java/com/typesafe/config/impl/TokenType.java +++ b/config/src/main/java/com/typesafe/config/impl/TokenType.java @@ -17,5 +17,5 @@ enum TokenType { NEWLINE, UNQUOTED_TEXT, SUBSTITUTION, - RESERVED_CHAR; + PROBLEM; } diff --git a/config/src/main/java/com/typesafe/config/impl/Tokenizer.java b/config/src/main/java/com/typesafe/config/impl/Tokenizer.java index 013c844b..b891b5e6 100644 --- a/config/src/main/java/com/typesafe/config/impl/Tokenizer.java +++ b/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) { - return parseError(message, null); + private ProblemException problem(String message) { + return problem("", message, null); } - private ConfigException parseError(String message, Throwable cause) { - return parseError(lineOrigin(), message, cause); + private ProblemException problem(String what, String message) { + 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, - String message) { - return parseError(origin, message, null); + private static ProblemException problem(ConfigOrigin origin, String what, String message, + boolean suggestQuotes, Throwable cause) { + 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 - + "' (if this is in a path, try quoting it with double quotes)", - e); + throw problem(s, "Invalid number: '" + s + "'", true /* suggestQuotes */, 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( - String.format( - "Malformed hex digits after \\u escape in string: '%s'", - digits), e); + throw problem(digits, String.format( + "Malformed hex digits after \\u escape in string: '%s'", digits), e); } } break; default: - throw parseError(String - .format("backslash followed by '%c', this is not a valid escape sequence", - escaped)); + throw problem( + asString(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) { - 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() { + private Token pullQuotedString() throws ProblemException { // 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"); diff --git a/config/src/main/java/com/typesafe/config/impl/Tokens.java b/config/src/main/java/com/typesafe/config/impl/Tokens.java index 2911dbc9..3a68eb44 100644 --- a/config/src/main/java/com/typesafe/config/impl/Tokens.java +++ b/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) { - super(TokenType.RESERVED_CHAR); + Problem(ConfigOrigin origin, String what, String message, boolean suggestQuotes, + 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) { - return token instanceof ReservedChar; + static boolean isProblem(Token token) { + return token instanceof Problem; } - static ConfigOrigin getReservedCharOrigin(Token token) { - if (token instanceof ReservedChar) { - return ((ReservedChar) token).origin(); + static ConfigOrigin getProblemOrigin(Token token) { + if (token instanceof Problem) { + 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) { - return new ReservedChar(origin, codepoint); + static Token newProblem(ConfigOrigin origin, String what, String message, + boolean suggestQuotes, Throwable cause) { + return new Problem(origin, what, message, suggestQuotes, cause); } static Token newUnquotedText(ConfigOrigin origin, String s) { diff --git a/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala b/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala index 70af96bf..24be722e 100644 --- a/config/src/test/scala/com/typesafe/config/impl/TokenizerTest.scala +++ b/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) { - intercept[ConfigException] { - tokenizeAsList(t) - } - } + val tokenized = tokenizeAsList(t) + val maybeProblem = tokenized.find(Tokens.isProblem(_)) + assertTrue(maybeProblem.isDefined) } } @@ -217,10 +216,14 @@ class TokenizerTest extends TestUtils { @Test def tokenizeReservedChars() { - val tokenized = tokenizeAsList("+`^?!@*&\\") - assertEquals(Seq(Tokens.START, tokenReserved('+'), tokenReserved('`'), - tokenReserved('^'), tokenReserved('?'), tokenReserved('!'), tokenReserved('@'), - tokenReserved('*'), tokenReserved('&'), tokenReserved('\\'), - Tokens.END), tokenized) + for (invalid <- "+`^?!@*&\\") { + val tokenized = tokenizeAsList(invalid.toString) + assertEquals(3, tokenized.size) + assertEquals(Tokens.START, tokenized(0)) + assertEquals(Tokens.END, tokenized(2)) + val problem = tokenized(1) + assertTrue("reserved char is a problem", Tokens.isProblem(problem)) + assertEquals("'" + invalid + "'", problem.toString()) + } } }