From 4170d6c7f684dc7bd19b2e4f3f227b3b6df8a2d3 Mon Sep 17 00:00:00 2001 From: mgravell Date: Fri, 11 Sep 2026 14:09:12 +0100 Subject: [PATCH 1/2] Say something at the mute overloads (DAP057), and open 1.1 34 public overloads told the consumer nothing at all. They work under JIT - the call-site just stays on vanilla Dapper - and fail after a native-AOT publish, with no build-time signal anywhere. That is the shape behind issues #112, #158 and #165. Every one of them is CommandDefinition-shaped, because the analyzer only inspects call-sites carrying SQL as a string argument and these hide it inside the struct. Two groups, two answers: - 21 are operations we *do* support, in a spelling we cannot read: new **DAP057**, reported from the generator (the analyzer cannot see them), naming the method and pointing at the overload that takes the SQL directly; - 12 are APIs unsupported in any spelling - ExecuteReader, multi-map, QueryMultiple. Those now get **DAP001 from the generator**, the same id their visible siblings already get from the analyzer, so both spellings behave alike. **Severity follows PublishAot.** Leaving a call-site on vanilla Dapper is a missed optimization under JIT and a latent publish-time crash under native AOT, so DAP057 is info in the first case and a warning in the second. The SDK sets EnableAotAnalyzer when PublishAot is on and it is compiler-visible by default, so this needs no build props of our own. Proved end to end: a net10.0 project reports info at default verbosity, and the same project with PublishAot=true reports warning. That way an upgrade does not bury a JIT project in new warnings, while an AOT project is told about every call-site that will not survive publishing. The surface report gains an `unsupported: CommandDefinition` bucket and collapses the undiagnosed-unsupported one, since nothing is undiagnosed any more. Silent skips go **34 -> 1**, and the remaining one is a reporting artifact rather than a gap: GetRowParser(concreteType) does different things depending on whether a call passes the Type, which a symbol-level report cannot say. Corpus (round 15 rig, net10.0): silent skips **25 -> 0**, refused-with-diagnostics 203 -> 228, handled unchanged at 432 of 736, and the suite still passes 729 of 800 - this adds diagnostics and changes no behaviour. version.json opens 1.1 with versionHeightOffset -1, so the first build is 1.1.0 rather than 1.1.1; confirmed by packing (Dapper.AOT.1.1.0-g.nupkg). --- docs/rules/DAP057.md | 53 ++++++++++++ notes/parity.md | 25 +++--- .../DapperAnalyzer.Diagnostics.cs | 2 + .../DapperInterceptorGenerator.cs | 60 +++++++++++-- .../CodeAnalysis/GlobalOptions.cs | 18 +++- .../Model/InterceptorEnvironment.cs | 12 ++- .../CodeAnalysis/ParseState.cs | 6 ++ test/Dapper.AOT.Test/ApiSurface.expected.txt | 78 +++++++++-------- .../ApiSurfaceCoverageTests.cs | 32 ++++++- .../CommandDefinitionDiagnosticTests.cs | 85 +++++++++++++++++++ .../CommandDefinitionOverloads.output.txt | 10 ++- version.json | 3 +- 12 files changed, 320 insertions(+), 64 deletions(-) create mode 100644 docs/rules/DAP057.md create mode 100644 test/Dapper.AOT.Test/CommandDefinitionDiagnosticTests.cs diff --git a/docs/rules/DAP057.md b/docs/rules/DAP057.md new file mode 100644 index 00000000..e0790a49 --- /dev/null +++ b/docs/rules/DAP057.md @@ -0,0 +1,53 @@ +# DAP057 + +CommandDefinition overload is not supported + +Dapper.AOT decides what to generate by reading the SQL at build time. The `CommandDefinition` +overloads carry their SQL *inside a struct*, constructed at execution time - so there is nothing +for the generator to read, and the call-site is left running vanilla Dapper. + +``` csharp +conn.Execute(new CommandDefinition(sql, args)); // DAP057 +``` + +**The fix**: use the overload that takes the SQL directly. + +``` csharp +conn.Execute(sql, args); +``` + +The named arguments of `CommandDefinition` map onto the longer overloads, so a call using +`transaction`, `commandTimeout`, `commandType` or `flags` has a direct equivalent: + +``` csharp +conn.Execute(sql, args, transaction, commandTimeout, commandType); +``` + +## Why this is a warning under native AOT, and information otherwise + +An unhandled call-site does not *break* - it keeps working exactly as vanilla Dapper always did, +by reflection and ref-emit. Under JIT that is a missed optimization, so this reports as +information. + +Under native AOT it is a latent crash: the reflection path this falls back to is precisely what +cannot survive publishing, and nothing else warns about it. So when the project sets +`PublishAot`, DAP057 is raised to a **warning** - the same fact, at the severity it deserves for +the person it will actually hurt. + +That means upgrading Dapper.AOT does not bury a JIT project in new warnings, while an AOT project +gets told about every call-site that will not make it. + +## Why not just support them? + +Nothing here is impossible - it would mean tracing the `CommandDefinition` back to its +construction site and recovering the SQL from there, which works when it is built inline and +does not when it arrives as a parameter, comes from a field, or is passed through a helper. The +overload would then be supported *sometimes*, which is a worse contract than supported never: +the failure would depend on how the argument happened to be spelled. + +Until that is settled, this diagnostic exists so the situation is at least *visible*. Previously +these call-sites were dropped in silence - they worked in development and failed after an AOT +publish, with nothing said at build time. + +See also [DAP001](DAP001.md), which reports the operations that are not supported in any +spelling. diff --git a/notes/parity.md b/notes/parity.md index 7bc34b72..80b4e882 100644 --- a/notes/parity.md +++ b/notes/parity.md @@ -54,21 +54,26 @@ Two levers change several complexity scores and are worth naming up front: Two independent measurements, because they answer different questions. **API surface** (`ApiSurface.expected.txt`, generated): of Dapper's public extension overloads -— 28 candidates, 18 `Type`-based non-goals (refused on purpose, with DAP056), 16 -unsupported-and-diagnosed, 12 unsupported-and-undiagnosed, 22 skipped silently, 9 never inspected -(helpers, correctly). So **34 still tell the consumer nothing**, almost all -`CommandDefinition`-shaped. That is a defect class of its own, separate from any missing feature: -it is not that these fail, it is that they fail *quietly*. +— 28 candidates, 18 `Type`-based non-goals (refused on purpose, with DAP056), 28 +unsupported-and-diagnosed, 21 `CommandDefinition`-shaped (refused, with DAP057), 9 never +inspected (helpers, correctly), and **1 still silent**. -**Behaviour** (Dapper suite, local SQL Server): **677 of 793** pass through generated code, with -**533 of 725** call-sites intercepted (73.5%). Note the denominator counts what the generator -examines, so the silently-skipped overloads above are inside it and the diagnosed ones are too. +That one is `GetRowParser(concreteType)`, and it is a reporting artifact rather than a gap: +the report classifies *symbols*, and what this overload does depends on whether a given call +actually passes the `Type` — which a symbol cannot say. Call-sites get DAP056 or are handled, as +appropriate. **The mute class is closed** (2026-09-11, was 40 at the start of the round). + +**Behaviour** (Dapper suite, local SQL Server): **729 of 800** pass through generated code, with +**432 of 736** call-sites handled (round 15, 2026-09-11; vanilla control 770/800). Note the +denominator counts what the generator examines, so the refused overloads above are inside it. +Call-site counts are rig-specific — see harness-baseline.md round 15 before comparing with any +earlier round. What stands between that and "all green", largest first: | # | what | where it shows up | size | | --- | --- | --- | --- | -| 0 | **say something at the 34 mute overloads** | 22 skipped silently + 12 unsupported-undiagnosed | small, and it is the cheapest safety win on the list: it turns a runtime AOT failure into a build warning without supporting anything new | +| ~~0~~ | ~~**say something at the mute overloads**~~ | **done 2026-09-11**: DAP057 for the 21 `CommandDefinition` spellings, DAP001 from the generator for the 12 it could not see. Severity follows `PublishAot` — info when a fallback to vanilla Dapper is a missed optimization, warning when it is a latent publish-time crash | — | | 1 | **multi-map** (`Query` + `splitOn`) | unsupported API - outside the 725 | large | | 2 | **`QueryMultiple` / `GridReader`** | unsupported API | large; needs a Dapper-side extension point first | | 3 | **corpus adoption of `[TypeHandler]`** | TypeHandlerTests x16/provider | a harness edit, not product work - but not all of it converts, see below | @@ -135,7 +140,7 @@ non-public members, and the "has no meaning" APIs warning - all in §7. | param filtering (only bind members named in SQL) + `SupportLegacyParameterTokens` | ❓ | med | low | AOT currently *includes* + warns (DAP236); on strict providers that's an error, so may need parity not preference | | UDTs (`UdtTypeHandler`, geo types) | ⚠️ | low | low | provider-specific, and now expressible: declare a handler for the type. No built-in, so a consumer supplies it | | XML types (`XmlDocument`/`XDocument`/`XElement`) | ⚠️ | low-med | low | expressible today by declaring a handler; vanilla registers these by default, so the open question is whether we ship built-in declarations rather than whether it *can* work | -| `CommandDefinition` overloads | ❌ | **high** | med | **21 of the 22 silently-skipped rows in the surface report are these** — the analyzer only inspects call-sites carrying SQL as a string argument, and these hide it inside the struct, so nothing is emitted *and nothing is reported*. (It was 27 before #214 moved the `Type`+`CommandDefinition` combinations into the non-goal bucket; the report is the count that matters, not this sentence.) Consumers get vanilla Dapper under JIT and a runtime failure under native AOT with no build-time signal (issues #112, #158, #165). External PR #153 proposes support; a diagnostic is worth having either way, and is cheaper | +| `CommandDefinition` overloads | ❌ | **high** | med | Still not generated — the analyzer only inspects call-sites carrying SQL as a string argument, and these hide it inside the struct. But **no longer silent**: DAP057 names each one (2026-09-11), at a severity that follows `PublishAot`. That closes the *safety* half of issues #112/#158/#165 — a consumer heading for native AOT is now told at build time instead of at publish. The *support* half is still open; external PR #153 proposes it. See the surface report for the current count | | `CommandFlags` (`Buffered`, `Pipelined`, `NoCache`) | ❓ (behaviour) | med | low-med | `NoCache` is **zero** (no cache to bypass); `Buffered` covered; `Pipelined` is a perf feature to verify — and all of it is moot at a call-site until the row above is fixed | | `commandTimeout` / `transaction` / `commandType` args | ✅ ❓ | — | — | verify `TableDirect` | | `CancellationToken` | ✅ | — | — | AOT extends Dapper here (DAP044/045) | diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs index 6d4db073..1494bd0d 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.Diagnostics.cs @@ -25,6 +25,8 @@ public static readonly DiagnosticDescriptor DapperAotTupleParameter = LibraryInfo("DAP014", "Tuple-type parameter", "Tuple-type parameters are not currently supported"), UntypedParameter = LibraryInfo("DAP015", "Untyped parameter", "The parameter type could not be resolved"), GenericTypeParameter = LibraryInfo("DAP016", "Generic type parameter", "Generic type parameters ({0}) are not currently supported"), + CommandDefinitionNotSupported = LibraryInfo("DAP057", "CommandDefinition overload is not supported", + "'{0}' passes its SQL inside a CommandDefinition, which Dapper.AOT cannot read at build time; use the overload that takes the SQL directly. This call-site is left on vanilla Dapper, which will not work under native AOT"), TypeBasedApiNotSupported = LibraryWarning("DAP056", "Type-based API is not supported", "'{0}' chooses the row type from a Type at execution time, which Dapper.AOT cannot generate for; use the generic overload so the type is known at build time, and the call-site is left on vanilla Dapper (which will not work under native AOT)"), DuplicateTypeHandler = LibraryWarning("DAP055", "Duplicate type-handler", diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs index 8c957230..ec991972 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs @@ -61,7 +61,8 @@ public override void Initialize(IncrementalGeneratorInitializationContext contex var nodes = context.SyntaxProvider.CreateSyntaxProvider(PreFilter, Parse) .Where(x => x is not null) .Select((x, _) => x!); - var env = context.CompilationProvider.Select(static (c, _) => CreateEnvironment(c)); + var env = context.CompilationProvider.Combine(context.AnalyzerConfigOptionsProvider) + .Select(static (pair, _) => CreateEnvironment(pair.Left, pair.Right.TargetsNativeAot())); var combined = env.Combine(nodes.Collect()); context.RegisterImplementationSourceOutput(combined, Generate); } @@ -169,19 +170,29 @@ private static InterceptedMethod ProjectMethod(IMethodSymbol method) } if (flags.HasAny(OperationFlags.NotAotSupported)) { - // not our API (yet); count it, so the scorecard stays honest. DAP001 comes - // from the analyzer - but only where the analyzer can see the call at all + // not our API (yet); count it, so the scorecard stays honest. DAP001 comes from + // the analyzer where it can see the call; where it cannot - the CommandDefinition + // overloads - the generator reports the same id, so both spellings behave alike + var visible = IsVisibleToAnalyzer(op.TargetMethod); return new SkippedSourceState(new LocationSnapshot(ie.GetLocation()), flags, - diagnosed: IsVisibleToAnalyzer(op.TargetMethod)); + diagnosed: true, + reason: visible ? SkipReason.None : SkipReason.UnsupportedInvisibleApi, + methodName: visible ? "" : op.TargetMethod.Name); } var location = DapperAnalyzer.SharedParseArgsAndFlags(ctx, op, ref flags, out var sql, out var argExpression, reportDiagnostic: null, out var resultType, exitFirstFailure: true); if (flags.HasAny(OperationFlags.DoNotGenerate)) { // the analyzer's identical pass told us to leave it alone - and will have said - // why, *if* the shape of this overload is one the analyzer inspects at all + // why, *if* the shape of this overload is one the analyzer inspects at all. + // Where it is not, the operation itself is supported and only this *spelling* + // is not, so say exactly that rather than dropping the call-site in silence + if (IsVisibleToAnalyzer(op.TargetMethod)) + { + return new SkippedSourceState(new LocationSnapshot(location), flags, diagnosed: true); + } return new SkippedSourceState(new LocationSnapshot(location), flags, - diagnosed: IsVisibleToAnalyzer(op.TargetMethod)); + diagnosed: true, SkipReason.CommandDefinition, op.TargetMethod.Name); } @@ -271,7 +282,7 @@ static string BuildParameterMap(in ParseState ctx, IInvocationOperation op, stri } - internal static InterceptorEnvironment CreateEnvironment(Compilation compilation) + internal static InterceptorEnvironment CreateEnvironment(Compilation compilation, bool targetsNativeAot = false) { var dbCommandTypes = IdentifyDbCommandTypes(compilation, out var needsCommandPrep); EquatableArray special = default; @@ -300,7 +311,8 @@ internal static InterceptorEnvironment CreateEnvironment(Compilation compilation baseFactoryCanConstruct: canConstruct, specialCommandTypes: special, systemObjectPlan: ParamPlan.Create(compilation.GetSpecialType(SpecialType.System_Object))!, - typeHandlers: GetTypeHandlers(compilation)); + typeHandlers: GetTypeHandlers(compilation), + targetsNativeAot: targetsNativeAot); } /// @@ -515,6 +527,27 @@ internal void Generate(in GenerateState ctx) refusedWithDiagnostics++; continue; } + if (skip.Reason == SkipReason.CommandDefinition) + { + // the operation is supported; only this *spelling* is not, because the SQL is + // inside the struct. Info when the consumer is not publishing native AOT (a + // missed optimization), warning when they are (a latent crash at publish) + ctx.ReportDiagnostic(Diagnostic.Create(DapperAnalyzer.Diagnostics.CommandDefinitionNotSupported, + skip.Location.AsLocation(), + ctx.TargetsNativeAot ? DiagnosticSeverity.Warning : DiagnosticSeverity.Info, + additionalLocations: null, properties: null, skip.MethodName)); + refusedWithDiagnostics++; + continue; + } + if (skip.Reason == SkipReason.UnsupportedInvisibleApi) + { + // DAP001, from here rather than the analyzer, which cannot see this overload - + // so both spellings of an unsupported API report the same thing + ctx.ReportDiagnostic(Diagnostic.Create(DapperAnalyzer.Diagnostics.UnsupportedMethod, + skip.Location.AsLocation(), skip.MethodName)); + unsupported++; + continue; + } if (skip.Flags.HasAny(OperationFlags.NotAotSupported)) unsupported++; else if (skip.Diagnosed) refusedWithDiagnostics++; else skippedSilently++; // nothing told the consumer; each one of these is a bug of ours @@ -1946,6 +1979,17 @@ internal enum SkipReason /// the one thing compile-time generation cannot follow. Declared non-goal, 2026-08-26. /// TypeBasedApi = 1, + /// + /// An overload carrying its SQL inside a CommandDefinition, for an operation we + /// otherwise support. The analyzer never sees these (no sql string parameter), so + /// the generator is what speaks - otherwise the drop is completely silent. + /// + CommandDefinition = 2, + /// + /// An API we do not support at all, on an overload the analyzer cannot see - so the + /// DAP001 its visible siblings get has to come from here instead. + /// + UnsupportedInvisibleApi = 3, } internal sealed class SkippedSourceState : SourceState diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs index f5200394..dc28a279 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs @@ -11,9 +11,25 @@ public static class Keys public const string GlobalOptions_DapperSqlSyntax = "dapper.sqlsyntax"; public const string GlobalOptions_DapperDebugSqlParseInputFlags = "dapper.debug_mode_sql_parse_input_flags"; // this is for test purposes; if you find and use this: don't blame me! public const string ProjectProperties_DapperSqlSyntax = "build_property.Dapper_SqlSyntax"; - + + /// + /// Set by the SDK when PublishAot is on, and compiler-visible by default - so we + /// can tell a native-AOT project from a JIT one without shipping any build props of our + /// own. PublishAot itself is *not* compiler-visible, which is why this stands in. + /// + public const string ProjectProperties_EnableAotAnalyzer = "build_property.EnableAotAnalyzer"; } + /// + /// Is the consuming project headed for native AOT? Decides whether a call-site we leave on + /// vanilla Dapper is a missed optimization or a latent publish-time crash. + /// + public static bool TargetsNativeAot(this AnalyzerConfigOptionsProvider? provider) + => provider is not null + && provider.GlobalOptions.TryGetValue(Keys.ProjectProperties_EnableAotAnalyzer, out var value) + && bool.TryParse(value, out var enabled) + && enabled; + public static bool TryGetSqlSyntax(this AnalyzerOptions? options, out SqlSyntax syntax) { if (options is not null) diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/Model/InterceptorEnvironment.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/Model/InterceptorEnvironment.cs index e2d4f480..dafb4ff3 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/Model/InterceptorEnvironment.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/Model/InterceptorEnvironment.cs @@ -18,11 +18,18 @@ internal sealed class InterceptorEnvironment : IEquatable TypeHandlers { get; } // [TypeHandler(...)] at module/assembly level + /// + /// Is this project headed for native AOT? Decides whether leaving a call-site on vanilla + /// Dapper is merely a missed optimization (info) or a latent publish-time crash (warning). + /// + public bool TargetsNativeAot { get; } + public InterceptorEnvironment(bool allowUnsafe, string? assemblyName, bool hasInterceptsLocationAttribute, bool needsCommandPrep, string? baseCommandFactoryName, bool baseFactoryCanConstruct, in EquatableArray specialCommandTypes, ParamPlan systemObjectPlan, - in EquatableArray typeHandlers) + in EquatableArray typeHandlers, bool targetsNativeAot = false) { + TargetsNativeAot = targetsNativeAot; AllowUnsafe = allowUnsafe; AssemblyName = assemblyName; HasInterceptsLocationAttribute = hasInterceptsLocationAttribute; @@ -43,7 +50,8 @@ public bool Equals(InterceptorEnvironment? other) => other is not null && BaseFactoryCanConstruct == other.BaseFactoryCanConstruct && SpecialCommandTypes.Equals(other.SpecialCommandTypes) && SystemObjectPlan.Equals(other.SystemObjectPlan) - && TypeHandlers.Equals(other.TypeHandlers); + && TypeHandlers.Equals(other.TypeHandlers) + && TargetsNativeAot == other.TargetsNativeAot; public override bool Equals(object? obj) => Equals(obj as InterceptorEnvironment); public override int GetHashCode() diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/ParseState.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/ParseState.cs index 39587ec5..046a865a 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/ParseState.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/ParseState.cs @@ -85,6 +85,12 @@ public GenerateState(SourceProductionContext ctx, in (InterceptorEnvironment Env public readonly InterceptorEnvironment Environment; public readonly GeneratorContext GeneratorContext = new(); + /// + /// Is the consuming project publishing native AOT? Some refusals are a missed optimization + /// there and a latent crash here, and report at different severities accordingly. + /// + public bool TargetsNativeAot => Environment.TargetsNativeAot; + internal void ReportDiagnostic(Diagnostic diagnostic) { if (proxy is not null) diff --git a/test/Dapper.AOT.Test/ApiSurface.expected.txt b/test/Dapper.AOT.Test/ApiSurface.expected.txt index 01cc4388..c3dd7750 100644 --- a/test/Dapper.AOT.Test/ApiSurface.expected.txt +++ b/test/Dapper.AOT.Test/ApiSurface.expected.txt @@ -14,6 +14,12 @@ Dispositions: not inspected outside the generator's name filter, so never examined - correct for helpers that need no interception, and worth checking nothing real is here + unsupported: CommandDefinition + the operation is supported, but not in this + spelling: the SQL lives inside the struct, where + build-time inspection cannot reach it. Reported by + the generator as DAP057 (a warning when the project + publishes native AOT, info otherwise) skipped silently dropped with nothing reported - the consumer gets vanilla Dapper under JIT and a runtime failure under native AOT, with no build-time signal. Every row here @@ -86,34 +92,17 @@ Dispositions: ReplaceLiterals(IDbCommand) SetTypeName(string) -## skipped silently (22) +## skipped silently (1) - Execute(CommandDefinition) - ExecuteAsync(CommandDefinition) - ExecuteScalar(CommandDefinition) - ExecuteScalar<1>(CommandDefinition) - ExecuteScalarAsync(CommandDefinition) - ExecuteScalarAsync<1>(CommandDefinition) GetRowParser<1>(Type?, int, int, bool) - Query<1>(CommandDefinition) - QueryAsync(CommandDefinition) - QueryAsync<1>(CommandDefinition) - QueryFirst<1>(CommandDefinition) - QueryFirstAsync(CommandDefinition) - QueryFirstAsync<1>(CommandDefinition) - QueryFirstOrDefault<1>(CommandDefinition) - QueryFirstOrDefaultAsync(CommandDefinition) - QueryFirstOrDefaultAsync<1>(CommandDefinition) - QuerySingle<1>(CommandDefinition) - QuerySingleAsync(CommandDefinition) - QuerySingleAsync<1>(CommandDefinition) - QuerySingleOrDefault<1>(CommandDefinition) - QuerySingleOrDefaultAsync(CommandDefinition) - QuerySingleOrDefaultAsync<1>(CommandDefinition) -## unsupported API (diagnosed) (16) +## unsupported API (diagnosed) (28) + ExecuteReader(CommandDefinition) + ExecuteReader(CommandDefinition, CommandBehavior) ExecuteReader(string, object?, IDbTransaction?, int?, CommandType?) + ExecuteReaderAsync(CommandDefinition) + ExecuteReaderAsync(CommandDefinition, CommandBehavior) ExecuteReaderAsync(string, object?, IDbTransaction?, int?, CommandType?) Query<3>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) Query<4>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) @@ -121,26 +110,43 @@ Dispositions: Query<6>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) Query<7>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) Query<8>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryAsync<3>(CommandDefinition, Func, string) QueryAsync<3>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryAsync<4>(CommandDefinition, Func, string) QueryAsync<4>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryAsync<5>(CommandDefinition, Func, string) QueryAsync<5>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryAsync<6>(CommandDefinition, Func, string) QueryAsync<6>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryAsync<7>(CommandDefinition, Func, string) QueryAsync<7>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryAsync<8>(CommandDefinition, Func, string) QueryAsync<8>(string, Func, object?, IDbTransaction?, bool, string, int?, CommandType?) + QueryMultiple(CommandDefinition) QueryMultiple(string, object?, IDbTransaction?, int?, CommandType?) + QueryMultipleAsync(CommandDefinition) QueryMultipleAsync(string, object?, IDbTransaction?, int?, CommandType?) -## unsupported API (undiagnosed) (12) +## unsupported: CommandDefinition (21) - ExecuteReader(CommandDefinition) - ExecuteReader(CommandDefinition, CommandBehavior) - ExecuteReaderAsync(CommandDefinition) - ExecuteReaderAsync(CommandDefinition, CommandBehavior) - QueryAsync<3>(CommandDefinition, Func, string) - QueryAsync<4>(CommandDefinition, Func, string) - QueryAsync<5>(CommandDefinition, Func, string) - QueryAsync<6>(CommandDefinition, Func, string) - QueryAsync<7>(CommandDefinition, Func, string) - QueryAsync<8>(CommandDefinition, Func, string) - QueryMultiple(CommandDefinition) - QueryMultipleAsync(CommandDefinition) + Execute(CommandDefinition) + ExecuteAsync(CommandDefinition) + ExecuteScalar(CommandDefinition) + ExecuteScalar<1>(CommandDefinition) + ExecuteScalarAsync(CommandDefinition) + ExecuteScalarAsync<1>(CommandDefinition) + Query<1>(CommandDefinition) + QueryAsync(CommandDefinition) + QueryAsync<1>(CommandDefinition) + QueryFirst<1>(CommandDefinition) + QueryFirstAsync(CommandDefinition) + QueryFirstAsync<1>(CommandDefinition) + QueryFirstOrDefault<1>(CommandDefinition) + QueryFirstOrDefaultAsync(CommandDefinition) + QueryFirstOrDefaultAsync<1>(CommandDefinition) + QuerySingle<1>(CommandDefinition) + QuerySingleAsync(CommandDefinition) + QuerySingleAsync<1>(CommandDefinition) + QuerySingleOrDefault<1>(CommandDefinition) + QuerySingleOrDefaultAsync(CommandDefinition) + QuerySingleOrDefaultAsync<1>(CommandDefinition) diff --git a/test/Dapper.AOT.Test/ApiSurfaceCoverageTests.cs b/test/Dapper.AOT.Test/ApiSurfaceCoverageTests.cs index ac0f29af..b9232deb 100644 --- a/test/Dapper.AOT.Test/ApiSurfaceCoverageTests.cs +++ b/test/Dapper.AOT.Test/ApiSurfaceCoverageTests.cs @@ -89,6 +89,12 @@ unsupported API (undiagnosed) refused, but nothing says so not inspected outside the generator's name filter, so never examined - correct for helpers that need no interception, and worth checking nothing real is here + unsupported: CommandDefinition + the operation is supported, but not in this + spelling: the SQL lives inside the struct, where + build-time inspection cannot reach it. Reported by + the generator as DAP057 (a warning when the project + publishes native AOT, info otherwise) skipped silently dropped with nothing reported - the consumer gets vanilla Dapper under JIT and a runtime failure under native AOT, with no build-time signal. Every row here @@ -137,13 +143,31 @@ private static string Classify(IMethodSymbol method) } // the analyzer only inspects (and so only reports on) call-sites carrying SQL as a - // string argument; an overload that hides it inside CommandDefinition is dropped mute - var diagnosed = DapperInterceptorGenerator.IsVisibleToAnalyzer(method); + // string argument. An overload that hides it inside CommandDefinition is invisible to + // it - so the *generator* reports those instead, and neither is mute any more + var visible = DapperInterceptorGenerator.IsVisibleToAnalyzer(method); if (flags.HasAny(OperationFlags.NotAotSupported)) { - return diagnosed ? "unsupported API (diagnosed)" : "unsupported API (undiagnosed)"; + // DAP001 either way: from the analyzer when it can see the call, from the generator + // when it cannot + return "unsupported API (diagnosed)"; } - return diagnosed ? "candidate" : "skipped silently"; + if (visible) return "candidate"; + if (TakesCommandDefinition(method)) return "unsupported: CommandDefinition"; + + // invisible to the analyzer, not CommandDefinition-shaped, and supportable at some + // call-sites but not others - only GetRowParser(concreteType) reaches here, and what + // it does depends on whether a given call passes the Type. A symbol cannot say + return "skipped silently"; + } + + private static bool TakesCommandDefinition(IMethodSymbol method) + { + foreach (var p in method.Parameters) + { + if (p.Type is { Name: "CommandDefinition", ContainingNamespace.Name: "Dapper" }) return true; + } + return false; } private static string Describe(IMethodSymbol method) diff --git a/test/Dapper.AOT.Test/CommandDefinitionDiagnosticTests.cs b/test/Dapper.AOT.Test/CommandDefinitionDiagnosticTests.cs new file mode 100644 index 00000000..7b99c4ce --- /dev/null +++ b/test/Dapper.AOT.Test/CommandDefinitionDiagnosticTests.cs @@ -0,0 +1,85 @@ +using Dapper.AOT.Test.TestCommon; +using Dapper.CodeAnalysis; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using Xunit; + +namespace Dapper.AOT.Test; + +/// +/// DAP057 reports the same fact at two different severities: leaving a call-site on vanilla +/// Dapper is a missed optimization under JIT and a latent publish-time crash under native AOT. +/// The interceptor goldens only cover the JIT case (no MSBuild properties in that harness), so +/// the promotion is pinned here. +/// +public class CommandDefinitionDiagnosticTests +{ + private const string Source = """ + using Dapper; + using System.Data.Common; + + [module: DapperAot] + + class SomeCode + { + public void Foo(DbConnection conn) + { + conn.Execute(new CommandDefinition("somesql")); + } + } + """; + + [Theory] + [InlineData(null, DiagnosticSeverity.Info)] // no PublishAot: a missed optimization + [InlineData("false", DiagnosticSeverity.Info)] + [InlineData("true", DiagnosticSeverity.Warning)] // PublishAot: this one will crash + public void SeverityFollowsPublishAot(string? enableAotAnalyzer, DiagnosticSeverity expected) + { + var diagnostic = Assert.Single(Run(enableAotAnalyzer).Where(static d => d.Id == "DAP057")); + Assert.Equal(expected, diagnostic.Severity); + Assert.Contains("CommandDefinition", diagnostic.GetMessage()); + } + + private static ImmutableArray Run(string? enableAotAnalyzer) + { + var compilation = RoslynTestHelpers.CreateCompilation(Source, "assembly", "input.cs"); + var driver = CSharpGeneratorDriver.Create( + [new DapperInterceptorGenerator().AsSourceGenerator()], + parseOptions: RoslynTestHelpers.ParseOptionsLatestLangVer, + optionsProvider: new OptionsProvider(enableAotAnalyzer)); + driver.RunGeneratorsAndUpdateCompilation(compilation, out _, out var diagnostics); + return diagnostics; + } + + // the SDK sets EnableAotAnalyzer when PublishAot is on, and it is compiler-visible by + // default; that is what the generator reads, so that is what the test supplies + private sealed class OptionsProvider(string? enableAotAnalyzer) : AnalyzerConfigOptionsProvider + { + public override AnalyzerConfigOptions GlobalOptions { get; } = new Options(enableAotAnalyzer); + public override AnalyzerConfigOptions GetOptions(SyntaxTree tree) => Options.Empty; + public override AnalyzerConfigOptions GetOptions(AdditionalText textFile) => Options.Empty; + + private sealed class Options(string? enableAotAnalyzer) : AnalyzerConfigOptions + { + public static readonly Options Empty = new(null); + + public override bool TryGetValue(string key, out string value) + { + if (enableAotAnalyzer is not null && key == Dapper.CodeAnalysis.GlobalOptions.Keys.ProjectProperties_EnableAotAnalyzer) + { + value = enableAotAnalyzer; + return true; + } + value = null!; + return false; + } + + public override IEnumerable Keys => enableAotAnalyzer is null + ? [] : [Dapper.CodeAnalysis.GlobalOptions.Keys.ProjectProperties_EnableAotAnalyzer]; + } + } +} diff --git a/test/Dapper.AOT.Test/Interceptors/CommandDefinitionOverloads.output.txt b/test/Dapper.AOT.Test/Interceptors/CommandDefinitionOverloads.output.txt index 5ade3665..7b996834 100644 --- a/test/Dapper.AOT.Test/Interceptors/CommandDefinitionOverloads.output.txt +++ b/test/Dapper.AOT.Test/Interceptors/CommandDefinitionOverloads.output.txt @@ -1,4 +1,10 @@ -Generator produced 1 diagnostics: +Generator produced 3 diagnostics: Hidden DAP000 L1 C1 -Dapper.AOT handled 1 of 3 enabled call-sites (0 unsupported API, 0 refused with diagnostics, 2 skipped silently) using 1 interceptors, 1 commands and 0 readers +Dapper.AOT handled 1 of 3 enabled call-sites (0 unsupported API, 2 refused with diagnostics, 0 skipped silently) using 1 interceptors, 1 commands and 0 readers + +Info DAP057 Interceptors/CommandDefinitionOverloads.input.cs L18 C24 +'Query' passes its SQL inside a CommandDefinition, which Dapper.AOT cannot read at build time; use the overload that takes the SQL directly. This call-site is left on vanilla Dapper, which will not work under native AOT + +Info DAP057 Interceptors/CommandDefinitionOverloads.input.cs L19 C24 +'Execute' passes its SQL inside a CommandDefinition, which Dapper.AOT cannot read at build time; use the overload that takes the SQL directly. This call-site is left on vanilla Dapper, which will not work under native AOT diff --git a/version.json b/version.json index 24082eee..01f9d210 100644 --- a/version.json +++ b/version.json @@ -1,6 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/dotnet/Nerdbank.GitVersioning/main/src/NerdBank.GitVersioning/version.schema.json", - "version": "1.0", + "version": "1.1", + "versionHeightOffset": -1, "assemblyVersion": "1.0.0.0", "publicReleaseRefSpec": [ "^refs/heads/main$", From a3061e171d1f232b54d42d607e8fc10bc878294f Mon Sep 17 00:00:00 2001 From: mgravell Date: Fri, 11 Sep 2026 14:31:17 +0100 Subject: [PATCH 2/2] DAP001 follows PublishAot too DAP057 warned under native AOT while DAP001 stayed info, so an AOT project was told about Execute(CommandDefinition) but not about QueryMultiple - both of which fail identically after publish. Same argument, same treatment: the analyzer gains an AnalyzerOptions overload of TargetsNativeAot, and both DAP001 report sites (analyzer for the visible spellings, generator for the ones it cannot see) promote to warning when the consumer is publishing native AOT. This turns out to matter more than tidiness. **Info-severity analyzer diagnostics do not surface in a normal `dotnet build`** - only in IDEs and at raised verbosity - so DAP001 was effectively being emitted into a void, and the surface report's "unsupported API (diagnosed)" bucket was true on paper and silent in practice. The promotion is what makes the unsupported surface visible to the people it will hurt; without it DAP057 would be the only thing an AOT publisher ever saw. Measured on the Dapper suite built with the AOT flag set (MSBuild double-lists, so halve these): 152 DAP001 + 50 DAP057, i.e. ~76 and ~25 real call-sites, every one of which would fail after publish. A JIT consumer sees no change at all. 12 lines across three files, because the TargetsNativeAot plumbing already existed and DAP001 has only two report sites. No golden churn - the test harness sets no MSBuild properties, so goldens stay info. Unit suites 380/380 on net8.0 and net10.0; corpus unchanged at 729 of 800. --- src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs | 6 +++++- .../CodeAnalysis/DapperInterceptorGenerator.cs | 4 +++- src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs | 4 ++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs index e626ad2b..a72e42a9 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs @@ -309,7 +309,11 @@ private void ValidateDapperMethod(in OperationAnalysisContext ctx, IOperation sq OnDapperAotHit(); // all good for AOT if (flags.HasAny(OperationFlags.NotAotSupported)) { - ctx.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnsupportedMethod, location, invoke.GetSignature())); + // left on vanilla Dapper: a missed optimization under JIT, a latent + // publish-time crash under native AOT - so it reports at both severities + ctx.ReportDiagnostic(Diagnostic.Create(Diagnostics.UnsupportedMethod, location, + ctx.Options.TargetsNativeAot() ? DiagnosticSeverity.Warning : DiagnosticSeverity.Info, + additionalLocations: null, properties: null, invoke.GetSignature())); } } else if (!aotAttribExists && !flags.HasAny(OperationFlags.NotAotSupported)) diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs index ec991972..8dc3687a 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/DapperInterceptorGenerator.cs @@ -544,7 +544,9 @@ internal void Generate(in GenerateState ctx) // DAP001, from here rather than the analyzer, which cannot see this overload - // so both spellings of an unsupported API report the same thing ctx.ReportDiagnostic(Diagnostic.Create(DapperAnalyzer.Diagnostics.UnsupportedMethod, - skip.Location.AsLocation(), skip.MethodName)); + skip.Location.AsLocation(), + ctx.TargetsNativeAot ? DiagnosticSeverity.Warning : DiagnosticSeverity.Info, + additionalLocations: null, properties: null, skip.MethodName)); unsupported++; continue; } diff --git a/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs b/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs index dc28a279..b9edbafd 100644 --- a/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs +++ b/src/Dapper.AOT.Analyzers/CodeAnalysis/GlobalOptions.cs @@ -20,6 +20,10 @@ public static class Keys public const string ProjectProperties_EnableAotAnalyzer = "build_property.EnableAotAnalyzer"; } + /// + public static bool TargetsNativeAot(this AnalyzerOptions? options) + => options?.AnalyzerConfigOptionsProvider.TargetsNativeAot() ?? false; + /// /// Is the consuming project headed for native AOT? Decides whether a call-site we leave on /// vanilla Dapper is a missed optimization or a latent publish-time crash.