Skip to content

Commit

Permalink
feat: support [Column] (#85)
Browse files Browse the repository at this point in the history
* implement model

* merge main

* support [column] attribute

* fix indents

* fix indent x2

* rollback

* fix `GetLocation(attributeName)` logic

* Update DAP043.md

* Update DAP042.md

* Update DAP043.md

* Update Inspection.cs

* add attr usage

---------

Co-authored-by: Marc Gravell <marc.gravell@gmail.com>
  • Loading branch information
DeagleGross and mgravell authored Dec 13, 2023
1 parent 643477a commit 636fd63
Show file tree
Hide file tree
Showing 17 changed files with 512 additions and 1,027 deletions.
41 changes: 41 additions & 0 deletions docs/rules/DAP042.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# DAP042

Usage of [`[Column]`](https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.schema.columnattribute)
attribute and `[DbValue]` attributes are conflicting - Dapper will choose either one or another's name override.

This conflict can lead to unexpected behavior, so we recommend to use only one of them.

Bad:

``` csharp
class MyType
{
[DbValue(Name = "MyOtherOtherName")]
[UseColumnAttribute]
[Column("MyOtherName")]
public int MyThisName { get; set; }
}
```

Good:

``` csharp
class MyType
{
[DbValue(Name = "MyOtherOtherName")]
// without [Column] and [UseColumnAttribute]
public int MyThisName { get; set; }
}
```

Another good:

``` csharp
class MyType
{
// without [DbValue]
[UseColumnAttribute]
[Column("MyOtherName")]
public int MyThisName { get; set; }
}
```
28 changes: 28 additions & 0 deletions docs/rules/DAP043.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# DAP043

If you are looking at `DAP043`, then most probably you wanted to use [`[Column]`](https://learn.microsoft.com/dotnet/api/system.componentmodel.dataannotations.schema.columnattribute)
attribute to override the name of the type member.

However, due to [historical reasons](https://stackoverflow.com/a/77073456) you need to add a "marker attribute" `[UseColumnAttribute]`
to explicitly let Dapper know you want to override the type member's name.

Bad:

``` csharp
class MyType
{
[Column("MyOtherName")]
public int MyThisName { get; set; }
}
```

Good:

``` csharp
class MyType
{
[UseColumnAttribute]
[Column("MyOtherName")]
public int MyThisName { get; set; }
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public static readonly DiagnosticDescriptor
ValueTypeSingleFirstOrDefaultUsage = LibraryWarning("DAP038", "Value-type single row 'OrDefault' usage", "Type '{0}' is a value-type; it will not be trivial to identify missing rows from {1}"),
FactoryMethodMultipleExplicit = LibraryError("DAP039", "Multiple explicit factory methods", "Only one factory method should be marked [ExplicitConstructor] for type '{0}'"),
ConstructorOverridesFactoryMethod = LibraryWarning("DAP041", "Constructor overrides factory method", "Type '{0}' has both constructor and factory method; Constructor will be used instead of a factory method"),
ParameterNameOverrideConflict = LibraryWarning("DAP042", "Parameter name override conflict", "A column name is specified via both [DbValue] and [Column]; '{0}' will be used"),
UseColumnAttributeNotSpecified = LibraryWarning("DAP043", "[Column] has no effect", "Attach the [UseColumnAttribute] attribute to make Dapper consider [Column]"),

// SQL parse specific
GeneralSqlError = SqlWarning("DAP200", "SQL error", "SQL error: {0}"),
Expand Down
30 changes: 30 additions & 0 deletions src/Dapper.AOT.Analyzers/CodeAnalysis/DapperAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,14 @@ private void ValidateDapperMethod(in OperationAnalysisContext ctx, IOperation sq
var resultMap = MemberMap.CreateForResults(resultType, location);
if (resultMap is not null)
{
if (resultMap.Members.Length != 0)
{
foreach (var member in resultMap.Members)
{
ValidateMember(member, onDiagnostic);
}
}

// check for single-row value-type usage
if (flags.HasAny(OperationFlags.SingleRow) && !flags.HasAny(OperationFlags.AtLeastOne)
&& resultMap.ElementType.IsValueType && !CouldBeNullable(resultMap.ElementType))
Expand Down Expand Up @@ -817,6 +825,28 @@ enum ParameterMode
? null : new(rowCountHint, rowCountHintMember?.Member.Name, batchSize, cmdProps);
}

static void ValidateMember(ElementMember member, Action<Diagnostic>? reportDiagnostic)
{
ValidateColumnAttribute();

void ValidateColumnAttribute()

This comment has been minimized.

Copy link
@dosper7

dosper7 Apr 10, 2024

Out of curiosity, what was the rationale of having a local function vs have the code in the ValidateMember code? Readability?

{
if (member.HasDbValueAttribute && member.ColumnAttributeData.IsCorrectUsage)
{
reportDiagnostic?.Invoke(Diagnostic.Create(Diagnostics.ParameterNameOverrideConflict,
location: member.GetLocation(Types.DbValueAttribute),
additionalLocations: member.AsAdditionalLocations(Types.ColumnAttribute, allowNonDapperLocations: true),
messageArgs: member.DbName));
}
if (member.ColumnAttributeData.ColumnAttribute == ColumnAttributeData.ColumnAttributeState.Specified
&& member.ColumnAttributeData.UseColumnAttribute == ColumnAttributeData.UseColumnAttributeState.NotSpecified)
{
reportDiagnostic?.Invoke(Diagnostic.Create(Diagnostics.UseColumnAttributeNotSpecified,
location: member.GetLocation(Types.ColumnAttribute, allowNonDapperLocations: true)));
}
}
}

internal static ImmutableArray<ElementMember>? SharedGetParametersToInclude(MemberMap? map, ref OperationFlags flags, string? sql, Action<Diagnostic>? reportDiagnostic, out SqlParseOutputFlags parseFlags)
{
SortedDictionary<string, ElementMember>? byDbName = null;
Expand Down
139 changes: 129 additions & 10 deletions src/Dapper.AOT.Analyzers/Internal/Inspection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,22 @@ public static bool IsDapperAttribute(AttributeData attrib)
}
};

public static AttributeData? GetAttribute(ISymbol? symbol, string attributeName)
{
if (symbol is not null)
{
foreach (var attrib in symbol.GetAttributes())
{
if (attrib.AttributeClass!.Name == attributeName)
{
return attrib;
}
}
}

return null;
}

public static AttributeData? GetDapperAttribute(ISymbol? symbol, string attributeName)
{
if (symbol is not null)
Expand Down Expand Up @@ -404,15 +420,33 @@ public enum ElementMemberKind
}
public readonly struct ElementMember
{
public Location[]? AsAdditionalLocations(string? attributeName = null)
public Location[]? AsAdditionalLocations(string? attributeName = null, bool allowNonDapperLocations = false)
{
var loc = GetLocation(attributeName);
var loc = GetLocation(attributeName, allowNonDapperLocations);
return loc is null ? null : [loc];
}

private readonly AttributeData? _dbValue;
public string DbName => TryGetAttributeValue(_dbValue, "Name", out string? name)
&& !string.IsNullOrWhiteSpace(name) ? name!.Trim() : CodeName;
public readonly ColumnAttributeData ColumnAttributeData { get; } = ColumnAttributeData.Default;
public string DbName
{
get
{
if (TryGetAttributeValue(_dbValue, "Name", out string? name) && !string.IsNullOrWhiteSpace(name))
{
// priority 1: [DbValue] attribute
return name!.Trim();
}

if (ColumnAttributeData.IsCorrectUsage)
{
// priority 2: [Column] attribute
return ColumnAttributeData.Name!;
}

return CodeName;
}
}
public string CodeName => Member.Name;
public ISymbol Member { get; }
public ITypeSymbol CodeType => Member switch
Expand Down Expand Up @@ -486,13 +520,15 @@ public enum ElementMemberFlags
public ElementMember(
ISymbol member,
AttributeData? dbValue,
ColumnAttributeData columnAttributeData,
ElementMemberKind kind,
ElementMemberFlags flags,
int? constructorParameterOrder,
int? factoryMethodParameterOrder)
{
_dbValue = dbValue;
_flags = flags;
ColumnAttributeData = columnAttributeData;

Member = member;
Kind = kind;
Expand All @@ -518,11 +554,51 @@ public override bool Equals(object obj) => obj is ElementMember other
}
return null;
}
public Location? GetLocation(string? attributeName = null)
public Location? GetLocation(string? attributeName = null, bool allowNonDapperLocations = false)
{
if (attributeName is null)
{
return GetLocation();
}

var result = allowNonDapperLocations
? GetAttribute(Member, attributeName)?.ApplicationSyntaxReference?.GetSyntax()?.GetLocation()
: GetDapperAttribute(Member, attributeName)?.ApplicationSyntaxReference?.GetSyntax()?.GetLocation();

return result ?? GetLocation();
}
}

internal struct ColumnAttributeData
{
public UseColumnAttributeState UseColumnAttribute { get; }
public ColumnAttributeState ColumnAttribute { get; }
public string? Name { get; }

public ColumnAttributeData(UseColumnAttributeState useColumnAttribute, ColumnAttributeState columnAttribute, string? name)
{
UseColumnAttribute = useColumnAttribute;
ColumnAttribute = columnAttribute;
Name = name;
}

public bool IsCorrectUsage
=> UseColumnAttribute is UseColumnAttributeState.NotSpecified or UseColumnAttributeState.Enabled
&& ColumnAttribute is ColumnAttributeState.Specified && !string.IsNullOrEmpty(Name);

public static ColumnAttributeData Default => new(UseColumnAttributeState.NotSpecified, ColumnAttributeState.NotSpecified, null);

public enum UseColumnAttributeState
{
NotSpecified,
Disabled,
Enabled
}

public enum ColumnAttributeState
{
var loc = attributeName is null ? null :
GetDapperAttribute(Member, attributeName)?.ApplicationSyntaxReference?.GetSyntax()?.GetLocation();
return loc ?? GetLocation();
NotSpecified,
Specified
}
}

Expand Down Expand Up @@ -775,12 +851,55 @@ internal static ImmutableArray<ElementMember> GetMembers(bool forParameters, ITy
flags |= ElementMember.ElementMemberFlags.IsExpandable;
}

var columnAttributeData = ParseColumnAttributeData(member);

// all good, then!
builder.Add(new(member, dbValue, kind, flags, constructorParameterOrder, factoryMethodParamOrder));
builder.Add(new(member, dbValue, columnAttributeData, kind, flags, constructorParameterOrder, factoryMethodParamOrder));
}
return builder.ToImmutable();
}

static ColumnAttributeData ParseColumnAttributeData(ISymbol? member)
{
if (member is null) return ColumnAttributeData.Default;
var useColumnAttributeState = ParseUseColumnAttributeState();
var (columnAttributeState, columnName) = ParseColumnAttributeState();

return new ColumnAttributeData(useColumnAttributeState, columnAttributeState, columnName);

(ColumnAttributeData.ColumnAttributeState state, string? columnName) ParseColumnAttributeState()
{
var columnAttribute = GetAttribute(member, Types.ColumnAttribute);
if (columnAttribute is null) return (ColumnAttributeData.ColumnAttributeState.NotSpecified, null);

if (TryGetAttributeValue(columnAttribute, "name", out string? name) && !string.IsNullOrWhiteSpace(name))
{
return (ColumnAttributeData.ColumnAttributeState.Specified, name);
}
else
{
// [Column] is specified, but value is null. Can happen for ctor overload:
// https://learn.microsoft.com/en-us/dotnet/api/system.componentmodel.dataannotations.schema.columnattribute.-ctor?view=net-7.0#system-componentmodel-dataannotations-schema-columnattribute-ctor
return (ColumnAttributeData.ColumnAttributeState.Specified, null);
}
}
ColumnAttributeData.UseColumnAttributeState ParseUseColumnAttributeState()
{
var useColumnAttribute = GetDapperAttribute(member, Types.UseColumnAttributeAttribute);
if (useColumnAttribute is null) return ColumnAttributeData.UseColumnAttributeState.NotSpecified;

if (TryGetAttributeValue(useColumnAttribute, "enable", out bool useColumnAttributeState))
{
return useColumnAttributeState ? ColumnAttributeData.UseColumnAttributeState.Enabled : ColumnAttributeData.UseColumnAttributeState.Disabled;
}
else
{
// default
return ColumnAttributeData.UseColumnAttributeState.Enabled;
}
}
}

static IReadOnlyDictionary<string, MethodParameter> ParseMethodParameters(IMethodSymbol constructorSymbol)
{
var parameters = new Dictionary<string, MethodParameter>(StringComparer.InvariantCultureIgnoreCase);
Expand Down Expand Up @@ -814,7 +933,7 @@ private static bool TryGetAttributeValue<T>(AttributeData? attrib, string name,
int index = 0;
foreach (var p in ctor.Parameters)
{
if (StringComparer.InvariantCultureIgnoreCase.Equals(p.Name == name))
if (StringComparer.InvariantCultureIgnoreCase.Equals(p.Name, name))
{
value = Parse(attrib.ConstructorArguments[index], out isNull);
return true;
Expand Down
2 changes: 2 additions & 0 deletions src/Dapper.AOT.Analyzers/Internal/Types.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ public const string
BindTupleByNameAttribute = nameof(BindTupleByNameAttribute),
DapperAotAttribute = nameof(DapperAotAttribute),
DbValueAttribute = nameof(DbValueAttribute),
ColumnAttribute = nameof(ColumnAttribute),
UseColumnAttributeAttribute = nameof(UseColumnAttributeAttribute),
RowCountAttribute = nameof(RowCountAttribute),
CacheCommandAttribute = nameof(CacheCommandAttribute),
IncludeLocationAttribute = nameof(IncludeLocationAttribute),
Expand Down
16 changes: 16 additions & 0 deletions src/Dapper.AOT/UseColumnAttributeAttribute.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using System;

namespace Dapper
{
/// <summary>
/// Specifies whether to use [System.ComponentModel.DataAnnotations.Schema.ColumnAttribute] for additional behavioral configuration
/// </summary>
[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)]
public class UseColumnAttributeAttribute : Attribute
{
/// <param name="enable">enable usage of [Column] attribute</param>
public UseColumnAttributeAttribute(bool enable = true)
{
}
}
}
1 change: 1 addition & 0 deletions test/Dapper.AOT.Test/Dapper.AOT.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
<PackageReference Include="Npgsql" />
<PackageReference Include="Oracle.ManagedDataAccess" Condition="'$(TargetFramework)'=='net48'" />
<PackageReference Include="Oracle.ManagedDataAccess.Core" Condition="'$(TargetFramework)'!='net48'" />
<PackageReference Include="System.ComponentModel.Annotations" />
<PackageReference Include="System.Data.Common" />
<PackageReference Include="System.Data.SqlClient" />
<PackageReference Include="Microsoft.Data.SqlClient" />
Expand Down
41 changes: 41 additions & 0 deletions test/Dapper.AOT.Test/Interceptors/ColumnAttribute.input.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using Dapper;
using System.Data.Common;

// needed for [Column] attribute
using System.ComponentModel.DataAnnotations.Schema;

[module: DapperAot]

public static class Foo
{
static void SomeCode(DbConnection connection, string bar, bool isBuffered)
{
_ = connection.Query<MyType>("def");
}

public class MyType
{
// default
public int A { get; set; }

// dbvalue
[DbValue(Name = "DbValue_B")]
public int B { get; set; }

// standard enabled
[UseColumnAttribute]
[Column("StandardColumn_C")]
public int C { get; set; }

// explicitly enabled
[UseColumnAttribute]
[Column("ExplicitColumn_D")]
public int D { get; set; }

// dbValue & Column enabled
[UseColumnAttribute]
[DbValue(Name = "DbValue_E")]
[Column("Column_E")]
public int E { get; set; }
}
}
Loading

0 comments on commit 636fd63

Please sign in to comment.