Repository navigation
[flutter_tools] Implement Configuration extension slice and flutter config integration - #191445
Conversation
…gration Implements core diagnostic models (ValidationResult, ValidationMessage) in package:flutter_tools_core, DiagnosticsExtension interface in package:flutter_tools_extension, LinuxExtensionDiagnostics in package:flutter_tools_extension_linux_prototype, ExtensionDoctorValidator, RpcDiagnosticsExtension, and IsolateDiagnosticsExtension host adapter integration in flutter_tools.
- Use firstWhere with fallback when deserializing ValidationType and ValidationMessageType from JSON in package:flutter_tools_core to handle unrecognized types gracefully. - Use pattern matching for extensionManager in DoctorValidatorsProvider instead of bang operator. - Add tests for unknown diagnostic enum types.
- Cache DiagnosticsExtensionClient per ExtensionConnection in ExtensionManager. - Safely handle title and diagnostics RPC responses with try/catch and type validation. - Pre-fetch diagnostics extension titles during Doctor.startValidatorTasks.
…ent and validation aggregation - Initialize diagnostics extension clients and fetch titles during ExtensionManager initialization. - Replace dynamic client map in ExtensionManager with an immutable list and assert on initialization. - Use exhaustive switch statement for ValidationType aggregation in ExtensionDoctorValidator. - Remove manual prefetch loop from Doctor.startValidatorTasks.
…ostics-slice # Conflicts: # packages/flutter_tools/lib/executable.dart
…ostics-slice # Conflicts: # packages/flutter_tools/lib/executable.dart
…onfig integration
…guration-slice # Conflicts: # packages/flutter_tools/lib/src/experimental/extension_manager.dart # packages/flutter_tools/packages/flutter_tools_core/lib/flutter_tools_core.dart # packages/flutter_tools/packages/flutter_tools_extension/lib/flutter_tools_extension.dart # packages/flutter_tools/packages/flutter_tools_extension_linux_prototype/lib/flutter_tools_extension_linux_prototype.dart # packages/flutter_tools/test/commands.shard/hermetic/tool_extensions_integration_test.dart # packages/flutter_tools/test/integration.shard/tool_extensions_test.dart
There was a problem hiding this comment.
Code Review
This pull request introduces support for tool extensions to dynamically register custom configuration options and feature flags in the flutter config command. It adds core models, RPC client adapters, and a dynamic argument parser mixin to rebuild the command's parser at runtime. Feedback focuses on improving robustness and preventing runtime crashes. Key recommendations include wrapping asynchronous RPC calls in try-catch blocks to handle remote isolate failures gracefully, caching fetched settings to avoid duplicate RPC requests, performing defensive checks against duplicate option and subcommand names during parser reconstruction, and safely handling potentially null RPC responses instead of using the null-assertion operator.
…n slice * Check for option and subcommand name collisions before dynamic parser registration. * Cache `_extensionSettingsGroups` in `ConfigCommand` to eliminate duplicate RPC queries during `settingsText`. * Wrap extension queries in `ExtensionConfiguration` with try-catch blocks to prevent individual extension errors from aborting all configuration resolution. * Handle null and malformed RPC responses safely in `ConfigurationExtensionClient`. * Add regression tests for conflicting options, failing extensions, and invalid RPC payloads.
…on slice - Initialize dynamic options prior to argument parsing in `FlutterCommandRunner` via robust command target resolution. - Extract `cloneParser` helper on `ExtensionArgParserMixin` for cleaner argument parser cloning. - Enhance RPC error handling and collection parsing with pattern matching in `ConfigurationExtensionClient`. - Simplify deserialization using Dart 3 switch expressions in `FeatureFlag` and `ConfigOption`. - Cache configuration extensions and enforce initialization lifecycle asserts in `ExtensionManager`.
Adopt modern Dart 3+ language features across the configuration extension slice: - Use map pattern matching and destructuring in `FeatureFlag.fromJson` and `ConfigOption.fromJson`. - Implement safe, generic `_fetchList<T>` RPC helper in `ConfigurationExtensionClient`. - Use exhaustive `switch (opt.type)` pattern matching in `cloneParser`. - Use `MapEntry(:key, :value)` destructuring for subcommand registration. - Replace verbose loops and conversions with collection comprehensions and spread operators. - Extract named constants for prototype config keys.
…estructuring Consolidate extension settings state to _extensionSettingsGroups in `ConfigCommand`: - Remove redundant `_extensionFeatureFlags` and `_extensionConfigOptions` fields. - Apply object destructuring patterns across all loops iterating over `ExtensionSettingsGroup`, `FeatureFlag`, and `ConfigOption` in `extensionArgParserCacheKey`, `buildDynamicArgParser`, `runCommand`, and `settingsText`. - Simplify dynamic parser cache key computation.
|
cc @chingjun. It looks like the Google testing is failing to pick up the G3Fix CL. Would you happen to know why that's happening? |
This is caused by a recent migration. Just sent a CL to fix. Will probably land next week |
| @override | ||
| String get title => 'Linux Custom Extension Prototype'; |
There was a problem hiding this comment.
I wonder if we should add a title argument to ToolExtensionEntryPoint.run? Currently the extension provides the title multiple times (ConfigurationExtension.title and DiagnosticsExtension.title).
There was a problem hiding this comment.
Good point! Centralizing the extension title at registration / capabilities level rather than on each service interface makes a lot of sense. Since ToolExtensionEntryPoint and capabilities were landed in Step 02, I'll follow up with a refactoring across the slices to consolidate extension metadata into the top-level registration.
…er option tests - Unify base parser creation into createBaseArgParser in ExtensionArgParserMixin and ConfigCommand. - Pre-clone baseArgParser inside ExtensionArgParserMixin.argParser before passing to buildDynamicArgParser. - Add re-entrancy assertion in argParser getter and assign _baseArgParser only after construction completes. - Document option skipping and help target resolution in FlutterCommandRunner. - Add unit tests in flutter_command_runner_test.dart for dynamic option runner initialization.
…eys with rebuildDynamicArgParser - Remove extensionArgParserCacheKey and cache invalidation logic from ExtensionArgParserMixin. - Add rebuildDynamicArgParser() to ExtensionArgParserMixin and call it in ConfigCommand.initializeDynamicOptions(). - Update runner unit tests in flutter_command_runner_test.dart.
…support subcommand hierarchies - Traverse parent command hierarchy in _initializeDynamicOptions to initialize all ExtensionArgParserMixin commands. - Add unit tests in flutter_command_runner_test.dart for subcommands, parent flags, inline option values, empty inline values, unknown flags, negated flags, and help subcommands.
Description
This PR implements Step 4 of Flutter Tools Extensibility: the Configuration extension slice and
flutter configintegration.Architectural Overview
package:flutter_tools_core:FeatureFlagandConfigOption.package:flutter_tools_extension:ConfigurationExtensioninterface and service contract (config.getTitle,config.getFeatureFlags,config.getConfigurations).package:flutter_tools_extension_linux_prototype:LinuxConfigurationExtensionin the Linux prototype extension.packages/flutter_tools:ConfigurationExtensionClientto query isolate extensions over RPC.ExtensionArgParserMixinto dynamically contribute CLI options toConfigCommand.ExtensionManager.configurationExtensionsand integrates active extension configuration settings and feature flags dynamically intoflutter config(settingsTextand arg parsing).Related Issues
Part of #190692
Tests
packages/flutter_tools/packages/flutter_tools_core/test/config_test.dartpackages/flutter_tools/packages/flutter_tools_extension_linux_prototype/test/linux_config_test.dartpackages/flutter_tools/test/general.shard/extension_protocol/config_service_test.dartpackages/flutter_tools/test/commands.shard/hermetic/config_test.dartpackages/flutter_tools/test/commands.shard/hermetic/tool_extensions_integration_test.dartpackages/flutter_tools/test/integration.shard/tool_extensions_test.dart