Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/plugin_platform_interface/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
///
Expand All @@ -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;
/// }
///
Expand All @@ -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`.
Expand All @@ -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(() {
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions packages/plugin_platform_interface/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make a version of this that still uses verifyToken so we're still exercising the codepath that >100 plugins are likely currently using, to make sure we don't break it later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Expand All @@ -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();
});
});
}