From 4e22176f1e4472fc6f3b9dae134eed26300b20a3 Mon Sep 17 00:00:00 2001 From: "moandji.ezana" Date: Mon, 16 Feb 2015 13:33:02 +0200 Subject: [PATCH] Improved duplicate table detection. Improved line reporting. --- .../java/com/moandjiezana/toml/Container.java | 9 +++ .../toml/IdentifierConverter.java | 1 + .../java/com/moandjiezana/toml/Results.java | 80 ++++++++++++++----- .../com/moandjiezana/toml/TomlParser.java | 6 +- .../moandjiezana/toml/ErrorMessagesTest.java | 10 +-- .../moandjiezana/toml/InlineTableTest.java | 18 +++-- 6 files changed, 92 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/moandjiezana/toml/Container.java b/src/main/java/com/moandjiezana/toml/Container.java index 834a891..0b1a24f 100644 --- a/src/main/java/com/moandjiezana/toml/Container.java +++ b/src/main/java/com/moandjiezana/toml/Container.java @@ -13,6 +13,15 @@ abstract class Container { static class Table extends Container { private final Map values = new HashMap(); + final String name; + + Table() { + this.name = null; + } + + public Table(String name) { + this.name = name; + } @Override boolean accepts(String key) { diff --git a/src/main/java/com/moandjiezana/toml/IdentifierConverter.java b/src/main/java/com/moandjiezana/toml/IdentifierConverter.java index d038ba5..b0a4444 100644 --- a/src/main/java/com/moandjiezana/toml/IdentifierConverter.java +++ b/src/main/java/com/moandjiezana/toml/IdentifierConverter.java @@ -20,6 +20,7 @@ class IdentifierConverter { quoted = !quoted; name.append('"'); } else if (c == '\n') { + index.decrementAndGet(); break; } else if (quoted) { name.append(c); diff --git a/src/main/java/com/moandjiezana/toml/Results.java b/src/main/java/com/moandjiezana/toml/Results.java index 02da5c7..ec5e577 100644 --- a/src/main/java/com/moandjiezana/toml/Results.java +++ b/src/main/java/com/moandjiezana/toml/Results.java @@ -3,8 +3,10 @@ package com.moandjiezana.toml; import java.util.ArrayDeque; import java.util.Deque; import java.util.HashSet; +import java.util.Iterator; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; class Results { @@ -13,7 +15,9 @@ class Results { private final StringBuilder sb = new StringBuilder(); void duplicateTable(String table, int line) { - sb.append("Duplicate table definition: [") + sb.append("Duplicate table definition on line ") + .append(line) + .append(": [") .append(table) .append("]\n"); } @@ -116,37 +120,41 @@ class Results { sb.append(other.sb); } } - Set tables = new HashSet(); + final Errors errors = new Errors(); - private Deque stack = new ArrayDeque(); + private final Set tables = new HashSet(); + private final Deque stack = new ArrayDeque(); Results() { - stack.push(new Container.Table()); + stack.push(new Container.Table("")); } - void addValue(String key, Object value) { + void addValue(String key, Object value, AtomicInteger line) { Container currentTable = stack.peek(); if (value instanceof Map) { - if (stack.size() == 1) { - startTables(Identifier.from(key, null)); + String path = getInlineTablePath(key); + if (path == null) { + startTable(key, line); + } else if (path.isEmpty()) { + startTables(Identifier.from(key, null), line); } else { - startTable(key); + startTables(Identifier.from(path, null), line); } @SuppressWarnings("unchecked") Map valueMap = (Map) value; for (Map.Entry entry : valueMap.entrySet()) { - addValue(entry.getKey(), entry.getValue()); + addValue(entry.getKey(), entry.getValue(), line); } stack.pop(); } else if (currentTable.accepts(key)) { currentTable.put(key, value); } else { - errors.duplicateKey(key, -1); + errors.duplicateKey(key, line != null ? line.get() : -1); } } - void startTableArray(Identifier identifier) { + void startTableArray(Identifier identifier, AtomicInteger line) { String tableName = identifier.getBareName(); while (stack.size() > 1) { stack.pop(); @@ -172,23 +180,23 @@ class Results { stack.push(nextTable); } else if (currentContainer.accepts(tablePart)) { Container newContainer = i == tableParts.length - 1 ? new Container.TableArray() : new Container.Table(); - addValue(tablePart, newContainer); + addValue(tablePart, newContainer, line); stack.push(newContainer); if (newContainer instanceof Container.TableArray) { stack.push(((Container.TableArray) newContainer).getCurrent()); } } else { - errors.duplicateTable(tableName, -1); + errors.duplicateTable(tableName, line.get()); break; } } } - void startTables(Identifier id) { + void startTables(Identifier id, AtomicInteger line) { String tableName = id.getBareName(); if (!tables.add(tableName)) { - errors.duplicateTable(tableName, -1); + errors.duplicateTable(tableName, line.get()); } while (stack.size() > 1) { @@ -206,7 +214,7 @@ class Results { stack.push(((Container.TableArray) stack.peek()).getCurrent()); } } else if (currentContainer.accepts(tablePart)) { - startTable(tablePart); + startTable(tablePart, line); } else { errors.duplicateTable(tableName, -1); break; @@ -224,11 +232,45 @@ class Results { return ((Container.Table) values).consume(); } - private Container startTable(String tableName) { - Container newTable = new Container.Table(); - addValue(tableName, newTable); + private Container startTable(String tableName, AtomicInteger line) { + Container newTable = new Container.Table(tableName); + addValue(tableName, newTable, line); stack.push(newTable); return newTable; } + + private String getInlineTablePath(String key) { + Iterator descendingIterator = stack.descendingIterator(); + StringBuilder sb = new StringBuilder(); + + while (descendingIterator.hasNext()) { + Container next = descendingIterator.next(); + if (next instanceof Container.TableArray) { + return null; + } + + Container.Table table = (Container.Table) next; + + if (table.name == null) { + break; + } + + if (sb.length() > 0) { + sb.append('.'); + } + + sb.append(table.name); + } + + if (sb.length() > 0) { + sb.append('.'); + } + + sb.append(key) + .insert(0, '[') + .append(']'); + + return sb.toString(); + } } \ No newline at end of file diff --git a/src/main/java/com/moandjiezana/toml/TomlParser.java b/src/main/java/com/moandjiezana/toml/TomlParser.java index cc8d329..fcfe3d7 100644 --- a/src/main/java/com/moandjiezana/toml/TomlParser.java +++ b/src/main/java/com/moandjiezana/toml/TomlParser.java @@ -35,9 +35,9 @@ class TomlParser { if (id.isKey()) { identifier = id; } else if (id.isTable()) { - results.startTables(id); + results.startTables(id, line); } else if (id.isTableArray()) { - results.startTableArray(id); + results.startTableArray(id, line); } } } else if (c == '\n') { @@ -51,7 +51,7 @@ class TomlParser { if (value instanceof Results.Errors) { results.errors.add((Results.Errors) value); } else { - results.addValue(identifier.getName(), value); + results.addValue(identifier.getName(), value, line); } } else if (value != null && !inComment && !Character.isWhitespace(c)) { results.errors.invalidTextAfterIdentifier(identifier, c, line.get()); diff --git a/src/test/java/com/moandjiezana/toml/ErrorMessagesTest.java b/src/test/java/com/moandjiezana/toml/ErrorMessagesTest.java index 1775b8b..87170d3 100644 --- a/src/test/java/com/moandjiezana/toml/ErrorMessagesTest.java +++ b/src/test/java/com/moandjiezana/toml/ErrorMessagesTest.java @@ -18,7 +18,7 @@ public class ErrorMessagesTest { @Test public void should_message_duplicate_table() throws Exception { - e.expectMessage("Duplicate table definition: [again]"); + e.expectMessage("Duplicate table definition on line 2: [again]"); new Toml().parse("[again]\n[again]"); } @@ -32,7 +32,7 @@ public class ErrorMessagesTest { @Test public void should_message_duplicate_key() throws Exception { - e.expectMessage("Duplicate key: k"); + e.expectMessage("Duplicate key on line 2: k"); new Toml().parse("k = 1\n k = 2"); } @@ -95,21 +95,21 @@ public class ErrorMessagesTest { @Test public void should_display_correct_line_number_with_literal_multiline_string() throws Exception { - e.expectMessage("on line 7"); + e.expectMessage("on line 8"); new Toml().parse("[table]\n\n k = '''abc\n\ndef\n'''\n # comment \n j = 4.\n l = 5"); } @Test public void should_display_correct_line_number_with_multiline_string() throws Exception { - e.expectMessage("on line 8"); + e.expectMessage("on line 9"); new Toml().parse("[table]\n\n k = \"\"\"\nabc\n\ndef\n\"\"\"\n # comment \n j = 4.\n l = 5"); } @Test public void should_display_correct_line_number_with_array() throws Exception { - e.expectMessage("on line 9"); + e.expectMessage("on line 10"); new Toml().parse("[table]\n\n k = [\"\"\"\nabc\n\ndef\n\"\"\"\n, \n # comment \n j = 4.,\n l = 5\n]"); } diff --git a/src/test/java/com/moandjiezana/toml/InlineTableTest.java b/src/test/java/com/moandjiezana/toml/InlineTableTest.java index 53bf549..a7237e2 100644 --- a/src/test/java/com/moandjiezana/toml/InlineTableTest.java +++ b/src/test/java/com/moandjiezana/toml/InlineTableTest.java @@ -190,7 +190,7 @@ public class InlineTableTest { @Test public void should_fail_when_duplicated_by_other_key() throws Exception { e.expect(IllegalStateException.class); - e.expectMessage("Duplicate key: tbl"); + e.expectMessage("Duplicate key on line 2: tbl"); new Toml().parse("tbl = { a = 1 }\n tbl = 1"); } @@ -198,7 +198,7 @@ public class InlineTableTest { @Test public void should_fail_when_duplicated_by_other_inline_table() throws Exception { e.expect(IllegalStateException.class); - e.expectMessage("Duplicate table definition: [tbl]"); + e.expectMessage("Duplicate table definition on line 2: [tbl]"); new Toml().parse("tbl = { a = 1 }\n tbl = {}"); } @@ -206,7 +206,7 @@ public class InlineTableTest { @Test public void should_fail_when_duplicated_by_top_level_table() throws Exception { e.expect(IllegalStateException.class); - e.expectMessage("Duplicate table definition: [tbl]"); + e.expectMessage("Duplicate table definition on line 2: [tbl]"); new Toml().parse("tbl = {}\n [tbl]"); } @@ -214,7 +214,7 @@ public class InlineTableTest { @Test public void should_fail_when_duplicates_second_level_table() throws Exception { e.expect(IllegalStateException.class); - e.expectMessage("Duplicate key: b"); + e.expectMessage("Duplicate table definition on line 3: [a.b]"); new Toml().parse("[a.b]\n [a]\n b = {}"); } @@ -222,8 +222,16 @@ public class InlineTableTest { @Test public void should_fail_when_inline_table_duplicates_table() throws Exception { e.expect(IllegalStateException.class); - e.expectMessage("Duplicate key: b"); + e.expectMessage("Duplicate table definition on line 3: [a.b]"); new Toml().parse("[a.b]\n [a]\n b = {}"); } + + @Test + public void should_fail_when_second_level_table_duplicates_inline_table() throws Exception { + e.expect(IllegalStateException.class); + e.expectMessage("Duplicate table definition on line 3: [a.b]"); + + new Toml().parse("[a]\n b = {}\n [a.b]"); + } }