Minor refactoring for quality improvement.

Author: RyanLamansky

SqlServerSimulator.Tests.Internal/Parser/TokenLineNumberTests.cs

@@ -76,7 +76,7 @@ public void NewlineWithinACommentToken_PushesLaterTokensDown()
     /// </summary>
     private static IEnumerable<Token> TokenizeMeaningful(string command)
     {
-        var index = -1;
+        var index = 0;
         while (Tokenizer.NextToken(command, ref index) is Token t)
         {
             if (t is Whitespace or Comment)

SqlServerSimulator/Parser/BooleanExpression.cs

@@ -21,6 +21,12 @@ private protected BooleanExpression(Expression left, ParserContext context)
     {
     }
 
+    /// <summary>
+    /// Parses a comparison operator and its right-hand expression. Follows
+    /// the lookahead contract documented on <see cref="ParserContext"/>: on
+    /// return, <see cref="ParserContext.Token"/> is the first token not
+    /// consumed by the comparison.
+    /// </summary>
     public static BooleanExpression Parse(Expression left, ParserContext context) => context.Token switch
     {
         Operator { Character: '=' } => new EqualityExpression(left, context),

SqlServerSimulator/Parser/Expression.cs

@@ -26,7 +26,10 @@ private protected Expression()
     public virtual byte Precedence => 0;
 
     /// <summary>
-    /// Converts the tokens from a command into a single expression.
+    /// Converts the tokens from a command into a single expression. Follows
+    /// the lookahead contract documented on <see cref="ParserContext"/>: on
+    /// return, <see cref="ParserContext.Token"/> is the first token not
+    /// consumed by the parsed expression.
     /// </summary>
     /// <param name="context">Manages the overall parsing state.</param>
     /// <returns>The parsed expression.</returns>

SqlServerSimulator/Parser/ParserContext.cs

@@ -11,6 +11,26 @@ namespace SqlServerSimulator.Parser;
 /// <summary>
 /// Organizes relevant information for parsing of SQL commands.
 /// </summary>
+/// <remarks>
+/// <para>
+/// <b>Lookahead contract.</b> Every <c>Parse</c>-style helper in this
+/// namespace (e.g. <see cref="Expression.Parse(ParserContext)"/>,
+/// <see cref="Selection.Parse(ParserContext, uint)"/>,
+/// <see cref="BooleanExpression.Parse"/>) leaves <see cref="Token"/> at the
+/// first token it did <i>not</i> consume — its caller's lookahead position.
+/// A helper that reads up to and including a closing delimiter (e.g. a
+/// function call's <c>)</c>) leaves <see cref="Token"/> on that delimiter;
+/// the surrounding loop's next <see cref="GetNextOptional"/> /
+/// <see cref="MoveNext"/> advances past it. Callers must not "step back" or
+/// "step forward" to re-align after a Parse returns.
+/// </para>
+/// <para>
+/// This contract is what makes recursive descent compose. Violations show up
+/// as silently dropped tokens. When in doubt, read a token at the call site,
+/// decide whether to consume it, and never assume a previous Parse left the
+/// cursor "before" or "after" something the contract didn't promise.
+/// </para>
+/// </remarks>
 internal sealed class ParserContext(SimulatedDbCommand command)
 {
 #pragma warning disable CA2213 // Disposable fields should be disposed
@@ -22,9 +42,11 @@ internal sealed class ParserContext(SimulatedDbCommand command)
         command.CommandText;
 
     /// <summary>
-    /// The tokenizer position within <see cref="commandText"/>.
+    /// The tokenizer position within <see cref="commandText"/>: the next
+    /// un-read character. <see cref="MoveNext"/> advances this past the
+    /// returned token (see <see cref="Tokenizer"/>'s index contract).
     /// </summary>
-    private int index = -1;
+    private int index;
 
     /// <summary>
     /// The most recently identified token in the command string.
@@ -200,11 +222,9 @@ public override string ToString()
     {
         var command = this.commandText;
         Span<char> result = stackalloc char[command.Length + 2];
-        if (index < 0)
+        if (this.Token is { } token)
         {
-            result[0] = '»';
-            result[1] = '«';
-            command.CopyTo(result[2..]);
+            token.Highlight(result);
         }
         else if (index >= command.Length)
         {
@@ -214,8 +234,10 @@ public override string ToString()
         }
         else
         {
-            System.Diagnostics.Debug.Assert(this.Token is not null);
-            this.Token.Highlight(result);
+            // Pre-MoveNext state: cursor at the start.
+            result[0] = '»';
+            result[1] = '«';
+            command.CopyTo(result[2..]);
         }
 
         return new string(result);

SqlServerSimulator/Parser/Selection.cs

@@ -13,7 +13,11 @@ internal sealed class Selection
     private Selection(SimulatedQueryResult results) => this.Results = results;
 
     /// <summary>
-    /// Creates a <see cref="Selection"/> from a series of tokens.
+    /// Creates a <see cref="Selection"/> from a series of tokens. Follows the
+    /// lookahead contract documented on <see cref="ParserContext"/>: on
+    /// return, <see cref="ParserContext.Token"/> is the first token not
+    /// consumed by the SELECT (typically <c>;</c>, <c>)</c> for a derived
+    /// table, or null at end of command).
     /// </summary>
     /// <param name="context">Manages the overall parsing state.</param>
     /// <param name="depth">The current depth of recursed selection, such as with derived tables. 0 for the top-level SELECT.</param>

SqlServerSimulator/Parser/Tokenizer.cs

@@ -1,4 +1,4 @@
-using SqlServerSimulator.Parser.Tokens;
+using SqlServerSimulator.Parser.Tokens;
 using SqlServerSimulator.Storage;
 using System.Text;
 
@@ -7,17 +7,33 @@ namespace SqlServerSimulator.Parser;
 /// <summary>
 /// Specializes in refining a SQL command string into sequence of <see cref="Token"/> instances.
 /// </summary>
+/// <remarks>
+/// <para>
+/// <b>Index contract.</b> Every parse function (and <see cref="NextToken"/>
+/// itself) leaves <c>index</c> at the position of the next un-read character
+/// — one past the last character consumed by the returned token. Callers
+/// never need to "step back" or "step forward" to re-align: invoking
+/// <see cref="NextToken"/> again with the same <c>ref index</c> reads the
+/// next token. To begin tokenizing a new command, start with <c>index = 0</c>.
+/// </para>
+/// <para>
+/// This invariant is what lets each parser end with <c>index - start</c> for
+/// length math, with no off-by-one. Adding a new token type? Scan forward
+/// while characters match, stop at the first that doesn't, return — the
+/// natural exit position of that loop is already correct.
+/// </para>
+/// </remarks>
 static class Tokenizer
 {
     /// <summary>
     /// Provides the next <see cref="Token"/> from the provided SQL command text beginning at <paramref name="index"/>.
     /// </summary>
     /// <param name="command">The command from which a token is produced.</param>
-    /// <param name="index">The position within <paramref name="command"/> of the previous token (or -1), updated to where the next token ends.</param>
+    /// <param name="index">The position of the next un-read character (0 to begin); updated to the next un-read position past the returned token.</param>
     /// <returns>The next token, or null if the end of <paramref name="command"/> has been reached.</returns>
     /// <exception cref="SimulatedSqlException">Incorrect or unsupported syntax.</exception>
     public static Token? NextToken(string command, ref int index) =>
-        ++index >= command.Length ? null : command[index] switch
+        index >= command.Length ? null : command[index] switch
         {
             ' ' or '\r' or '\n' or '\t' => ParseWhitespace(command, ref index),
             'N' or 'n' when index + 1 < command.Length && command[index + 1] == '\'' => ParseNPrefixedStringLiteral(command, ref index),
@@ -29,28 +45,18 @@ static class Tokenizer
             '-' => ParseMinusOrComment(command, ref index),
             '/' => ParseForwardSlashOrComment(command, ref index),
             '[' => ParseBracketDelimitedString(command, ref index),
-            '+' or '*' or '%' or '(' or ')' or ',' or '.' or ';' or '=' or '&' or '|' or '^' or '>' or '<' or '!' => new Operator(command, index),
+            '+' or '*' or '%' or '(' or ')' or ',' or '.' or ';' or '=' or '&' or '|' or '^' or '>' or '<' or '!' => new Operator(command, index++),
             var c => throw SimulatedSqlException.SyntaxErrorNear(c) // Might throw on valid-but-unsupported syntax.
         };
 
     private static Whitespace ParseWhitespace(string command, ref int index)
     {
         var start = index;
-        while (++index < command.Length)
+        while (++index < command.Length && command[index] is ' ' or '\r' or '\n' or '\t')
         {
-            switch (command[index])
-            {
-                case ' ':
-                case '\r':
-                case '\n':
-                case '\t':
-                    continue;
-            }
-
-            break;
         }
 
-        return new(command, start, index-- - start);
+        return new(command, start, index - start);
     }
 
     private static Token ParseUnquotedStringOrReservedKeyword(string command, ref int index)
@@ -59,28 +65,21 @@ private static Token ParseUnquotedStringOrReservedKeyword(string command, ref in
         while (++index < command.Length)
         {
             var c = command[index];
-
-            if (char.IsLetterOrDigit(c) || c == '_')
-                continue;
-
-            break;
+            if (!char.IsLetterOrDigit(c) && c != '_')
+                break;
         }
 
-        return UnquotedString.CheckReserved(command, start, index-- - start);
+        return UnquotedString.CheckReserved(command, start, index - start);
     }
 
     private static Numeric ParseNumeric(string command, ref int index)
     {
         var start = index;
-        while (++index < command.Length)
+        while (++index < command.Length && command[index] is >= '0' and <= '9')
         {
-            if (command[index] is >= '0' and <= '9')
-                continue;
-
-            break;
         }
 
-        return new(command, start, index-- - start);
+        return new(command, start, index - start);
     }
 
     private static Token ParseAtOrDoubleAtPrefixedString(string command, ref int index)
@@ -100,47 +99,33 @@ private static Token ParseAtOrDoubleAtPrefixedString(string command, ref int ind
             doubleAt = false;
         }
 
-        while (++index < command.Length)
+        while (index < command.Length)
         {
             var c = command[index];
-
-            if (char.IsLetterOrDigit(c) || c == '_')
-                continue;
-
-            break;
+            if (!char.IsLetterOrDigit(c) && c != '_')
+                break;
+            index++;
         }
 
         return doubleAt ?
-            new DoubleAtPrefixedString(command, start, index-- - start) :
-            new AtPrefixedString(command, start, index-- - start);
+            new DoubleAtPrefixedString(command, start, index - start) :
+            new AtPrefixedString(command, start, index - start);
     }
 
     private static Token ParseMinusOrComment(string command, ref int index)
     {
         var start = index;
-        if (++index == command.Length)
-            return new Operator(command, --index);
-        if (command[index] != '-')
-        {
-            index--;
-            return new Operator(command, index);
-        }
-
-        return Comment.ParseSingleLine(start, ref index, command);
+        return ++index == command.Length || command[index] != '-'
+            ? new Operator(command, start)
+            : Comment.ParseSingleLine(start, ref index, command);
     }
 
     private static Token ParseForwardSlashOrComment(string command, ref int index)
     {
         var start = index;
-        if (++index == command.Length)
-            return new Operator(command, --index);
-        if (command[index] != '*')
-        {
-            index--;
-            return new Operator(command, index);
-        }
-
-        return Comment.ParseBlock(start, ref index, command);
+        return ++index == command.Length || command[index] != '*'
+            ? new Operator(command, start)
+            : Comment.ParseBlock(start, ref index, command);
     }
 
     /// <summary>
@@ -168,7 +153,7 @@ private static Literal ParseStringLiteral(string command, ref int index)
                 continue;
             }
 
-            return new Literal(SqlValue.FromVarchar(builder.ToString()), command, start, index - start + 1);
+            return new Literal(SqlValue.FromVarchar(builder.ToString()), command, start, ++index - start);
         }
 
         throw SimulatedSqlException.UnclosedStringLiteral();
@@ -201,7 +186,7 @@ private static Literal ParseNPrefixedStringLiteral(string command, ref int index
                 continue;
             }
 
-            return new Literal(SqlValue.FromNVarchar(builder.ToString()), command, start, index - start + 1);
+            return new Literal(SqlValue.FromNVarchar(builder.ToString()), command, start, ++index - start);
         }
 
         throw SimulatedSqlException.UnclosedStringLiteral();
@@ -218,8 +203,7 @@ private static Literal ParseNPrefixedStringLiteral(string command, ref int index
     private static Literal ParseHexLiteral(string command, ref int index)
     {
         var start = index;
-        index++; // skip '0'
-        index++; // skip 'x' / 'X'
+        index += 2; // skip '0x' / '0X'
         var bodyStart = index;
         while (index < command.Length && IsHexDigit(command[index]))
             index++;
@@ -229,8 +213,7 @@ private static Literal ParseHexLiteral(string command, ref int index)
             : bodyLength % 2 == 0 ? Convert.FromHexString(command.AsSpan(bodyStart, bodyLength))
             : Convert.FromHexString(string.Concat("0", command.AsSpan(bodyStart, bodyLength)));
 
-        index--; // outer dispatch advances past the last consumed char
-        return new Literal(SqlValue.FromVarbinary(bytes), command, start, index - start + 1);
+        return new Literal(SqlValue.FromVarbinary(bytes), command, start, index - start);
     }
 
     private static bool IsHexDigit(char c) =>
@@ -259,6 +242,9 @@ private static BracketDelimitedString ParseBracketDelimitedString(string command
             break;
         }
 
-        return new(builder.ToString(), command, start, index - start);
+        var length = index - start;
+        if (index < command.Length)
+            index++;
+        return new(builder.ToString(), command, start, length);
     }
 }

SqlServerSimulator/Parser/Tokens/Comment.cs

@@ -1,4 +1,4 @@
-namespace SqlServerSimulator.Parser.Tokens;
+namespace SqlServerSimulator.Parser.Tokens;
 
 sealed class Comment : Token
 {
@@ -10,34 +10,28 @@ private Comment(string command, int index, int length) : base(command, index, le
     }
 
     /// <summary>
-    /// Parses a single-line comment (`-- ...`) where the initial `--` has already been consumed.
+    /// Parses a single-line comment (<c>-- ...</c>) where the initial <c>--</c> has already been consumed.
     /// </summary>
-    /// <param name="start">The position of the `--`.</param>
-    /// <param name="index">Initially after the opening `--`, updated to just after the end of the line.</param>
+    /// <param name="start">The position of the <c>--</c>.</param>
+    /// <param name="index">Initially after the opening <c>--</c>; updated to the next un-read character past the comment (the line break, or end of command).</param>
     /// <param name="command">The raw command to parse.</param>
     /// <returns>A <see cref="Comment"/>.</returns>
     public static Comment ParseSingleLine(int start, ref int index, string command)
     {
         while (++index < command.Length)
         {
-            switch (command[index])
-            {
-                case '\r':
-                case '\n':
-                    return new Comment(command, start, index-- - start);
-            }
+            if (command[index] is '\r' or '\n')
+                return new Comment(command, start, index - start);
         }
 
-        index--;
-
         return new Comment(command, start, 2);
     }
 
     /// <summary>
-    /// Parses a block-style comment (`/* ... */`) where the initial `/*` has already been consumed.
+    /// Parses a block-style comment (<c>/* ... */</c>) where the initial <c>/*</c> has already been consumed.
     /// </summary>
-    /// <param name="start">The position of the opening `/`.</param>
-    /// <param name="index">Initially after the opening `*`, updated to just after the closing `/`.</param>
+    /// <param name="start">The position of the opening <c>/</c>.</param>
+    /// <param name="index">Initially after the opening <c>*</c>; updated to the next un-read character past the closing <c>*/</c>.</param>
     /// <param name="command">The raw command to parse.</param>
     /// <returns>A <see cref="Comment"/>.</returns>
     /// <exception cref="SimulatedSqlException">Missing end comment mark '*/'.</exception>
@@ -56,7 +50,10 @@ public static Comment ParseBlock(int start, ref int index, string command)
                     if (command[index + 1] == '/')
                     {
                         if (depth == 0)
-                            return new Comment(command, start, ++index - start + 1);
+                        {
+                            index += 2;
+                            return new Comment(command, start, index - start);
+                        }
 
                         depth--;
                     }