Skip to content

Commit

Permalink
Improved exception messages.
Browse files Browse the repository at this point in the history 
Improved exception message that communicates that registrations of
collections are separated from other registrations. (fixes #115)
  • Loading branch information
dot_net_junkie committed Sep 3, 2015
1 parent 37ee9d0 commit 259f0fd
Show file tree
Hide file tree
Showing 5 changed files with 189 additions and 14 deletions.
114 changes: 114 additions & 0 deletions SimpleInjector.NET.Tests.Unit/GetInstanceTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,120 @@ public void GetInstance_ResolvingAbstractTypeAfterGettingTheRegistrationWithMiss
action);
}

[TestMethod]
public void GetInstance_NoRegistrationExistsButRegistrationForCollectionDoes_ThrowsExceptionReferingToThatCollectionRegistration()
{
// Arrange
var container = new Container();

container.RegisterCollection(typeof(ILogger), new[] { typeof(NullLogger) });

// Act
Action action = () => container.GetInstance<ILogger>();

// Assert
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(@"
No registration for type ILogger could be found.
There is, however, a registration for IEnumerable<ILogger>;
Did you mean to call GetAllInstances<ILogger>() or depend on IEnumerable<ILogger>?"
.TrimInside(),
action);
}

[TestMethod]
public void GetInstance_ResolvingATypeDependingOnAnUnregisteredTypeWhileACollectionRegistrationExists_ThrowsExceptionReferingToThatRegistration()
{
// Arrange
var container = new Container();

container.RegisterCollection(typeof(ILogger), new[] { typeof(NullLogger) });

// Act
Action action = () => container.GetInstance<ComponentDependingOn<ILogger>>();

// Assert
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(@"
There is, however, a registration for IEnumerable<ILogger>;
Did you mean to depend on IEnumerable<ILogger>?"
.TrimInside(),
action);
}

[TestMethod]
public void GetInstance_NoRegistrationFExistsAndNoCollectionRegistrationEither_ExceptionMessageDoesNotReferToOtherRegistrations()
{
// Arrange
var container = new Container();

// Act
Action action = () => container.GetInstance<ILogger>();

// Assert
AssertThat.ThrowsWithExceptionMessageDoesNotContain<ActivationException>(
"GetInstance<ILogger>()",
action);

AssertThat.ThrowsWithExceptionMessageDoesNotContain<ActivationException>(
"GetAllInstances<ILogger>()",
action);
}

[TestMethod]
public void GetAllInstances_NoRegistrationForCollectionButRegistrationForSingleElementExists_ThrowsExceptionReferingToThatRegistration()
{
// Arrange
var container = new Container();

container.Register<ILogger, NullLogger>();

// Act
Action action = () => container.GetAllInstances<ILogger>();

// Assert
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(@"
No registration for type IEnumerable<ILogger> could be found.
There is, however, a registration for ILogger;
Did you mean to call GetInstance<ILogger>() or depend on ILogger?"
.TrimInside(),
action);
}

[TestMethod]
public void GetInstance_ResolvingATypeDependingOnAnUnregisteredCollectionWhileASingleRegistrationExists_ThrowsExceptionReferingToThatRegistration()
{
// Arrange
var container = new Container();

container.Register<ILogger, NullLogger>();

// Act
Action action = () => container.GetInstance<ComponentDependingOn<IEnumerable<ILogger>>>();

// Assert
AssertThat.ThrowsWithExceptionMessageContains<ActivationException>(
"There is, however, a registration for ILogger; Did you mean to depend on ILogger?",
action);
}

[TestMethod]
public void GetAllInstances_NoRegistrationForCollectionAndNoSingleRegistrationEither_ExceptionMessageDoesNotReferToOtherRegistrations()
{
// Arrange
var container = new Container();

// Act
Action action = () => container.GetAllInstances<ILogger>();

// Assert
AssertThat.ThrowsWithExceptionMessageDoesNotContain<ActivationException>(
"Did you mean to call GetInstance<ILogger>()",
action);

AssertThat.ThrowsWithExceptionMessageDoesNotContain<ActivationException>(
"GetAllInstances<ILogger>()",
action);
}

//// Seems like there are tests missing, but all other cases are already covered by other test classes.

public class SomeGenericNastyness<TBla>
Expand Down
4 changes: 3 additions & 1 deletion SimpleInjector.NET/Container.Common.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,9 @@ internal void ThrowParameterTypeMustBeRegistered(InjectionTargetInfo target)
throw new ActivationException(
StringResources.ParameterTypeMustBeRegistered(
target,
this.GetNumberOfConditionalRegistrationsFor(target.TargetType)));
this.GetNumberOfConditionalRegistrationsFor(target.TargetType),
this.ContainsOneToOneRegistrationForCollectionType(target.TargetType),
this.ContainsCollectionRegistrationFor(target.TargetType)));
}

