Assertion bug: 'a is! Set && b is! Set': iterableEquals doesn't support Sets

[franzaps]

I have a nested object that, when compared, calls objectsEquals internally and in turn iterableEquals.

In my case, a is Iterable and b is Set, so the following code calls iterableEquals:

  } else if (a is Set && b is Set) {
    return setEquals(a, b);
  } else if (a is Iterable && b is Iterable) {
    return iterableEquals(a, b);
  } 

However, inside iterableEquals I get the assertion error because 'a is! Set && b is! Set' is true.

[felangel]

Hi @franzaps 👋
Thanks for opening an issue!

Can you please provide a minimal reproduction sample? I'm happy to push a fix asap!

[ShakyaCsun]

Minimal reproduction sample:

import 'package:equatable/equatable.dart';

class Example extends Equatable {
  const Example(this.iterable);

  final Iterable<int> iterable;

  @override
  List<Object> get props => [iterable];
}

void main() {
  print(Example([]) == Example({})); // Throws assertion error
}

Potential fix would be to remove the assertion and make sure that iterableEquals supports Sets as well by unifying setEquals into iterableEquals with additional checks to ensure equality checks between Set and List/Iterable returns false:

bool iterableEquals(Iterable<Object?> a, Iterable<Object?> b) {
  if (identical(a, b)) return true;
  if (a.length != b.length) return false;
  if (a is Set && b is Set) {
    for (final element in a) {
      if (!b.any((e) => objectsEquals(element, e))) return false;
    }
    return true;
  }
  if (a is Set != b is Set) {
    // If either a or b is a Set, but the other isn't, return false.
    return false;
  }
  // Maybe add this check too, but it would break existing functionality where
  // Iterable and List are considered equal if the elements are in same order.
  if (a is List != b is List) {
    return false;
  }
  for (var i = 0; i < a.length; i++) {
    if (!objectsEquals(a.elementAt(i), b.elementAt(i))) return false;
  }
  return true;
}

Let me know how to proceed with comparing List to Set and List/Set to Iterable with same elements, and I'd be happy to send a PR.

[felangel]

@ShakyaCsun thanks for the context and suggested fix. It seems totally reasonable to me and I'm happy to review a PR 😄

[ShakyaCsun]

print(Example([1]) == Example([1].map((e) => e))); // returns true currently

Should I add the List and Iterable checks as well so that the above would return false? I think the List and Iterable being equal is a bug but maybe that's intended.

[felangel]

print(Example([1]) == Example([1].map((e) => e))); // returns true currently
Should I add the List and Iterable checks as well so that the above would return false? I think the List and Iterable being equal is a bug but maybe that's intended.

Yeah that's fine I'll think about it some more but it seems most consistent with the remaining behavior to consider List and Iterable as different.

[javlon92]

Subject: Question Regarding Equatable Implementation and Class Modifiers

Hello @felangel,

I had a quick question regarding Dart's Equatable implementation. Since both abstract class Equatable and mixin EquatableMixin essentially provide the same functionality, would it be possible to combine them into a single abstract mixin class Equatable, or is there a specific reason they are kept separate?

I came across this idea while reading the Dart documentation on class modifiers, and I wanted to make sure I'm not missing anything conceptually or technically.

Thank you in advance for your time!