Skip to content

Commit 8bade15

Browse files
matanlureykorca0220
authored andcommitted
Add a warning on usage of dartPluginClass: 'none'. (flutter#172315)
Towards flutter#57497. Supersedes flutter#171922 based on @stuartmorgan-g's advice for a warning release. I'll CP this into `3.35` (beta) so that we can clean it up on `master` anytime.
1 parent af75b57 commit 8bade15

File tree

3 files changed

+121
-6
lines changed

3 files changed

+121
-6
lines changed

packages/flutter_tools/lib/src/flutter_plugins.dart

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1659,7 +1659,19 @@ bool _hasPluginInlineImpl(
16591659
/// Determine if the plugin provides an inline Dart implementation.
16601660
bool _hasPluginInlineDartImpl(Plugin plugin, String platformKey) {
16611661
final DartPluginClassAndFilePair? platformInfo = plugin.pluginDartClassPlatforms[platformKey];
1662-
return platformInfo != null && platformInfo.dartClass != 'none';
1662+
if (platformInfo == null) {
1663+
return false;
1664+
}
1665+
if (platformInfo.dartClass == 'none') {
1666+
// TODO(matanlurey): Remove as part of https://github.com/flutter/flutter/issues/57497.
1667+
globals.printWarning(
1668+
'Use of `dartPluginClass: none` (${plugin.name}) is deprecated, and will '
1669+
'be removed in the next stable version. See '
1670+
'https://github.com/flutter/flutter/issues/57497 for details.',
1671+
);
1672+
return false;
1673+
}
1674+
return true;
16631675
}
16641676

16651677
/// Get the resolved [Plugin] `resolution` from the [candidates] serving as

packages/flutter_tools/lib/src/platform_plugins.dart

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:yaml/yaml.dart';
66

77
import 'base/common.dart';
88
import 'base/file_system.dart';
9+
import 'globals.dart' as globals;
910

1011
/// Constant for 'pluginClass' key in plugin maps.
1112
const kPluginClass = 'pluginClass';
@@ -367,8 +368,18 @@ class MacOSPlugin extends PluginPlatform implements NativeOrDartPlugin, DarwinPl
367368
);
368369
}
369370

370-
// Treat 'none' as not present. See https://github.com/flutter/flutter/issues/57497.
371-
final String? pluginClass = yaml[kPluginClass] == 'none' ? null : yaml[kPluginClass] as String?;
371+
final String? pluginClass;
372+
if (yaml[kPluginClass] == 'none') {
373+
// TODO(matanlurey): Remove as part of https://github.com/flutter/flutter/issues/57497.
374+
globals.printWarning(
375+
'Use of `dartPluginClass: none` ($name) is deprecated, and will be '
376+
'removed in the next stable version. See '
377+
'https://github.com/flutter/flutter/issues/57497 for details.',
378+
);
379+
pluginClass = null;
380+
} else {
381+
pluginClass = yaml[kPluginClass] as String?;
382+
}
372383