/// <summary>Releases all instances that are cached by the <see cref="Container"/> object.</summary>
Expand Down
19 changes: 17 additions & 2 deletions SimpleInjector.NET/Container.Resolving.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -755,10 +755,25 @@ private void ThrowMissingInstanceProducerException(Type serviceType)
throw new ActivationException(StringResources.OpenGenericTypesCanNotBeResolved(serviceType));
}

throw new ActivationException(
StringResources.NoRegistrationForTypeFound(serviceType, this.HasRegistrations));
throw new ActivationException(StringResources.NoRegistrationForTypeFound(
serviceType,
this.HasRegistrations,
this.ContainsOneToOneRegistrationForCollectionType(serviceType),
this.ContainsCollectionRegistrationFor(serviceType)));
}

private bool ContainsOneToOneRegistrationForCollectionType(Type collectionServiceType) =>
IsGenericCollectionType(collectionServiceType) &&
this.ContainsExplicitRegistrationFor(collectionServiceType.GetGenericArguments()[0]);

// NOTE: MakeGenericType will fail for IEnumerable<T> when T is a pointer.
private bool ContainsCollectionRegistrationFor(Type serviceType) =>
!IsGenericCollectionType(serviceType) && !serviceType.IsPointer &&
this.ContainsExplicitRegistrationFor(typeof(IEnumerable<>).MakeGenericType(serviceType));

private bool ContainsExplicitRegistrationFor(Type serviceType) =>
this.GetRegistrationEvenIfInvalid(serviceType, InjectionConsumerInfo.Root, false) != null;

private void ThrowNotConstructableException(Type concreteType)
{
string exceptionMessage;
Expand Down
62 changes: 52 additions & 10 deletions SimpleInjector.NET/StringResources.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,14 @@ internal static string DelegateForTypeThrewAnException(Type serviceType) =>
string.Format(CultureInfo.InvariantCulture,
"The registered delegate for type {0} threw an exception.", serviceType.ToFriendlyName());

internal static string NoRegistrationForTypeFound(Type serviceType, bool containerHasRegistrations) =>
internal static string NoRegistrationForTypeFound(Type serviceType, bool containerHasRegistrations,
bool containerHasRelatedOneToOneMapping, bool containerHasRelatedCollectionMapping) =>
string.Format(CultureInfo.InvariantCulture,
"No registration for type {0} could be found.{1}",
"No registration for type {0} could be found.{1}{2}{3}",
serviceType.ToFriendlyName(),
ContainsHasNoRegistrationsAddition(containerHasRegistrations));
ContainerHasNoRegistrationsAddition(containerHasRegistrations),
DidYouMeanToCallGetInstanceInstead(containerHasRelatedOneToOneMapping, serviceType),
DidYouMeanToCallGetAllInstancesInstead(containerHasRelatedCollectionMapping, serviceType));

internal static string OpenGenericTypesCanNotBeResolved(Type serviceType) =>
string.Format(CultureInfo.InvariantCulture,
Expand Down Expand Up @@ -157,22 +160,27 @@ internal static string CollectionTypeAlreadyRegistered(Type serviceType) =>
nameof(ContainerOptions),
nameof(ContainerOptions.AllowOverridingRegistrations));

internal static string ParameterTypeMustBeRegistered(InjectionTargetInfo target, int numberOfConditionals) =>
internal static string ParameterTypeMustBeRegistered(InjectionTargetInfo target, int numberOfConditionals,
bool hasRelatedOneToOneMapping, bool hasRelatedCollectionMapping) =>
target.Parameter != null
? string.Format(CultureInfo.InvariantCulture,
"The constructor of type {0} contains the parameter with name '{1}' and type {2} that " +
"is not registered. Please ensure {2} is registered, or change the constructor of {0}.{3}",
"is not registered. Please ensure {2} is registered, or change the constructor of {0}.{3}{4}{5}",
target.Member.DeclaringType.ToFriendlyName(),
target.Name,
target.TargetType.ToFriendlyName(),
GetAdditionalInformationAboutExistingConditionalRegistrations(target, numberOfConditionals))
GetAdditionalInformationAboutExistingConditionalRegistrations(target, numberOfConditionals),
DidYouMeanToDependOnNonCollectionInstead(hasRelatedOneToOneMapping, target.TargetType),
DidYouMeanToDependOnCollectionInstead(hasRelatedCollectionMapping, target.TargetType))
: string.Format(CultureInfo.InvariantCulture,
"Type {0} contains the property with name '{1}' and type {2} that is not registered. " +
"Please ensure {2} is registered, or change {0}.{3}",
"Please ensure {2} is registered, or change {0}.{3}{4}{5}",
target.Member.DeclaringType.ToFriendlyName(),
target.Name,
target.TargetType.ToFriendlyName(),
GetAdditionalInformationAboutExistingConditionalRegistrations(target, numberOfConditionals));
GetAdditionalInformationAboutExistingConditionalRegistrations(target, numberOfConditionals),
DidYouMeanToDependOnNonCollectionInstead(hasRelatedOneToOneMapping, target.TargetType),
DidYouMeanToDependOnCollectionInstead(hasRelatedCollectionMapping, target.TargetType));

