PlatformInterface._instanceToken needs reworking due to an upcoming dart language change

[stereotype441]

The dart language team will soon be changing the behavior of the language so that an invocation of a private class members won't get dispatched to a noSuchMethod method in another library; instead an exception will be thrown. This is a necessary prerequisite to allowing promotion of private fields (dart-lang/language#2020). It also closes an important privacy loophole in the language, by ensuring that the user can see all the possible behaviors of a private member invocation without having to look outside of the library.

This has an important consequence for PlatformInterface (from the plugin_platform_interface package), which insists that it be subclassed by clients via extends rather than implements, and detects the difference by reading from the private field _instanceToken and checking whether the expected value was returned. Today, that technique works some of the time (it relies on the implementing class having an implementation of noSuchMethod that completes without throwing an exception), but after the upcoming language change it will always throw a yet-to-be-defined exception.

I believe we should change PlatformInterface._instanceToken to use an expando (similar to what was done in #109238), so that PlatformInterface will be able to reliably detect whether it is subclassed by extends or implements, both before and after the upcoming language change.

In the process we'll also need to update several brittle tests that rely on the invocation of _instanceToken being dispatched to the standard Object.noSuchMethod method, and thus expect a NoSuchMethodError to be thrown. Changing PlatformInterface._instanceToken to use an expando will prevent the NoSuchMethodError from occurring, and instead PlatformInterface._verify will throw an AssertionError. These tests need to be updated so that they don't care what particular kind of exception is thrown. The tests I've found that require fixing are in the following repos:

• https://github.com/firebase/flutterfire
• https://github.com/Baseflow/flutter-geocoding
• https://github.com/Baseflow/flutter-geolocator
• https://github.com/Lyokone/flutterlocation
• https://github.com/flutter/plugins

To avoid breakages, I'll submit PRs to these repos before landing the change to PlatformInterface.

[goderbauer]

I believe you're talking about this class here, which is from the plugin_platform_interface package and not part of the Flutter framework itself:

https://github.com/flutter/plugins/blob/395dc14b27e0ba1bb02a65168b510e55fa7fc09d/packages/plugin_platform_interface/lib/plugin_platform_interface.dart#L42

I don't actually have too much context about how this class is used. Maybe @stuartmorgan or someone from his team can help out?

[stereotype441]

I believe you're talking about this class here, which is from the plugin_platform_interface package and not part of the Flutter framework itself:

https://github.com/flutter/plugins/blob/395dc14b27e0ba1bb02a65168b510e55fa7fc09d/packages/plugin_platform_interface/lib/plugin_platform_interface.dart#L42

You're right, thanks. Edited the issue to clarify.

[stuartmorgan]

which insists that it be subclassed by clients via extends rather than implements, and detects the difference by reading from the private field _instanceToken and checking whether the expected value was returned. Today, that technique works some of the time (it relies on the implementing class having an implementation of noSuchMethod that completes without throwing an exception), but after the upcoming language change it will always throw a yet-to-be-defined exception.

Summarizing from discussion elsewhere since I was confused about this: the key functionality of verify*, which is completing without an exception only when extends is used, works all of the time, and will continue to work even after the change. The part that only sometimes works and will never work in the future is actually throwing an AssertionError rather than some other error (like NoSuchMethodError as explained above).

So it's already the case that the behavior doesn't match the documentation, and the difference will only ever affect tests since the whole point here is that this is to prevent people from shipping code that uses implements (which it will accomplish regardless of the specific Error type).

But we should match the documented behavior (and provide the better error message that our own assertion error contains), so changing to an Expando seems reasonable to me.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.