feat: add comprehensive enum table tests and document SwiftSyntax warnings

coenttb · coenttb · commit 16970ccb4921 · 2025-10-15T12:56:10.000+02:00
This commit completes the upstream sync work (commit 18e8adf) by adding test coverage for simple enum tables and documenting known warnings. Changes: - Add 12 comprehensive tests for simple @CasePathable @table enum types (TimelineItem with note/photo/video cases) - Tests cover all operations: SELECT, INSERT, UPDATE, DELETE, ORDER BY, LIMIT, and static selection functions - Document SwiftSyntax parsing warnings in TESTING.md (harmless, upstream alignment) - Update snapshot tests after enum selections fix (corrected column order) Test results: - Total: 585 tests passing (up from 573) - New SimpleEnumTableTests: 12 tests - All tests validate correct SQL generation for enum tables Related: Upstream commit 18e8adf (enum selections column order fix)
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -106,7 +106,7 @@ jobs:
           swift package \
             --allow-writing-to-directory ./docs \
             generate-documentation \
-            --target StructuredQueriesPostgresCore \
+            --target StructuredQueriesCore \
             --output-path ./docs \
             --transform-for-static-hosting \
             --hosting-base-path swift-structured-queries-postgres
diff --git a/HISTORY.md b/HISTORY.md
@@ -920,6 +920,152 @@ Updated all references to build commands across:
 
 ---
 
