NullReferenceException thrown when attempting to build unmet expectation message that includes an anonymous parameter about fakeiteasy HOT 7 CLOSED

Hi, @kaeedo. Thanks for the kind words for FakeItEasy, and sorry you're having a less than ideal time.

Regarding #232, no updates. The original idea was proposed by a community member, and no contribution was forthcoming. I'm not a F# user at all, and so have little insight (or use for the project). @thomaslevesque is more aware of the F# technology and may have additional insights, but the issue has never been a high priority. I'd've consider closing it, except we're not against the concept, just resource-poor. Is it something you'd like to tackle?

I've run your repro and see the problem (and at some level, the cause). For those playing along at home, the F# output is

[xUnit.net 00:00:02.55]     Tests.Fake it easy with mocking [FAIL]
  Failed Tests.Fake it easy with mocking [87 ms]
  Error Message:
   System.ArgumentNullException : Value cannot be null. (Parameter 'value')
  Stack Trace:
   at FakeItEasy.Guard.AgainstNull(Object argument, String argumentName) in C:\projects\fakeiteasy\src\FakeItEasy\Guard.cs:line 24
   at FakeItEasy.DefaultOutputWriter.Write(String value) in C:\projects\fakeiteasy\src\FakeItEasy\DefaultOutputWriter.cs:line 26
   at FakeItEasy.Expressions.CallConstraintDescriber.WriteArgumentsListString(IOutputWriter writer, MethodInfo method, IEnumerable`1 argumentConstraints) in C:\projects\fakeiteasy\src\FakeItEasy\Expressions\CallConstraintDescriber.cs:line 70
   at FakeItEasy.Expressions.CallConstraintDescriber.DescribeCallOn(IOutputWriter writer, Object callTarget, MethodInfo method, IEnumerable`1 argumentConstraints) in C:\projects\fakeiteasy\src\FakeItEasy\Expressions\CallConstraintDescriber.cs:line 27
   …

The problem appears at https://github.com/FakeItEasy/FakeItEasy/blob/7.3.1/src/FakeItEasy/Expressions/CallConstraintDescriber.cs#L78, where we try to write the call constraint:

writer.Write(parameter.Name!).Write(": ");

parameter.Name is null.

Naively changing to

writer.Write(parameter.Name ?? "unnamed parameter").Write(": ");

Gives this error:

[xUnit.net 00:00:00.70]     Tests.Fake it easy with mocking [FAIL]
  Failed Tests.Fake it easy with mocking [84 ms]
  Error Message:
   FakeItEasy.ExpectationException :

  Assertion failed for the following call:
    Program+IDatabase.Save(unnamed parameter: <Ignored>)
  Expected to find it once exactly but no calls were made to the fake object.

Slightly better, but lacking in that the parameter (as I think of it; I know that F# has different concepts, and I am not an F# user) was not originally unnamed. It was user:

interface IDatabase with
    member _.Save user =
        let id, username = user
        ()

We should probably explore and try to see if there's a better alternative. Maybe there is some information that can be gleaned from the call.

I can take a look sometime, but if you have any interest, do feel free.

from fakeiteasy.

Actually, I see what the problem is: the Database implementation specifies a parameter name, but the interface doesn't! If you just fix the interface to specify the parameter name, it should work as expected:

type IDatabase =
    abstract member Save: user: (int * string) -> unit

(we still need to fix the case where we don't have the parameter name, though)

from fakeiteasy.

🥇 Thanks for the contribution @kaeedo. Look for your name in the release notes!

from fakeiteasy.

Looks like F# doesn't emit reflection metadata for parameter names...

This code prints <null>:

let param = typedefof<IDatabase>.GetMethod("Save").GetParameters()[0]
printfn "%A" (param.Name)

There's not much we can do about it... we can't guess the name if it's not accessible through reflection.

However, we should handle the case where we don't have the parameter name. We had assumed it would never be null; that's why we have the ! in parameter.Name!, but it was clearly was a mistake, and ParameterInfo.Name is nullable for a good reason. We should either use a fallback name, or not include the name at all in the output.

from fakeiteasy.

HAHA. i JUST discovered the same fix. This is nice because this lets us update our own code by specifying the name in the interface, without needing a library fix.

I think it still makes sense to include a fallback message in the library side of things for when the parameter name is missing.

I've pushed a commit to my repo that includes a param name.

Thanks for looking into it

from fakeiteasy.

@kaeedo, as @thomaslevesque says, whatever else happens, FakeItEasy shouldn't throw an exception. I have a half-baked fix in my sandbox and will continue with it.

from fakeiteasy.

This change has been released as part of FakeItEasy 7.4.0.

from fakeiteasy.