Skip to content

Commit 95cb7ca

Browse files
matanlureymatanlurey
authored
Finish removing --explicit-package-dependencies. (flutter#169950)
Closes flutter#48918. Closes flutter#102983.
1 parent d408b2a commit 95cb7ca

File tree

12 files changed

+141
-156
lines changed

12 files changed

+141
-156
lines changed

‎dev/benchmarks/test_apps/stocks/l10n.yaml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ output-localization-file: stock_strings.dart
1414
## `template-arb-file` describes the template arb file that the tool
1515
## will use to check and validate the remaining arb files when
1616
## generating Flutter's localization files.
17-
synthetic-package: false
1817
template-arb-file: stocks_en.arb
1918
## setting `nullable-getter` to false generates a non-nullable
2019
## StockStrings getter. This removes the need for adding null checks

‎packages/flutter_tools/lib/src/commands/generate_localizations.dart

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import '../artifacts.dart';
88
import '../base/common.dart';
99
import '../base/file_system.dart';
1010
import '../base/logger.dart';
11-
import '../features.dart';
1211
import '../localizations/gen_l10n.dart';
1312
import '../localizations/localizations_utils.dart';
1413
import '../runner/flutter_command.dart';
@@ -36,16 +35,16 @@ class GenerateLocalizationsCommand extends FlutterCommand {
3635
argParser.addOption(
3736
'output-dir',
3837
help:
39-
'The directory where the generated localization classes will be written '
40-
'if the synthetic-package flag is set to false.\n'
41-
'\n'
42-
'If output-dir is specified and the synthetic-package flag is enabled, '
43-
'this option will be ignored by the tool.\n'
38+
'The directory where the generated localization classes will be written.'
4439
'\n'
4540
'The app must import the file specified in the "--output-localization-file" '
4641
'option from this directory. If unspecified, this defaults to the same '
4742
'directory as the input directory specified in "--arb-dir".',
4843
);
44+
argParser.addFlag(
45+
'synthetic-package',
46+
help: 'DEPRECATED. This flag cannot be enabled and should be removed.',
47+
);
4948
argParser.addOption(
5049
'template-arb-file',
5150
help:
@@ -149,19 +148,6 @@ class GenerateLocalizationsCommand extends FlutterCommand {
149148
'\n'
150149
'When null, the JSON file will not be generated.',
151150
);
152-
argParser.addFlag(
153-
'synthetic-package',
154-
defaultsTo: !featureFlags.isExplicitPackageDependenciesEnabled,
155-
help:
156-
'Determines whether or not the generated output files will be '
157-
'generated as a synthetic package or at a specified directory in '
158-
'the Flutter project.\n'
159-
'\n'
160-
'DEPRECATED: https://flutter.dev/to/flutter-gen-deprecation.\n'
161-
'\n'
162-
'When synthetic-package is set to false, it will generate the '
163-
'localizations files in the directory specified by arb-dir by default.\n',
164-
);
165151
argParser.addOption(
166152
'project-dir',
167153
valueHelp: 'absolute/path/to/flutter/project',

‎packages/flutter_tools/lib/src/features.dart

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ abstract class FeatureFlags {
5353
/// Whether Swift Package Manager dependency management is enabled.
5454
bool get isSwiftPackageManagerEnabled => false;
5555

56-
/// Whether explicit package dependency management is enabled.
57-
bool get isExplicitPackageDependenciesEnabled => false;
58-
5956
/// Whether a particular feature is enabled for the current channel.
6057
///
6158
/// Prefer using one of the specific getters above instead of this API.

‎packages/flutter_tools/lib/src/flutter_features.dart

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,6 @@ mixin FlutterFeatureFlagsIsEnabled implements FeatureFlags {
5151

5252
@override
5353
bool get isSwiftPackageManagerEnabled => isEnabled(swiftPackageManager);
54-
55-
// TODO(matanlurey): Remove this when the flag is no longer in use.
56-
//
57-
// Setting this always to true prevents `false` from ever being returned in
58-
// production code, but still allows tests to override it so that they can be
59-
// migrated (or deleted) one at a time instead of a single large PR.
60-
@override
61-
bool get isExplicitPackageDependenciesEnabled => true;
6254
}
6355

6456
interface class FlutterFeatureFlags extends FeatureFlags with FlutterFeatureFlagsIsEnabled {

‎packages/flutter_tools/lib/src/flutter_plugins.dart

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,7 @@ Future<List<Plugin>> findPlugins(FlutterProject project, {bool throwOnError = tr
130130
packageName,
131131
dependency.rootUri,
132132
project.manifest.dependencies,
133-
isDevDependency:
134-
featureFlags.isExplicitPackageDependenciesEnabled && dependency.isExclusiveDevDependency,
133+
isDevDependency: dependency.isExclusiveDevDependency,
135134
fileSystem: fs,
136135
);
137136
if (plugin != null) {
@@ -1160,8 +1159,7 @@ void _createPlatformPluginSymlinks(
11601159
///
11611160
/// Assumes `pub get` has been executed since last change to `pubspec.yaml`.
11621161
///
1163-
/// If [FeatureFlags.isExplicitPackageDependenciesEnabled] is `true`,
1164-
/// plugins that are development-only dependencies might be labeled or,
1162+
/// Plugins that are development-only dependencies might be labeled or,
11651163
/// depending on the platform, omitted from metadata or platform-specific artifacts.
11661164
Future<void> refreshPluginsList(
11671165
FlutterProject project, {

‎packages/flutter_tools/lib/src/localizations/localizations_utils.dart

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:yaml/yaml.dart';
88
import '../base/common.dart';
99
import '../base/file_system.dart';
1010
import '../base/logger.dart';
11+
import '../globals.dart' as globals;
1112
import '../runner/flutter_command.dart';
1213
import 'gen_l10n_types.dart';
1314
import 'language_subtag_registry.dart';
@@ -487,6 +488,22 @@ LocalizationOptions parseLocalizationsOptionsFromYAML({
487488
logger.printError('Expected ${file.path} to contain a map, instead was $yamlNode');
488489
throw Exception();
489490
}
491+
const String kSyntheticPackage = 'synthetic-package';
492+
const String kFlutterGenNotice = 'http://flutter.dev/to/flutter-gen-deprecation';
493+
final bool? syntheticPackage = _tryReadBool(yamlNode, kSyntheticPackage, logger);
494+
if (syntheticPackage != null) {
495+
if (syntheticPackage) {
496+
throwToolExit(
497+
'${file.path}: Cannot enable "$kSyntheticPackage", this feature has '
498+
'been removed. See $kFlutterGenNotice.',
499+
);
500+
} else {
501+
logger.printWarning(
502+
'${file.path}: The argument "$kSyntheticPackage" no longer has any '
503+
'effect and should be removed. See $kFlutterGenNotice',
504+
);
505+
}
506+
}
490507
return LocalizationOptions(
491508
arbDir: _tryReadFilePath(yamlNode, 'arb-dir', logger, fileSystem) ?? defaultArbDir,
492509
outputDir: _tryReadFilePath(yamlNode, 'output-dir', logger, fileSystem),
@@ -523,6 +540,21 @@ LocalizationOptions parseLocalizationsOptionsFromCommand({
523540
required FlutterCommand command,
524541
required String defaultArbDir,
525542
}) {
543+
const String kSyntheticPackage = 'synthetic-package';
544+
const String kFlutterGenNotice = 'http://flutter.dev/to/flutter-gen-deprecation';
545+
if (command.argResults!.wasParsed(kSyntheticPackage)) {
546+
if (command.boolArg(kSyntheticPackage)) {
547+
throwToolExit(
548+
'Cannot enable "$kSyntheticPackage", this feature has been removed. '
549+
'See $kFlutterGenNotice.',
550+
);
551+
} else {
552+
globals.logger.printWarning(
553+
'The argument "$kSyntheticPackage" no longer has any effect and should '
554+
'be removed. See $kFlutterGenNotice',
555+
);
556+
}
557+
}
526558
return LocalizationOptions(
527559
arbDir: command.stringArg('arb-dir') ?? defaultArbDir,
528560
outputDir: command.stringArg('output-dir'),

‎packages/flutter_tools/lib/src/runner/flutter_command.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1888,9 +1888,7 @@ abstract class FlutterCommand extends Command<void> {
18881888
if (!shouldRunPub) {
18891889
return;
18901890
}
1891-
await project.regeneratePlatformSpecificTooling(
1892-
releaseMode: featureFlags.isExplicitPackageDependenciesEnabled && releaseMode,
1893-
);
1891+
await project.regeneratePlatformSpecificTooling(releaseMode: releaseMode);
18941892
}
18951893

18961894
/// The set of development artifacts required for this command.

‎packages/flutter_tools/test/commands.shard/hermetic/generate_localizations_test.dart

Lines changed: 37 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -69,76 +69,6 @@ void main() {
6969
},
7070
);
7171

72-
testUsingContext(
73-
'not using synthetic packages (explicitly)',
74-
() async {
75-
final Directory l10nDirectory = fileSystem.directory(fileSystem.path.join('lib', 'l10n'));
76-
final File arbFile = l10nDirectory.childFile('app_en.arb')..createSync(recursive: true);
77-
78-
arbFile.writeAsStringSync('''
79-
{
80-
"helloWorld": "Hello, World!",
81-
"@helloWorld": {
82-
"description": "Sample description"
83-
}
84-
}''');
85-
fileSystem.file('pubspec.yaml').writeAsStringSync('''
86-
flutter:
87-
generate: true''');
88-
89-
final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
90-
fileSystem: fileSystem,
91-
logger: logger,
92-
artifacts: artifacts,
93-
processManager: processManager,
94-
);
95-
await createTestCommandRunner(command).run(<String>['gen-l10n', '--no-synthetic-package']);
96-
97-
expect(l10nDirectory.existsSync(), true);
98-
expect(l10nDirectory.childFile('app_localizations_en.dart').existsSync(), true);
99-
expect(l10nDirectory.childFile('app_localizations.dart').existsSync(), true);
100-
},
101-
overrides: <Type, Generator>{
102-
FileSystem: () => fileSystem,
103-
ProcessManager: () => FakeProcessManager.any(),
104-
},
105-
);
106-
107-
testUsingContext(
108-
'not using synthetic packages (due to --explicit-package-dependencies)',
109-
() async {
110-
final Directory l10nDirectory = fileSystem.directory(fileSystem.path.join('lib', 'l10n'));
111-
final File arbFile = l10nDirectory.childFile('app_en.arb')..createSync(recursive: true);
112-
113-
arbFile.writeAsStringSync('''
114-
{
115-
"helloWorld": "Hello, World!",
116-
"@helloWorld": {
117-
"description": "Sample description"
118-
}
119-
}''');
120-
fileSystem.file('pubspec.yaml').writeAsStringSync('''
121-
flutter:
122-
generate: true''');
123-
124-
final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
125-
fileSystem: fileSystem,
126-
logger: logger,
127-
artifacts: artifacts,
128-
processManager: processManager,
129-
);
130-
await createTestCommandRunner(command).run(<String>['gen-l10n']);
131-
132-
expect(l10nDirectory.existsSync(), true);
133-
expect(l10nDirectory.childFile('app_localizations_en.dart').existsSync(), true);
134-
expect(l10nDirectory.childFile('app_localizations.dart').existsSync(), true);
135-
},
136-
overrides: <Type, Generator>{
137-
FileSystem: () => fileSystem,
138-
ProcessManager: () => FakeProcessManager.any(),
139-
},
140-
);
141-
14272
testUsingContext(
14373
'throws error when arguments are invalid',
14474
() async {
@@ -590,7 +520,7 @@ format: false
590520
);
591521

592522
testUsingContext(
593-
'throw when generate: false and uses synthetic package when run with l10n.yaml',
523+
'throw when generate: false when run with l10n.yaml',
594524
() async {
595525
final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
596526
..createSync(recursive: true);
@@ -636,7 +566,7 @@ format: false
636566
);
637567

638568
testUsingContext(
639-
'throw when generate: false and uses synthetic package when run via commandline options',
569+
'throw when generate: false when run via commandline options',
640570
() async {
641571
final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
642572
..createSync(recursive: true);
@@ -667,8 +597,7 @@ format: false
667597
processManager: processManager,
668598
);
669599
expect(
670-
() async =>
671-
createTestCommandRunner(command).run(<String>['gen-l10n', '--synthetic-package']),
600+
() async => createTestCommandRunner(command).run(<String>['gen-l10n']),
672601
throwsToolExit(
673602
message:
674603
'Attempted to generate localizations code without having the flutter: generate flag turned on.',
@@ -689,13 +618,44 @@ format: false
689618
processManager: processManager,
690619
);
691620
expect(
692-
() async => createTestCommandRunner(
693-
command,
694-
).run(<String>['gen-l10n', '--synthetic-package', 'false']),
621+
() async => createTestCommandRunner(command).run(<String>['gen-l10n', 'false']),
695622
throwsToolExit(message: 'Unexpected positional argument "false".'),
696623
);
697624
});
698625

626+
testUsingContext('throws error when synthetic-package is provided', () async {
627+
final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
628+
fileSystem: fileSystem,
629+
logger: logger,
630+
artifacts: artifacts,
631+
processManager: processManager,
632+
);
633+
await expectLater(
634+
() async => createTestCommandRunner(command).run(<String>['gen-l10n', '--synthetic-package']),
635+
throwsToolExit(message: 'synthetic-package'),
636+
);
637+
});
638+
639+
testUsingContext(
640+
'prints warning when --no-synthetic-package is provided',
641+
() async {
642+
final GenerateLocalizationsCommand command = GenerateLocalizationsCommand(
643+
fileSystem: fileSystem,
644+
logger: logger,
645+
artifacts: artifacts,
646+
processManager: processManager,
647+
);
648+
fileSystem
649+
.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))
650+
.createSync(recursive: true);
651+
final File pubspecFile = fileSystem.file('pubspec.yaml')..createSync();
652+
pubspecFile.writeAsStringSync(BasicProjectWithFlutterGen().pubspec);
653+
await createTestCommandRunner(command).run(<String>['gen-l10n', '--no-synthetic-package']);
654+
expect(logger.warningText, contains('synthetic-package'));
655+
},
656+
overrides: <Type, Generator>{Logger: () => logger},
657+
);
658+
699659
group(AppResourceBundle, () {
700660
testWithoutContext("can be parsed without FormatException when it's content is empty", () {
701661
final File arbFile = fileSystem.file(fileSystem.path.join('lib', 'l10n', 'app_en.arb'))

‎packages/flutter_tools/test/commands.shard/permeable/packages_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ workspace:
387387
);
388388

389389
testUsingContext(
390-
'get generates normal files when l10n.yaml has synthetic-package: false',
390+
'get generates files into lib/l10n',
391391
() async {
392392
final String projectPath = await createProject(
393393
tempDir,
@@ -403,7 +403,7 @@ flutter:
403403
generate: true
404404
''');
405405
projectDir.childFile('pubspec.yaml').writeAsStringSync(pubspecFileContent);
406-
projectDir.childFile('l10n.yaml').writeAsStringSync('synthetic-package: false');
406+
projectDir.childFile('l10n.yaml').createSync();
407407
await runCommandIn(projectPath, 'get');
408408
expect(
409409
projectDir

0 commit comments

Comments
 (0)