From 72634e0c404ca0cef18227dec5f8eaa4b6445998 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Wed, 5 Jan 2022 17:43:43 -0800 Subject: [PATCH] [plugin_platform_interface] Add a new method `verify` that prevents use 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 https://github.com/flutter/flutter/issues/96178. --- .../plugin_platform_interface/CHANGELOG.md | 5 ++ .../lib/plugin_platform_interface.dart | 34 ++++++-- .../plugin_platform_interface/pubspec.yaml | 10 +-- .../test/plugin_platform_interface_test.dart | 85 ++++++++++++++++--- 4 files changed, 109 insertions(+), 25 deletions(-) diff --git a/packages/plugin_platform_interface/CHANGELOG.md b/packages/plugin_platform_interface/CHANGELOG.md index af79d119c5f6..950da032e10f 100644 --- 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 index d9bd88168422..f12fd3c39cc7 100644 --- 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 index 0b4b1782b526..0559b8daee8f 100644 --- 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 index 967fa79d6dc3..a00462ffad50 100644 --- 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())); - }); + group('`verify`', () { + test('prevents implementation with `implements`', () { + expect(() { + SamplePluginPlatform.instance = ImplementsSamplePluginPlatform(); + }, throwsA(isA())); + }); + + 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())); + }); }); - 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())); + }); + + test('allows mocking with `implements`', () { + final VerifyTokenPluginPlatform mock = + ImplementsVerifyTokenPluginPlatformUsingMockPlatformInterfaceMixin(); + VerifyTokenPluginPlatform.instance = mock; + }); + + test('allows extending', () { + VerifyTokenPluginPlatform.instance = ExtendsVerifyTokenPluginPlatform(); + }); }); }