Skip to content

Commit 980417e

Browse files
reidbakerTecHaxter
authored andcommitted
[Tool] Add ability to check dependencies independently of dev-dependencies, exclude integration_test from dependencies (flutter#6446)
Create a linter that ensures that `integration_test` is not used in dependencies. Will be paired with a change to documentation ``` If you are considering adding an external dependency: Consider other options, and discuss with #hackers-ecosystem in Discord. * If you add a dev_dependency on an external package, pin it to a specific version if at all possible. * If you add a dependency on an external package in an example/, pin it to a specific version if at all possible. * Some dependencies should only be linked as dev dependencies like integration_test ``` Related to flutter/flutter/issues/145992
1 parent a2449ad commit 980417e

File tree

2 files changed

+75
-1
lines changed

2 files changed

+75
-1
lines changed

script/tool/lib/src/pubspec_check_command.dart

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
356356
'a topic. Add "$topicName" to the "topics" section.';
357357
}
358358
}
359-
359+
360360
// Validates topic names according to https://dart.dev/tools/pub/pubspec#topics
361361
final RegExp expectedTopicFormat = RegExp(r'^[a-z](?:-?[a-z0-9]+)*$');
362362
final Iterable<String> invalidTopics = topics.where((String topic) =>
@@ -542,6 +542,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
542542
// there are any that aren't allowed.
543543
String? _checkDependencies(Pubspec pubspec) {
544544
final Set<String> badDependencies = <String>{};
545+
// Shipped dependencies.
545546
for (final Map<String, Dependency> dependencies
546547
in <Map<String, Dependency>>[
547548
pubspec.dependencies,
@@ -553,6 +554,19 @@ class PubspecCheckCommand extends PackageLoopingCommand {
553554
}
554555
});
555556
}
557+
558+
// Ensure that dev-only dependencies aren't in `dependencies`.
559+
const List<String> devOnlyDependencies = <String>['integration_test'];
560+
// Non-published packages like pidgeon subpackages are allowed to violate
561+
// the dev only dependencies rule.
562+
if (pubspec.publishTo != 'none') {
563+
pubspec.dependencies.forEach((String name, Dependency dependency) {
564+
if (devOnlyDependencies.contains(name)) {
565+
badDependencies.add(name);
566+
}
567+
});
568+
}
569+
556570
if (badDependencies.isEmpty) {
557571
return null;
558572
}
@@ -563,6 +577,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
563577
}
564578

565579
// Checks whether a given dependency is allowed.
580+
// Defaults to false.
566581
bool _shouldAllowDependency(String name, Dependency dependency) {
567582
if (dependency is PathDependency || dependency is SdkDependency) {
568583
return true;

script/tool/test/pubspec_check_command_test.dart

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,6 +1734,65 @@ ${_topicsSection()}
17341734
]),
17351735
);
17361736
});
1737+
1738+
test('fails when integration_test is used in non dev dependency',
1739+
() async {
1740+
final RepositoryPackage package =
1741+
createFakePackage('a_package', packagesDir, examples: <String>[]);
1742+
1743+
package.pubspecFile.writeAsStringSync('''
1744+
${_headerSection('a_package')}
1745+
${_environmentSection()}
1746+
${_dependenciesSection(<String>['integration_test: \n sdk: flutter'])}
1747+
${_devDependenciesSection()}
1748+
${_topicsSection()}
1749+
''');
1750+
1751+
Error? commandError;
1752+
final List<String> output = await runCapturingPrint(runner, <String>[
1753+
'pubspec-check',
1754+
], errorHandler: (Error e) {
1755+
commandError = e;
1756+
});
1757+
1758+
expect(commandError, isA<ToolExit>());
1759+
expect(
1760+
output,
1761+
containsAllInOrder(<Matcher>[
1762+
contains(
1763+
'The following unexpected non-local dependencies were found:\n'
1764+
' integration_test\n'
1765+
'Please see https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#Dependencies '
1766+
'for more information and next steps.'),
1767+
]),
1768+
);
1769+
});
1770+
1771+
test('passes when integration_test is used in non published package',
1772+
() async {
1773+
final RepositoryPackage package =
1774+
createFakePackage('a_package', packagesDir, examples: <String>[]);
1775+
1776+
package.pubspecFile.writeAsStringSync('''
1777+
${_headerSection('a_package', publishable: false)}
1778+
${_environmentSection()}
1779+
${_dependenciesSection(<String>['integration_test: \n sdk: flutter'])}
1780+
${_devDependenciesSection()}
1781+
${_topicsSection()}
1782+
''');
1783+
1784+
final List<String> output = await runCapturingPrint(runner, <String>[
1785+
'pubspec-check',
1786+
]);
1787+
1788+
expect(
1789+
output,
1790+
containsAllInOrder(<Matcher>[
1791+
contains('Running for a_package...'),
1792+
contains('Ran for'),
1793+
]),
1794+
);
1795+
});
17371796
});
17381797
});
17391798

0 commit comments

Comments
 (0)