Skip to content

Commit

Permalink
Merge pull request #585 from SteveDunn/allow-unvalidated-instances-to…
Browse files Browse the repository at this point in the history
…-be-created

Allow unvalidated instances to be created.
Implements #221
  • Loading branch information
SteveDunn authored May 3, 2024
2 parents 0154535 + 845ca35 commit 64f45cc
Show file tree
Hide file tree
Showing 17 changed files with 3,428 additions and 28 deletions.
1 change: 1 addition & 0 deletions src/Vogen/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@

Rule ID | Category | Severity | Notes
--------|----------|----------|-------
VOG027 | Usage | Error | DoNotUseNewAnalyzer
1 change: 1 addition & 0 deletions src/Vogen/Diagnostics/RuleIdentifiers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ public static class RuleIdentifiers
public const string DuplicateTypesFound = "VOG024";
public const string DoNotUseReflection = "VOG025";
public const string DoNotDeriveFromVogenAttributes = "VOG026";
public const string IncorrectUseOfInstanceField = "VOG027";
}
52 changes: 51 additions & 1 deletion src/Vogen/Rules/DoNotUseNewAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,18 @@ public class DoNotUseNewAnalyzer : DiagnosticAnalyzer
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);
private static readonly DiagnosticDescriptor _rule2 = new DiagnosticDescriptor(
RuleIdentifiers.IncorrectUseOfInstanceField,
"Instance fields should be declared as public and static",
"Type '{0}' cannot be constructed as a field with 'new' as it should be public and static",
RuleCategories.Usage,
DiagnosticSeverity.Error,
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, _rule2);

public override void Initialize(AnalysisContext context)
{
Expand All @@ -43,9 +54,48 @@ private static void AnalyzeExpression(OperationAnalysisContext context)
if (c.Type is not INamedTypeSymbol symbol) return;

if (!VoFilter.IsTarget(symbol)) return;

var instanceFieldState = IsAPublicStaticFieldInAValueObject();

if (instanceFieldState == InstanceField.NotValid)
{
context.ReportDiagnostic(DiagnosticsCatalogue.BuildDiagnostic(_rule2, symbol.Name, context.Operation.Syntax.GetLocation()));

return;
}

if (instanceFieldState == InstanceField.Valid) return;

var diagnostic = DiagnosticsCatalogue.BuildDiagnostic(_rule, symbol.Name, context.Operation.Syntax.GetLocation());

context.ReportDiagnostic(diagnostic);

return;

InstanceField IsAPublicStaticFieldInAValueObject()
{
var cs = context.ContainingSymbol as IFieldSymbol;
if (cs is null) return InstanceField.NotApplicable;
var type = cs.ContainingType;
var isVo = VoFilter.IsTarget(type);
if (!isVo)
{
return InstanceField.NotApplicable;
}

if (cs.DeclaredAccessibility != Accessibility.Public) return InstanceField.NotValid;
if (!cs.IsStatic) return InstanceField.NotValid;

return InstanceField.Valid;
}

}

internal enum InstanceField
{
Valid,
NotValid,
NotApplicable

}
}
1 change: 1 addition & 0 deletions tests/AnalyzerTests/AnalyzerTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<ProjectReference Include="..\..\src\Vogen.CodeFixers\Vogen.CodeFixers.csproj" />
<ProjectReference Include="..\..\src\Vogen.SharedTypes\Vogen.SharedTypes.csproj" />
<ProjectReference Include="..\Shared\Shared.csproj" />
<ProjectReference Include="..\SnapshotTests\SnapshotTests.csproj" />
</ItemGroup>
</Project>

Expand Down
163 changes: 139 additions & 24 deletions tests/AnalyzerTests/DoNotUseNewAnalyzerTests.cs
Original file line number Diff line number Diff line change
@@ -1,26 +1,46 @@
using System.Collections;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Testing;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Shared;
using Vogen;
using VerifyCS = AnalyzerTests.Verifiers.CSharpAnalyzerVerifier<Vogen.Rules.DoNotUseNewAnalyzer>;
// ReSharper disable CoVariantArrayConversion

