Skip to content

Commit

Permalink
Merge pull request #577 from SteveDunn/574-improve-performance-and-sw…
Browse files Browse the repository at this point in the history
…itch-to-forattributewithmetadataname

574 improve performance and switch to forattributewithmetadataname
  • Loading branch information
SteveDunn authored Apr 28, 2024
2 parents c8d8a2e + 8e766ff commit 49795e3
Show file tree
Hide file tree
Showing 62 changed files with 1,022 additions and 284 deletions.
3 changes: 3 additions & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
###############################################################################
* text=auto
*.sh eol=lf
*.verified.txt text eol=lf working-tree-encoding=UTF-8
*.verified.xml text eol=lf working-tree-encoding=UTF-8
*.verified.json text eol=lf working-tree-encoding=UTF-8

###############################################################################
# Set default behavior for command prompt diff.
Expand Down
26 changes: 11 additions & 15 deletions docs/nuget-readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ The Value Objects wrap simple primitives such as `int`, `string`, `double` etc.
To get started, add this package, and add a type such as:

```csharp
[ValueObject(typeof(int))]
[ValueObject<int>]
public partial struct CustomerId
{
}
Expand All @@ -23,16 +23,16 @@ You can now treat `CustomerId` as you would an `int` and there is very little pe
And your method signatures change from:

```charp
public void HandleCustomer(int customerId)`
void HandleCustomer(int customerId)
```

to

```charp
public void HandleCustomer(CustomerId customerId)`
void HandleCustomer(CustomerId customerId)
```

The Source Generator generates code for things like creating the object and for performing equality.
The Source Generator generates code for things like creating the object and for performing equality.

Value Objects help combat Primitive Obsession. Primitive Obsession means being obsessed with primitives. It is a Code Smell that degrades the quality of software.

Expand Down Expand Up @@ -65,7 +65,7 @@ var customerId = CustomerId.From(42);
`CustomerId` is declared as:

```csharp
[ValueObject(typeof(int))]
[ValueObject<int>]
public partial struct CustomerId
{
}
Expand All @@ -75,7 +75,7 @@ That's all you need to do to switch from a primitive to a Value Object.
Here it is again with some validation

