Skip to content

Commit 2259b67

Browse files
amirhmormih
authored andcommitted
Forbid ... implements UrlLauncherPlatform (flutter#2230)
Fail setting `UrlLauncherPlatform.instance` to something that `implements` (rather than `extends`) `UrlLauncherPlatform`. I'm not bumping the major version as the breaking change policy for platform interface is that we're only guaranteeing to not break classes that `extend` it. We leave a backdoor for mockito mocks which can do: ```dart class MockUrlLauncherPlatform extends Mock implements UrlLauncherPlatform {} test('', () { MockUrlLauncherPlatform mock = MockUrlLauncherPlatform(); when(mock.isMock).thenReturn(true); UrlLauncherPlatform.instance = mock; } ```
1 parent 29ab35e commit 2259b67

File tree

4 files changed

+83
-18
lines changed

4 files changed

+83
-18
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
## 1.0.1
2+
3+
* Enforce that UrlLauncherPlatform isn't implemented with `implements`.
4+
5+
## 1.0.0
6+
7+
* Initial release.

packages/url_launcher/url_launcher_platform_interface/lib/url_launcher_platform_interface.dart

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,27 +4,50 @@
44

55
import 'dart:async';
66

7-
import 'package:meta/meta.dart' show required;
7+
import 'package:meta/meta.dart' show required, visibleForTesting;
88

99
import 'method_channel_url_launcher.dart';
1010

1111
/// The interface that implementations of url_launcher must implement.
1212
///
13-
/// Platform implementations that live in a separate package should extend this
14-
/// class rather than implement it as `url_launcher` does not consider newly
15-
/// added methods to be breaking changes. Extending this class (using `extends`)
16-
/// ensures that the subclass will get the default implementation, while
17-
/// platform implementations that `implements` this interface will be broken by
18-
/// newly added [UrlLauncherPlatform] methods.
13+
/// Platform implementations should extend this class rather than implement it as `url_launcher`
14+
/// does not consider newly added methods to be breaking changes. Extending this class
15+
/// (using `extends`) ensures that the subclass will get the default implementation, while
16+
/// platform implementations that `implements` this interface will be broken by newly added
17+
/// [UrlLauncherPlatform] methods.
1918
abstract class UrlLauncherPlatform {
19+
/// Only mock implementations should set this to true.
20+
///
21+
/// Mockito mocks are implementing this class with `implements` which is forbidden for anything
22+
/// other than mocks (see class docs). This property provides a backdoor for mockito mocks to
23+
/// skip the verification that the class isn't implemented with `implements`.
24+
@visibleForTesting
25+
bool get isMock => false;
26+
2027
/// The default instance of [UrlLauncherPlatform] to use.
2128
///
2229
/// Platform-specific plugins should override this with their own
2330
/// platform-specific class that extends [UrlLauncherPlatform] when they
2431
/// register themselves.
2532
///
2633
/// Defaults to [MethodChannelUrlLauncher].
27-
static UrlLauncherPlatform instance = MethodChannelUrlLauncher();
34+
static UrlLauncherPlatform _instance = MethodChannelUrlLauncher();
35+
36+
static UrlLauncherPlatform get instance => _instance;
37+
38+
// TODO(amirh): Extract common platform interface logic.
39+
// https://github.com/flutter/flutter/issues/43368
40+
static set instance(UrlLauncherPlatform instance) {
41+
if (!instance.isMock) {
42+
try {
43+
instance._verifyProvidesDefaultImplementations();
44+
} on NoSuchMethodError catch (_) {
45+
throw AssertionError(
46+
'Platform interfaces must not be implemented with `implements`');
47+
}
48+
}
49+
_instance = instance;
50+
}
2851

2952
/// Returns `true` if this platform is able to launch [url].
3053
Future<bool> canLaunch(String url) {
@@ -51,4 +74,12 @@ abstract class UrlLauncherPlatform {
5174
Future<void> closeWebView() {
5275
throw UnimplementedError('closeWebView() has not been implemented.');
5376
}
77+
78+
// This method makes sure that UrlLauncher isn't implemented with `implements`.
79+
//
80+
// See class doc for more details on why implementing this class is forbidden.
81+
//
82+
// This private method is called by the instance setter, which fails if the class is
83+
// implemented with `implements`.
84+
void _verifyProvidesDefaultImplementations() {}
5485
}

packages/url_launcher/url_launcher_platform_interface/pubspec.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ author: Flutter Team <flutter-dev@googlegroups.com>
44
homepage: https://github.com/flutter/plugins/tree/master/packages/url_launcher/url_launcher_platform_interface
55
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
66
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
7-
version: 1.0.0
7+
version: 1.0.1
88

99
dependencies:
1010
flutter:
@@ -14,6 +14,7 @@ dependencies:
1414
dev_dependencies:
1515
flutter_test:
1616
sdk: flutter
17+
mockito: ^4.1.1
1718

1819
environment:
1920
sdk: ">=2.0.0-dev.28.0 <3.0.0"

packages/url_launcher/url_launcher_platform_interface/test/method_channel_url_launcher_test.dart

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,41 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'package:mockito/mockito.dart';
56
import 'package:flutter/services.dart';
67
import 'package:flutter_test/flutter_test.dart';
7-
import 'package:url_launcher_platform_interface/method_channel_url_launcher.dart';
8-
import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart';
8+
9+
import '../lib/method_channel_url_launcher.dart';
10+
import '../lib/url_launcher_platform_interface.dart';
911

1012
void main() {
11-
group('$MethodChannelUrlLauncher', () {
12-
TestWidgetsFlutterBinding.ensureInitialized();
13+
TestWidgetsFlutterBinding.ensureInitialized();
14+
15+
group('$UrlLauncherPlatform', () {
16+
test('$MethodChannelUrlLauncher() is the default instance', () {
17+
expect(UrlLauncherPlatform.instance,
18+
isInstanceOf<MethodChannelUrlLauncher>());
19+
});
20+
21+
test('Cannot be implemented with `implements`', () {
22+
expect(() {
23+
UrlLauncherPlatform.instance = ImplementsUrlLauncherPlatform();
24+
}, throwsA(isInstanceOf<AssertionError>()));
25+
});
26+
27+
test('Can be mocked with `implements`', () {
28+
final ImplementsUrlLauncherPlatform mock =
29+
ImplementsUrlLauncherPlatform();
30+
when(mock.isMock).thenReturn(true);
31+
UrlLauncherPlatform.instance = mock;
32+
});
1333

34+
test('Can be extended', () {
35+
UrlLauncherPlatform.instance = ExtendsUrlLauncherPlatform();
36+
});
37+
});
38+
39+
group('$MethodChannelUrlLauncher', () {
1440
const MethodChannel channel =
1541
MethodChannel('plugins.flutter.io/url_launcher');
1642
final List<MethodCall> log = <MethodCall>[];
@@ -24,11 +50,6 @@ void main() {
2450
log.clear();
2551
});
2652

27-
test('is the default $UrlLauncherPlatform instance', () {
28-
expect(UrlLauncherPlatform.instance,
29-
isInstanceOf<MethodChannelUrlLauncher>());
30-
});
31-
3253
test('canLaunch', () async {
3354
await launcher.canLaunch('http://example.com/');
3455
expect(
@@ -258,3 +279,8 @@ void main() {
258279
});
259280
});
260281
}
282+
283+
class ImplementsUrlLauncherPlatform extends Mock
284+
implements UrlLauncherPlatform {}
285+
286+
class ExtendsUrlLauncherPlatform extends UrlLauncherPlatform {}

0 commit comments

Comments
 (0)