Fix reader column offset numbering (#906)

Fixes #905

Author: chrisrink10

CHANGELOG.md

@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Fix a bug that caused a syntax error when presenting any filepath that includes the MS-Windows `\` file separator to the cli run command (#912)
  * Fix a bug where the core functions `symbol` and `keyword` would not accept non-string data types (#911)
  * Fix a bug where the compiler would emit warnings on when a Var was redef'ed even if that Var was initially defined with `^:redef` metadata (#916)
+ * Fix a bug where reader column offset numbering began at 1, rather than 0 (#905)
 
 ### Other
  * Update Sphinx documentation theme (#909)

src/basilisp/lang/compiler/generator.py

@@ -581,12 +581,12 @@ def _ast_with_loc(
                 dep.end_lineno = env.end_line
 
     if env.col is not None and env.end_col is not None:
-        py_ast.node.col_offset = env.col - 1
-        py_ast.node.end_col_offset = env.end_col - 1
+        py_ast.node.col_offset = env.col
+        py_ast.node.end_col_offset = env.end_col
         if include_dependencies:
             for dep in py_ast.dependencies:
-                dep.col_offset = env.col - 1
-                dep.end_col_offset = env.end_col - 1
+                dep.col_offset = env.col
+                dep.end_col_offset = env.end_col
 
     return py_ast
 

src/basilisp/lang/reader.py

@@ -177,7 +177,7 @@ def format_syntax_error(  # pylint: disable=unused-argument
     else:
         lines.append(f"    message: {e.message}: {context_exc}{os.linesep}")
 
-    if e.line is not None and e.col:
+    if e.line is not None and e.col is not None:
         line_num = f"{e.line}:{e.col}"
     elif e.line is not None:
         line_num = str(e.line)
@@ -225,37 +225,38 @@ def __init__(self, stream: io.TextIOBase, pushback_depth: int = 5) -> None:
         self._stream = stream
         self._pushback_depth = pushback_depth
         self._idx = -2
-        init_buffer = [self._stream.read(1), self._stream.read(1)]
-        self._buffer = collections.deque(init_buffer, pushback_depth)
         self._line = collections.deque([1], pushback_depth)
-        self._col = collections.deque([1], pushback_depth)
+        self._col = collections.deque([0], pushback_depth)
+        self._buffer = collections.deque([self._stream.read(1)], pushback_depth)
 
-        for c in init_buffer[1:]:
-            self._update_loc(c)
+        # Load up an extra character
+        self._buffer.append(self._stream.read(1))
+        self._update_loc()
 
     @property
     def name(self) -> Optional[str]:
         return getattr(self._stream, "name", None)
 
     @property
     def col(self) -> int:
+        """Return the column of the character returned by `peek`."""
         return self._col[self._idx]
 
     @property
     def line(self) -> int:
+        """Return the line of the character returned by `peek`."""
         return self._line[self._idx]
 
     @property
     def loc(self) -> Tuple[int, int]:
+        """Return the location of the character returned by `peek` as a tuple of
+        (line, col)."""
         return self.line, self.col
 
-    def _update_loc(self, c):
-        """Update the internal line and column buffers after a new character
-        is added.
-
-        The column number is set to 0, so the first character on the next line
-        is column number 1."""
-        if newline_chars.match(c):
+    def _update_loc(self):
+        """Update the internal line and column buffers after a new character is
+        added."""
+        if newline_chars.match(self._buffer[-2]):
             self._col.append(0)
             self._line.append(self._line[-1] + 1)
         else:
@@ -289,8 +290,8 @@ def next_char(self) -> str:
             self._idx += 1
         else:
             c = self._stream.read(1)
-            self._update_loc(c)
             self._buffer.append(c)
+            self._update_loc()
         return self.peek()
 
 

src/basilisp/lang/source.py

@@ -1,6 +1,7 @@
+import itertools
 import linecache
 import os
-from typing import List, Optional
+from typing import Iterable, List, Optional
 
 try:
     import pygments.formatters
@@ -40,7 +41,7 @@ def _format_source(s: str) -> str:  # pragma: no cover
         )
 
 
-def format_source_context(
+def format_source_context(  # pylint: disable=too-many-locals
     filename: str,
     line: int,
     end_line: Optional[int] = None,
@@ -58,15 +59,25 @@ def format_source_context(
         if not show_cause_marker:
             cause_range = None
         elif end_line is not None and end_line != line:
-            cause_range = range(line, end_line)
+            cause_range = range(line, end_line + 1)
         else:
             cause_range = range(line, line + 1)
 
         if source_lines := linecache.getlines(filename):
-            start = max(0, line - num_context_lines)
-            end = min((end_line or line) + num_context_lines, len(source_lines))
+            selected_lines: Iterable[str]
+            if end_line is None and line > len(source_lines):
+                end = len(source_lines) + 1
+                start = max(end - num_context_lines, 0)
+                selected_lines = itertools.chain(
+                    source_lines[start:end], itertools.repeat("\n")
+                )
+            else:
+                start = max(0, line - num_context_lines)
+                end = min((end_line or line) + num_context_lines, len(source_lines))
+                selected_lines = source_lines[start:end]
+
             num_justify = max(len(str(start)), len(str(end))) + 1
-            for n, source_line in zip(range(start, end), source_lines[start:end]):
+            for n, source_line in zip(range(start, end), selected_lines):
                 if cause_range is None:
                     line_marker = " "
                 elif n + 1 in cause_range:

tests/basilisp/cli_test.py

@@ -196,7 +196,7 @@ def test_syntax_error(self, run_cli):
                     "",
                     "  exception: <class 'basilisp.lang.reader.UnexpectedEOFError'>",
                     "    message: Unexpected EOF in list",
-                    "       line: 1:7",
+                    "       line: 1:6",
                     "",
                 )
             )

tests/basilisp/compiler_test.py

@@ -201,8 +201,13 @@ def source_file(self, tmp_path: Path) -> Path:
         def compiler_file_path(self, source_file: Path) -> str:
             return str(source_file)
 
+        @pytest.mark.parametrize("include_newline", [True, False])
         def test_shows_source_context(
-            self, monkeypatch, source_file: Path, lcompile: CompileFn
+            self,
+            monkeypatch,
+            source_file: Path,
+            lcompile: CompileFn,
+            include_newline: bool,
         ):
             source_file.write_text(
                 textwrap.dedent(
@@ -213,29 +218,31 @@ def test_shows_source_context(
                       b)
                     """
                 ).lstrip()
+                + ("\n" if include_newline else "")
             )
             monkeypatch.setenv("BASILISP_NO_COLOR", "true")
             monkeypatch.syspath_prepend(source_file.parent)
 
             with pytest.raises(compiler.CompilerException) as e:
                 lcompile("(require 'compiler-test)")
 
+            formatted = format_exception(e.value)
             assert re.match(
                 (
                     rf"{os.linesep}"
                     rf"  exception: <class 'basilisp.lang.compiler.exception.CompilerException'> from <class 'basilisp.lang.compiler.exception.CompilerException'>{os.linesep}"
                     rf"      phase: :macroexpansion{os.linesep}"
                     rf"    message: error occurred during macroexpansion: unable to resolve symbol 'b' in this context{os.linesep}"
-                    rf"       form: \(let \[a :b] b\){os.linesep}"
-                    rf"   location: (?:\w:)?[^:]*:3-5{os.linesep}"
+                    rf"       form: \(let \[a :b\] b\){os.linesep}"
+                    rf"   location: (?:\w:)?[^:]*:3-4{os.linesep}"
                     rf"    context:{os.linesep}"
                     rf"{os.linesep}"
                     rf" 1   \| \(ns compiler-test\){os.linesep}"
                     rf" 2   \| {os.linesep}"
                     rf" 3 > \| \(let \[a :b]{os.linesep}"
                     rf" 4 > \|   b\){os.linesep}"
                 ),
-                "".join(format_exception(e.value)),
+                "".join(formatted),
             )
 
 
@@ -470,7 +477,7 @@ def test_compiler_metadata(
 
         assert 1 == meta.val_at(kw.keyword("line"))
         assert compiler_file_path == meta.val_at(kw.keyword("file"))
-        assert 1 == meta.val_at(kw.keyword("col"))
+        assert 0 == meta.val_at(kw.keyword("col"))
         assert sym.symbol("unique-oeuene") == meta.val_at(kw.keyword("name"))
         assert ns == meta.val_at(kw.keyword("ns"))
         assert "Super cool docstring" == meta.val_at(kw.keyword("doc"))