namespace AnalyzerTests
{
public class DoNotUseNewAnalyzerTests
{
// A pattern for 'placeholders' for errors. These are stripped out when running tests
// that require both the user source and generated source.
private static readonly Regex _placeholderPattern = new(@"{\|#\d+:", RegexOptions.Compiled);

private class Types : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
{
yield return new[] {"partial class"};
yield return new[] {"partial struct"};
yield return new[] {"readonly partial struct"};
yield return new[] {"partial record class"};
yield return new[] {"partial record struct"};
yield return new[] {"readonly partial record struct"};
yield return ["partial class"];
yield return ["partial struct"];
yield return ["readonly partial struct"];
yield return ["partial record class"];
yield return ["partial record struct"];
yield return ["readonly partial record struct"];
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
}

private class Classes : IEnumerable<object[]>
{
public IEnumerator<object[]> GetEnumerator()
{
yield return ["partial class"];
yield return ["partial record class"];
}

IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
Expand Down Expand Up @@ -104,26 +124,119 @@ await Run(
[ClassData(typeof(Types))]
public async Task Disallow_new_from_local_function(string type)
{
var source = $@"
using Vogen;
namespace Whatever;
var source = $$"""
[ValueObject]
public {type} MyVo {{ }}
using Vogen;
namespace Whatever;
public class Test {{
public Test() {{
MyVo Get() => {{|#0:new MyVo()|}};
MyVo Get2() => {{|#1:new()|}};
}}
}}
";
[ValueObject]
public {{type}} MyVo { }
public class Test {
public Test() {
MyVo Get() => {|#0:new MyVo()|};
MyVo Get2() => {|#1:new()|};
}
}
""";

await Run(
source,
WithDiagnostics("VOG010", DiagnosticSeverity.Error, "MyVo", 0, 1));
}

[Theory]
[ClassData(typeof(Types))]
public async Task Allow_as_public_static_field_in_a_VO(string type)
{
var userSource = $$"""
using Vogen;
namespace Whatever
{
[ValueObject]
public {{type}} MyVo {
public static MyVo Unspecified = new MyVo(-1);
}
}
""";
string[] sources = CombineUserAndGeneratedSource(userSource);

await Run(sources, Enumerable.Empty<DiagnosticResult>());
}

[Theory]
[ClassData(typeof(Types))]
public async Task Disallow_as_private_static_field_in_a_VO(string type)
{
var userSource = $$"""
using Vogen;
namespace Whatever
{
[ValueObject]
public {{type}} MyVo {
private static MyVo Unspecified1 = {|#0:new MyVo(-1)|};
private static MyVo Unspecified2 = {|#1:new(-1)|};
}
}
""";

string[] sources = CombineUserAndGeneratedSource(userSource);

await Run(sources, WithDiagnostics("VOG027", DiagnosticSeverity.Error, "MyVo", 0, 1));
}

[Theory]
[ClassData(typeof(Classes))]
public async Task Disallow_as_non_static_field_in_a_VO(string type)
{
var userSource = $$"""
using Vogen;
namespace Whatever
{
[ValueObject]
public {{type}} MyVo {
public MyVo Unspecified1 = {|#0:new MyVo(-1)|};
public MyVo Unspecified2 = {|#1:new(-1)|};
}
}
""";

string[] sources = CombineUserAndGeneratedSource(userSource);

await Run(sources, WithDiagnostics("VOG027", DiagnosticSeverity.Error, "MyVo", 0, 1));
}

private static string[] CombineUserAndGeneratedSource(string userSource)
{
var r = MetadataReference.CreateFromFile(typeof(ValueObjectAttribute).Assembly.Location);

var strippedSource = _placeholderPattern.Replace(userSource, string.Empty).Replace("|}", string.Empty);

(ImmutableArray<Diagnostic> Diagnostics, string GeneratedSource) output = new ProjectBuilder()
.WithSource(strippedSource)
.WithTargetFramework(TargetFramework.Net8_0)
.GetGeneratedOutput<ValueObjectGenerator>(true, r);

if (output.Diagnostics.Length > 0)
{
throw new AssertFailedException(
$"""
Expected user source to be error and generated code to be free from errors:
User source: {userSource}
Errors: {string.Join(",", output.Diagnostics.Select(d => d.ToString()))}
""");
}

return [userSource, output.GeneratedSource];
}

[Theory]
[ClassData(typeof(Types))]
public async Task Disallow_new_from_func(string type)
Expand Down Expand Up @@ -189,19 +302,21 @@ private static IEnumerable<DiagnosticResult> WithDiagnostics(string code, Diagno
}
}

private async Task Run(string source, IEnumerable<DiagnosticResult> expected)
private async Task Run(string source, IEnumerable<DiagnosticResult> expected) => await Run([source], expected);

private async Task Run(string[] sources, IEnumerable<DiagnosticResult> expected)
{
var test = new VerifyCS.Test
{
TestState =
{
Sources = { source },
},

CompilerDiagnostics = CompilerDiagnostics.Errors,
ReferenceAssemblies = References.Net80AndOurs.Value,
};

foreach (var eachSource in sources)
{
test.TestState.Sources.Add(eachSource);
}

test.ExpectedDiagnostics.AddRange(expected);

await test.RunAsync();
Expand Down
2 changes: 1 addition & 1 deletion tests/AnalyzerTests/TestRunner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ private void RunOn(bool ignoreInitialCompilationErrors,
}
}

private static (ImmutableArray<Diagnostic> Diagnostics, string Output) GetGeneratedOutput(
private static (ImmutableArray<Diagnostic> Diagnostics, string GeneratedSource) GetGeneratedOutput(
string source,
TargetFramework targetFramework,
bool ignoreInitialCompilationErrors)
Expand Down
13 changes: 13 additions & 0 deletions tests/ConsumerTests/Instances/Types.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
namespace ConsumerTests.Instances;

[ValueObject]
public partial class IntWithNewedUpInstanceFields
{
public static IntWithNewedUpInstanceFields Invalid = new(-1);
public static IntWithNewedUpInstanceFields Unspecified = new(-2);

// Error VOG027 : Type 'IntWithNewedUpInstanceFields' cannot be constructed as a field with 'new' as it should be public and static
// public IntWithNewedUpInstanceFields NotStatic = new(-2);

// Error VOG027 : Type 'IntWithNewedUpInstanceFields' cannot be constructed as a field with 'new' as it should be public and static
// internal static IntWithNewedUpInstanceFields NotPublic = new(-2);
}

[ValueObject(typeof(int))]
[Instance(name: "Invalid", value: -1)]
[Instance(name: "Unspecified", value: -2)]
Expand Down
2 changes: 1 addition & 1 deletion tests/Shared/ProjectBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public ProjectBuilder WithNugetPackages(IEnumerable<NuGetPackage> packages)
return this;
}

public (ImmutableArray<Diagnostic> Diagnostics, string Output) GetGeneratedOutput<T>(
public (ImmutableArray<Diagnostic> Diagnostics, string GeneratedSource) GetGeneratedOutput<T>(
bool ignoreInitialCompilationErrors,
MetadataReference? valueObjectAttributeMetadata = null)
where T : IIncrementalGenerator, new()
Expand Down
21 changes: 21 additions & 0 deletions tests/SnapshotTests/InstanceFields/InstanceFieldGenerationTests.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Threading.Tasks;
using Shared;
using VerifyXunit;
using Vogen;

Expand Down Expand Up @@ -29,6 +30,26 @@ public partial struct BooleanThing
.RunOnAllFrameworks();
}

[Fact]
public Task Instances_can_be_newed_up_in_the_wrapper_itself()
{
var source = $$"""
using Vogen;
namespace Whatever;
[ValueObject(typeof(int))]
public partial class MyVo
{
public static readonly MyVo Unspecified = new MyVo(-1);
}
""";
return new SnapshotRunner<ValueObjectGenerator>()
.WithSource(source)
.IgnoreInitialCompilationErrors()
.RunOnAllFrameworks();
}

[Fact]
public Task Instance_names_can_have_reserved_keywords()
{
Expand Down
Loading

0 comments on commit 64f45cc

Please sign in to comment.