Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support [Column] #85

Merged
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()
{
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))
DeagleGross marked this conversation as resolved.
Show resolved Hide resolved
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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the allowed usage here? can it be applied at the module level, or is it only respected when attached to the same type? are more than one allowed? can we add [AttributeUsage(...)] here accordingly

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added such an expression.
[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field | AttributeTargets.Parameter, AllowMultiple = false)]

let me know if such AttributeUsage is correct

{
/// <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
Loading