Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ParsingException by including cursor location when possible #166

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ plugins {
// Use the build script defined in buildSrc

dependencies {
compileOnly("org.jetbrains:annotations:+")
testImplementation(project(":test-shared"))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import com.electronwill.nightconfig.core.utils.IntDeque;

import java.util.concurrent.atomic.AtomicLong;

/**
* Abstract base class for CharacterInputs.
*
Expand All @@ -12,6 +14,8 @@ public abstract class AbstractInput implements CharacterInput {
* Contains the peeked characters that haven't been read (by the read methods) yet.
*/
protected final IntDeque deque = new IntDeque();
private final AtomicLong line = new AtomicLong();
private final AtomicLong column = new AtomicLong();

/**
* Tries to parse the next character without taking care of the peek deque.
Expand All @@ -29,24 +33,59 @@ public abstract class AbstractInput implements CharacterInput {
*/
protected abstract char directReadChar();

protected void advance(int r) {
if (r == '\n')
advanceLine();
else if (r == '\r')
column.set(0);
else advanceColumn();
}

protected void advanceLine() {
line.incrementAndGet();
column.set(0);
}

protected void advanceColumn() {
column.incrementAndGet();
}

@Override
public long line() {
return line.get();
}

@Override
public long column() {
return column.get();
}

@Override
public int read() {
int r;
if (!deque.isEmpty()) {
return deque.removeFirst();
r = deque.removeFirst();
} else {
r = directRead();
}
return directRead();
advance(r);
return r;
}

@Override
public char readChar() {
char r;
if (!deque.isEmpty()) {
int next = deque.removeFirst();
if (next == -1) {
throw ParsingException.notEnoughData();
}
return (char)next;
r = (char)next;
} else {
r = directReadChar();
}
return directReadChar();
advance(r);
return r;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
*
* @author TheElectronWill
*/
public interface CharacterInput {
public interface CharacterInput extends Cursor {
/**
* Reads the next character.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,14 @@ protected CoderResult decodeLoop(ByteBuffer in, CharBuffer out) {
// detect UTF-16 BE and LE BOMs: wrong encoding!
if (b1 == 0xFE && b2 == 0xFF) {
if (utf8Only) {
throw new ParsingException(
throw new ParsingException(null,
"Invalid input: it begins with an UTF-16 BE byte-order mark, but it should be plain UTF-8.");
}
setupDecoder(StandardCharsets.UTF_16BE);
newPosition += 2;
} else if (b1 == 0xFF && b2 == 0xFE) {
if (utf8Only) {
throw new ParsingException(
throw new ParsingException(null,
"Invalid input: it begins with an UTF-16 LE byte-order mark, but it should be plain UTF-8.");
}
setupDecoder(StandardCharsets.UTF_16LE);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package com.electronwill.nightconfig.core.io;

public interface Cursor {
long line();

long column();
}
Original file line number Diff line number Diff line change
@@ -1,24 +1,54 @@
package com.electronwill.nightconfig.core.io;

import org.jetbrains.annotations.Nullable;

/**
* Thrown when a parsing operation fails.
*
* @author TheElectronWill
*/
public class ParsingException extends RuntimeException {
public ParsingException(String message) {
private final long line, column;

public ParsingException(@Nullable Cursor positionReference, String message) {
super(message);

this.line = positionReference == null ? -1 : positionReference.line();
this.column = positionReference == null ? -1 : positionReference.column();
}

public ParsingException(String message, Throwable cause) {
public ParsingException(@Nullable Cursor positionReference, String message, Throwable cause) {
super(message, cause);

this.line = positionReference == null ? -1 : positionReference.line();
this.column = positionReference == null ? -1 : positionReference.column();
}

@Override
public String getMessage() {
String pos = "";
boolean hasLine = line != -1;
boolean hasColumn = column != -1;
boolean hasAny = hasLine || hasColumn;
if (hasAny)
pos += " [";
if (hasLine)
pos += "line " + line;
if (hasColumn) {
if (hasLine)
pos += ", ";
pos += "pos " + column;
}
if (hasAny)
pos += "]";
return super.getMessage() + pos;
}

public static ParsingException readFailed(Throwable cause) {
return new ParsingException("Failed to parse data from Reader", cause);
return new ParsingException(null, "Failed to parse data from Reader", cause);
}

public static ParsingException notEnoughData() {
return new ParsingException("Not enough data available");
return new ParsingException(null, "Not enough data available");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.electronwill.nightconfig.core.io;

import org.jetbrains.annotations.Nullable;

/**
* Serialization utilities.
*
Expand Down Expand Up @@ -48,7 +50,7 @@ public static int arrayIndexOf(char[] array, char element) {
* @param base the base of the number
* @return the long value represented by the CharsWrapper
*/
public static long parseLong(CharsWrapper chars, int base) {
public static long parseLong(CharsWrapper chars, int base, @Nullable Cursor cursor) {
// Optimized lightweight parsing
int offset = chars.offset;
boolean negative = false;
Expand All @@ -64,7 +66,7 @@ public static long parseLong(CharsWrapper chars, int base) {
for (int i = chars.limit - 1; i >= offset; i--) {
int digitValue = Character.digit(array[i], base);
if (digitValue == -1) {//invalid digit in the specified base
throw new ParsingException("Invalid value: " + chars);
throw new ParsingException(cursor, "Invalid value: " + chars);
}
value += digitValue * coefficient;
coefficient *= base;
Expand All @@ -75,12 +77,13 @@ public static long parseLong(CharsWrapper chars, int base) {
/**
* Parses a CharsWrapper that represents an int value in the specified base.
*
* @param chars the CharsWrapper representing an int
* @param base the base of the number
* @param chars the CharsWrapper representing an int
* @param base the base of the number
* @param cursor
* @return the int value represented by the CharsWrapper
*/
public static int parseInt(CharsWrapper chars, int base) {
return (int)parseLong(chars, base);
public static int parseInt(CharsWrapper chars, int base, @Nullable Cursor cursor) {
return (int)parseLong(chars, base, cursor);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ class UtilsTest {

@Test
void parseLong() {
assertEquals(123456789L, Utils.parseLong(new CharsWrapper("123456789"), 10));
assertEquals(0, Utils.parseLong(new CharsWrapper("0"), 10));
assertEquals(-123456789L, Utils.parseLong(new CharsWrapper("-123456789"), 10));
assertEquals(0xff, Utils.parseLong(new CharsWrapper("ff"), 16));
assertEquals(123456789L, Utils.parseLong(new CharsWrapper("123456789"), 10, null));
assertEquals(0, Utils.parseLong(new CharsWrapper("0"), 10, null));
assertEquals(-123456789L, Utils.parseLong(new CharsWrapper("-123456789"), 10, null));
assertEquals(0xff, Utils.parseLong(new CharsWrapper("ff"), 16, null));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public void parse(Reader reader, Config destination, ParsingMode parsingMode) {
put(parsed, destination, parsingMode);
}
} catch (Exception e) {
throw new ParsingException("HOCON parsing failed", e);
throw new ParsingException(null, "HOCON parsing failed", e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public Object parseDocument(Reader reader, Config configModel) {
// If data is empty && we accept empty data => return empty config
return configModel.createSubConfig();
} else {
throw new ParsingException("No json data: input is empty");
throw new ParsingException(input, "No json data: input is empty");
}
}
char firstChar = input.readCharAndSkip(SPACES);
Expand All @@ -103,7 +103,7 @@ public Object parseDocument(Reader reader, Config configModel) {
if (firstChar == '[') {
return parseArray(input, new ArrayList<>(), ParsingMode.MERGE, configModel.createSubConfig());
}
throw new ParsingException("Invalid first character for a json document: " + firstChar);
throw new ParsingException(input, "Invalid first character for a json document: " + firstChar);
}

/**
Expand All @@ -127,12 +127,12 @@ public void parse(Reader reader, Config destination, ParsingMode parsingMode) {
// If data is empty && we accept empty data => let the config as it is
return;
} else {
throw new ParsingException("No json data: input is empty");
throw new ParsingException(input, "No json data: input is empty");
}
}
char firstChar = input.readCharAndSkip(SPACES);
if (firstChar != '{') {
throw new ParsingException("Invalid first character for a json object: " + firstChar);
throw new ParsingException(input, "Invalid first character for a json object: " + firstChar);
}
if (destination instanceof ConcurrentConfig) {
((ConcurrentConfig)destination).bulkUpdate(view -> {
Expand Down Expand Up @@ -190,12 +190,12 @@ public void parseList(Reader reader, List<?> destination, ParsingMode parsingMod
// If data is empty && we accept empty data => let the config as it is
return;
} else {
throw new ParsingException("No json data: input is empty");
throw new ParsingException(input, "No json data: input is empty");
}
}
char firstChar = input.readCharAndSkip(SPACES);
if (firstChar != '[') {
throw new ParsingException("Invalid first character for a json array: " + firstChar);
throw new ParsingException(input, "Invalid first character for a json array: " + firstChar);
}
parseArray(input, destination, parsingMode, configModel);
}
Expand All @@ -205,19 +205,19 @@ private <T extends Config> T parseObject(CharacterInput input, T config, Parsing
if (kfirst == '}') {
return config;
} else if (kfirst != '"') {
throw new ParsingException("Invalid beginning of a key: " + kfirst);
throw new ParsingException(input, "Invalid beginning of a key: " + kfirst);
}
parseKVPair(input, config, parsingMode);
while (true) {
char vsep = input.readCharAndSkip(SPACES);
if (vsep == '}') {// end of the object
return config;
} else if (vsep != ',') {
throw new ParsingException("Invalid value separator: " + vsep);
throw new ParsingException(input, "Invalid value separator: " + vsep);
}
kfirst = input.readCharAndSkip(SPACES);
if (kfirst != '"') {
throw new ParsingException("Invalid beginning of a key: " + kfirst);
throw new ParsingException(input, "Invalid beginning of a key: " + kfirst);
}
parseKVPair(input, config, parsingMode);
}
Expand All @@ -227,7 +227,7 @@ private void parseKVPair(CharacterInput input, Config config, ParsingMode parsin
List<String> key = Collections.singletonList(parseString(input)); // the list is necessary if there are dots in the key
char sep = input.readCharAndSkip(SPACES);
if (sep != ':') {
throw new ParsingException("Invalid key-value separator: " + sep);
throw new ParsingException(input, "Invalid key-value separator: " + sep);
}

char vfirst = input.readCharAndSkip(SPACES);
Expand All @@ -249,7 +249,7 @@ private <T> List<T> parseArray(CharacterInput input, List<T> list, ParsingMode p
if (next == ']') {// end of the array
return list;
} else if (next != ',') {// invalid separator
throw new ParsingException("Invalid value separator: " + valueFirst);
throw new ParsingException(input, "Invalid value separator: " + valueFirst);
}
}
}
Expand Down Expand Up @@ -279,7 +279,7 @@ private Number parseNumber(CharacterInput input) {
if (chars.contains('.') || chars.contains('e') || chars.contains('E')) {// must be a double
return Utils.parseDouble(chars);
}
long l = Utils.parseLong(chars, 10);
long l = Utils.parseLong(chars, 10, input);
int small = (int)l;
if (l == small) {// small value => return an int instead of a long
return small;
Expand All @@ -290,23 +290,23 @@ private Number parseNumber(CharacterInput input) {
private boolean parseTrue(CharacterInput input) {
CharsWrapper chars = input.readChars(3);
if (!chars.contentEquals(TRUE_LAST)) {
throw new ParsingException("Invalid value: t" + chars + " - expected boolean true");
throw new ParsingException(input, "Invalid value: t" + chars + " - expected boolean true");
}
return true;
}

private boolean parseFalse(CharacterInput input) {
CharsWrapper chars = input.readChars(4);
if (!chars.contentEquals(FALSE_LAST)) {
throw new ParsingException("Invalid value: f" + chars + " - expected boolean false");
throw new ParsingException(input, "Invalid value: f" + chars + " - expected boolean false");
}
return false;
}

private Object parseNull(CharacterInput input) {
CharsWrapper chars = input.readChars(3);
if (!chars.contentEquals(NULL_LAST)) {
throw new ParsingException("Invaid value: n" + chars + " - expected null");
throw new ParsingException(input, "Invaid value: n" + chars + " - expected null");
}
return null;
}
Expand Down Expand Up @@ -346,9 +346,9 @@ private char escape(char c, CharacterInput input) {
return '\t';
case 'u':
CharsWrapper chars = input.readChars(4);
return (char)Utils.parseInt(chars, 16);
return (char)Utils.parseInt(chars, 16, input);
default:
throw new ParsingException("Invalid escapement: \\" + c);
throw new ParsingException(input, "Invalid escapement: \\" + c);
}
}
}
3 changes: 2 additions & 1 deletion toml/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ multiRelease {

dependencies {
implementation(project(":core"))
testImplementation(project(":test-shared"))
compileOnly("org.jetbrains:annotations:+")
testImplementation(project(":test-shared"))
}
Loading