Skip to content

Commit

Permalink
Apply review comments
Browse files Browse the repository at this point in the history
- use string.Empty for null value from IDescribeSpecification, rather than
  falling back to ToString(). This supports the contract that
  IDescribeSpecification will be used if implemented. Replacing
  null string.Empty with matches the documented
  behaviour of IDescribeNonMatches.
- updated IDescribeSpecification code docs.
- removed GenericToNonGenericMatcherProxyWithDescribe `ToString` as it
  can use the GenericToNonGenericMatcherProxy superclass implementation.
- update ArgumentSpecification to also support IDescribeSpecification
  for its matcher.
- Replace linq with Array.ConvertAll rather than requiring an extra
  ToArray conversion.
  • Loading branch information
dtchepak committed Nov 10, 2024
1 parent 4460556 commit 50401f7
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 14 deletions.
7 changes: 3 additions & 4 deletions src/NSubstitute/Core/Arguments/ArgumentMatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ private class GenericToNonGenericMatcherProxy<T>(IArgumentMatcher<T> matcher) :
public bool IsSatisfiedBy(object? argument) => _matcher.IsSatisfiedBy((T?)argument!);

public override string ToString() =>
(_matcher as IDescribeSpecification)?.DescribeSpecification() ?? _matcher.ToString() ?? "";
_matcher is IDescribeSpecification describe
? describe.DescribeSpecification() ?? string.Empty
: _matcher.ToString() ?? string.Empty;
}

private class GenericToNonGenericMatcherProxyWithDescribe<T> : GenericToNonGenericMatcherProxy<T>, IDescribeNonMatches
Expand All @@ -51,9 +53,6 @@ public GenericToNonGenericMatcherProxyWithDescribe(IArgumentMatcher<T> matcher)
}

public string DescribeFor(object? argument) => ((IDescribeNonMatches)_matcher).DescribeFor(argument);

public override string ToString() =>
(_matcher as IDescribeSpecification)?.DescribeSpecification() ?? _matcher.ToString() ?? "";
}

private class DefaultValueContainer<T>
Expand Down
5 changes: 4 additions & 1 deletion src/NSubstitute/Core/Arguments/ArgumentSpecification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ public string FormatArgument(object? argument)
: ArgumentFormatter.Default.Format(argument, highlight: !isSatisfiedByArg);
}

public override string ToString() => matcher.ToString() ?? string.Empty;
public override string ToString() =>
matcher is IDescribeSpecification describe
? describe.DescribeSpecification()
: matcher.ToString() ?? string.Empty;

public IArgumentSpecification CreateCopyMatchingAnyArgOfType(Type requiredType)
{
Expand Down
8 changes: 5 additions & 3 deletions src/NSubstitute/Core/CallSpecification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,11 @@ public IEnumerable<ArgumentMatchInfo> NonMatchingArguments(ICall call)

public override string ToString()
{
var argSpecsAsStrings = _argumentSpecifications.Select(x =>
(x as IDescribeSpecification)?.DescribeSpecification() ?? x.ToString() ?? string.Empty
).ToArray();
var argSpecsAsStrings = Array.ConvertAll(_argumentSpecifications, x =>
x is IDescribeSpecification describe
? describe.DescribeSpecification() ?? string.Empty
: x.ToString() ?? string.Empty
);
return CallFormatter.Default.Format(GetMethodInfo(), argSpecsAsStrings);
}

Expand Down
6 changes: 3 additions & 3 deletions src/NSubstitute/Core/IDescribeSpecification.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ namespace NSubstitute.Core;
/// </summary>
public interface IDescribeSpecification
{

/// <summary>
/// A concise description of the conditions required to match this specification.
/// A concise description of the conditions required to match this specification, or <see cref="string.Empty"/>
/// if a detailed description can not be provided.
/// </summary>
/// <returns></returns>
/// <returns>Description of the specification, or <see cref="string.Empty" /> if no description can be provided.</returns>
string DescribeSpecification();
}
16 changes: 13 additions & 3 deletions tests/NSubstitute.Acceptance.Specs/ArgumentMatching.cs
Original file line number Diff line number Diff line change
Expand Up @@ -862,11 +862,21 @@ public void Should_describe_spec_for_custom_arg_matcher_when_implemented()
{
var ex = Assert.Throws<ReceivedCallsException>(() =>
{
_something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher()));
_something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher("DescribeSpec")));
});
Assert.That(ex.Message, Contains.Substring("Add(23, DescribeSpec)"));
}

[Test]
public void Should_use_empty_string_for_null_describe_spec_for_custom_arg_matcher_when_implemented()
{
var ex = Assert.Throws<ReceivedCallsException>(() =>
{
_something.Received().Add(23, ArgumentMatcher.Enqueue(new CustomDescribeSpecMatcher(null)));
});
Assert.That(ex.Message, Contains.Substring("Add(23, )"));
}

class CustomMatcher : IArgumentMatcher, IDescribeNonMatches, IArgumentMatcher<int>
{
public string DescribeFor(object argument) => "failed";
Expand All @@ -875,8 +885,8 @@ class CustomMatcher : IArgumentMatcher, IDescribeNonMatches, IArgumentMatcher<in
public override string ToString() => "Custom match";
}

class CustomDescribeSpecMatcher : CustomMatcher, IDescribeSpecification
class CustomDescribeSpecMatcher(string description) : CustomMatcher, IDescribeSpecification
{
public string DescribeSpecification() => "DescribeSpec";
public string DescribeSpecification() => description;
}
}

0 comments on commit 50401f7

Please sign in to comment.