Skip to content

Commit

Permalink
refactor generate path (#56)
Browse files Browse the repository at this point in the history
* tidy up test framework

* DAP005tests

* getting into the testing swing

* all the refactoring; VB happening!

* sql rules

* tests for sql syntax selection

* DAP203,DAP205,DAP205

* link

* more sql

* all the other SQL rules

* don't worry about ending GO

* allow joins in 223/224; handle sub-queries in 225

* nit

* DAP027,DAP028

* DAP001, DAP005

* DAP038

* general UnknownError DAP999

* ack delta

* fix #52

* 25/26

* rework constructor code

* ack drift

* top/fetch

* 234/235

* err

* more null

* nit

* allow serialization ctors; be lazier re options

* fix false positive failure on parameter handling

* intermediate

* more verifiers - 006,007,011,012,013,014,015,016

* verifier/docs DAP017
  • Loading branch information
mgravell authored Sep 21, 2023
1 parent 3349d60 commit 46760b8
Show file tree
Hide file tree
Showing 210 changed files with 5,812 additions and 3,189 deletions.
2 changes: 1 addition & 1 deletion Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<DelaySign>False</DelaySign>
<Description>Build time (AOT) tools for Dapper</Description>
<AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)Dapper.AOT.snk</AssemblyOriginatorKeyFile>
<LangVersion>9</LangVersion>
<Nullable>enable</Nullable>
<RootNamespace>Dapper</RootNamespace>
<PackageLicenseExpression>Apache-2.0</PackageLicenseExpression>
Expand All @@ -26,6 +25,7 @@
<DefaultLanguage>en-US</DefaultLanguage>
<IncludeSymbols>false</IncludeSymbols>
<AnalysisMode>latest-Recommended</AnalysisMode>
<LangVersion>preview</LangVersion>
<Features>($Features);strict</Features>
</PropertyGroup>
<ItemGroup>
Expand Down
3 changes: 3 additions & 0 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
<PackageVersion Include="Microsoft.Build.Utilities.Core" Version="17.7.2" />
<!-- 4.8 would be nice, but: let's try to offer down-level compiler support -->
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.4.0" />
<PackageVersion Include="Microsoft.CodeAnalysis.VisualBasic.Workspaces" Version="4.4.0" />
<PackageVersion Include="Microsoft.Data.SqlClient" Version="5.1.1" />
<PackageVersion Include="Microsoft.EntityFrameworkCore" Version="7.0.11" />
<PackageVersion Include="Microsoft.EntityFrameworkCore.SqlServer" Version="7.0.11" />
Expand All @@ -30,5 +31,7 @@

<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.Analyzer.Testing.XUnit" Version="1.1.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.CSharp.CodeFix.Testing.XUnit" Version="1.1.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.VisualBasic.Analyzer.Testing.XUnit" Version="1.1.1" />
<PackageVersion Include="Microsoft.CodeAnalysis.VisualBasic.CodeFix.Testing.XUnit" Version="1.1.1" />
</ItemGroup>
</Project>
7 changes: 7 additions & 0 deletions docs/rules/DAP001.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# DAP001

We're focusing on delivering the most common Dapper scenarios first; not everything currently has AOT support.

The good news is: we'll just let regular vanilla Dapper handle the code, so it should still work fine. Just; without AOT. Sorry.

