Skip to content

Commit

Permalink
Fixed objects created by bound factories should not have lifecycle
Browse files Browse the repository at this point in the history
Added distinct test project to cover more edge cases
Improved getting factory targets from type
  • Loading branch information
hugener committed Apr 30, 2024
1 parent 566fb13 commit 80b0c83
Show file tree
Hide file tree
Showing 157 changed files with 1,361 additions and 765 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ An IDisposable/IAsyncDisposable object is considered owned by a factory in the f

## Not implemented yet:
* First Beta
* Fix nuget package need interface project explicitly
* Binding property factory method (implicit/explicit)
* Place generator dependencies in own namespace
* Generating documentation
* IServiceProvider support (ASP.NET)
Expand Down
1 change: 1 addition & 0 deletions Source/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
<PackageVersion Include="System.Collections.Immutable" Version="6.0.0" />
<PackageVersion Include="System.Threading.Tasks" Version="4.3.0" />
<PackageVersion Include="System.ValueTuple" Version="4.5.0" />
<PackageVersion Include="System.Memory" Version="4.5.5" />
<PackageVersion Include="Verify.NUnit" Version="24.1.0" />
<PackageVersion Include="Verify.SourceGenerators" Version="2.2.0" />
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
Expand All @@ -22,7 +22,9 @@
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="System.Memory" />
<PackageReference Include="System.ValueTuple" />
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
<PackageReference Include="Sundew.DiscriminatedUnions" />
<PackageReference Include="Sundew.Testing.CodeAnalysis" />
<PackageReference Include="System.Collections.Immutable" />
<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
},
"Roslyn Component": {
"commandName": "DebugRoslynComponent",
"targetProject": "..\\TestProjects\\Success\\Success.csproj"
"targetProject": "..\\TestProjects\\DistinctSuccess\\DistinctSuccess.csproj"
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,4 @@ namespace Sundew.Injection.Generator.Stages.CodeGeneration;
using Sundew.Injection.Generator.Stages.CodeGeneration.Syntax;
using Sundew.Injection.Generator.TypeSystem;

internal readonly record struct GeneratedTypeDeclaration(NamedType ImplementationType, NamedType? InterfaceType, ValueArray<FactoryTargetDeclaration> CreateMethods);
internal readonly record struct GeneratedTypeDeclaration(NamedType ImplementationType, NamedType? InterfaceType, ValueArray<FactoryTargetDeclaration> FactoryTargets);
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,12 @@ namespace Sundew.Injection.Generator.Stages.Features.Factory.CodeGenerationStage
using Sundew.Injection.Generator.Stages.Features.Factory.CodeGenerationStage.Model;
using Sundew.Injection.Generator.Stages.Features.Factory.ResolveGraphStage.Nodes;

internal class SingleInstancePerObjectGenerator
internal class SingleInstancePerObjectGenerator(
GeneratorFeatures generatorFeatures,
GeneratorContext generatorContext)
{
private readonly GeneratorFeatures generatorFeatures;
private readonly GeneratorContext generatorContext;

public SingleInstancePerObjectGenerator(
GeneratorFeatures generatorFeatures,
GeneratorContext generatorContext)
{
this.generatorFeatures = generatorFeatures;
this.generatorContext = generatorContext;
}
private readonly GeneratorFeatures generatorFeatures = generatorFeatures;
private readonly GeneratorContext generatorContext = generatorContext;

public FactoryNode VisitSingleInstancePerObject(
SingleInstancePerObjectInjectionNode singleInstancePerObjectInjectionNode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public R<InjectionTree, ImmutableList<InjectionStageError>> Build(Binding bindin
private R<InjectionModel, ImmutableList<InjectionStageError>> GetInjectionModel(
Binding binding,
InjectionNode? dependantInjectionNode,
(Type Type, string Name, TypeMetadata TypeMetadata)? parameterOption,
(Type Type, string Name, TypeMetadata Metadata)? parameterOption,
CancellationToken cancellationToken)
{
var factoryConstructorParameters = ImmutableList.CreateBuilder<FactoryConstructorParameter>();
Expand All @@ -72,8 +72,6 @@ private R<InjectionModel, ImmutableList<InjectionStageError>> GetInjectionModel(
var creation = creationResult.Value;
factoryConstructorParameters.AddRange(creation.FactoryConstructorParameters);

BooleanHelper.SetIfTrue(ref needsLifecycleHandling, creation.NeedsLifecycleHandling);

var creationInjectionNode = this.CreateInjectionNode(
binding.TargetType,
binding.ReferencedType,
Expand All @@ -89,10 +87,11 @@ private R<InjectionModel, ImmutableList<InjectionStageError>> GetInjectionModel(
targetReferenceType,
this.GetParameterSource(targetReferenceType, parameter.Name, errors),
parameter.Name,
parameter.TypeMetadata,
parameter.Metadata,
scope is Scope.NewInstance,
scope is Scope.SingleInstancePerFactory,
dependantInjectionNode?.GetInjectionNodeName())));
BooleanHelper.SetIfTrue(ref needsLifecycleHandling, creation.NeedsLifecycleHandling);

foreach (var parameter in binding.Method.Parameters)
{
Expand All @@ -104,7 +103,7 @@ private R<InjectionModel, ImmutableList<InjectionStageError>> GetInjectionModel(
break;
case SingleParameter singleParameter:
{
var injectionModelResult = this.GetInjectionModel(singleParameter.Binding, creationInjectionNode, (parameter.Type, parameter.Name, parameter.TypeMetadata), cancellationToken);
var injectionModelResult = this.GetInjectionModel(singleParameter.Binding, creationInjectionNode, (parameter.Type, parameter.Name, parameter.TypeMetadata with { HasLifecycle = singleParameter.Binding.HasLifecycle }), cancellationToken);

errors.AddErrors(injectionModelResult);

Expand Down Expand Up @@ -146,7 +145,7 @@ private R<InjectionModel, ImmutableList<InjectionStageError>> GetInjectionModel(
new NewInstanceInjectionNode(
parameter.Type,
parameter.Type,
defaultParameter.TypeMetadata.HasLifetime && parameter.Type.IsValueType,
defaultParameter.TypeMetadata.HasLifecycle && parameter.Type.IsValueType,
new RecordList<InjectionNode>(),
defaultParameter.Literal != null ? CreationSource._LiteralValue(defaultParameter.Literal.ToString()) : CreationSource._DefaultValue(defaultParameter.Type),
null,
Expand Down Expand Up @@ -207,7 +206,7 @@ private R<CreationModel, ImmutableList<InjectionStageError>> GetCreationSource(
case ScopeError scopeError:
return R.Error(ImmutableList.Create(InjectionStageError._ScopeError(scopeError.CurrentType, scopeError.CurrentScope, Root, scopeError.Dependant.Scope.ToString())));
case SingleParameter singleParameter:
var injectionModelResult = this.GetInjectionModel(singleParameter.Binding, dependantInjectionNode, (singleParameter.Binding.TargetType, singleParameter.Binding.TargetType.Name, TypeMetadata: instance.ContainingTypeMetadata), cancellationToken);
var injectionModelResult = this.GetInjectionModel(singleParameter.Binding, dependantInjectionNode, (singleParameter.Binding.TargetType, singleParameter.Binding.TargetType.Name, Metadata: instance.ContainingTypeMetadata), cancellationToken);
return injectionModelResult.With(injectionModel =>
new CreationModel(CreationSource._InstanceMethodCall(bindingMethod.ContainingType, bindingMethod, injectionModel.InjectionNode, instance.IsProperty), injectionModel.NeedsLifecycleHandling, injectionModel.FactoryConstructorParameters));
case MultiItemParameter multiItemParameter:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public ResolvedBinding TryCreateSingleParameter(BindingRegistration bindingRegis
bindingRegistration.ReferencedType,
bindingRegistration.Scope,
bindingRegistration.Method,
bindingRegistration.TargetType.Metadata.HasLifetime,
bindingRegistration.TargetType.Metadata.HasLifecycle,
bindingRegistration.IsInjectable,
bindingRegistration.IsNewOverridable);
var newResolvedBinding = ResolvedBinding.SingleParameter(newBinding);
Expand Down Expand Up @@ -77,7 +77,7 @@ public ResolvedBinding TryCreateGenericSingleParameter(Type interfaceType, Close
public ResolvedBinding TryCreateMultiItemParameter(Type requestedType, Type elementType, ValueArray<BindingRegistration> resolvedBindingRegistrations, bool isArrayRequired)
{
var bindings = resolvedBindingRegistrations.Select(x =>
new Binding(x.TargetType.Type, elementType, x.Scope, x.Method, x.TargetType.Metadata.HasLifetime, x.IsInjectable, x.IsNewOverridable)).ToArray();
new Binding(x.TargetType.Type, elementType, x.Scope, x.Method, x.TargetType.Metadata.HasLifecycle, x.IsInjectable, x.IsNewOverridable)).ToArray();

bindingsTypeRegistrar.Register(requestedType.Id, default, bindings, true);
return this.CreateMultiItemParameter(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ string GetName()
factoryMethodRegistration.Return.Type,
factoryMethodRegistration.Scope,
factoryMethodRegistration.Method with { Name = factoryMethodName },
factoryMethodRegistration.Target.Metadata.HasLifetime,
factoryMethodRegistration.Target.Metadata.HasLifecycle,
false,
factoryMethodRegistration.IsNewOverridable);
return new BindingRoot(binding, factoryMethodRegistration.Accessibility, factoryMethodRegistration.Return.Type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,6 @@ void PickBindingScope(Binding binding)

break;
case SingleParameter singleParameter:
if (singleParameter.Binding.Method.Kind is MethodKind.Instance instance)
{
this.ResolveBindingScopes(this.bindingResolver.ResolveBinding(singleParameter.Binding.Method.ContainingType, instance.ContainingTypeMetadata, instance.ContainingTypeDefaultConstructor, default), dependant, errors);

this.UpdateBindingScope(singleParameter.Binding, dependant, errors);
}

PickBindingScope(singleParameter.Binding);
break;
case MultiItemParameter multiItemParameter:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,27 +33,27 @@ internal static IncrementalValuesProvider<ValueArray<ResolvedTypeResolverDefinit
var (resolverCreationDefinitions, generatedFactoryDeclarations, compilationData) = tuple;
foreach (var generatedFactoryDeclaration in generatedFactoryDeclarations)
{
nameRegistry.Register(generatedFactoryDeclaration.ImplementationType.Name, generatedFactoryDeclaration.CreateMethods);
nameRegistry.Register(generatedFactoryDeclaration.ImplementationType.Name, generatedFactoryDeclaration.FactoryTargets);
if (generatedFactoryDeclaration.InterfaceType.TryGetValue(out var factoryInterfaceType))
{
nameRegistry.Register(factoryInterfaceType.Name, generatedFactoryDeclaration.CreateMethods);
nameRegistry.Register(factoryInterfaceType.Name, generatedFactoryDeclaration.FactoryTargets);
}
}

token.ThrowIfCancellationRequested();
var factoryTypeAndCreateMethodsResolver = new FactoryTypeAndCreateMethodsResolver(nameRegistry);
IEnumerable<(ResolverCreationDefinition ResolverCreationDefinition, IEnumerable<(Type Type, ValueArray<FactoryTargetDeclaration> CreateMethods)> CreateMethods)> resolveCreationDefinitionResults = resolverCreationDefinitions
IEnumerable<(ResolverCreationDefinition ResolverCreationDefinition, IEnumerable<(Type Type, ValueArray<FactoryTargetDeclaration> FactoryTargets)> FactoryTargets)> resolveCreationDefinitionResults = resolverCreationDefinitions
.Select(resolverCreationDefinition => (resolverCreationDefinition,
createMethods: resolverCreationDefinition.FactoryRegistrations
.Select(resolverCreationDefinitionResult => factoryTypeAndCreateMethodsResolver.ResolveFactoryRegistration(resolverCreationDefinitionResult))));

return resolveCreationDefinitionResults.Select(resolvedCreationDefinition =>
{
var (resolverCreationDefinition, createMethods) = resolvedCreationDefinition;
var (resolverCreationDefinition, factoryTargets) = resolvedCreationDefinition;
return new ResolvedTypeResolverDefinition(
resolverCreationDefinition.ResolverType,
true,
createMethods.Select(x => new ResolvedFactoryRegistration(x.Type, x.CreateMethods))
factoryTargets.Select(x => new ResolvedFactoryRegistration(x.Type, x.FactoryTargets))
.ToValueArray(),
resolverCreationDefinition.Accessibility);
}).ToValueArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void AddBinding(UnboundGenericType type, GenericBindingRegistration genericBindi
bindingList.Add(genericBinding);
}

var genericBinding = new GenericBindingRegistration(implementation.Type, scope, genericMethod, Injection.Accessibility.Internal, implementation.TypeMetadata.HasLifetime, false);
var genericBinding = new GenericBindingRegistration(implementation.Type, scope, genericMethod, Injection.Accessibility.Internal, implementation.TypeMetadata.HasLifecycle, false);
AddBinding(implementation.Type.ToUnboundGenericType(), genericBinding);
foreach (var @interface in interfaces)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static void AddFactoryMethodFromTypeSymbol(

if (factoryMethod == null)
{
var createMethodResult = CreateMethod(analysisContext, implementationTypeSymbolWithLocation.TypeSymbol);
var createMethodResult = GetFactoryTarget(analysisContext, implementationTypeSymbolWithLocation.TypeSymbol);
if (createMethodResult.IsError)
{
analysisContext.CompiletimeInjectionDefinitionBuilder.AddDiagnostic(Diagnostics.InfiniteRecursionError, implementationTypeSymbolWithLocation, createMethodResult.Error.GetErrorText());
Expand Down Expand Up @@ -104,7 +104,7 @@ public static void AddDefaultFactoryMethodFromTypeSymbol(
if (typeSymbol.IsInstantiable() && interfaceType.DefaultConstructor.TryGetValue(out var method))
{
analysisContext.CompiletimeInjectionDefinitionBuilder.Bind(ImmutableArray.Create(interfaceType.Type), interfaceType, method, new ScopeContext(Scope._Auto, ScopeSelection.Implicit), false, isNewOverridable);
var createMethodResult = CreateMethod(analysisContext, typeSymbol);
var createMethodResult = GetFactoryTarget(analysisContext, typeSymbol);
if (createMethodResult.IsError)
{
analysisContext.CompiletimeInjectionDefinitionBuilder.AddDiagnostic(Diagnostics.InfiniteRecursionError, new SymbolErrorWithLocation(createMethodResult.Error, typeSymbolWithLocation.Location));
Expand Down Expand Up @@ -143,7 +143,7 @@ private static ValueArray<FactoryTarget> GetFactoryTargets(ITypeSymbol factoryTy
switch (symbol)
{
case IMethodSymbol methodSymbol:
if (methodSymbol.MethodKind != Microsoft.CodeAnalysis.MethodKind.Constructor
if (methodSymbol.MethodKind == Microsoft.CodeAnalysis.MethodKind.DeclareMethod
&& symbol.GetAttributes().All(x => x.AttributeClass?.ToDisplayString() != KnownTypesProvider.IndirectFactoryTargetName)
&& !symbol.MetadataName.Contains(Dispose))
{
Expand All @@ -168,7 +168,7 @@ private static ValueArray<FactoryTarget> GetFactoryTargets(ITypeSymbol factoryTy
.ToValueArray();
}

private static R<Method, SymbolError> CreateMethod(AnalysisContext analysisContext, ITypeSymbol implementationType)
private static R<Method, SymbolError> GetFactoryTarget(AnalysisContext analysisContext, ITypeSymbol implementationType)
{
var defaultConstructor = TypeHelper.GetDefaultConstructorMethod(implementationType);
if (defaultConstructor != null)
Expand Down
Loading

0 comments on commit 80b0c83

Please sign in to comment.