373384
return MacOSPlugin(
374385
name: name,
@@ -446,9 +457,14 @@ class WindowsPlugin extends PluginPlatform implements NativeOrDartPlugin, Varian
446457

447458
factory WindowsPlugin.fromYaml(String name, YamlMap yaml) {
448459
assert(validate(yaml));
449-
// Treat 'none' as not present. See https://github.com/flutter/flutter/issues/57497.
450460
var pluginClass = yaml[kPluginClass] as String?;
451461
if (pluginClass == 'none') {
462+
// TODO(matanlurey): Remove as part of https://github.com/flutter/flutter/issues/57497.
463+
globals.printWarning(
464+
'Use of `dartPluginClass: none` ($name) is deprecated, and will be '
465+
'removed in the next stable version. See '
466+
'https://github.com/flutter/flutter/issues/57497 for details.',
467+
);
452468
pluginClass = null;
453469
}
454470
final variants = <PluginPlatformVariant>{};
@@ -564,10 +580,22 @@ class LinuxPlugin extends PluginPlatform implements NativeOrDartPlugin {
564580
);
565581
}
566582

583+
final String? pluginClass;
584+
if (yaml[kPluginClass] == 'none') {
585+
// TODO(matanlurey): Remove as part of https://github.com/flutter/flutter/issues/57497.
586+
globals.printWarning(
587+
'Use of `dartPluginClass: none` ($name) is deprecated, and will be '
588+
'removed in the next stable version. See '
589+
'https://github.com/flutter/flutter/issues/57497 for details.',
590+
);
591+
pluginClass = null;
592+
} else {
593+
pluginClass = yaml[kPluginClass] as String?;
594+
}
595+
567596
return LinuxPlugin(
568597
name: name,
569-
// Treat 'none' as not present. See https://github.com/flutter/flutter/issues/57497.
570-
pluginClass: yaml[kPluginClass] == 'none' ? null : yaml[kPluginClass] as String?,
598+
pluginClass: pluginClass,
571599
dartPluginClass: dartPluginClass,
572600
dartFileName: dartFileName,
573601
ffiPlugin: yaml[kFfiPlugin] as bool? ?? false,

packages/flutter_tools/test/general.shard/build_system/targets/dart_plugin_registrant_test.dart

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
import 'package:file/memory.dart';
6+
import 'package:file_testing/file_testing.dart';
67
import 'package:flutter_tools/src/artifacts.dart';
78
import 'package:flutter_tools/src/base/file_system.dart';
89
import 'package:flutter_tools/src/base/logger.dart';
@@ -54,12 +55,33 @@ environment:
5455
flutter: ">=1.20.0"
5556
''';
5657

58+
/// Returns a `pubspec.yaml` where `$platform` uses `dartPluginClass: 'none'`.
59+
String samplePluginPubspecWithDartPluginClassNone({required String platform}) =>
60+
'''
61+
name: path_provider_$platform
62+
description: $platform implementation of the path_provider plugin
63+
// version: 2.0.1
64+
// homepage: https://github.com/flutter/plugins/tree/main/packages/path_provider/path_provider_$platform
65+
flutter:
66+
plugin:
67+
implements: path_provider
68+
platforms:
69+
$platform:
70+
dartPluginClass: none
71+
pluginClass: none
72+
environment:
73+
sdk: ^3.7.0-0
74+
flutter: ">=1.20.0"
75+
''';
76+
5777
void main() {
5878
group('Dart plugin registrant', () {
5979
late FileSystem fileSystem;
80+
late BufferLogger logger;
6081

6182
setUp(() {
6283
fileSystem = MemoryFileSystem.test();
84+
logger = BufferLogger.test();
6385
});
6486

6587
testWithoutContext('skipped based on environment.generateDartPluginRegistry', () async {
@@ -158,6 +180,59 @@ name: path_provider_example
158180
},
159181
);
160182

183+
for (final platform in ['linux', 'macos', 'windows']) {
184+
testUsingContext(
185+
'$platform treats dartPluginClass: "none" as omitted',
186+
() async {
187+
final Directory projectDir = fileSystem.directory('project')..createSync();
188+
final environment = Environment.test(
189+
fileSystem.currentDirectory,
190+
projectDir: projectDir,
191+
artifacts: Artifacts.test(),
192+
fileSystem: fileSystem,
193+
logger: BufferLogger.test(),
194+
processManager: FakeProcessManager.any(),
195+
defines: <String, String>{
196+
kTargetFile: projectDir.childDirectory('lib').childFile('main.dart').absolute.path,
197+
},
198+
generateDartPluginRegistry: true,
199+
);
200+
201+
writePackageConfigFiles(
202+
directory: projectDir,
203+
mainLibName: 'path_provider_example',
204+
packages: <String, String>{'path_provider_$platform': '/path_provider_$platform'},
205+
languageVersions: <String, String>{'path_provider_example': '2.12'},
206+
);
207+
208+
projectDir.childFile('pubspec.yaml').writeAsStringSync(_kSamplePubspecFile);
209+
210+
projectDir.childDirectory('lib').childFile('main.dart').createSync(recursive: true);
211+
212+
environment.fileSystem.currentDirectory
213+
.childDirectory('path_provider_$platform')
214+
.childFile('pubspec.yaml')
215+
..createSync(recursive: true)
216+
..writeAsStringSync(samplePluginPubspecWithDartPluginClassNone(platform: platform));
217+
218+
final FlutterProject testProject = FlutterProject.fromDirectoryTest(projectDir);
219+
await DartPluginRegistrantTarget.test(testProject).build(environment);
220+
221+
final File generatedMain = projectDir
222+
.childDirectory('.dart_tool')
223+
.childDirectory('flutter_build')
224+
.childFile('dart_plugin_registrant.dart');
225+
expect(generatedMain, isNot(exists));
226+
expect(logger.warningText, contains('Use of `dartPluginClass: none`'));
227+
},
228+
overrides: {
229+
Logger: () => logger,
230+
ProcessManager: () => FakeProcessManager.any(),
231+
Pub: ThrowingPub.new,
232+
},
233+
);
234+
}
235+
161236
testUsingContext(
162237
'regenerates dart_plugin_registrant.dart',
163238
() async {

0 commit comments

Comments
 (0)