Skip to content

Commit 8b499d5

Browse files
committed
fix: remediate second-round review findings (A/#4, B/#1/#7/#8/#9)
Addresses review.md classifications from session 2: - A/#4 Correctness: extend HasQuarryContextAttributeSyntactic with an AliasQualifiedNameSyntax branch so [global::QuarryContextAttribute] and extern-alias attribute forms pass the syntactic pre-filter. - B/#1 Plan: drop RawSqlTypeInfo.FullyQualifiedResultTypeName — written but never read; ResultTypeName already carries the final display form (FQN for nested, short for not). - B/#7 Tests: add alias-qualified attribute QRY044 coverage. - B/#8 Tests: add struct-with-init-only-property QRY043 coverage. - B/#9 Tests: add mixed RawSqlScalarAsync<int> + failing RawSqlAsync<T> test exercising the scalar branch in PipelineOrchestrator + FileEmitter. 3259 tests pass (+3 new).
1 parent be7da67 commit 8b499d5

8 files changed

Lines changed: 215 additions & 64 deletions

File tree

_sessions/259-docs-dx-friction-points/review.md

Lines changed: 80 additions & 44 deletions
Large diffs are not rendered by default.

_sessions/259-docs-dx-friction-points/workflow.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ remote: https://github.com/Dtronix/Quarry.git
66
base-branch: master
77

88
## State
9-
phase: REVIEW
9+
phase: REMEDIATE
1010
status: active
1111
issue: #259
1212
pr: #260
@@ -64,3 +64,5 @@ Add `IsNestedType` (bool) and `FullyQualifiedResultTypeName` (string) to `RawSql
6464
| 1 | 2026-04-22 | - | INTAKE→DESIGN→PLAN: baseline green (3242 tests), design decisions recorded, plan.md approved. Auto-transition to IMPLEMENT. |
6565
| 2 | 2026-04-23 | - | Resume. Worktree had been pruned from disk; recreated at `../259-docs-dx-friction-points/` from `origin/259-docs-dx-friction-points`. PR #260 verified MERGEABLE/CLEAN, CI run 24820105388 SUCCESS. Branch 8 ahead, 0 behind origin/master — no rebase needed. Remediation already committed (`be224dd`, `25f0b5e`). REMEDIATE step 8: awaiting user finalize confirmation. |
6666
| 3 | 2026-04-23 | - | Back-step REMEDIATE → REVIEW at user request (declined finalize). Keep review.md + Decisions intact; re-run analysis pass over the full branch diff including remediation commits `be224dd` and `25f0b5e`. Classifications reset — rerun classification pass after. No targeted focus — general re-review. |
67+
| 4 | 2026-04-23 | - | REVIEW session 2 complete: 13 findings, classified 1A/4B/0C/8D (accepted Rec). Transition to REMEDIATE. A: #4 AliasQualifiedNameSyntax in `HasQuarryContextAttributeSyntactic`. B: #1 drop dead `FullyQualifiedResultTypeName`, #7 alias-qualified attribute test, #8 struct init-only row test, #9 `RawSqlScalarAsync<T>` QRY043 suppression test. |
68+
| 5 | 2026-04-23 | - | Session 2 remediation complete: A/#4 + B/#1/#7/#8/#9 addressed. Build clean; 3259 tests pass (+3 new: Analyzers 110, Migration 201, Quarry 2948). Next: commit + rebase on origin/master + push + verify CI. |

src/Quarry.Analyzers.Tests/InterceptorsNamespacesAnalyzerTests.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,31 @@ public partial class AppDb : QuarryContext
155155
Assert.That(diagnostics[0].GetMessage(), Does.Contain("MyApp.Data"));
156156
}
157157

158+
[Test]
159+
public async Task EmitsQRY044_WhenAttributeUsesAliasQualifiedName()
160+
{
161+
// `[global::QuarryContextAttribute]` decomposes into AliasQualifiedNameSyntax at the
162+
// attribute's top-level name, which the syntactic pre-filter must handle explicitly —
163+
// otherwise the analyzer silently skips the class and the diagnostic never fires.
164+
// This also covers extern-alias-qualified attribute forms.
165+
var source = @"
166+
using Quarry;
167+
using QuarryContextAttribute = Quarry.QuarryContextAttribute;
168+
169+
namespace MyApp.Data
170+
{
171+
[global::QuarryContextAttribute(Dialect = SqlDialect.SQLite)]
172+
public partial class AppDb : QuarryContext
173+
{
174+
}
175+
}";
176+
177+
var diagnostics = await GetDiagnosticsAsync(source, "Quarry.Generated");
178+
Assert.That(diagnostics, Has.Length.EqualTo(1));
179+
Assert.That(diagnostics[0].GetMessage(), Does.Contain("AppDb"));
180+
Assert.That(diagnostics[0].GetMessage(), Does.Contain("MyApp.Data"));
181+
}
182+
158183
[Test]
159184
public async Task NoDiagnostic_ForNonContextClass()
160185
{

src/Quarry.Analyzers/InterceptorsNamespacesAnalyzer.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ private static bool HasQuarryContextAttributeSyntactic(ClassDeclarationSyntax cl
7878
var name = attr.Name switch
7979
{
8080
QualifiedNameSyntax q => q.Right.Identifier.Text,
81+
AliasQualifiedNameSyntax a => a.Name.Identifier.Text,
8182
SimpleNameSyntax s => s.Identifier.Text,
8283
_ => null
8384
};

src/Quarry.Generator/Models/RawSqlTypeInfo.cs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ public RawSqlTypeInfo(
1515
bool hasCancellationToken = false,
1616
string? scalarReaderMethod = null,
1717
string? sqlLiteral = null,
18-
bool isNestedType = false,
19-
string? fullyQualifiedResultTypeName = null)
18+
bool isNestedType = false)
2019
{
2120
ResultTypeName = resultTypeName;
2221
TypeKind = typeKind;
@@ -25,7 +24,6 @@ public RawSqlTypeInfo(
2524
ScalarReaderMethod = scalarReaderMethod;
2625
SqlLiteral = sqlLiteral;
2726
IsNestedType = isNestedType;
28-
FullyQualifiedResultTypeName = fullyQualifiedResultTypeName ?? resultTypeName;
2927
}
3028

3129
/// <summary>
@@ -71,13 +69,6 @@ public RawSqlTypeInfo(
7169
/// </summary>
7270
public bool IsNestedType { get; }
7371

74-
/// <summary>
75-
/// The <c>global::</c>-prefixed fully qualified name of the result type. Always
76-
/// resolves to a valid C# type reference regardless of <c>using</c> directives.
77-
/// Falls back to <see cref="ResultTypeName"/> when not separately supplied.
78-
/// </summary>
79-
public string FullyQualifiedResultTypeName { get; }
80-
8172
public bool Equals(RawSqlTypeInfo? other)
8273
{
8374
if (other is null) return false;
@@ -88,7 +79,6 @@ public bool Equals(RawSqlTypeInfo? other)
8879
&& ScalarReaderMethod == other.ScalarReaderMethod
8980
&& SqlLiteral == other.SqlLiteral
9081
&& IsNestedType == other.IsNestedType
91-
&& FullyQualifiedResultTypeName == other.FullyQualifiedResultTypeName
9282
&& EqualityHelpers.SequenceEqual(Properties, other.Properties);
9383
}
9484

src/Quarry.Generator/Parsing/DisplayClassEnricher.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,7 @@ private static void EnrichRawSqlTypeInfo(
249249
rawSqlTypeInfo.HasCancellationToken,
250250
rawSqlTypeInfo.ScalarReaderMethod,
251251
sqlLiteral,
252-
rawSqlTypeInfo.IsNestedType,
253-
rawSqlTypeInfo.FullyQualifiedResultTypeName);
252+
rawSqlTypeInfo.IsNestedType);
254253
}
255254

256255
site.RawSqlTypeInfo = rawSqlTypeInfo;
@@ -372,8 +371,7 @@ private static RawSqlTypeInfo PatchWithColumnMetadata(
372371
typeInfo.HasCancellationToken,
373372
typeInfo.ScalarReaderMethod,
374373
typeInfo.SqlLiteral,
375-
typeInfo.IsNestedType,
376-
typeInfo.FullyQualifiedResultTypeName);
374+
typeInfo.IsNestedType);
377375
}
378376

379377
/// <summary>

src/Quarry.Generator/Parsing/UsageSiteDiscovery.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4032,8 +4032,7 @@ internal static RawSqlTypeInfo ResolveRawSqlTypeInfo(ITypeSymbol typeSymbol, boo
40324032
System.Array.Empty<RawSqlPropertyInfo>(),
40334033
hasCancellationToken,
40344034
scalarReaderMethod,
4035-
isNestedType: false,
4036-
fullyQualifiedResultTypeName: typeName);
4035+
isNestedType: false);
40374036
}
40384037

40394038
// T is a class/struct with properties — enumerate public settable properties
@@ -4089,8 +4088,7 @@ internal static RawSqlTypeInfo ResolveRawSqlTypeInfo(ITypeSymbol typeSymbol, boo
40894088
RawSqlTypeKind.Dto,
40904089
properties,
40914090
hasCancellationToken,
4092-
isNestedType: isNestedType,
4093-
fullyQualifiedResultTypeName: typeName);
4091+
isNestedType: isNestedType);
40944092
}
40954093

40964094
/// <summary>

src/Quarry.Tests/RawSqlGeneratorPipelineTests.cs

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,107 @@ public async Task Test(TestDbContext db)
629629
Assert.That(qry043!.GetMessage(), Does.Contain("interface"));
630630
}
631631

632+
[Test]
633+
public void RawSqlAsync_StructRowWithInitOnlyProperty_EmitsQRY043()
634+
{
635+
// Structs skip the ctor branch in CheckRowEntityMaterializability (C# always
636+
// synthesizes a public parameterless ctor). Pin that the property-loop path
637+
// still rejects an init-only property on a struct row type.
638+
var source = @"
639+
using Quarry;
640+
using System.Threading.Tasks;
641+
642+
namespace TestApp;
643+
644+
public struct UserRow
645+
{
646+
public int Id { get; init; }
647+
public string Name { get; init; }
648+
}
649+
650+
public class UserSchema : Schema
651+
{
652+
public static string Table => ""users"";
653+
public Key<int> UserId => Identity();
654+
}
655+
656+
[QuarryContext(Dialect = SqlDialect.SQLite)]
657+
public partial class TestDbContext : QuarryContext
658+
{
659+
public partial IEntityAccessor<User> Users();
660+
}
661+
662+
public class Service
663+
{
664+
public async Task Test(TestDbContext db)
665+
{
666+
var rows = await db.RawSqlAsync<UserRow>(""SELECT Id, Name FROM users"");
667+
foreach (var r in rows) { _ = r; }
668+
}
669+
}
670+
";
671+
672+
var compilation = CreateCompilation(source);
673+
var (diagnostics, _) = RunGeneratorWithDiagnostics(compilation);
674+
675+
var qry043 = diagnostics.FirstOrDefault(d => d.Id == "QRY043");
676+
Assert.That(qry043, Is.Not.Null, "QRY043 should fire for a struct row with init-only properties");
677+
Assert.That(qry043!.GetMessage(), Does.Contain("init-only"));
678+
}
679+
680+
[Test]
681+
public void RawSqlScalarAsync_DoesNotEmitQRY043_AndEmitsInterceptor_WhenMixedWithFailingRawSqlAsync()
682+
{
683+
// Pins the `Kind is RawSqlAsync or RawSqlScalarAsync` branch in
684+
// PipelineOrchestrator.CollectTranslatedDiagnostics and FileEmitter for the
685+
// scalar case: a failing RawSqlAsync<UserRow> must emit QRY043 exactly once
686+
// while the sibling RawSqlScalarAsync<int> still produces a valid interceptor
687+
// (scalars skip the materializability check entirely).
688+
var source = @"
689+
using Quarry;
690+
using System.Threading.Tasks;
691+
692+
namespace TestApp;
693+
694+
public sealed record UserRow(int Id, string Name);
695+
696+
public class UserSchema : Schema
697+
{
698+
public static string Table => ""users"";
699+
public Key<int> UserId => Identity();
700+
}
701+
702+
[QuarryContext(Dialect = SqlDialect.SQLite)]
703+
public partial class TestDbContext : QuarryContext
704+
{
705+
public partial IEntityAccessor<User> Users();
706+
}
707+
708+
public class Service
709+
{
710+
public async Task Test(TestDbContext db)
711+
{
712+
var rows = await db.RawSqlAsync<UserRow>(""SELECT Id, Name FROM users"");
713+
foreach (var r in rows) { _ = r; }
714+
var count = await db.RawSqlScalarAsync<int>(""SELECT COUNT(*) FROM users"");
715+
}
716+
}
717+
";
718+
719+
var compilation = CreateCompilation(source);
720+
var (diagnostics, result) = RunGeneratorWithDiagnostics(compilation);
721+
722+
var qry043s = diagnostics.Where(d => d.Id == "QRY043").ToList();
723+
Assert.That(qry043s, Has.Count.EqualTo(1),
724+
"QRY043 should fire once for the failing RawSqlAsync<UserRow> and not for the RawSqlScalarAsync<int>");
725+
Assert.That(qry043s[0].GetMessage(), Does.Contain("UserRow"));
726+
727+
var code = GetInterceptorsCode(result);
728+
Assert.That(code, Is.Not.Null, "Should still generate an interceptors file for the scalar call");
729+
Assert.That(code, Does.Contain("RawSqlScalarAsyncWithConverter"),
730+
"Scalar interceptor must still emit despite the sibling QRY043");
731+
}
732+
632733
[Test]
633734
public void RawSqlAsync_PlainClassRow_DoesNotEmitQRY043()
634735
{

0 commit comments

Comments
 (0)