Skip to content
21 changes: 20 additions & 1 deletion script/tool/lib/src/pubspec_check_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
'a topic. Add "$topicName" to the "topics" section.';
}
}

// Validates topic names according to https://dart.dev/tools/pub/pubspec#topics
final RegExp expectedTopicFormat = RegExp(r'^[a-z](?:-?[a-z0-9]+)*$');
final Iterable<String> invalidTopics = topics.where((String topic) =>
Expand Down Expand Up @@ -542,6 +542,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
// there are any that aren't allowed.
String? _checkDependencies(Pubspec pubspec) {
final Set<String> badDependencies = <String>{};
// Shipped dependencies.
for (final Map<String, Dependency> dependencies
in <Map<String, Dependency>>[
pubspec.dependencies,
Expand All @@ -553,6 +554,23 @@ class PubspecCheckCommand extends PackageLoopingCommand {
}
});
}

// Ensure that dev-only dependencies aren't in `dependencies`.
const List<String> devOnlyDependencies = <String>['integration_test'];
// pigeon/platform_tests/shared_test_plugin_code is allowed to violate
// the dev only dependencies rule beause pidgeon has generated tests that
// are intended to ship to customers.
if (pubspec.name == 'shared_test_plugin_code') {
Copy link
Contributor Author

@reidbaker reidbaker Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stuartmorgan requesting re-review because I am not a fan of how I exempted pidgeon from the new check and am hoping you have a better idea.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just conditionalize this whole new check on being publishable, and remove the warning:

if (pubspec.publishTo != 'none') {

printWarning(
' Skipping dev-only dependencies check for ${pubspec.name}');
} else {
pubspec.dependencies.forEach((String name, Dependency dependency) {
if (devOnlyDependencies.contains(name)) {
badDependencies.add(name);
}
});
}

if (badDependencies.isEmpty) {
return null;
}
Expand All @@ -563,6 +581,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
}

// Checks whether a given dependency is allowed.
// Defaults to false.
bool _shouldAllowDependency(String name, Dependency dependency) {
if (dependency is PathDependency || dependency is SdkDependency) {
return true;
Expand Down
64 changes: 64 additions & 0 deletions script/tool/test/pubspec_check_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1734,6 +1734,70 @@ ${_topicsSection()}
]),
);
});

test('fails when integration_test is used in non dev dependency',
() async {
final RepositoryPackage package =
createFakePackage('a_package', packagesDir, examples: <String>[]);

package.pubspecFile.writeAsStringSync('''
${_headerSection('a_package')}
${_environmentSection()}
${_dependenciesSection(<String>['integration_test: \n sdk: flutter'])}
${_devDependenciesSection()}
${_topicsSection()}
''');

Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[
'pubspec-check',
], errorHandler: (Error e) {
commandError = e;
});

expect(commandError, isA<ToolExit>());
expect(
output,
containsAllInOrder(<Matcher>[
contains(
'The following unexpected non-local dependencies were found:\n'
' integration_test\n'
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
'for more information and next steps.'),
]),
);
});

test('passes when integration_test is used in shared_test_plugin_code',
() async {
final RepositoryPackage package = createFakePackage(
'shared_test_plugin_code', packagesDir,
examples: <String>[]);

package.pubspecFile.writeAsStringSync('''
${_headerSection('shared_test_plugin_code')}
${_environmentSection()}
${_dependenciesSection(<String>['integration_test: \n sdk: flutter'])}
${_devDependenciesSection()}
${_topicsSection()}
''');

Error? commandError;
final List<String> output = await runCapturingPrint(runner, <String>[
'pubspec-check',
], errorHandler: (Error e) {
commandError = e;
});

expect(
output,
containsAllInOrder(<Matcher>[
contains('Running for shared_test_plugin_code...'),
contains(
'Skipping dev-only dependencies check for shared_test_plugin_code'),
]),
);
});
});
});

Expand Down