Repository: jhy/jsoup
Pull request #1624 (2021-08-29 04:29:07)

Added support for tracking newlines in CharacterReader
This allows parse errors to be reported more intuitively.


Patch:
diff --git a/CHANGES b/CHANGES
index 1adcd101..bb18b2fc 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
 jsoup changelog
 
 *** Release 1.14.3 [PENDING]
+  * Improvement: added support in CharacterReader to track newlines, so that parse errors can be reported more
+    intuitively.
+    <https://github.com/jhy/jsoup/pull/1624>
+
   * Bugfix: the OSGi bundle meta-data incorrectly set a version on the import of java.annotation (used as a build-time
     dependency for nullability assertions).
     <https://github.com/jhy/jsoup/issues/1616>
diff --git a/src/main/java/org/jsoup/parser/CharacterReader.java b/src/main/java/org/jsoup/parser/CharacterReader.java
index ec80df96..af27c7aa 100644
--- a/src/main/java/org/jsoup/parser/CharacterReader.java
+++ b/src/main/java/org/jsoup/parser/CharacterReader.java
@@ -3,10 +3,13 @@ package org.jsoup.parser;
 import org.jsoup.UncheckedIOException;
 import org.jsoup.helper.Validate;
 
+import javax.annotation.Nullable;
 import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Locale;
 
 /**
@@ -29,6 +32,9 @@ public final class CharacterReader {
     private static final int stringCacheSize = 512;
     private String[] stringCache = new String[stringCacheSize]; // holds reused strings in this doc, to lessen garbage
 
+    @Nullable private ArrayList<Integer> newlinePositions = null; // optionally track the pos() position of newlines - scans during bufferUp()
+    private int lineNumberOffset = 1; // line numbers start at 1; += newlinePosition[indexof(pos)]
+
     public CharacterReader(Reader input, int sz) {
         Validate.notNull(input);
         Validate.isTrue(input.markSupported());
@@ -98,6 +104,7 @@ public final class CharacterReader {
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
+        scanBufferForNewlines();
     }
 
     /**
@@ -108,6 +115,104 @@ public final class CharacterReader {
         return readerPos + bufPos;
     }
 
+    /**
+     Enables or disables line number tracking. By default, will be <b>off</b>.Tracking line numbers improves the
+     legibility of parser error messages, for example. Tracking should be enabled before any content is read to be of
+     use.
+
+     @param track set tracking on|off
+     @since 1.14.3
+     */
+    public void trackNewlines(boolean track) {
+        if (track && newlinePositions == null) {
+            newlinePositions = new ArrayList<>(maxBufferLen / 80); // rough guess of likely count
+            scanBufferForNewlines(); // first pass when enabled; subsequently called during bufferUp
+        }
+        else if (!track)
+            newlinePositions = null;
+    }
+
+    /**
+     Check if the tracking of newlines is enabled.
+     @return the current newline tracking state
+     @since 1.14.3
+     */
+    public boolean isTrackNewlines() {
+        return newlinePositions != null;
+    }
+
+    /**
+     Get the current line number (that the reader has consumed to). Starts at line #1.
+     @return the current line number, or 1 if line tracking is not enabled.
+     @since 1.14.3
+     @see #trackNewlines(boolean)
+     */
+    public int lineNumber() {
+        if (!isTrackNewlines())
+            return 1;
+
+        int i = lineNumIndex();
+        if (i == -1)
+            return lineNumberOffset; // first line
+        if (i < 0)
+            return Math.abs(i) + lineNumberOffset - 1;
+        return i + lineNumberOffset + 1;
+    }
+
+    /**
+     Get the current column number (that the reader has consumed to). Starts at column #1.
+     @return the current column number
+     @since 1.14.3
+     @see #trackNewlines(boolean)
+     */
+    int columnNumber() {
+        if (!isTrackNewlines())
+            return pos() + 1;
+
+        int i = lineNumIndex();
+        if (i == -1)
+            return pos() + 1;
+        if (i < 0)
+            i = Math.abs(i) - 2;
+        return pos() - newlinePositions.get(i) + 1;
+    }
+
+    /**
+     Get a formatted string representing the current line and cursor positions. E.g. <code>5:10</code> indicating line
+     number 5 and column number 10.
+     @return line:col position
+     @since 1.14.3
+     @see #trackNewlines(boolean)
+     */
+    String cursorPos() {
+        return lineNumber() + ":" + columnNumber();
+    }
+
+    private int lineNumIndex() {
+        if (!isTrackNewlines()) return 0;
+        return Collections.binarySearch(newlinePositions, pos());
+    }
+
+    /**
+     Scans the buffer for newline position, and tracks their location in newlinePositions.
+     */
+    private void scanBufferForNewlines() {
+        if (!isTrackNewlines())
+            return;
+
+        lineNumberOffset += newlinePositions.size();
+        int lastPos = newlinePositions.size() > 0 ? newlinePositions.get(newlinePositions.size() -1) : -1;
+        newlinePositions.clear();
+        if (lastPos != -1) {
+            newlinePositions.add(lastPos); // roll the last pos to first, for cursor num after buffer
+            lineNumberOffset--; // as this takes a position
+        }
+        for (int i = bufPos; i < bufLength; i++) {
+            if (charBuf[i] == '\n')
+                newlinePositions.add(1 + readerPos + i);
+        }
+    }
+
     /**
      * Tests if all the content has been read.
      * @return true if nothing left to read.
diff --git a/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java b/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java
index b276858a..cdf1b797 100644
--- a/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java
+++ b/src/main/java/org/jsoup/parser/HtmlTreeBuilder.java
@@ -204,7 +204,7 @@ public class HtmlTreeBuilder extends TreeBuilder {
 
     void error(HtmlTreeBuilderState state) {
         if (parser.getErrors().canAddError())
-            parser.getErrors().add(new ParseError(reader.pos(), "Unexpected token [%s] when in state [%s]", currentToken.tokenType(), state));
+            parser.getErrors().add(new ParseError(reader, "Unexpected token [%s] when in state [%s]", currentToken.tokenType(), state));
     }
 
     Element insert(final Token.StartTag startTag) {
diff --git a/src/main/java/org/jsoup/parser/ParseError.java b/src/main/java/org/jsoup/parser/ParseError.java
index dfa09005..feccea68 100644
--- a/src/main/java/org/jsoup/parser/ParseError.java
+++ b/src/main/java/org/jsoup/parser/ParseError.java
@@ -5,16 +5,31 @@ package org.jsoup.parser;
  */
 public class ParseError {
     private int pos;
+    private String cursorPos;
     private String errorMsg;
 
+    ParseError(CharacterReader reader, String errorMsg) {
+        pos = reader.pos();
+        cursorPos = reader.cursorPos();
+        this.errorMsg = errorMsg;
+    }
+
+    ParseError(CharacterReader reader, String errorFormat, Object... args) {
+        pos = reader.pos();
+        cursorPos = reader.cursorPos();
+        this.errorMsg = String.format(errorFormat, args);
+    }
+
     ParseError(int pos, String errorMsg) {
         this.pos = pos;
+        cursorPos = String.valueOf(pos);
         this.errorMsg = errorMsg;
     }
 
     ParseError(int pos, String errorFormat, Object... args) {
-        this.errorMsg = String.format(errorFormat, args);
         this.pos = pos;
+        cursorPos = String.valueOf(pos);
+        this.errorMsg = String.format(errorFormat, args);
     }
 
     /**
@@ -33,8 +48,16 @@ public class ParseError {
         return pos;
     }
 
+    /**
+     Get the formatted line:column cursor position where the error occured.
+     @return line:number cursor position
+     */
+    public String getCursorPos() {
+        return cursorPos;
+    }
+
     @Override
     public String toString() {
-        return pos + ": " + errorMsg;
+        return "<" + cursorPos + ">: " + errorMsg;
     }
 }
diff --git a/src/main/java/org/jsoup/parser/Parser.java b/src/main/java/org/jsoup/parser/Parser.java
index 8ff9667e..efc5f3c4 100644
--- a/src/main/java/org/jsoup/parser/Parser.java
+++ b/src/main/java/org/jsoup/parser/Parser.java
@@ -93,6 +93,7 @@ public class Parser {
     /**
      * Retrieve the parse errors, if any, from the last parse.
      * @return list of parse errors, up to the size of the maximum errors tracked.
+     * @see #setTrackErrors(int)
      */
     public ParseErrorList getErrors() {
         return errors;
diff --git a/src/main/java/org/jsoup/parser/Tokeniser.java b/src/main/java/org/jsoup/parser/Tokeniser.java
index 05a95c8c..68de3488 100644
--- a/src/main/java/org/jsoup/parser/Tokeniser.java
+++ b/src/main/java/org/jsoup/parser/Tokeniser.java
@@ -276,22 +276,22 @@ final class Tokeniser {
 
     void error(TokeniserState state) {
         if (errors.canAddError())
-            errors.add(new ParseError(reader.pos(), "Unexpected character '%s' in input state [%s]", reader.current(), state));
+            errors.add(new ParseError(reader, "Unexpected character '%s' in input state [%s]", reader.current(), state));
     }
 
     void eofError(TokeniserState state) {
         if (errors.canAddError())
-            errors.add(new ParseError(reader.pos(), "Unexpectedly reached end of file (EOF) in input state [%s]", state));
+            errors.add(new ParseError(reader, "Unexpectedly reached end of file (EOF) in input state [%s]", state));
     }
 
     private void characterReferenceError(String message) {
         if (errors.canAddError())
-            errors.add(new ParseError(reader.pos(), "Invalid character reference: %s", message));
+            errors.add(new ParseError(reader, "Invalid character reference: %s", message));
     }
 
     void error(String errorMsg) {
         if (errors.canAddError())
-            errors.add(new ParseError(reader.pos(), errorMsg));
+            errors.add(new ParseError(reader, errorMsg));
     }
 
     boolean currentNodeInHtmlNS() {
diff --git a/src/main/java/org/jsoup/parser/TreeBuilder.java b/src/main/java/org/jsoup/parser/TreeBuilder.java
index 42617f52..a27ff35d 100644
--- a/src/main/java/org/jsoup/parser/TreeBuilder.java
+++ b/src/main/java/org/jsoup/parser/TreeBuilder.java
@@ -40,6 +40,7 @@ abstract class TreeBuilder {
         this.parser = parser;
         settings = parser.settings();
         reader = new CharacterReader(input);
+        reader.trackNewlines(parser.isTrackErrors()); // when tracking errors, enable newline tracking for better error reports
         currentToken = null;
         tokeniser = new Tokeniser(reader, parser.getErrors());
         stack = new ArrayList<>(32);
@@ -139,7 +140,7 @@ abstract class TreeBuilder {
     protected void error(String msg) {
         ParseErrorList errors = parser.getErrors();
         if (errors.canAddError())
-            errors.add(new ParseError(reader.pos(), msg));
+            errors.add(new ParseError(reader, msg));
     }
 
     /**


Test patch:
diff --git a/src/test/java/org/jsoup/parser/CharacterReaderTest.java b/src/test/java/org/jsoup/parser/CharacterReaderTest.java
index ba34d47a..a362b1e6 100644
--- a/src/test/java/org/jsoup/parser/CharacterReaderTest.java
+++ b/src/test/java/org/jsoup/parser/CharacterReaderTest.java
@@ -1,8 +1,10 @@
 package org.jsoup.parser;
 
+import org.jsoup.integration.ParseTest;
 import org.junit.jupiter.api.Test;
 
 import java.io.BufferedReader;
+import java.io.IOException;
 import java.io.StringReader;
 
 import static org.junit.jupiter.api.Assertions.*;
@@ -336,4 +338,117 @@ public class CharacterReaderTest {
         assertTrue(r.isEmpty());
     }
 
+    @Test public void canEnableAndDisableLineNumberTracking() {
+        CharacterReader reader = new CharacterReader("Hello!");
+        assertFalse(reader.isTrackNewlines());
+        reader.trackNewlines(true);
+        assertTrue(reader.isTrackNewlines());
+        reader.trackNewlines(false);
+        assertFalse(reader.isTrackNewlines());
+    }
+
+    @Test public void canTrackNewlines() {
+        StringBuilder builder = new StringBuilder();
+        builder.append("<foo>\n<bar>\n<qux>\n");
+        while (builder.length() < maxBufferLen)
+            builder.append("Lorem ipsum dolor sit amet, consectetur adipiscing elit.");
+        builder.append("[foo]\n[bar]");
+        String content = builder.toString();
+
+        CharacterReader noTrack = new CharacterReader(content);
+        assertFalse(noTrack.isTrackNewlines());
+        CharacterReader track = new CharacterReader(content);
+        track.trackNewlines(true);
+        assertTrue(track.isTrackNewlines());
+
+        // check that no tracking works as expected (pos is 0 indexed, line number stays at 1, col is pos+1)
+        assertEquals(0, noTrack.pos());
+        assertEquals(1, noTrack.lineNumber());
+        assertEquals(1, noTrack.columnNumber());
+        noTrack.consumeTo("<qux>");
+        assertEquals(12, noTrack.pos());
+        assertEquals(1, noTrack.lineNumber());
+        assertEquals(13, noTrack.columnNumber());
+        assertEquals("1:13", noTrack.cursorPos());
+        // get over the buffer
+        while (!noTrack.matches("[foo]"))
+            noTrack.consumeTo("[foo]");
+        assertEquals(32778, noTrack.pos());
+        assertEquals(1, noTrack.lineNumber());
+        assertEquals(noTrack.pos()+1, noTrack.columnNumber());
+        assertEquals("1:32779", noTrack.cursorPos());
+
+        // and the line numbers: "<foo>\n<bar>\n<qux>\n"
+        assertEquals(0, track.pos());
+        assertEquals(1, track.lineNumber());
+        assertEquals(1, track.columnNumber());
+
+        track.consumeTo('\n');
+        assertEquals(1, track.lineNumber());
+        assertEquals(6, track.columnNumber());
+        track.consume();
+        assertEquals(2, track.lineNumber());
+        assertEquals(1, track.columnNumber());
+
+        assertEquals("<bar>", track.consumeTo('\n'));
+        assertEquals(2, track.lineNumber());
+        assertEquals(6, track.columnNumber());
+
+        assertEquals("\n", track.consumeTo("<qux>"));
+        assertEquals(12, track.pos());
+        assertEquals(3, track.lineNumber());
+        assertEquals(1, track.columnNumber());
+        assertEquals("3:1", track.cursorPos());
+        assertEquals("<qux>", track.consumeTo('\n'));
+        assertEquals("3:6", track.cursorPos());
+        // get over the buffer
+        while (!track.matches("[foo]"))
+            track.consumeTo("[foo]");
+        assertEquals(32778, track.pos());
+        assertEquals(4, track.lineNumber());
+        assertEquals(32761, track.columnNumber());
+        assertEquals("4:32761", track.cursorPos());
+        track.consumeTo('\n');
+        assertEquals("4:32766", track.cursorPos());
+
+        track.consumeTo("[bar]");
+        assertEquals(5, track.lineNumber());
+        assertEquals("5:1", track.cursorPos());
+        track.consumeToEnd();
+        assertEquals("5:6", track.cursorPos());
+    }
+
+    @Test public void countsColumnsOverBufferWhenNoNewlines() {
+        StringBuilder builder = new StringBuilder();
+        while (builder.length() < maxBufferLen * 4)
+            builder.append("Lorem ipsum dolor sit amet, consectetur adipiscing elit.");
+        String content = builder.toString();
+        CharacterReader reader = new CharacterReader(content);
+        reader.trackNewlines(true);
+
+        assertEquals("1:1", reader.cursorPos());
+        while (!reader.isEmpty())
+            reader.consume();
+        assertEquals(131096, reader.pos());
+        assertEquals(reader.pos() + 1, reader.columnNumber());
+        assertEquals(1, reader.lineNumber());
+    }
+
+    @Test public void linenumbersAgreeWithEditor() throws IOException {
+        String content = ParseTest.getFileAsString(ParseTest.getFile("/htmltests/large.html"));
+        CharacterReader reader = new CharacterReader(content);
+        reader.trackNewlines(true);
+
+        String scan = "<p>VESTIBULUM"; // near the end of the file
+        while (!reader.matches(scan))
+            reader.consumeTo(scan);
+
+        assertEquals(280218, reader.pos());
+        assertEquals(1002, reader.lineNumber());
+        assertEquals(1, reader.columnNumber());
+        reader.consumeTo(' ');
+        assertEquals(1002, reader.lineNumber());
+        assertEquals(14, reader.columnNumber());
+    }
+
 }
diff --git a/src/test/java/org/jsoup/parser/HtmlParserTest.java b/src/test/java/org/jsoup/parser/HtmlParserTest.java
index 2d27e531..3177a334 100644
--- a/src/test/java/org/jsoup/parser/HtmlParserTest.java
+++ b/src/test/java/org/jsoup/parser/HtmlParserTest.java
@@ -835,29 +835,29 @@ public class HtmlParserTest {
     }
 
     @Test public void tracksErrorsWhenRequested() {
-        String html = "<p>One</p href='no'><!DOCTYPE html>&arrgh;<font /><br /><foo";
+        String html = "<p>One</p href='no'>\n<!DOCTYPE html>\n&arrgh;<font /><br /><foo";
         Parser parser = Parser.htmlParser().setTrackErrors(500);
         Document doc = Jsoup.parse(html, "http://example.com", parser);
 
         List<ParseError> errors = parser.getErrors();
         assertEquals(5, errors.size());
-        assertEquals("20: Attributes incorrectly present on end tag", errors.get(0).toString());
-        assertEquals("35: Unexpected token [Doctype] when in state [InBody]", errors.get(1).toString());
-        assertEquals("36: Invalid character reference: invalid named reference", errors.get(2).toString());
-        assertEquals("50: Tag cannot be self closing; not a void tag", errors.get(3).toString());
-        assertEquals("61: Unexpectedly reached end of file (EOF) in input state [TagName]", errors.get(4).toString());
+        assertEquals("<1:21>: Attributes incorrectly present on end tag", errors.get(0).toString());
+        assertEquals("<2:16>: Unexpected token [Doctype] when in state [InBody]", errors.get(1).toString());
+        assertEquals("<3:2>: Invalid character reference: invalid named reference", errors.get(2).toString());
+        assertEquals("<3:16>: Tag cannot be self closing; not a void tag", errors.get(3).toString());
+        assertEquals("<3:27>: Unexpectedly reached end of file (EOF) in input state [TagName]", errors.get(4).toString());
     }
 
     @Test public void tracksLimitedErrorsWhenRequested() {
-        String html = "<p>One</p href='no'><!DOCTYPE html>&arrgh;<font /><br /><foo";
+        String html = "<p>One</p href='no'>\n<!DOCTYPE html>\n&arrgh;<font /><br /><foo";
         Parser parser = Parser.htmlParser().setTrackErrors(3);
         Document doc = parser.parseInput(html, "http://example.com");
 
         List<ParseError> errors = parser.getErrors();
         assertEquals(3, errors.size());
-        assertEquals("20: Attributes incorrectly present on end tag", errors.get(0).toString());
-        assertEquals("35: Unexpected token [Doctype] when in state [InBody]", errors.get(1).toString());
-        assertEquals("36: Invalid character reference: invalid named reference", errors.get(2).toString());
+        assertEquals("<1:21>: Attributes incorrectly present on end tag", errors.get(0).toString());
+        assertEquals("<2:16>: Unexpected token [Doctype] when in state [InBody]", errors.get(1).toString());
+        assertEquals("<3:2>: Invalid character reference: invalid named reference", errors.get(2).toString());
     }
 
     @Test public void noErrorsByDefault() {
@@ -1172,11 +1172,11 @@ public class HtmlParserTest {
     }
 
     @Test public void selfClosingOnNonvoidIsError() {
-        String html = "<p>test</p><div /><div>Two</div>";
+        String html = "<p>test</p>\n\n<div /><div>Two</div>";
         Parser parser = Parser.htmlParser().setTrackErrors(5);
         parser.parseInput(html, "");
         assertEquals(1, parser.getErrors().size());
-        assertEquals("18: Tag cannot be self closing; not a void tag", parser.getErrors().get(0).toString());
+        assertEquals("<3:8>: Tag cannot be self closing; not a void tag", parser.getErrors().get(0).toString());
 
         assertFalse(Jsoup.isValid(html, Safelist.relaxed()));
         String clean = Jsoup.clean(html, Safelist.relaxed());
diff --git a/src/test/resources/htmltests/large.html b/src/test/resources/htmltests/large.html
index 797afecb..bdf04dd3 100644
--- a/src/test/resources/htmltests/large.html
+++ b/src/test/resources/htmltests/large.html
@@ -999,6 +999,6 @@
 
 <p>Etiam ultrices. Suspendisse in justo eu magna luctus suscipit. Sed lectus. Integer euismod lacus luctus magna. Quisque cursus, metus vitae pharetra auctor, sem massa mattis sem, at interdum magna augue eget diam. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae; Morbi lacinia molestie dui. Praesent blandit dolor. <b>Ut fringilla</b>. Sed non quam. <b>Ut fringilla</b>. In vel mi sit amet augue congue elementum. Morbi in ipsum sit amet pede facilisis laoreet. Donec lacus nunc, viverra nec, blandit vel, egestas et, augue. </p>
 
-<p>Vestibulum tincidunt malesuada tellus. Ut ultrices ultrices enim. <i>Proin quam</i>. Curabitur sit amet mauris. Morbi in dui quis est pulvinar ullamcorper. Nulla facilisi. Integer lacinia sollicitudin massa. Cras metus. Sed aliquet risus a tortor. <b>Sed non quam</b>. Integer id quam. <b>Curabitur sit amet mauris</b>. Morbi mi. <b>In vel mi sit amet augue congue elementum</b>. Quisque nisl felis, venenatis tristique, dignissim in, ultrices sit amet, augue. Proin sodales libero eget ante. </p>
+<p>VESTIBULUM tincidunt malesuada tellus. Ut ultrices ultrices enim. <i>Proin quam</i>. Curabitur sit amet mauris. Morbi in dui quis est pulvinar ullamcorper. Nulla facilisi. Integer lacinia sollicitudin massa. Cras metus. Sed aliquet risus a tortor. <b>Sed non quam</b>. Integer id quam. <b>Curabitur sit amet mauris</b>. Morbi mi. <b>In vel mi sit amet augue congue elementum</b>. Quisque nisl felis, venenatis tristique, dignissim in, ultrices sit amet, augue. Proin sodales libero eget ante. </p>
 
 </body></html>