nunit.analyzers icon indicating copy to clipboard operation
nunit.analyzers copied to clipboard

Not correctly marked incompatible types for EqualTo constraint

Open alexandra142 opened this issue 2 years ago • 2 comments

Hello,

I am writing to notify you that your analyzer mark a code as not compatible types for EqualTo, but it is not true .

for code:

Assert.That(result, Is.EqualTo(Error.EntraAssignmentNotFound));

Where result is type of Response :

public class Response
    {
        public HttpStatusCode StatusCode => Error?.StatusCode ?? SuccessStatus;
        public Error Error { get; }

        private HttpStatusCode SuccessStatus { get; }

        protected Response(Error error)
        {
            Error = error;
        }

        protected Response()
        {
            SuccessStatus = HttpStatusCode.OK;
        }

        protected Response(HttpStatusCode statusCode)
        {
            SuccessStatus = statusCode;
        }

        public static Response<T> Result<T>(T value)
            => new Response<T>(value);

        public static Response<T> Result<T>(T value, HttpStatusCode successStatus)
            => new Response<T>(value, successStatus);

        public bool IsError => (int)StatusCode > 299;

        public override string ToString()
        {
            return new
            {
                StatusCode,
                Error,
            }.ToString();
        }

        public static implicit operator Response(Error error) => new Response(error);
    }

And Error is:

 public sealed class Error : CommunicationErrorResponse, IEquatable<Error>
  {
      public HttpStatusCode StatusCode { get; }

      public Error(HttpStatusCode statusCode, CommunicationError response , TimeSpan? retryAfter , string  wwAuthenticateHeaderValuel)
      {
          StatusCode = statusCode;
          Error = response;
          RetryAfter = retryAfter;
          _wwwAuthenticateHeaderValue = wwwAuthenticateHeaderValue;
      }

      public bool Equals(Error other)
      {
          if (ReferenceEquals(null, other))
          {
              return false;
          }

          if (ReferenceEquals(this, other))
          {
              return true;
          }

          return StatusCode == other.StatusCode
                 && RetryAfter.Equals(other.RetryAfter)
                 && Equals(Error, other.Error);
      }

      public bool Equals(Error other, bool strict)
      {
          if (strict)
          {
              return Equals(other);
          }

          return StatusCode == other?.StatusCode
                 && Error.Code == other.Error.Code
                 && Error.Message == other.Error.Message;
      }

      public override bool Equals(object obj)
      {
          if (obj is Response response)
          {
              return Equals(response.Error);
          }

          return ReferenceEquals(this, obj) || Equals(obj as Error);
      }

      public override int GetHashCode()
      {
          return HashCode.Combine((int)StatusCode, RetryAfter, Error);
      }
  }

And CommunicationErrorResponse is:

 public class CommunicationErrorResponse
  {
      /// <summary>
      /// The Communication Services error.
      /// </summary>
      [SwaggerRequired]
      public CommunicationError Error { get; set; }
  }

alexandra142 avatar Mar 14 '24 22:03 alexandra142

@alexandra142 Thanks for reporting.

Using your code extracts (with some parameters removed), I made this test:

Error entryAssignmnentNotFound = new(HttpStatusCode.NotFound, CommunicationError.Some);
Response response= entryAssignmnentNotFound;

Assert.That(response.Equals(entryAssignmnentNotFound), Is.True);

This test fails because Response.Equals(object?) knows nothing about Error. So the NUnit.Analyzer is correct that these are not compatible.

However, the reverse passes, because Error.Equals(object?) knows about Response.

Assert.That(entryAssignmnentNotFound.Equals(response), Is.True);

This makes your code violating the definition of equality that 'Response == Error' implies 'Error == Response'

The symmetric property: x.Equals(y) returns the same value as y.Equals(x).

See how to implement equals

However, the Analyzer assumes that Equal(object?) tests against instance of the type defining the method not possible other types. The Analyzer has no access to your implementation to see what other types it allows comparing with.

In initially though you could add a reciprocal implicit conversion to Response (and drop the test on Response in Error.Equals(object?):

public static implicit operator Error?(Response response) => response.Error;

But that doesn't seem to work neither when calling Response.Equals(Error) nor in Assert.That(..., Is.EqualTo).

The compiler seems to prefer the Equals(object?) overload over the implicit conversion.

In this case, I suggest to remove the cyclic-dependency between Response and Error, by removing Response logic from Error and call the test like:

Assert.That(result.Error, Is.EqualTo(Error.EntraAssignmentNotFound));

manfred-brands avatar Mar 15 '24 03:03 manfred-brands

@alexandra142 I think that it would work if Error is implementing IEquatable<Result> as the Analyzer (like NUnit) looks for implementation of where actualType implement IEquatable<_expectedType_> and vice versa.

manfred-brands avatar Apr 10 '24 10:04 manfred-brands