Key improvements (#7719)

ValueKey and ObjectKey shouldn't be == with subclasses.
Fixes https://github.com/flutter/flutter/issues/3107

Clean up toString for the keys a bit.

Add a test for keys.

packages/flutter/lib/src/widgets/framework.dart

@@ -52,6 +52,12 @@ abstract class LocalKey extends Key {
 ///
 /// A [ValueKey<T>] is equal to another [ValueKey<T>] if, and only if, their
 /// values are [operator==].
+///
+/// This class can be subclassed to create value keys that will not be equal to
+/// other value keys that happen to use the same value. If the subclass is
+/// private, this results in a value key type that cannot collide with keys from
+/// other sources, which could be useful, for example, if the keys are being
+/// used as fallbacks in the same scope as keys supplied from another widget.
 class ValueKey<T> extends LocalKey {
   /// Creates a key that delgates its [operator==] to the given value.
   const ValueKey(this.value);
@@ -61,19 +67,28 @@ class ValueKey<T> extends LocalKey {
 
   @override
   bool operator ==(dynamic other) {
-    if (other is! ValueKey<T>)
+    if (other.runtimeType != runtimeType)
       return false;
     final ValueKey<T> typedOther = other;
     return value == typedOther.value;
   }
 
   @override
-  int get hashCode => value.hashCode;
+  int get hashCode => hashValues(runtimeType, value);
 
   @override
-  String toString() => '[\'$value\']';
+  String toString() {
+    final String valueString = T == String ? '<\'$value\'>' : '<$value>';
+    // The crazy on the next line is a workaround for
+    // https://github.com/dart-lang/sdk/issues/28548
+    if (runtimeType == new _TypeLiteral<ValueKey<T>>().type)
+      return '[$valueString]';
+    return '[$T $valueString]';
+  }
 }
 
+class _TypeLiteral<T> { Type get type => T; }
+
 /// A key that is only equal to itself.
 class UniqueKey extends LocalKey {
   /// Creates a key that is equal only to itself.
@@ -96,17 +111,21 @@ class ObjectKey extends LocalKey {
 
   @override
   bool operator ==(dynamic other) {
-    if (other is! ObjectKey)
+    if (other.runtimeType != runtimeType)
       return false;
     final ObjectKey typedOther = other;
     return identical(value, typedOther.value);
   }
 
   @override
-  int get hashCode => identityHashCode(value);
+  int get hashCode => hashValues(runtimeType, identityHashCode(value));
 
   @override
-  String toString() => '[${value.runtimeType}(${value.hashCode})]';
+  String toString() {
+    if (runtimeType == ObjectKey)
+      return '[${value.runtimeType}@${value.hashCode}]';
+    return '[$runtimeType ${value.runtimeType}@${value.hashCode}]';
+  }
 }
 
 /// Signature for a callback when a global key is removed from the tree.
@@ -286,13 +305,19 @@ class LabeledGlobalKey<T extends State<StatefulWidget>> extends GlobalKey<T> {
   final String _debugLabel;
 
   @override
-  String toString() => '[GlobalKey ${_debugLabel != null ? _debugLabel : hashCode}]';
+  String toString() {
+    if (this.runtimeType == LabeledGlobalKey)
+      return '[GlobalKey ${_debugLabel ?? hashCode}]';
+    return '[$runtimeType ${_debugLabel ?? hashCode}]';
+  }
 }
 
 /// A global key that takes its identity from the object used as its value.
 ///
 /// Used to tie the identity of a widget to the identity of an object used to
 /// generate that widget.
+///
+/// Any [GlobalObjectKey] created for the same value will match.
 @optionalTypeArgs
 class GlobalObjectKey<T extends State<StatefulWidget>> extends GlobalKey<T> {
   /// Creates a global key that uses [identical] on [value] for its [operator==].
@@ -303,7 +328,7 @@ class GlobalObjectKey<T extends State<StatefulWidget>> extends GlobalKey<T> {
 
   @override
   bool operator ==(dynamic other) {
-    if (other is! GlobalObjectKey<T>)
+    if (other.runtimeType != runtimeType)
       return false;
     final GlobalObjectKey<T> typedOther = other;
     return identical(value, typedOther.value);
@@ -313,7 +338,7 @@ class GlobalObjectKey<T extends State<StatefulWidget>> extends GlobalKey<T> {
   int get hashCode => identityHashCode(value);
 
   @override
-  String toString() => '[$runtimeType ${value.runtimeType}(${value.hashCode})]';
+  String toString() => '[$runtimeType ${value.runtimeType}@${value.hashCode}]';
 }
 
 /// This class is a work-around for the "is" operator not accepting a variable value as its right operand

packages/flutter/test/widgets/key_test.dart

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_test/flutter_test.dart';
+import 'package:flutter/widgets.dart';
+
+class TestValueKey<T> extends ValueKey<T> {
+  const TestValueKey(T value) : super(value);
+}
+
+class NotEquals {
+  const NotEquals();
+  @override
+  bool operator ==(dynamic other) => false;
+  @override
+  int get hashCode => 0;
+}
+
+void main() {
+  testWidgets('Keys', (WidgetTester tester) async {
+    expect(new ValueKey<int>(3) == new ValueKey<int>(3), isTrue);
+    expect(new ValueKey<num>(3) == new ValueKey<int>(3), isFalse);
+    expect(new ValueKey<int>(3) == new ValueKey<int>(2), isFalse);
+    expect(const ValueKey<double>(double.NAN) == const ValueKey<double>(double.NAN), isFalse);
+    
+    expect(new Key('') == new ValueKey<String>(''), isTrue);
+    expect(new ValueKey<String>('') == new ValueKey<String>(''), isTrue);
+    expect(new TestValueKey<String>('') == new ValueKey<String>(''), isFalse);
+    expect(new TestValueKey<String>('') == new TestValueKey<String>(''), isTrue);
+
+    expect(new ValueKey<String>('') == new ValueKey<dynamic>(''), isFalse);
+    expect(new TestValueKey<String>('') == new TestValueKey<dynamic>(''), isFalse);
+    
+    expect(new UniqueKey() == new UniqueKey(), isFalse);
+    LocalKey k = new UniqueKey();
+    expect(new UniqueKey() == new UniqueKey(), isFalse);
+    expect(k == k, isTrue);
+    
+    expect(new ValueKey<LocalKey>(k) == new ValueKey<LocalKey>(k), isTrue);
+    expect(new ValueKey<LocalKey>(k) == new ValueKey<UniqueKey>(k), isFalse);
+    expect(new ObjectKey(k) == new ObjectKey(k), isTrue);
+
+    expect(new ValueKey<NotEquals>(const NotEquals()) == new ValueKey<NotEquals>(const NotEquals()), isFalse);
+    expect(new ObjectKey(const NotEquals()) == new ObjectKey(const NotEquals()), isTrue);
+    
+    expect(new ObjectKey(const Object()) == new ObjectKey(const Object()), isTrue);
+    expect(new ObjectKey(new Object()) == new ObjectKey(new Object()), isFalse);
+
+    expect(new ValueKey<bool>(true), hasOneLineDescription);
+    expect(new UniqueKey(), hasOneLineDescription);
+    expect(new ObjectKey(true), hasOneLineDescription);
+    expect(new GlobalKey(), hasOneLineDescription);
+    expect(new GlobalKey(debugLabel: 'hello'), hasOneLineDescription);
+    expect(new GlobalObjectKey(true), hasOneLineDescription);
+  });
+}