```csharp
[ValueObject(typeof(int))]
[ValueObject<int>]
public partial struct CustomerId
{
private static Validation Validate(int value) =>
Expand All @@ -86,12 +86,12 @@ public partial struct CustomerId
This allows us to have more _strongly typed_ domain objects instead of primitives, which makes the code easier to read and enforces better method signatures, so instead of:

``` cs
public void DoSomething(int customerId, int supplierId, int amount)
void DoSomething(int customerId, int supplierId, int amount)
```
we can have:

``` cs
public void DoSomething(CustomerId customerId, SupplierId supplierId, Amount amount)
void DoSomething(CustomerId customerId, SupplierId supplierId, Amount amount)
```

Now, callers can't mess up the ordering of parameters and accidentally pass us a Supplier ID in place of a Customer ID.
Expand All @@ -115,11 +115,11 @@ This adds a `<PackageReference>` to your project. You can additionally mark the

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net6.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
</PropertyGroup>

<!-- Add the package -->
<PackageReference Include="Vogen" Version="1.0.9"
<PackageReference Include="Vogen" Version="4.0.0"
PrivateAssets="all" ExcludeAssets="runtime" />
<!-- -->

Expand All @@ -130,21 +130,17 @@ This adds a `<PackageReference>` to your project. You can additionally mark the

Here's the benchmarks comparing a native int to a ValueObject:

```ini
| Method | Mean | Error | StdDev | Ratio | Allocated |
|------------------------ |---------:|---------:|---------:|------:|----------:|
| UsingIntNatively | 17.04 ns | 0.253 ns | 0.014 ns | 1.00 | - |
| UsingValueObjectStruct | 19.76 ns | 2.463 ns | 0.135 ns | 1.16 | - |
```

There's hardly any speed overhead, and no memory overhead.

The next most common scenario is using a VO class to represent a natits are:
The next most common scenario is using a VO to represent a string:

```ini
| Method | Mean | Error | StdDev | Ratio | Allocated |
|------------------------- |---------:|---------:|--------:|------:|----------:|
| UsingStringNatively | 204.4 ns | 8.09 ns | 0.44 ns | 1.00 | 256 B |
| UsingValueObjectAsClass | 250.7 ns | 29.97 ns | 1.64 ns | 1.23 | 328 B |
| UsingValueObjectAsStruct | 248.9 ns | 18.82 ns | 1.03 ns | 1.22 | 304 B |
```
12 changes: 0 additions & 12 deletions samples/Vogen.Examples/ModuleInitializerAttribute.cs

This file was deleted.

2 changes: 1 addition & 1 deletion samples/Vogen.Examples/Vogen.Examples.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net7.0</TargetFramework>
<TargetFramework>net8.0</TargetFramework>
<IsPackable>false</IsPackable>
<LangVersion>preview</LangVersion>
<UseLocallyBuiltPackage>true</UseLocallyBuiltPackage>
Expand Down
10 changes: 9 additions & 1 deletion src/Vogen/AnalyzerReleases.Shipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,12 @@ Rule ID | Category | Severity | Notes
AddValidationMethod | Usage | Info | AddValidationAnalyzer
VOG010 | Usage | Error | DoNotUseNewAnalyzer
VOG009 | Usage | Error | DoNotUseDefaultAnalyzer
VOG025 | Usage | Error | DoNotUseReflection
VOG025 | Usage | Error | DoNotUseReflection

## Release 4.0.1

### New Rules

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
VOG026 | Usage | Warning | DoNotDeriveFromVogenAttributes
1 change: 1 addition & 0 deletions src/Vogen/Diagnostics/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,5 @@ public static class RuleIdentifiers
public const string InstanceValueCannotBeConverted = "VOG023";
public const string DuplicateTypesFound = "VOG024";
public const string DoNotUseReflection = "VOG025";
public const string DoNotDeriveFromVogenAttributes = "VOG026";
}
92 changes: 70 additions & 22 deletions src/Vogen/ManageAttributes.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
Expand All @@ -16,20 +17,21 @@ internal static class ManageAttributes
/// If some are specified, then they are validated.
/// If anything is invalid, a compilation error is raised.
/// </summary>
/// <param name="defaults"></param>
/// <param name="compilation"></param>
/// <param name="ctx"></param>
/// <returns></returns>
public static VogenConfigurationBuildResult GetDefaultConfigFromGlobalAttribute(
ImmutableArray<AttributeSyntax> defaults,
Compilation compilation)
GeneratorAttributeSyntaxContext ctx)
{
if (defaults.IsDefaultOrEmpty)
var assemblyAttributes = ctx.TargetSymbol.GetAttributes();

if (assemblyAttributes.IsDefaultOrEmpty)
{
// No global defaults
return VogenConfigurationBuildResult.Null;
}

return GetDefaultConfigFromGlobalAttribute(compilation);
AttributeData attr = assemblyAttributes.ElementAt(0);

return BuildConfigurationFromAttributes.TryBuildFromVogenDefaultsAttribute(attr);
}

/// <summary>
Expand All @@ -40,10 +42,9 @@ public static VogenConfigurationBuildResult GetDefaultConfigFromGlobalAttribute(
/// </summary>
/// <param name="compilation"></param>
/// <returns></returns>
public static VogenConfigurationBuildResult GetDefaultConfigFromGlobalAttribute(
Compilation compilation)
public static VogenConfigurationBuildResult GetDefaultConfigFromGlobalAttribute(Compilation compilation)
{
var assemblyAttributes = compilation.Assembly.GetAttributes();
ImmutableArray<AttributeData> assemblyAttributes = compilation.Assembly.GetAttributes();
if (assemblyAttributes.IsDefaultOrEmpty)
{
return VogenConfigurationBuildResult.Null;
Expand Down Expand Up @@ -74,27 +75,74 @@ public static VogenConfigurationBuildResult GetDefaultConfigFromGlobalAttribute(
/// </summary>
/// <param name="context"></param>
/// <returns>The syntax of the attribute if it matches the global defaults attribute, otherwise null.</returns>
public static AttributeSyntax? TryGetAssemblyLevelDefaultsAttribute(GeneratorSyntaxContext context)
public static AttributeSyntax? TryGetAssemblyLevelDefaultsAttribute(GeneratorAttributeSyntaxContext context)
{
// we know the node is a AttributeListSyntax thanks to IsSyntaxTargetForGeneration
var attributeListSyntax = (AttributeListSyntax) context.Node;
ImmutableArray<AttributeData> assemblyAttributes = context.TargetSymbol.GetAttributes();

foreach (AttributeSyntax attributeSyntax in attributeListSyntax.Attributes)
if (assemblyAttributes.IsDefaultOrEmpty)
{
if (context.SemanticModel.GetSymbolInfo(attributeSyntax).Symbol is not IMethodSymbol attributeSymbol)
return null;
}

foreach (AttributeData? attribute in assemblyAttributes)
{
var attrClass = attribute.AttributeClass;

if (!(attrClass?.Name is "VogenDefaultsAttribute" or "VogenDefaults" &&
attrClass.ToDisplayString() == "Vogen.VogenDefaultsAttribute"))
{
continue;
}

SyntaxNode? syntax = attribute.ApplicationSyntaxReference?.GetSyntax();

INamedTypeSymbol attributeContainingTypeSymbol = attributeSymbol.ContainingType;
string fullName = attributeContainingTypeSymbol.ToDisplayString();

if (fullName == "Vogen.VogenDefaultsAttribute")
{
return attributeSyntax;
}
return syntax as AttributeSyntax;
}

return null;
}

/// <summary>
/// Tries to get the syntax element for any matching attribute that might exist in the provided context.
/// </summary>
/// <param name="context"></param>
/// <param name="cancellationToken"></param>
/// <returns>The syntax of the attribute if it matches the global defaults attribute, otherwise null.</returns>
public static AttributeSyntax? TryGetAssemblyLevelDefaultsAttribute2(GeneratorAttributeSyntaxContext context,
CancellationToken cancellationToken)
{
var attributes = context.Attributes;
if (attributes.IsDefaultOrEmpty)
{
return null;
}

AttributeData a = attributes.ElementAt(0);

var n = a.ApplicationSyntaxReference?.GetSyntax() as AttributeSyntax;
return n;
// ImmutableArray<AttributeData> assemblyAttributes = context.TargetSymbol.GetAttributes();
//
// if (assemblyAttributes.IsDefaultOrEmpty)
// {
// return null;
// }
//
// foreach (AttributeData? attribute in assemblyAttributes)
// {
// var attrClass = attribute.AttributeClass;
//
// if (!(attrClass?.Name is "VogenDefaultsAttribute" or "VogenDefaults" &&
// attrClass.ToDisplayString() == "Vogen.VogenDefaultsAttribute"))
// {
// continue;
// }
//
// SyntaxNode? syntax = attribute.ApplicationSyntaxReference?.GetSyntax();
//
// return syntax as AttributeSyntax;
// }

//return null;
}
}
49 changes: 49 additions & 0 deletions src/Vogen/Rules/DoNotDeriveFromVogenAttributes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System.Collections.Immutable;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Vogen.Diagnostics;

namespace Vogen.Rules;

[DiagnosticAnalyzer(LanguageNames.CSharp)]
public class DoNotDeriveFromVogenAttributesAnalyzer : DiagnosticAnalyzer
{
// ReSharper disable once ArrangeObjectCreationWhenTypeEvident - current bug in Roslyn analyzer means it
// won't find this and will report:
// "error RS2002: Rule 'XYZ123' is part of the next unshipped analyzer release, but is not a supported diagnostic for any analyzer"
private static readonly DiagnosticDescriptor _rule = new DiagnosticDescriptor(
RuleIdentifiers.DoNotDeriveFromVogenAttributes,
"Deriving from a Vogen attribute will be disallowed in a future release, use a type alias instead if you're using C# 12 or greater",
"Type '{0}' should not derive from a Vogen attribute",
RuleCategories.Usage,
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
description:
"The value object is created with a new expression. This can lead to invalid value objects in your domain. Use the From method instead.");

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(_rule);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.Analyze | GeneratedCodeAnalysisFlags.ReportDiagnostics);

context.EnableConcurrentExecution();

context.RegisterSymbolAction(AnalyzeSymbol, SymbolKind.NamedType);
}

private static void AnalyzeSymbol(SymbolAnalysisContext context)
{
var symbol = (INamedTypeSymbol) context.Symbol;

var a1 = context.Compilation.GetTypeByMetadataName("Vogen.ValueObjectAttribute");
var a2 = context.Compilation.GetTypeByMetadataName("Vogen.VogenDefaultsAttribute");

if (symbol.DerivesFrom(a1) || symbol.DerivesFrom(a2))
{
var diagnostic = Diagnostic.Create(_rule, symbol.Locations[0], symbol.Name);
context.ReportDiagnostic(diagnostic);
}
}
}
Loading

0 comments on commit 49795e3

Please sign in to comment.