[plugin_platform_interface] Add a new method verify that prevents u…

…se of `const Object()` as token. (#4640) - Introduce `verify`, a more future-proof name. It throws `AssertionError` if `const Object()` used as the instance's token. - Soft-deprecate `verifyToken` with a comment. It will actually deprecated in a future release, to avoid breaking tests with this minor change. - Update documentation for `PlatformInterface` to show new usage of `verify` and other cosmetic fixes. - Add a test for the new assertion. Fixes flutter/flutter#96178.
flutter · Jan 6, 2022 · 72634e0 · stuartmorgan · Jan 7, 2022 · 72634e0
1 parent cb381ce
commit 72634e0
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 25 deletions.
diff --git a/packages/plugin_platform_interface/CHANGELOG.md b/packages/plugin_platform_interface/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 2.1.0
+
+* Introduce `verify`, which prevents use of `const Object()` as instance token.
+* Add a comment indicating that `verifyToken` will be deprecated in a future release.
+
 ## 2.0.2
 
 * Update package description.

diff --git a/packages/plugin_platform_interface/lib/plugin_platform_interface.dart b/packages/plugin_platform_interface/lib/plugin_platform_interface.dart
@@ -12,7 +12,7 @@ import 'package:meta/meta.dart';
 /// implemented using `extends` instead of `implements`.
 ///
 /// Platform interface classes are expected to have a private static token object which will be
-/// be passed to [verifyToken] along with a platform interface object for verification.
+/// be passed to [verify] along with a platform interface object for verification.
 ///
 /// Sample usage:
 ///
@@ -22,14 +22,14 @@ import 'package:meta/meta.dart';
 ///
 ///   static UrlLauncherPlatform _instance = MethodChannelUrlLauncher();
 ///
-///   static const Object _token = Object();
+///   static final Object _token = Object();
 ///
 ///   static UrlLauncherPlatform get instance => _instance;
 ///
 ///   /// Platform-specific plugins should set this with their own platform-specific
 ///   /// class that extends [UrlLauncherPlatform] when they register themselves.
 ///   static set instance(UrlLauncherPlatform instance) {
-///     PlatformInterface.verifyToken(instance, _token);
+///     PlatformInterface.verify(instance, _token);
 ///     _instance = instance;
 ///   }
 ///
@@ -40,13 +40,16 @@ import 'package:meta/meta.dart';
 /// to include the [MockPlatformInterfaceMixin] for the verification to be temporarily disabled. See
 /// [MockPlatformInterfaceMixin] for a sample of using Mockito to mock a platform interface.
 abstract class PlatformInterface {
-  /// Pass a private, class-specific `const Object()` as the `token`.
+  /// Constructs a PlatformInterface, for use only in constructors of abstract
+  /// derived classes.
+  ///
+  /// @param token The same, non-`const` `Object` that will be passed to `verify`.
   PlatformInterface({required Object token}) : _instanceToken = token;
 
   final Object? _instanceToken;
 
-  /// Ensures that the platform instance has a token that matches the
-  /// provided token and throws [AssertionError] if not.
+  /// Ensures that the platform instance was constructed with a non-`const` token
+  /// that matches the provided token and throws [AssertionError] if not.
   ///
   /// This is used to ensure that implementers are using `extends` rather than
   /// `implements`.
@@ -56,7 +59,22 @@ abstract class PlatformInterface {
   ///
   /// This is implemented as a static method so that it cannot be overridden
   /// with `noSuchMethod`.
+  static void verify(PlatformInterface instance, Object token) {
+    if (identical(instance._instanceToken, const Object())) {
+      throw AssertionError('`const Object()` cannot be used as the token.');
+    }
+    _verify(instance, token);
+  }
+
+  /// Performs the same checks as `verify` but without throwing an
+  /// [AssertionError] if `const Object()` is used as the instance token.
+  ///
+  /// This method will be deprecated in a future release.
   static void verifyToken(PlatformInterface instance, Object token) {
+    _verify(instance, token);
+  }
+
+  static void _verify(PlatformInterface instance, Object token) {
     if (instance is MockPlatformInterfaceMixin) {
       bool assertionsEnabled = false;
       assert(() {
@@ -78,12 +96,12 @@ abstract class PlatformInterface {
 
 /// A [PlatformInterface] mixin that can be combined with mockito's `Mock`.
 ///
-/// It passes the [PlatformInterface.verifyToken] check even though it isn't
+/// It passes the [PlatformInterface.verify] check even though it isn't
 /// using `extends`.
 ///
 /// This class is intended for use in tests only.
 ///
-/// Sample usage (assuming UrlLauncherPlatform extends [PlatformInterface]:
+/// Sample usage (assuming `UrlLauncherPlatform` extends [PlatformInterface]):
 ///
 /// ```dart
 /// class UrlLauncherPlatformMock extends Mock

diff --git a/packages/plugin_platform_interface/pubspec.yaml b/packages/plugin_platform_interface/pubspec.yaml
@@ -9,13 +9,13 @@ issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+
 #
 # This package is used as a second level dependency for many plugins, a major version bump here
 # is guaranteed to lead the ecosystem to a version lock (the first plugin that upgrades to version
-# 2 of this package cannot be used with any other plugin that have not yet migrated).
+# 3 of this package cannot be used with any other plugin that have not yet migrated).
 #
 # Please consider carefully before bumping the major version of this package, ideally it should only
-# be done when absolutely necessary and after the ecosystem has already migrated to 1.X.Y version
-# that is forward compatible with 2.0.0 (ideally the ecosystem have migrated to depend on:
-# `plugin_platform_interface: >=1.X.Y <3.0.0`).
-version: 2.0.2
+# be done when absolutely necessary and after the ecosystem has already migrated to 2.X.Y version
+# that is forward compatible with 3.0.0 (ideally the ecosystem have migrated to depend on:
+# `plugin_platform_interface: >=2.X.Y <4.0.0`).
+version: 2.1.0
 
 environment:
   sdk: ">=2.12.0 <3.0.0"

diff --git a/packages/plugin_platform_interface/test/plugin_platform_interface_test.dart b/packages/plugin_platform_interface/test/plugin_platform_interface_test.dart
@@ -12,7 +12,7 @@ class SamplePluginPlatform extends PlatformInterface {
   static final Object _token = Object();
 
   static set instance(SamplePluginPlatform instance) {
-    PlatformInterface.verifyToken(instance, _token);
+    PlatformInterface.verify(instance, _token);
     // A real implementation would set a static instance field here.
   }
 }
@@ -26,20 +26,81 @@ class ImplementsSamplePluginPlatformUsingMockPlatformInterfaceMixin extends Mock
 
 class ExtendsSamplePluginPlatform extends SamplePluginPlatform {}
 
+class ConstTokenPluginPlatform extends PlatformInterface {
+  ConstTokenPluginPlatform() : super(token: _token);
+
+  static const Object _token = Object(); // invalid
+
+  static set instance(ConstTokenPluginPlatform instance) {
+    PlatformInterface.verify(instance, _token);
+  }
+}
+
+class ExtendsConstTokenPluginPlatform extends ConstTokenPluginPlatform {}
+
+class VerifyTokenPluginPlatform extends PlatformInterface {
+  VerifyTokenPluginPlatform() : super(token: _token);
+
+  static final Object _token = Object();
+
+  static set instance(VerifyTokenPluginPlatform instance) {
+    PlatformInterface.verifyToken(instance, _token);
+    // A real implementation would set a static instance field here.
+  }
+}
+
+class ImplementsVerifyTokenPluginPlatform extends Mock
+    implements VerifyTokenPluginPlatform {}
+
+class ImplementsVerifyTokenPluginPlatformUsingMockPlatformInterfaceMixin
+    extends Mock
+    with MockPlatformInterfaceMixin
+    implements VerifyTokenPluginPlatform {}
+
+class ExtendsVerifyTokenPluginPlatform extends VerifyTokenPluginPlatform {}
+
 void main() {
-  test('Cannot be implemented with `implements`', () {
-    expect(() {
-      SamplePluginPlatform.instance = ImplementsSamplePluginPlatform();
-    }, throwsA(isA<AssertionError>()));
-  });
+  group('`verify`', () {
+    test('prevents implementation with `implements`', () {
+      expect(() {
+        SamplePluginPlatform.instance = ImplementsSamplePluginPlatform();
+      }, throwsA(isA<AssertionError>()));
+    });
+
+    test('allows mocking with `implements`', () {
+      final SamplePluginPlatform mock =
+          ImplementsSamplePluginPlatformUsingMockPlatformInterfaceMixin();
+      SamplePluginPlatform.instance = mock;
+    });
 
-  test('Can be mocked with `implements`', () {
-    final SamplePluginPlatform mock =
-        ImplementsSamplePluginPlatformUsingMockPlatformInterfaceMixin();
-    SamplePluginPlatform.instance = mock;
+    test('allows extending', () {
+      SamplePluginPlatform.instance = ExtendsSamplePluginPlatform();
+    });
+
+    test('prevents `const Object()` token', () {
+      expect(() {
+        ConstTokenPluginPlatform.instance = ExtendsConstTokenPluginPlatform();
+      }, throwsA(isA<AssertionError>()));
+    });
   });
 
-  test('Can be extended', () {
-    SamplePluginPlatform.instance = ExtendsSamplePluginPlatform();
+  // Tests of the earlier, to-be-deprecated `verifyToken` method
+  group('`verifyToken`', () {
+    test('prevents implementation with `implements`', () {
+      expect(() {
+        VerifyTokenPluginPlatform.instance =
+            ImplementsVerifyTokenPluginPlatform();
+      }, throwsA(isA<AssertionError>()));
+    });
+
+    test('allows mocking with `implements`', () {
+      final VerifyTokenPluginPlatform mock =
+          ImplementsVerifyTokenPluginPlatformUsingMockPlatformInterfaceMixin();
+      VerifyTokenPluginPlatform.instance = mock;
+    });
+
+    test('allows extending', () {
+      VerifyTokenPluginPlatform.instance = ExtendsVerifyTokenPluginPlatform();
+    });
   });
 }