From d6bf1447f4942f2a62e3021202d77f1c2c588eee Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Thu, 24 Aug 2023 14:54:59 -0700 Subject: [PATCH] Update the tool to know about all our new platforms (#132423) ...and add a test so we remember to keep it in sync. --- dev/bots/analyze.dart | 81 +++++++++++++++++++ .../flutter/lib/src/foundation/binding.dart | 29 +++---- .../flutter/lib/src/foundation/platform.dart | 11 ++- .../lib/src/resident_runner.dart | 24 +++--- .../general.shard/resident_runner_test.dart | 8 +- .../general.shard/terminal_handler_test.dart | 12 +-- 6 files changed, 122 insertions(+), 43 deletions(-) diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index 9e87d94f0e..39f739277f 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart @@ -87,6 +87,9 @@ Future run(List arguments) async { foundError(['The analyze.dart script must be run with --enable-asserts.']); } + printProgress('TargetPlatform tool/framework consistency'); + await verifyTargetPlatform(flutterRoot); + printProgress('No Double.clamp'); await verifyNoDoubleClamp(flutterRoot); @@ -266,6 +269,84 @@ class _DoubleClampVisitor extends RecursiveAstVisitor { } } +Future verifyTargetPlatform(String workingDirectory) async { + final File framework = File('$workingDirectory/packages/flutter/lib/src/foundation/platform.dart'); + final Set frameworkPlatforms = {}; + List lines = framework.readAsLinesSync(); + int index = 0; + while (true) { + if (index >= lines.length) { + foundError(['${framework.path}: Can no longer find TargetPlatform enum.']); + return; + } + if (lines[index].startsWith('enum TargetPlatform {')) { + index += 1; + break; + } + index += 1; + } + while (true) { + if (index >= lines.length) { + foundError(['${framework.path}: Could not find end of TargetPlatform enum.']); + return; + } + String line = lines[index].trim(); + final int comment = line.indexOf('//'); + if (comment >= 0) { + line = line.substring(0, comment); + } + if (line == '}') { + break; + } + if (line.isNotEmpty) { + if (line.endsWith(',')) { + frameworkPlatforms.add(line.substring(0, line.length - 1)); + } else { + foundError(['${framework.path}:$index: unparseable line when looking for TargetPlatform values']); + } + } + index += 1; + } + final File tool = File('$workingDirectory/packages/flutter_tools/lib/src/resident_runner.dart'); + final Set toolPlatforms = {}; + lines = tool.readAsLinesSync(); + index = 0; + while (true) { + if (index >= lines.length) { + foundError(['${tool.path}: Can no longer find nextPlatform logic.']); + return; + } + if (lines[index].trim().startsWith('const List platforms = [')) { + index += 1; + break; + } + index += 1; + } + while (true) { + if (index >= lines.length) { + foundError(['${tool.path}: Could not find end of nextPlatform logic.']); + return; + } + final String line = lines[index].trim(); + if (line.startsWith("'") && line.endsWith("',")) { + toolPlatforms.add(line.substring(1, line.length - 2)); + } else if (line == '];') { + break; + } else { + foundError(['${tool.path}:$index: unparseable line when looking for nextPlatform values']); + } + index += 1; + } + final Set frameworkExtra = frameworkPlatforms.difference(toolPlatforms); + if (frameworkExtra.isNotEmpty) { + foundError(['TargetPlatform has some extra values not found in the tool: ${frameworkExtra.join(", ")}']); + } + final Set toolExtra = toolPlatforms.difference(frameworkPlatforms); + if (toolExtra.isNotEmpty) { + foundError(['The nextPlatform logic in the tool has some extra values not found in TargetPlatform: ${toolExtra.join(", ")}']); + } +} + /// Verify that we use clampDouble instead of Double.clamp for performance reasons. /// /// We currently can't distinguish valid uses of clamp from problematic ones so diff --git a/packages/flutter/lib/src/foundation/binding.dart b/packages/flutter/lib/src/foundation/binding.dart index e898b808e5..a663cf9716 100644 --- a/packages/flutter/lib/src/foundation/binding.dart +++ b/packages/flutter/lib/src/foundation/binding.dart @@ -561,33 +561,22 @@ abstract class BindingBase { name: FoundationServiceExtensions.platformOverride.name, callback: (Map parameters) async { if (parameters.containsKey('value')) { - switch (parameters['value']) { - case 'android': - debugDefaultTargetPlatformOverride = TargetPlatform.android; - case 'fuchsia': - debugDefaultTargetPlatformOverride = TargetPlatform.fuchsia; - case 'iOS': - debugDefaultTargetPlatformOverride = TargetPlatform.iOS; - case 'linux': - debugDefaultTargetPlatformOverride = TargetPlatform.linux; - case 'macOS': - debugDefaultTargetPlatformOverride = TargetPlatform.macOS; - case 'windows': - debugDefaultTargetPlatformOverride = TargetPlatform.windows; - case 'default': - default: - debugDefaultTargetPlatformOverride = null; + final String value = parameters['value']!; + debugDefaultTargetPlatformOverride = null; + for (final TargetPlatform candidate in TargetPlatform.values) { + if (candidate.name == value) { + debugDefaultTargetPlatformOverride = candidate; + break; + } } _postExtensionStateChangedEvent( FoundationServiceExtensions.platformOverride.name, - defaultTargetPlatform.toString().substring('$TargetPlatform.'.length), + defaultTargetPlatform.name, ); await reassembleApplication(); } return { - 'value': defaultTargetPlatform - .toString() - .substring('$TargetPlatform.'.length), + 'value': defaultTargetPlatform.name, }; }, ); diff --git a/packages/flutter/lib/src/foundation/platform.dart b/packages/flutter/lib/src/foundation/platform.dart index d01d0305c9..60d90b22d7 100644 --- a/packages/flutter/lib/src/foundation/platform.dart +++ b/packages/flutter/lib/src/foundation/platform.dart @@ -35,7 +35,7 @@ import '_platform_io.dart' // // When adding support for a new platform (e.g. Windows Phone, Raspberry Pi), // first create a new value on the [TargetPlatform] enum, then add a rule for -// selecting that platform here. +// selecting that platform in `_platform_io.dart` and `_platform_web.dart`. // // It would be incorrect to make a platform that isn't supported by // [TargetPlatform] default to the behavior of another platform, because doing @@ -47,6 +47,15 @@ TargetPlatform get defaultTargetPlatform => platform.defaultTargetPlatform; /// The platform that user interaction should adapt to target. /// /// The [defaultTargetPlatform] getter returns the current platform. +/// +/// When using the "flutter run" command, the "o" key will toggle between +/// values of this enum when updating [debugDefaultTargetPlatformOverride]. +/// This lets one test how the application will work on various platforms +/// without having to switch emulators or physical devices. +// +// When you add values here, make sure to also add them to +// nextPlatform() in flutter_tools/lib/src/resident_runner.dart so that +// the tool can support the new platform for its "o" option. enum TargetPlatform { /// Android: android, diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart index a3de310f5a..ec1d2f4e60 100644 --- a/packages/flutter_tools/lib/src/resident_runner.dart +++ b/packages/flutter_tools/lib/src/resident_runner.dart @@ -1877,19 +1877,17 @@ class DebugConnectionInfo { /// These values must match what is available in /// `packages/flutter/lib/src/foundation/binding.dart`. String nextPlatform(String currentPlatform) { - switch (currentPlatform) { - case 'android': - return 'iOS'; - case 'iOS': - return 'fuchsia'; - case 'fuchsia': - return 'macOS'; - case 'macOS': - return 'android'; - default: - assert(false); // Invalid current platform. - return 'android'; - } + const List platforms = [ + 'android', + 'iOS', + 'windows', + 'macOS', + 'linux', + 'fuchsia', + ]; + final int index = platforms.indexOf(currentPlatform); + assert(index >= 0, 'unknown platform "$currentPlatform"'); + return platforms[(index + 1) % platforms.length]; } /// A launcher for the devtools debugger and analysis tool. diff --git a/packages/flutter_tools/test/general.shard/resident_runner_test.dart b/packages/flutter_tools/test/general.shard/resident_runner_test.dart index 6723487850..bd93c7c85f 100644 --- a/packages/flutter_tools/test/general.shard/resident_runner_test.dart +++ b/packages/flutter_tools/test/general.shard/resident_runner_test.dart @@ -2210,9 +2210,11 @@ flutter: testUsingContext('nextPlatform moves through expected platforms', () { expect(nextPlatform('android'), 'iOS'); - expect(nextPlatform('iOS'), 'fuchsia'); - expect(nextPlatform('fuchsia'), 'macOS'); - expect(nextPlatform('macOS'), 'android'); + expect(nextPlatform('iOS'), 'windows'); + expect(nextPlatform('windows'), 'macOS'); + expect(nextPlatform('macOS'), 'linux'); + expect(nextPlatform('linux'), 'fuchsia'); + expect(nextPlatform('fuchsia'), 'android'); expect(() => nextPlatform('unknown'), throwsAssertionError); }); diff --git a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart b/packages/flutter_tools/test/general.shard/terminal_handler_test.dart index e234825ede..e6a38e5d39 100644 --- a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart +++ b/packages/flutter_tools/test/general.shard/terminal_handler_test.dart @@ -464,10 +464,10 @@ void main() { method: 'ext.flutter.platformOverride', args: { 'isolateId': '1', - 'value': 'fuchsia', + 'value': 'windows', }, jsonResponse: { - 'value': 'fuchsia', + 'value': 'windows', }, ), // Request 2. @@ -496,7 +496,7 @@ void main() { await terminalHandler.processTerminalInput('o'); await terminalHandler.processTerminalInput('O'); - expect(terminalHandler.logger.statusText, contains('Switched operating system to fuchsia')); + expect(terminalHandler.logger.statusText, contains('Switched operating system to windows')); expect(terminalHandler.logger.statusText, contains('Switched operating system to iOS')); }); @@ -518,10 +518,10 @@ void main() { method: 'ext.flutter.platformOverride', args: { 'isolateId': '1', - 'value': 'fuchsia', + 'value': 'windows', }, jsonResponse: { - 'value': 'fuchsia', + 'value': 'windows', }, ), // Request 2. @@ -550,7 +550,7 @@ void main() { await terminalHandler.processTerminalInput('o'); await terminalHandler.processTerminalInput('O'); - expect(terminalHandler.logger.statusText, contains('Switched operating system to fuchsia')); + expect(terminalHandler.logger.statusText, contains('Switched operating system to windows')); expect(terminalHandler.logger.statusText, contains('Switched operating system to iOS')); });