avoid_implementing_value_types seems misguided?

[Hixie]

In the documentation for avoid_implementing_value_types it says:

When using implements, you do not inherit the method body of ==, making it nearly impossible to follow the contract of ==

...but as far as I can tell, that's just not true. Whether you can inherit from the superclass or not, to implement the equivalence relation contract, you have to include runtimeType in your operator == check, and once you do that, there's no problem with using operator == with implements.

Indeed, none of the examples that are given in https://dart.dev/tools/linter-rules/avoid_implementing_value_types include the runtimeType in their operator ==, which seems concerning (they use is instead, which is insufficient for implementing the contract, because a subclass could add fields).

[lrhn]

The opposing view, as usual, is that if you don't check runtimeType, another class can implement the interface and be compatible with equality.

It can also be incompatible, but it shouldn't.

import "dart:math"; 

class Point {
  final double x, y;
  Point(this.x, this.y);
  int get hashCode => Object.hash(x, y);
  bool operator==(Object other) => other is Point && x == other.x && y == other.y;
  String toString() => "($x, $y)";
}

class PolarPoint implements Point {  
  final double _angle, _magnitude;
  PolarPoint(double angle, double magnitude) : _angle = angle.remainder(pi * 2), _magnitude = magnitude;
  double get x => cos(_angle) * _magnitude;
  double get y => sin(_angle) * _magnitude;
  int get hashCode => Object.hash(x, y);
  bool operator==(Object other) => other is Point && x == other.x && y == other.y;
  String toString() => "($x, $y)";
}

void main() {
  var angle = asin(3/4);
  var magnitude = 5.0;
  // I compute the point coordinates.  
  var p1 = Point(cos(angle) * magnitude, sin(angle) * magnitude);
  // Or I use a PolarPoint to do it for me.
  var p2 = PolarPoint(angle, magnitude);
  print(p1 == p2); // - doesn't matter, they're equal.
}

My personal recommendation is that any class which overrides == is made final, that avoids the issue.

If you have a class which defines an equality, and allows subclasses which have different equalities, you should have made the original class abstract, and created a final subclass that people can instantiate instead.
The same class is just not suited to be both an extensible superclass and a source of instances with equality.

You can use runtimeType checks instead, but then you prevent other types from having compatible equalities.

(I can see why a widget-hierarchy or similar, with subtyping and change detection, may have different affordances than the code I normally write..)

[eernstg]

I think we need an auxiliary concept in order to explain what runtimeType is doing for us in an implementation of operator ==. I see runtimeType in this context as a special case of a broader concept which is "projected structural equality". The word "structural" is just there to indicate that this is about the state, not the identity. But the word "projected" refers to the distinction that matters here:

Projected structural equality is an asymmetric operation where one object o1 determines whether or not some other object o2 is equal to o1 with respect to the set of properties that the class of o1 considers significant for equality. In other words, o1 throws away everything in o2 that it doesn't know (or care) about, and then compares structurally.

The canonically incorrect (non-symmetric) version of == is this one:

class Point1D {
  final int x;
  
  Point1D(this.x);
  
  @override
  bool operator ==(other) => other is Point1D && other.x == x;
  
  @override
  int get hashCode => Object.hash(Point1D, x);
}

class Point2D extends Point1D {
  final int y;
  
  Point2D(super.x, this.y);

  @override
  bool operator ==(other) => other is Point2D && other.x == x && other.y == y;
  
  @override
  int get hashCode => Object.hash(Point2D, x, y);
}

void main() {
  Point1D p1 = Point1D(1), p2 = Point2D(1, 2);
  print('Receiver is 1D: ${p1 == p2}, 2D: ${p2 == p1}'); // Is true, false.
}

The asymmetry arises because this is really a projected structural equality test: Point1D tests other according to x, and uses other is Point1D to ensure that other has the required state, and that it is nominally related (it is not just an arbitrary thing that has an x), and then it ignores everything else about other. Similarly Point2D fits the same description, but in the actual invocation it rejects the Point1D because it doesn't have the required state—we can't even hope to have a successful structural comparison of the projection, because the projection doesn't exist.

In any case, this implementation of == projects the state of other to the known properties, and ignores anything extra that other might have.