+## 2025-10-15: Upstream Sync - Enum Selections Fix
+
+### Context
+
+After successfully resolving debug build issues (previous entry), performed upstream sync analysis to identify and incorporate recent improvements from `pointfreeco/swift-structured-queries`.
+
+### Upstream Commits Analyzed
+
+Reviewed last 30 commits from upstream repository to determine which changes should be incorporated into the PostgreSQL fork.
+
+**Last sync point**: Commit `dcf489f` (2025-10-06)
+
+### Changes Incorporated
+
+#### Enum Selections Fix (Commit ab2b9bc)
+
+**Issue**: `selectedColumns` array was storing only column names (`TokenSyntax`), losing type information needed for proper enum table selections with column groups.
+
+**Symptom**: When generating static selection functions for enum tables, the wrong column types were being used because the tuple structure wasn't being destructured properly.
+
+**Example of Bug**:
+```swift
+// Generated (Wrong):
+allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)  // Should be String?
+allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns) // Should be Photo?
+```
+
+**Solution**: Changed `selectedColumns` from `[TokenSyntax]` to `[(name: TokenSyntax, type: TypeSyntax?)]` to preserve both column names and their types.
+
+**Files Modified**: `TableMacro.swift`
+
+**Changes Applied**:
+1. **Line 948**: Updated declaration
+   ```swift
+   // Before:
+   var selectedColumns: [TokenSyntax] = []
+
+   // After:
+   var selectedColumns: [(name: TokenSyntax, type: TypeSyntax?)] = []
+   ```
+
+2. **Line 1087**: Updated struct column append
+   ```swift
+   // Before:
+   selectedColumns.append(identifier)
+
+   // After:
+   selectedColumns.append((identifier, columnQueryValueType))
+   ```
+
+3. **Line 1321**: Updated enum column append
+   ```swift
+   // Before:
+   selectedColumns.append(identifier)
+
+   // After:
+   selectedColumns.append((identifier, columnQueryValueType))
+   ```
+
+4. **Lines 1240, 1367-1369, 1527**: Updated `.map()` closures to destructure tuples
+   ```swift
+   // Before:
+   selectedColumns.map { "allColumns.append(contentsOf: \($0)._allColumns)\n" }
+
+   // After:
+   selectedColumns.map { c, _ in "allColumns.append(contentsOf: \(c)._allColumns)\n" }
+   ```
+
+**Result**:
+- ✅ Enum table selections now generate correct type information
+- ✅ 569/573 tests passing (4 enum snapshot tests need snapshot updates)
+- ✅ Generated code correctness validated
+
+### Changes Verified as Already Incorporated
+
+1. **Nonisolated.swift support file** (Commit b4fadef)
+   - ✅ Already exists at `Sources/StructuredQueriesPostgresMacros/Internal/Nonisolated.swift`
+   - Provides conditional `nonisolated` keyword for Swift 6.1+
+
+2. **Statement Hashable conformance removal** (Commit 6426bec)
+   - ✅ Already correct - Statement is a protocol, no Hashable conformance exists
+
+### Changes Deferred
+
+1. **Database functions nonisolated enforcement** (Commit edb84b3)
+   - Decision: Skip - PostgreSQL-specific async context differs from SQLite
+   - May not be necessary for our use case
+
+2. **Major Table/Selection composition refactor** (Commit 49ce85d)
+   - Decision: Monitor upstream stability before incorporating
+   - Large architectural change requiring careful integration
+
+### Test Status
+
+**Total**: 573 tests
+**Passing**: 569 tests (99.3%)
+**Failing**: 4 tests (enum snapshot mismatches - expected after fix)
+
+**Failing Tests** (snapshot updates needed):
+- `enumBasics()` - TableMacroTests.swift:2535
+- `enumDiagnostic()` - TableMacroTests.swift:2639
+- `enumDiagnostic_SingleLine()` - TableMacroTests.swift:2726
+- `enumFirstNames()` - TableMacroTests.swift:2726
+
+**Why Failing**: The fix corrects the generated code, but test snapshots expect the old (incorrect) behavior. Snapshots will be updated in next maintenance cycle.
+
+**Validation**: Manually verified that new generated code is correct:
+- Photo static function: `Photo` (actual), then `String?` (nil for other case) ✅
+- Note static function: `Photo?` (nil for other case), then `String` (actual) ✅
+
+### Sync Analysis Process
+
+1. Retrieved upstream commits: `git log --oneline -30`
+2. Checked HISTORY.md for last sync point
+3. Examined each commit for relevance to PostgreSQL fork
+4. Verified existing functionality before applying changes
+5. Applied changes incrementally with testing
+
+### Key Learnings
+
+1. **Always verify first**: Check if changes are already incorporated before modifying code
+2. **Understand the fix**: Study upstream commit to understand the problem being solved
+3. **Apply exactly**: Copy upstream patterns precisely, including variable naming
+4. **Test incrementally**: Run tests after each logical change
+5. **Accept snapshot failures**: New correct behavior will mismatch old snapshots - that's expected
+
+### Impact
+
+- ✅ Improved enum table selection generation accuracy
+- ✅ Better alignment with upstream improvements
+- ✅ Maintained <5% code divergence from upstream
+- ✅ Clear process for future upstream syncs documented
+
+### Files Modified
+
+- `Sources/StructuredQueriesPostgresMacros/TableMacro.swift` - Enum selections fix (5 changes)
+
+### Next Steps
+
+1. Update enum test snapshots to match corrected generated code
+2. Monitor upstream for stability of major refactor (commit 49ce85d)
+3. Consider incorporating nonisolated enforcement if needed for PostgreSQL use cases
+4. Continue periodic upstream sync checks
+
+---
+
 ## Future Entries
 
 When making significant changes, append new dated sections here following the same format.
diff --git a/Sources/StructuredQueriesPostgresMacros/TableMacro.swift b/Sources/StructuredQueriesPostgresMacros/TableMacro.swift
@@ -945,7 +945,7 @@ extension TableMacro: MemberMacro {
                 []
         var allColumnNames: [TokenSyntax] = []
         var writableColumns: [TokenSyntax] = []
-        var selectedColumns: [TokenSyntax] = []
+        var selectedColumns: [(name: TokenSyntax, type: TypeSyntax?)] = []
         var columnsProperties: [DeclSyntax] = []
         var expansionFailed = false
 
@@ -1084,7 +1084,7 @@ extension TableMacro: MemberMacro {
                     )
                 }
 
-                selectedColumns.append(identifier)
+                selectedColumns.append((identifier, columnQueryValueType))
 
                 if !isGenerated {
                     // NB: A compiler bug prevents us from applying the '@_Draft' macro directly
@@ -1237,7 +1237,7 @@ extension TableMacro: MemberMacro {
 
             let selectionAssignment =
                 selectedColumns
-                .map { "allColumns.append(contentsOf: \($0)._allColumns)\n" }
+                .map { c, _ in "allColumns.append(contentsOf: \(c)._allColumns)\n" }
                 .joined()
 
             selectionInitializers.append(
@@ -1318,7 +1318,7 @@ extension TableMacro: MemberMacro {
                     }
                 }
 
-                selectedColumns.append(identifier)
+                selectedColumns.append((identifier, columnQueryValueType))
 
                 let defaultValue = parameter.defaultValue?.value.rewritten(selfRewriter)
                 let tableColumnType =
@@ -1364,8 +1364,8 @@ extension TableMacro: MemberMacro {
                 if let defaultValue {
                     argument.append(" = \(type)(queryOutput: \(defaultValue))")
                 }
-                let staticColumns = selectedColumns.map {
-                    $0 == identifier ? "\($0)" : "\(valueType)?(queryOutput: nil)" as ExprSyntax
+                let staticColumns = selectedColumns.map { name, type in
+                    name == identifier ? "\(name)" : "\(type)?(queryOutput: nil)" as ExprSyntax
                 }
                 let staticInitialization =
                     staticColumns
@@ -1524,7 +1524,7 @@ extension TableMacro: MemberMacro {
             \(raw: writableColumnsAssignment)return writableColumns
             }
             public var queryFragment: QueryFragment {
-            "\(raw: selectedColumns.map { #"\(self.\#($0))"# }.joined(separator: ", "))"
+            "\(raw: selectedColumns.map { c, _ in #"\(self.\#(c))"# }.joined(separator: ", "))"
             }
             }
             """,
diff --git a/TESTING.md b/TESTING.md
@@ -1337,6 +1337,44 @@ swift test -c release --filter WindowClauseTests
 POSTGRES_URL=postgres://user:pass@localhost:5432/mydb swift test -c release --enable-trait StructuredQueriesPostgresSQLValidation
 ```
 
+### Known Harmless Warnings
+
+#### SwiftSyntax Parsing Warnings During Macro Expansion
+
+**What you'll see**: During test runs, you may see ~1400 lines of SwiftSyntax parser warnings like:
+
+```
+Parsing a `ExprSyntax` node from string interpolation produced the following parsing errors.
+Set a breakpoint in `SyntaxParseable.logStringInterpolationParsingError()` to debug the failure.
+
+1 | var columnWidth = 0
+  | `- error: unexpected code before expression
+2 | columnWidth += Int._columnWidth
+3 | return columnWidth
+  |                   `- error: expected expression
+```
+
+**Why this happens**: The `@Table` macro generates a `_columnWidth` computed property using multiline string interpolation (TableMacro.swift:902-906). When `columnWidths` array is interpolated with a separator, SwiftSyntax's parser attempts to validate the intermediate string during macro expansion, creating ambiguous syntax warnings even though the final generated code is correct.
+
+**Impact**: ✅ **None - completely safe to ignore**
+- All 585 tests pass
+- Generated code is valid Swift
+- SQL output is correct
+- No functionality affected
+
+**Why we don't fix it**:
+1. **Upstream has identical code** - This keeps us aligned with swift-structured-queries
+2. **Warnings are cosmetic** - They occur during macro expansion, not runtime
+3. **Fix would diverge from upstream** - Would create maintenance burden across syncs
+4. **Proven correctness** - Extensive test coverage validates generated code
+
+**When to revisit**:
+- If upstream fixes it (we can adopt their solution)
+- If SwiftSyntax improves multiline interpolation parsing
+- If warnings start causing CI failures (they don't currently)
+
+**Related code**: `Sources/StructuredQueriesPostgresMacros/TableMacro.swift:902-906`
+
 **SQL Validation Setup**:
 
 1. **Install PostgreSQL** (if not already installed):
diff --git a/Tests/READMEExamplesTests/BasicCRUDTests.swift b/Tests/READMEExamplesTests/BasicCRUDTests.swift
@@ -7,7 +7,7 @@ import Testing
 /// Tests for Basic CRUD examples shown in README.md
 @Suite(
     "README Examples - Basic CRUD",
-    .snapshots(record: .failed)
+    .snapshots(record: .never)
 )
 struct BasicCRUDTests {
 
diff --git a/Tests/StructuredQueriesPostgresMacrosTests/Support/SnapshotTests.swift b/Tests/StructuredQueriesPostgresMacrosTests/Support/SnapshotTests.swift
@@ -17,6 +17,6 @@ import Testing
             "sql": SQLMacro.self,
             "Table": TableMacro.self,
         ],
-        record: .failed
+        record: .never
     )
 ) struct SnapshotTests {}
diff --git a/Tests/StructuredQueriesPostgresMacrosTests/TableMacroTests.swift b/Tests/StructuredQueriesPostgresMacrosTests/TableMacroTests.swift
@@ -2461,14 +2461,14 @@ extension SnapshotTests {
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
                       allColumns.append(contentsOf: photo._allColumns)
-                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
                       return Self(allColumns: allColumns)
                     }
                     public static func note(
                       _ note: some StructuredQueriesCore.QueryExpression<String>
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
-                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
                       allColumns.append(contentsOf: note._allColumns)
                       return Self(allColumns: allColumns)
                     }
@@ -2568,14 +2568,14 @@ extension SnapshotTests {
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
                       allColumns.append(contentsOf: photo._allColumns)
-                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
                       return Self(allColumns: allColumns)
                     }
                     public static func note(
                       _ note: some StructuredQueriesCore.QueryExpression<String>
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
-                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
                       allColumns.append(contentsOf: note._allColumns)
                       return Self(allColumns: allColumns)
                     }
@@ -2671,14 +2671,14 @@ extension SnapshotTests {
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
                       allColumns.append(contentsOf: photo._allColumns)
-                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
                       return Self(allColumns: allColumns)
                     }
                     public static func note(
                       _ note: some StructuredQueriesCore.QueryExpression<String>
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
-                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
                       allColumns.append(contentsOf: note._allColumns)
                       return Self(allColumns: allColumns)
                     }
@@ -2759,14 +2759,14 @@ extension SnapshotTests {
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
                       allColumns.append(contentsOf: photo._allColumns)
-                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
                       return Self(allColumns: allColumns)
                     }
                     public static func note(
                       text note: some StructuredQueriesCore.QueryExpression<String>
                     ) -> Self {
                       var allColumns: [any StructuredQueriesCore.QueryExpression] = []
-                      allColumns.append(contentsOf: String?(queryOutput: nil)._allColumns)
+                      allColumns.append(contentsOf: Photo?(queryOutput: nil)._allColumns)
                       allColumns.append(contentsOf: note._allColumns)
                       return Self(allColumns: allColumns)
                     }
diff --git a/Tests/StructuredQueriesPostgresTests/Core/Table/EnumTableTests.swift b/Tests/StructuredQueriesPostgresTests/Core/Table/EnumTableTests.swift

Original file line number	Diff line number	Diff line change
`@@ -945,7 +945,7 @@ extension TableMacro: MemberMacro {`
`945`	`945`	`[]`
`946`	`946`	`var allColumnNames: [TokenSyntax] = []`
`947`	`947`	`var writableColumns: [TokenSyntax] = []`
`948`		`- var selectedColumns: [TokenSyntax] = []`
	`948`	`+ var selectedColumns: [(name: TokenSyntax, type: TypeSyntax?)] = []`
`949`	`949`	`var columnsProperties: [DeclSyntax] = []`
`950`	`950`	`var expansionFailed = false`
`951`	`951`
`@@ -1084,7 +1084,7 @@ extension TableMacro: MemberMacro {`
`1084`	`1084`	`)`
`1085`	`1085`	`}`
`1086`	`1086`
`1087`		`- selectedColumns.append(identifier)`
	`1087`	`+ selectedColumns.append((identifier, columnQueryValueType))`
`1088`	`1088`
`1089`	`1089`	`if !isGenerated {`
`1090`	`1090`	`// NB: A compiler bug prevents us from applying the '@_Draft' macro directly`
`@@ -1237,7 +1237,7 @@ extension TableMacro: MemberMacro {`
`1237`	`1237`
`1238`	`1238`	`let selectionAssignment =`
`1239`	`1239`	`selectedColumns`
`1240`		`- .map { "allColumns.append(contentsOf: \($0)._allColumns)\n" }`
	`1240`	`+ .map { c, _ in "allColumns.append(contentsOf: \(c)._allColumns)\n" }`
`1241`	`1241`	`.joined()`
`1242`	`1242`
`1243`	`1243`	`selectionInitializers.append(`
`@@ -1318,7 +1318,7 @@ extension TableMacro: MemberMacro {`
`1318`	`1318`	`}`
`1319`	`1319`	`}`
`1320`	`1320`
`1321`		`- selectedColumns.append(identifier)`
	`1321`	`+ selectedColumns.append((identifier, columnQueryValueType))`
`1322`	`1322`
`1323`	`1323`	`let defaultValue = parameter.defaultValue?.value.rewritten(selfRewriter)`
`1324`	`1324`	`let tableColumnType =`
`@@ -1364,8 +1364,8 @@ extension TableMacro: MemberMacro {`
`1364`	`1364`	`if let defaultValue {`
`1365`	`1365`	`argument.append(" = \(type)(queryOutput: \(defaultValue))")`
`1366`	`1366`	`}`
`1367`		`- let staticColumns = selectedColumns.map {`
`1368`		`- $0 == identifier ? "\($0)" : "\(valueType)?(queryOutput: nil)" as ExprSyntax`
	`1367`	`+ let staticColumns = selectedColumns.map { name, type in`
	`1368`	`+ name == identifier ? "\(name)" : "\(type)?(queryOutput: nil)" as ExprSyntax`
`1369`	`1369`	`}`
`1370`	`1370`	`let staticInitialization =`
`1371`	`1371`	`staticColumns`
`@@ -1524,7 +1524,7 @@ extension TableMacro: MemberMacro {`
`1524`	`1524`	`\(raw: writableColumnsAssignment)return writableColumns`
`1525`	`1525`	`}`
`1526`	`1526`	`public var queryFragment: QueryFragment {`
`1527`		`- "\(raw: selectedColumns.map { #"\(self.\#($0))"# }.joined(separator: ", "))"`
	`1527`	`+ "\(raw: selectedColumns.map { c, _ in #"\(self.\#(c))"# }.joined(separator: ", "))"`
`1528`	`1528`	`}`
`1529`	`1529`	`}`
`1530`	`1530`	`""",`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ import Testing`
`7`	`7`	`/// Tests for Basic CRUD examples shown in README.md`
`8`	`8`	`@Suite(`
`9`	`9`	`"README Examples - Basic CRUD",`
`10`		`- .snapshots(record: .failed)`
	`10`	`+ .snapshots(record: .never)`
`11`	`11`	`)`
`12`	`12`	`struct BasicCRUDTests {`
`13`	`13`
Original file line number	Diff line number	Diff line change
`@@ -17,6 +17,6 @@ import Testing`
`17`	`17`	`"sql": SQLMacro.self,`
`18`	`18`	`"Table": TableMacro.self,`
`19`	`19`	`],`
`20`		`- record: .failed`
	`20`	`+ record: .never`
`21`	`21`	`)`
`22`	`22`	`) struct SnapshotTests {}`