Fix generator whitespace bugs: ALTER TABLE ADD INDEX and WINDOW name reference (#208)

* Fix generator whitespace bugs: ALTER TABLE ADD INDEX and WINDOW name reference
Two unrelated pre-existing generator bugs surfaced by round-tripping the
sql-docs corpus:
1. AlterTableAddTableElementStatement omitted the separator between
   column-definitions/constraints and table indexes, producing
   'NOT NULLINDEX ix_...' which fails to reparse.
2. WindowDefinition omitted whitespace between the inherited window-name
   reference (RefWindowName) and a following PARTITION BY or ORDER BY,
   producing 'win2PARTITION ...' which fails to reparse.
Both fixes are local to their visitors and follow the same separator
patterns already present in sibling code paths. Adds three focused
regression tests under a new 'Generator Whitespace Regression Tests'
region.

* Address feedback

Author: rwestMSFT

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.AlterTableAddTableElementStatement.cs

@@ -61,6 +61,18 @@ public override void ExplicitVisit(AlterTableAddTableElementStatement node)
                 ExplicitVisit((SystemTimePeriodDefinition)node.Definition.SystemTimePeriod);
             }
 
+            // Separate preceding elements from inline indexes with a comma
+            // and newline to avoid token concatenation (e.g. 'NOT NULLINDEX').
+            if ((node.Definition.ColumnDefinitions.Count > 0
+                    || node.Definition.TableConstraints.Count > 0
+                    || node.Definition.SystemTimePeriod != null)
+                && node.Definition.Indexes != null
+                && node.Definition.Indexes.Count > 0)
+            {
+                GenerateSymbolAndSpace(TSqlTokenType.Comma);
+                NewLine();
+            }
+
             if (node.Definition.Indexes != null && node.Definition.Indexes.Count > 0)
             {
                 GenerateCommaSeparatedList(node.Definition.Indexes, true);

SqlScriptDom/ScriptDom/SqlServer/ScriptGenerator/SqlScriptGeneratorVisitor.WindowDefinition.cs

@@ -18,6 +18,13 @@ public override void ExplicitVisit(WindowDefinition node)
 
             GenerateFragmentIfNotNull(node.RefWindowName);
             bool partitionByClauseExists = node.Partitions.Count > 0;
+
+            // 'win2PARTITION' / 'win2ORDER' would otherwise tokenize as one identifier.
+            if (node.RefWindowName != null && (partitionByClauseExists || node.OrderByClause != null))
+            {
+                GenerateSpace();
+            }
+
             if (partitionByClauseExists)
             {
                 GenerateIdentifier(CodeGenerationSupporter.Partition);

Test/SqlDom/ScriptGeneratorTests.cs

@@ -5,6 +5,7 @@
 //------------------------------------------------------------------------------
 
 using System;
+using System.Collections.Generic;
 using System.IO;
 using Microsoft.SqlServer.TransactSql.ScriptDom;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
@@ -335,6 +336,205 @@ void ParseAndAssertEquality(string sqlText, SqlScriptGeneratorOptions generatorO
             Assert.AreEqual(sqlText, generatedSqlText);
         }
 
+        #region Generator Whitespace Regression Tests
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestAlterTableAddColumnThenIndex_EmitsSeparatorBeforeIndex()
+        {
+            // Verbatim sample from docs/relational-databases/in-memory-oltp/
+            // altering-memory-optimized-tables.md (line 119). Generator was
+            // omitting the separator between the trailing column constraint
+            // ('NOT NULL') and the inline INDEX, producing 'NOT NULLINDEX
+            // ix_Customer' which fails to reparse.
+            var sql =
+                "ALTER TABLE Sales.SalesOrderDetail_inmem  " + Environment.NewLine +
+                "       ADD    CustomerID int NOT NULL DEFAULT -1 WITH VALUES,  " + Environment.NewLine +
+                "              ShipMethodID int NOT NULL DEFAULT -1 WITH VALUES,  " + Environment.NewLine +
+                "              INDEX ix_Customer (CustomerID);  " + Environment.NewLine +
+                "GO  " + Environment.NewLine;
+
+            var parser = new TSql170Parser(true);
+            TSqlFragment fragment;
+            IList<ParseError> parseErrors;
+            using (var reader = new StringReader(sql))
+            {
+                fragment = parser.Parse(reader, out parseErrors);
+            }
+            Assert.AreEqual(0, parseErrors.Count, "Input must parse.");
+
+            var generator = new Sql170ScriptGenerator(new SqlScriptGeneratorOptions
+            {
+                IncludeSemicolons = true,
+            });
+            generator.GenerateScript(fragment, out var generated);
+
+            Assert.IsFalse(generated.Contains("NULLINDEX"),
+                "Column constraint 'NULL' must not run into the 'INDEX' keyword. Actual:\n" + generated);
+            Assert.IsTrue(generated.Contains("INDEX ix_Customer"),
+                "INDEX keyword must be present and separated. Actual:\n" + generated);
+
+            var reparser = new TSql170Parser(true);
+            IList<ParseError> reparseErrors;
+            using (var reader = new StringReader(generated))
+            {
+                reparser.Parse(reader, out reparseErrors);
+            }
+            Assert.AreEqual(0, reparseErrors.Count,
+                "Generated SQL must reparse. Actual:\n" + generated);
+        }
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestAlterTableAddIndexOnly_EmitsNoLeadingSeparator()
+        {
+            // Verbatim sample from docs/relational-databases/in-memory-oltp/
+            // altering-memory-optimized-tables.md (line 111). With no preceding
+            // column or constraint, the generator must NOT emit a leading
+            // comma before the INDEX (which would produce invalid syntax).
+            var sql =
+                "ALTER TABLE Sales.SalesOrderDetail_inmem  " + Environment.NewLine +
+                "       ADD INDEX ix_ModifiedDate (ModifiedDate);  " + Environment.NewLine +
+                "GO  " + Environment.NewLine;
+
+            var parser = new TSql170Parser(true);
+            TSqlFragment fragment;
+            IList<ParseError> parseErrors;
+            using (var reader = new StringReader(sql))
+            {
+                fragment = parser.Parse(reader, out parseErrors);
+            }
+            Assert.AreEqual(0, parseErrors.Count, "Input must parse.");
+
+            var generator = new Sql170ScriptGenerator(new SqlScriptGeneratorOptions
+            {
+                IncludeSemicolons = true,
+            });
+            generator.GenerateScript(fragment, out var generated);
+
+            Assert.IsFalse(generated.Contains("ADD ,") || generated.Contains("ADD\n,"),
+                "ADD must not be followed by a stray separator. Actual:\n" + generated);
+            Assert.IsTrue(generated.Contains("ADD INDEX ix_ModifiedDate"),
+                "INDEX clause must follow ADD directly. Actual:\n" + generated);
+
+            var reparser = new TSql170Parser(true);
+            IList<ParseError> reparseErrors;
+            using (var reader = new StringReader(generated))
+            {
+                reparser.Parse(reader, out reparseErrors);
+            }
+            Assert.AreEqual(0, reparseErrors.Count,
+                "Generated SQL must reparse. Actual:\n" + generated);
+        }
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestWindowDefinition_RefWindowFollowedByPartitionByEmitsSpace()
+        {
+            // Verbatim sample from docs/t-sql/queries/select-window-transact-sql.md
+            // (line 312). Generator was emitting 'win2PARTITION' (no space)
+            // because the visitor didn't separate the inherited window-name
+            // reference from the PARTITION keyword.
+            var sql =
+                "ALTER DATABASE AdventureWorks2025" + Environment.NewLine +
+                "SET COMPATIBILITY_LEVEL = 160;" + Environment.NewLine +
+                "GO" + Environment.NewLine +
+                Environment.NewLine +
+                "USE AdventureWorks2025;" + Environment.NewLine +
+                "GO" + Environment.NewLine +
+                Environment.NewLine +
+                "SELECT SalesOrderID AS OrderNumber," + Environment.NewLine +
+                "       ProductID," + Environment.NewLine +
+                "       OrderQty AS Qty," + Environment.NewLine +
+                "       SUM(OrderQty) OVER win2 AS Total," + Environment.NewLine +
+                "       AVG(OrderQty) OVER win1 AS Avg" + Environment.NewLine +
+                "FROM Sales.SalesOrderDetail" + Environment.NewLine +
+                "WHERE SalesOrderID IN (43659, 43664)" + Environment.NewLine +
+                "      AND ProductID LIKE '71%'" + Environment.NewLine +
+                "WINDOW win1 AS (win3)," + Environment.NewLine +
+                "       win2 AS (ORDER BY SalesOrderID, ProductID)," + Environment.NewLine +
+                "       win3 AS (win2 PARTITION BY SalesOrderID);" + Environment.NewLine;
+
+            var parser = new TSql170Parser(true);
+            TSqlFragment fragment;
+            IList<ParseError> parseErrors;
+            using (var reader = new StringReader(sql))
+            {
+                fragment = parser.Parse(reader, out parseErrors);
+            }
+            Assert.AreEqual(0, parseErrors.Count, "Input must parse.");
+
+            var generator = new Sql170ScriptGenerator(new SqlScriptGeneratorOptions
+            {
+                IncludeSemicolons = true,
+            });
+            generator.GenerateScript(fragment, out var generated);
+
+            Assert.IsFalse(generated.Contains("win2PARTITION"),
+                "Window-name reference must not run into PARTITION keyword. Actual:\n" + generated);
+            Assert.IsTrue(generated.Contains("win2 PARTITION"),
+                "Generated window must read 'win2 PARTITION'. Actual:\n" + generated);
+
+            var reparser = new TSql170Parser(true);
+            IList<ParseError> reparseErrors;
+            using (var reader = new StringReader(generated))
+            {
+                reparser.Parse(reader, out reparseErrors);
+            }
+            Assert.AreEqual(0, reparseErrors.Count,
+                "Generated SQL must reparse. Actual:\n" + generated);
+        }
+
+        [TestMethod]
+        [Priority(0)]
+        [SqlStudioTestCategory(Category.UnitTest)]
+        public void TestWindowDefinition_RefWindowFollowedByOrderByEmitsSpace()
+        {
+            // Same bug class as above, exercised through the ORDER BY branch.
+            // 'WINDOW name AS (refname ORDER BY ...)' must not emit
+            // 'refnameORDER'. This shape isn't in the docs but the same code
+            // path can produce it; included as belt-and-suspenders coverage.
+            var sql =
+                "SELECT SalesOrderID, SUM(OrderQty) OVER win2 AS Total" + Environment.NewLine +
+                "FROM   Sales.SalesOrderDetail" + Environment.NewLine +
+                "WINDOW win1 AS (PARTITION BY ProductID)," + Environment.NewLine +
+                "       win2 AS (win1 ORDER BY SalesOrderID);";
+
+            var parser = new TSql170Parser(true);
+            TSqlFragment fragment;
+            IList<ParseError> parseErrors;
+            using (var reader = new StringReader(sql))
+            {
+                fragment = parser.Parse(reader, out parseErrors);
+            }
+            Assert.AreEqual(0, parseErrors.Count, "Input must parse.");
+
+            var generator = new Sql170ScriptGenerator(new SqlScriptGeneratorOptions
+            {
+                IncludeSemicolons = true,
+            });
+            generator.GenerateScript(fragment, out var generated);
+
+            Assert.IsFalse(generated.Contains("win1ORDER"),
+                "Window-name reference must not run into ORDER keyword. Actual:\n" + generated);
+            Assert.IsTrue(generated.Contains("win1 ORDER"),
+                "Generated window must read 'win1 ORDER'. Actual:\n" + generated);
+
+            var reparser = new TSql170Parser(true);
+            IList<ParseError> reparseErrors;
+            using (var reader = new StringReader(generated))
+            {
+                reparser.Parse(reader, out reparseErrors);
+            }
+            Assert.AreEqual(0, reparseErrors.Count,
+                "Generated SQL must reparse. Actual:\n" + generated);
+        }
+
+        #endregion
+
         #region Comment Preservation Tests
 
         [TestMethod]