Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not correctly marked incompatible types for EqualTo constraint #706

Open
alexandra142 opened this issue Mar 14, 2024 · 2 comments
Open

Not correctly marked incompatible types for EqualTo constraint #706

alexandra142 opened this issue Mar 14, 2024 · 2 comments

Comments

@alexandra142
Copy link

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; }
  }
@manfred-brands
Copy link
Member

@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
Copy link
Member

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants