-
Notifications
You must be signed in to change notification settings - Fork 29.1k
Description
We need to migrate the flutter
tool's codebase to null-safe Dart.
This will be a long-term effort and is blocked on issues around dependencies that haven't migrated and figuring out a solution to our reliance on mockito.
Migrating the Flutter Tool to null safety
jonahwilliams@
Background
The flutter command line tool is an approximately 125,000 line Dart application (with an additional 90,000 lines of test code) that powers the Flutter development/build cycle, as well as the underlying support used by IDE plugins and google3. While migration to null safety of this tool does not block user applications from migrating to null safety, in the interest of moving the ecosystem forward, making it easier to contribute to the tool, and reducing common errors caused by NPEs, the tool must eventually migrate to null safe Dart.
Overview
Migration Process
Prerequisites
- All dependencies are migrated to null safety (In Progress). See Appendix for a list of all unmigrated deps.
- Plan for migration to null safety is drafted (In Progress)
Process
- Flutter tool SDK constraint is set to 2.12 and all files are opted into 2.8 ([flutter_tools] opt all flutter tool libraries and tests out of null safety. #74832)
- Begin removing mockito usage and enforce ban on additional mockito tests
- Begin migrating individual libraries with no dependencies on unmigrated code (?)
- Application entrypoint is migrated
- Blocked on migration of all dependencies
- Test code without mockito dependencies is migrated
- Test code with mockito dependencies is migrated.
Challenges
The Flutter tool code base is large enough that a one-shot dart migrate PR will be essentially un-reviewable. As of the writing of this document, dart migrate does support partial migrations, which would allow the tool to more gradually opt in. While the Flutter tool is layered, it is not as structured as the framework - the majority of the libraries are in lib/src and there are significant circular imports between these files.
The Flutter tool unit tests make extensive use of mockito 4.X, which has required breaking changes in 5.X (that introduce codegen) for null safe Dart. These tests would have to be substantially rewritten, or additional dependencies introduced to support mockito 5.X
Work Estimates
Note: this excludes the cost/time for migration of external dependencies
The Flutter framework code is as of writing 366,880 lines of Dart. The first PR to migrate foundation to null safety (#61188 ) was opened on Jul 15, 2020. The last PR to migrate material (#66633) landed on Sept 28, 2020. This was approximately 73 working days, with the work primarily performed by the contributor a14n and reviewed by multiple members of the Flutter team. With a bit of napkin math, assuming a similar pace of work at ~5,000 lines a day on average, that would take the tool 25 working days to complete. This does not account for significant pre-work that may have been done by a14n that I have not counted, nor can it account for substantial differences in the type of codebase between the Flutter framework and CLI tool (namely the lack of layering in the latter).
The difficulty of the test migrating is harder to estimate. As of writing there are 365 instances of "extends Mock" in the Flutter tools test shard. In order to migrate the Flutter framework tests to null safety, a handful of tests that made limited use of mockito were re-written. Based on my experience there, I would estimate that it could take a few hours for trivial usage, and multiple days for substantial rewrite. With some more napkin math, and based on 130 tests that use mockito, that could take between 30 - 130 working days to fully rewrite.
Unless more individuals contribute or more time is dedicated to the migration, it will take an extremely long time. For example, If I were to devote my ~8 hours a week of heads down time alone to migration, this could take until August, 2023!
Priorities and Planning
The Flutter tools team does not currently have many spare cycles to dedicate to migration (See estimate above). At best, we could devote a % of "code health" time to it, and keep track of overall progress to estimate a final completion date. Based on my experience trying to remove most Zone injected dynamic values (similar widespread cleanup effort, less analysis assistance), this is likely to take an indeterminate amount of time - as other high priority issues will continually interrupt progress. This makes approaches such as a manual migration or migration off of mockito tests infeasible in the short term (2021).
In order to make the process easier, the tools team will begin enforcing a total ban on additional mockito usage (i.e. new imports, mocks, extends) on a given date.
Migration Date Start
The Flutter tool could begin either partially migrating to null safety and/or removing mockito dependencies immediately. Many of the analyzer dependencies, for example, are transitive through the test_core dependency which is only exposed through the flutter_test command. This may allow libraries in base/
or /android
(for example) to migrate ahead of that.
Similarly, removal of mockito usage can begin immediately - but unless we can proactively ban further usage of it in tests, we risk backsliding significantly.
The date where all packages are migrated to null-safety isn't yet clear. The largest individual dependencies are analyzer (transitive through test), dwds, and devtools (transitive through dwds). At the time of writing, none of these packages have been migrated yet.
Appendix
For more information on the breaking changes in mockito, see https://github.com/dart-lang/mockito/blob/master/NULL_SAFETY_README.md
For an up to date listing of the currently migrated tool dependencies, the best source is the pubspec.yaml itself. (There are too many to otherwise count).
Outstanding non-null-safe Tool Files
NOTE: re-generate this list with grep --recursive --files-with-matches 'dart = 2\.8' flutter_tools | grep -v '\.dart_tool' | sort | awk '{print " - "$1}' | pbcopy
:
- flutter_tools/bin/flutter_tools.dart
- flutter_tools/bin/fuchsia_asset_builder.dart
- flutter_tools/bin/fuchsia_tester.dart
- flutter_tools/test/commands.shard/hermetic/analyze_continuously_test.dart
- flutter_tools/test/commands.shard/hermetic/analyze_suggestion_test.dart
- flutter_tools/test/commands.shard/hermetic/analyze_test.dart
- flutter_tools/test/commands.shard/hermetic/assemble_test.dart
- flutter_tools/test/commands.shard/hermetic/attach_test.dart
- flutter_tools/test/commands.shard/hermetic/build_darwin_framework_test.dart
- flutter_tools/test/commands.shard/hermetic/build_fuchsia_test.dart
- flutter_tools/test/commands.shard/hermetic/build_ios_test.dart
- flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart
- flutter_tools/test/commands.shard/hermetic/build_linux_test.dart
- flutter_tools/test/commands.shard/hermetic/build_macos_test.dart
- flutter_tools/test/commands.shard/hermetic/build_test.dart
- flutter_tools/test/commands.shard/hermetic/build_web_test.dart
- flutter_tools/test/commands.shard/hermetic/build_windows_test.dart
- flutter_tools/test/commands.shard/hermetic/clean_test.dart
- flutter_tools/test/commands.shard/hermetic/config_test.dart
- flutter_tools/test/commands.shard/hermetic/create_usage_test.dart
- flutter_tools/test/commands.shard/hermetic/custom_devices_test.dart
- flutter_tools/test/commands.shard/hermetic/daemon_test.dart
- flutter_tools/test/commands.shard/hermetic/devices_test.dart
- flutter_tools/test/commands.shard/hermetic/doctor_test.dart
- flutter_tools/test/commands.shard/hermetic/downgrade_test.dart
- flutter_tools/test/commands.shard/hermetic/drive_test.dart
- flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart
- flutter_tools/test/commands.shard/hermetic/ide_config_test.dart
- flutter_tools/test/commands.shard/hermetic/install_test.dart
- flutter_tools/test/commands.shard/hermetic/logs_test.dart
- flutter_tools/test/commands.shard/hermetic/precache_test.dart
- flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart
- flutter_tools/test/commands.shard/hermetic/pub_get_test.dart
- flutter_tools/test/commands.shard/hermetic/run_test.dart
- flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart
- flutter_tools/test/commands.shard/hermetic/shell_completion_test.dart
- flutter_tools/test/commands.shard/hermetic/symbolize_test.dart
- flutter_tools/test/commands.shard/hermetic/test_test.dart
- flutter_tools/test/commands.shard/hermetic/update_packages_test.dart
- flutter_tools/test/commands.shard/hermetic/upgrade_test.dart
- flutter_tools/test/commands.shard/permeable/build_aar_test.dart
- flutter_tools/test/commands.shard/permeable/build_apk_test.dart
- flutter_tools/test/commands.shard/permeable/build_appbundle_test.dart
- flutter_tools/test/commands.shard/permeable/build_bundle_test.dart
- flutter_tools/test/commands.shard/permeable/create_test.dart
- flutter_tools/test/commands.shard/permeable/devices_test.dart
- flutter_tools/test/commands.shard/permeable/format_test.dart
- flutter_tools/test/commands.shard/permeable/migrate_abandon_test.dart
- flutter_tools/test/commands.shard/permeable/migrate_apply_test.dart
- flutter_tools/test/commands.shard/permeable/migrate_status_test.dart
- flutter_tools/test/commands.shard/permeable/packages_test.dart
- flutter_tools/test/commands.shard/permeable/upgrade_test.dart
- flutter_tools/test/general.shard/android/gradle_errors_test.dart
- flutter_tools/test/general.shard/application_package_test.dart
- flutter_tools/test/general.shard/args_test.dart
- flutter_tools/test/general.shard/base/logger_test.dart
- flutter_tools/test/general.shard/commands/build_test.dart
- flutter_tools/test/general.shard/dart_plugin_test.dart
- flutter_tools/test/general.shard/drive/drive_service_test.dart
- flutter_tools/test/general.shard/drive/web_driver_service_test.dart
- flutter_tools/test/general.shard/emulator_test.dart
- flutter_tools/test/general.shard/fuchsia/fuchsia_device_start_test.dart
- flutter_tools/test/general.shard/fuchsia/fuchsia_device_test.dart
- flutter_tools/test/general.shard/hot_test.dart
- flutter_tools/test/general.shard/ios/ios_device_start_nonprebuilt_test.dart
- flutter_tools/test/general.shard/project_test.dart
- flutter_tools/test/general.shard/resident_runner_test.dart
- flutter_tools/test/general.shard/resident_web_runner_cold_test.dart
- flutter_tools/test/general.shard/resident_web_runner_test.dart
- flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart
- flutter_tools/test/general.shard/runner/flutter_command_test.dart
- flutter_tools/test/general.shard/runner/runner_test.dart
- flutter_tools/test/general.shard/tester/flutter_tester_test.dart
- flutter_tools/test/general.shard/vmservice_test.dart
- flutter_tools/test/general.shard/web/devfs_web_test.dart
- flutter_tools/test/general.shard/web/golden_comparator_process_test.dart
- flutter_tools/test/general.shard/web/golden_comparator_test.dart
- flutter_tools/test/general.shard/web/migrations/scrub_generated_plugin_registrant_test.dart
- flutter_tools/test/general.shard/web/web_asset_server_test.dart
- flutter_tools/test/general.shard/web/web_expression_compiler_test.dart
- flutter_tools/test/integration.shard/test_data/basic_project.dart
- flutter_tools/test/web.shard/sdk_web_configuration_test.dart
(last updated July 11, 2022 82 outstanding unmigrated libraries)
Metadata
Metadata
Assignees
Labels
Type
Projects
Status