We could obviously make == symmetric if we test both directions of the projection. We would then have a projectedEquals instance member in Object, and Object.== would perform the symmetric test (projecting from both sides), and all other classes would override projectedEquals (and they wouldn't override ==).

(Of course, this makes identity equality much more expensive because it would call Object.projectedEquals twice, and that method would do the actual identity test. But this is about the underlying principles, not about performance, so let's ignore the discussion about possible optimizations.)

Here's the example again, with that change to class Object emulated by means of a new ProjectedEquals class:

abstract class ProjectedEquals { // `projectedEquals` should have been declared in `Object`.
  bool projectedEquals(ProjectedEquals other) { /*... primitive identity test ...*/ }

  @override
  bool operator ==(covariant ProjectedEquals other) =>
      projectedEquals(other) && other.projectedEquals(this);
}

class Point1D extends ProjectedEquals {
  final int x;

  Point1D(this.x);

  @override
  bool projectedEquals(Object other) => other is Point1D && other.x == x;

  @override
  int get hashCode => Object.hash(Point1D, x);
}

class Point2D extends Point1D {
  final int y;

  Point2D(super.x, this.y);

  @override
  bool projectedEquals(Object other) =>
      other is Point2D && other.x == x && other.y == y;

  @override
  int get hashCode => Object.hash(Point2D, x, y);
}

void main() {
  Point1D p1 = Point1D(1), p2 = Point2D(1, 2);
  print('Receiver is 1D: ${p1 == p2}, 2D: ${p2 == p1}'); // Is false, false.
}

The overriding implementations of projectedEquals are systematic (so they could be generated by indicating that "this is a class that has structural equality" and "these properties are significant for equality").

Now, to revisit runtimeType, the point is that runtimeType == other.runtimeType serves to confirm that both objects will necessarily perform the same projection (that is, they're testing the same set of properties), and this is sufficient to prevent the asymmetry issue. So if we use runtimeType == other.runtimeType then we're covering a subset of the cases where symmetric projected equalities hold, we just need to accept that equality can't deviate from the precise run-time type.

However, allowing objects with "slightly" different run-time types to be equal is definitely a meaningful choice.

In general, each class that uses a structural equality criterion would subscribe to a specific projection, which is given by (1) the type T used in other is T, and (2) the set of tested properties.

It is perfectly meaningful for a class to test for a proper supertype of the enclosing class: This is appropriate in the case where a given supertype of the current type specifies the properties required for equality test (and the current type may differ in many ways, but it still delegates the responsibility to that supertype for specifying which properties to test).

class Point1DImpl implements Point1D {
  final int x;

  Point1DImpl(this.x);

  ... // Stuff.

  @override
  bool projectedEquals(Object other) => other is Point1D && other.x == x; // NB: Not `is Point1DImpl`.

  @override
  int get hashCode => Object.hash(Point1D, x); // Again, we're comparing "at" Point1D.
}

We might certainly conclude that it is impractical to do anything like this, in order to ensure symmetry when we allow objects with slightly different run-time types to be equal. However, I do think that it paints a bigger picture where the use of runtimeType == other.runtimeType fits in as a special case.

Is it fair to say that avoid_implementing_value_types is misguided because it is impractical to allow structural equality among objects whose run-time type isn't precisely identical?

[Hixie]

I agree with much of what has been said on this issue, but all of it is consistent with the lint being wrong, IMHO. For example, PolarPoint from @lrhn's example above seems like a perfectly valid use of operator == to me, but it would violate the lint. Conversely, the class Point2D extends Point1D example from @eernstg above violates the symmetry principle we're arguing for here, but would not violate the lint (despite using the lint's purported best practices).

I think there's lots of valid ways to implement equality. I think they largely center around conventions that have to be agreed ahead of time, e.g. that if a class uses is as the type check in operator == that subclasses must not add fields, or that instead of using operator == a hierarchy should rely on something like projectedEquals, or that the instance type should be considered part of equality (and thus use runtimeType). The key for this issue is just that the lint is not really supporting any of these and is therefore, IMHO, just wrong.

[matanlurey]

Consideration: I originally authored this lint before there was final in the language.

I personally never use runtimeType comparisons, with Dart 3x I either make my class final and use is, or don't implement == or hashCode. In either case, I'd be open to either changing how this lint works, or deprecating it.

The original intent was "value types should be created or extended, not implemented" to avoid folks creating mocks or fakes for types that really should just be used directly. We could definitely create better lints for that purpose, IMO, and I'd be happy to be involved.

[eernstg]

Interestingly, the original intent is pretty much the same as "value classes should be base".

[matanlurey]

Yeah, though not 100%.

This lint does not instruct how to write value classes (I think that would be useful, FWIW), but rather how to use the large corpus of existing value classes that were written pre-Dart 3.x.