internal static string TypeMustHaveASinglePublicConstructor(Type serviceType) =>
string.Format(CultureInfo.InvariantCulture,
Expand Down Expand Up @@ -232,7 +240,7 @@ internal static string ImplicitRegistrationCouldNotBeMadeForType(Type serviceTyp
string.Format(CultureInfo.InvariantCulture,
"No registration for type {0} could be found and an implicit registration could not be made.{1}",
serviceType.ToFriendlyName(),
ContainsHasNoRegistrationsAddition(containerHasRegistrations));
ContainerHasNoRegistrationsAddition(containerHasRegistrations));

internal static string DefaultScopedLifestyleCanNotBeSetWithLifetimeScoped() =>
string.Format(CultureInfo.InvariantCulture,
Expand Down Expand Up @@ -764,10 +772,44 @@ private static string SuppliedTypeIsNotGenericExplainingAlternatives(Type type,
registeringElement,
nameof(Container.RegisterCollection));

private static object ContainsHasNoRegistrationsAddition(bool containerHasRegistrations) =>
private static object ContainerHasNoRegistrationsAddition(bool containerHasRegistrations) =>
containerHasRegistrations
? string.Empty
: " Please note that the container instance you are resolving from contains no " +
"registrations. Could it be that you accidentally created a new -and empty- container?";

private static object DidYouMeanToCallGetInstanceInstead(bool hasRelatedOneToOneMapping,
Type collectionServiceType) =>
hasRelatedOneToOneMapping
? string.Format(CultureInfo.InvariantCulture,
" There is, however, a registration for {0}; Did you mean to call GetInstance<{0}>() " +
"or depend on {0}?",
collectionServiceType.GetGenericArguments()[0].ToFriendlyName())
: string.Empty;

private static object DidYouMeanToDependOnNonCollectionInstead(bool hasRelatedOneToOneMapping,
Type collectionServiceType) =>
hasRelatedOneToOneMapping
? string.Format(CultureInfo.InvariantCulture,
" There is, however, a registration for {0}; Did you mean to depend on {0}?",
collectionServiceType.GetGenericArguments()[0].ToFriendlyName())
: string.Empty;

private static object DidYouMeanToCallGetAllInstancesInstead(bool hasCollection, Type serviceType) =>
hasCollection
? string.Format(CultureInfo.InvariantCulture,
" There is, however, a registration for {0}; Did you mean to call " +
"GetAllInstances<{1}>() or depend on {0}?",
typeof(IEnumerable<>).MakeGenericType(serviceType).ToFriendlyName(),
serviceType.ToFriendlyName())
: string.Empty;

private static object DidYouMeanToDependOnCollectionInstead(bool hasCollection, Type serviceType) =>
hasCollection
? string.Format(CultureInfo.InvariantCulture,
" There is, however, a registration for {0}; Did you mean to depend on {0}?",
typeof(IEnumerable<>).MakeGenericType(serviceType).ToFriendlyName(),
serviceType.ToFriendlyName())
: string.Empty;
}
}
4 changes: 3 additions & 1 deletion changes.txt
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
Version [Version Number] ([Friendly Version Number]) [Release Date]


Version 3.0.5
Version 3.0.5 (v3.0.5 RTM) 2015-09-03

Bug fixes:
-[Core] Unconditional generic registration with type constraint could not be made when
AllowOverridingRegistrations was set to true. (fixes #116)
-[Core] Improved exception message that communicates that registrations of collections are separated
from other registrations. (fixes #115)


Version 3.0.4 (v3.0.4 RTM) 2015-08-31
Expand Down

0 comments on commit 259f0fd

Please sign in to comment.