If you think a particular API deserves attention: [*let us know*](https://github.com/DapperLib/DapperAOT/issues)..
21 changes: 21 additions & 0 deletions docs/rules/DAP006.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# DAP006

In C#, tuple-types like `(int id, string name)` are implemented by the compiler using trickery; that trickery is passed *outwards* to *consuming*
code, but that name information (`"id"` and `"name"`) is not available at runtime and the names are not passed *inwards* at runtime to library code.

Because of this, **vanilla** (runtime-only) Dapper can't really use such values for parameters. Since this would involve "boxing" *anyway*, this isn't
really a problem (although Dapper.AOT *can* see the names at build-time, and can act correctly).

Short version: don't use this syntax for parameters. Perhaps use anonymous types instead.

Bad:

``` csharp
conn.Execute("somesql", (id: 42, name: "abc"));
```

Good:

``` csharp
conn.Execute("somesql", new { id = 42, name = "abc" });
```
18 changes: 18 additions & 0 deletions docs/rules/DAP007.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# DAP007

The [`CommandType`](https://learn.microsoft.com/dotnet/api/system.data.commandtype) in ADO.NET is quite important. If Dapper
can *see* your command-type at compile-time, but knows that it isn't something it understands, then it isn't sure what you
want to do. If you genuinely have a need to use an unknown command-type, maybe [log an issue](https://github.com/DapperLib/DapperAOT/issues),
explaining the context!

Bad:

``` csharp
conn.Execute("somesql", commandType: (CommandType)42);
```

Good:

``` csharp
conn.Execute("somesql", commandType: CommandType.StoredProcedure);
```
7 changes: 7 additions & 0 deletions docs/rules/DAP009.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# DAP009

Huh; I guess we missed an overload, and you found a parameter for Dapper that we aren't currently expecting.

Sorry, mea cupla.

Please [log an issue](https://github.com/DapperLib/DapperAOT/issues) with example code; we want to fix this!
22 changes: 22 additions & 0 deletions docs/rules/DAP011.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# DAP011

Historically, Dapper has bound tuples *positionally*, because it does not have access to name metadata.

It looks like you have bind-by-name semantics enabled (`[BindTupleByName]`), and you're using named tuples - so you
probably expect this query to be bound ... by name; however, this query is going to be executed by vanilla non-AOT Dapper,
so: that won't happen.

Suggestions:

- remove `[BindTupleByName]` or use `[BindTupleByName(false)]` to explicitly acknowledge that you're using positional binding
- find out why Dapper.AOT can't help with this method, and fix that - maybe add `[DapperAot]`?
- use a regular non-tuple type - maybe a `struct record`
- remove the names from the tuple (because: they don't have any effect here)

As an example of the `struct record` suggestion:

``` csharp
var data = conn.Query<MyData>("somesql");
// ...
public readonly record struct MyData(string name, int id);
```
19 changes: 19 additions & 0 deletions docs/rules/DAP012.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# DAP012

As explained in [DAP006](DAP006), Dapper.AOT can make use of the names in tuple syntax (for example `(id: 42, name: "abc")`). However, because this represents
a change from runtime-only Dapper, we want to be sure that this is what you *wanted*.

To help clarify your intent, please add `[BindTupleByName]` or `[BindTupleByName(false)]` to enable/disable use of name data. Or alternatively,
just use an anonymous type instead of a tuple:

Ambiguous (unless `BindTupleByNameAttribute` is specified):

``` csharp
var data = conn.Query<MyData>("somesql", (id: 42, name: "abc"));
```

Always clear:

``` csharp
var data = conn.Query<MyData>("somesql", new { id = 42, name = "abc" });
```
20 changes: 20 additions & 0 deletions docs/rules/DAP013.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# DAP013

Dapper.AOT does not currently support tuple-type results. It absolutely *will*; I just haven't got there yet.

This includes both positional and nomimal usage, i.e. `Query<(int, string)>` and `Query<(int id, string name)>`.

Suggestions:

- use a regular non-tuple type - maybe a `struct record`
- use vanilla Dapper by adding `[DapperAot(false)]`
- use a non-tuple type as the result, noting that this will be bound by position **only** (see [DAP011](DAP011))


As an example of the `struct record` suggestion:

``` csharp
var data = conn.Query<MyData>("somesql");
// ...
public readonly record struct MyData(string name, int id);
```
18 changes: 18 additions & 0 deletions docs/rules/DAP014.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# DAP014

As explained in [DAP006](DAP006), Dapper.AOT can make use of the names in tuple syntax (for example `(id: 42, name: "abc")`). That's the theory. In practice,
we haven't implemented that just yet, so ... it isn't going to work. Hey, we're human.

Maybe just use an anonymous type instead?

Not yet supported:

``` csharp
var data = conn.Query<MyData>("somesql", (id: 42, name: "abc"));
```

Good:

``` csharp
var data = conn.Query<MyData>("somesql", new { id = 42, name = "abc" });
```
24 changes: 24 additions & 0 deletions docs/rules/DAP015.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# DAP015

To handle parameters efficiently, Dapper.AOT first needs to understand what you're doing. If your parameter data is `object`, `dynamic` etc,
then we can't do that at compile-time, so: Dapper.AOT can't help.

Either used typed parameters, or disable Dapper.AOT (`[DapperAot(false)]`).

If you're using `DynamicParameters`, note that this is often *wildly* overused. If your usage looks like

``` csharp
var args = new DynamicParameters();
args.Add("id", 42);
args.Add("name", "Fred");
conn.Execute("somesql", args);
```

then you can make this *much* more efficient (and Dapper.AOT-friendly) with simply:

``` csharp
conn.Execute("somesql", new { id = 42, name = "Fred" });
```

If you're using `DynamicParameters` for things like output-parameters, Dapper.AOT has new options for that; you can use `[DbValue(Direction = ...)]` on
properties of a custom type.
7 changes: 7 additions & 0 deletions docs/rules/DAP016.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# DAP016

As discussed in [DAP015](DAP015), Dapper.AOT wants to understand your data at compile-time, and it can't do that when generics are involved.

Specifically, you can't use a `T args` (or `TSomething args`) as the parameters to a Dapper call.

Either use non-generic arguments when calling Dapper, or disable Dapper.AOT with `[DapperAot(false)]`.
21 changes: 21 additions & 0 deletions docs/rules/DAP017.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# DAP017

Dapper.AOT works by generating code to handle your types; to do that, those types need to be accessible to the generated code; that means they must have
at least `internal` accessibility.

Since Dapper.AOT is adding code to your assembly, this problem is limited to nested types that are `private`, `protected`, `private protected`, or similar -
basically anywhere where *any other part of your code* couldn't mention the type by name.

Suggestions:

- make the type `internal`:
- `private` => `internal`
- `protected` => `protected internal`
- disable Dapper.AOT (`[DapperAot(false)]`)



> Can Dapper.AOT not just generate the code inside the relevant types via `partial` types?
No; generators are limited to placing their output in pre-defined namespaces, which precludes generating code directly inside your types.
We may be able to revisit this at a later date by having you add additional configuration to your project file, allowing Dapper.AOT to generate inside *your* namespaces.
24 changes: 24 additions & 0 deletions docs/rules/DAP025.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# DAP025

It looks like you're using an `Execute` operation (which doesn't expect rows) to
invoke something that *definitely has a query*. Those results will be
silently ignored, which probably isn't what you intended. Did you mean to use
a `Query` API?

Bad:

``` c#
conn.Execute("""
select Id, Name
from SomeLookupTable
""");
```

Good:

``` c#
var rows = conn.Query<MyLookup>("""
select Id, Name
from SomeLookupTable
""");
```
33 changes: 33 additions & 0 deletions docs/rules/DAP026.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# DAP026

It looks like you're using a `Query` operation (which expects rows) to
invoke something that *definitely does not have a query*. This won't work; did you
mean to use an `Execute` API, or fetch something?

Bad:

``` c#
var id = conn.QuerySingle<int>("""
insert SomeLookupTable (Name)
values ('North')
""");
```

Good:

``` c#
var id = conn.QuerySingle<int>("""
insert SomeLookupTable (Name)
output inserted.Id
values ('North')
""");
```

Also fine:

``` c#
conn.Execute("""
insert SomeLookupTable (Name)
values ('North')
""");
```
21 changes: 21 additions & 0 deletions docs/rules/DAP027.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# DAP027

The `Query` APIs allow multiple rows to be returned, which is *fine* - but if you only want a single row, there are
scenario-specific APIs that allow for significant additional optimizations. The
`QueryFirst`, `QueryFirstOrDefault`, `QuerySingle` and `QuerySingleOrDefault` work as you would expect, and compare directly
to the corresponding LINQ [`First`](https://learn.microsoft.com/dotnet/api/system.linq.enumerable.first),
[`FirstOrDefault`](https://learn.microsoft.com/dotnet/api/system.linq.enumerable.firstordefault),
[`Single`](https://learn.microsoft.com/dotnet/api/system.linq.enumerable.single)
and [`SingleOrDefault`](https://learn.microsoft.com/dotnet/api/system.linq.enumerable.singleordefault) methods.

Bad:

``` c#
var order = conn.Query<Order>(sql, args).First();
```

Good:

``` c#
var order = conn.QueryFirst<Order>(sql, args);
```
19 changes: 19 additions & 0 deletions docs/rules/DAP028.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# DAP028

The `Query` APIs allow optional buffering, so it returns the value as `IEnumerable<T>`. However, the default is "buffered", which means that
most of the time, the result *actually is* a `List<T>`. If you call `.ToList()` on that, you create an **additional** `List<T>` with the same
contents, which is unnecessary.

To avoid this, Dapper provides an `AsList()` method, which gives you the *existing* `List<T>` if it is one, otherwise it creates one.

Bad:

``` c#
var orders = conn.Query<Order>(sql, args).ToList();
```

Good:

``` c#
var orders = conn.Query<Order>(sql, args).AsList();
```
29 changes: 29 additions & 0 deletions docs/rules/DAP035.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# DAP035

It looks like you have multiple constructors on a type marked `[ExplicitConstructor]`; that's just confusing!
Pick one, and remove the attribute from the others. This should *probably* be the one with the most parameters.

Bad:

``` csharp
class MyType
{
[ExplicitConstructor]
public MyType(int id, string name) { }

[ExplicitConstructor]
public MyType(DateTime when, int id, string name) { }
}
```

Good:

``` csharp
class MyType
{
public MyType(int id, string name) { }

[ExplicitConstructor]
public MyType(DateTime when, int id, string name) { }
}
```
Loading

0 comments on commit 46760b8

Please sign in to comment.