Skip to content

Commit

Permalink
[Tool] Add ability to check dependencies independently of dev-depende…
Browse files Browse the repository at this point in the history
…ncies, exclude integration_test from dependencies (#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
  • Loading branch information
reidbaker authored Apr 3, 2024
1 parent c1216f5 commit 3ff376b
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 1 deletion.
17 changes: 16 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,19 @@ class PubspecCheckCommand extends PackageLoopingCommand {
}
});
}

// Ensure that dev-only dependencies aren't in `dependencies`.
const List<String> devOnlyDependencies = <String>['integration_test'];
// Non-published packages like pidgeon subpackages are allowed to violate
// the dev only dependencies rule.
if (pubspec.publishTo != 'none') {
pubspec.dependencies.forEach((String name, Dependency dependency) {
if (devOnlyDependencies.contains(name)) {
badDependencies.add(name);
}
});
}

if (badDependencies.isEmpty) {
return null;
}
Expand All @@ -563,6 +577,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
59 changes: 59 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,65 @@ ${_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 non published package',
() async {
final RepositoryPackage package =
createFakePackage('a_package', packagesDir, examples: <String>[]);

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

final List<String> output = await runCapturingPrint(runner, <String>[
'pubspec-check',
]);

expect(
output,
containsAllInOrder(<Matcher>[
contains('Running for a_package...'),
contains('Ran for'),
]),
);
});
});
});

Expand Down

0 comments on commit 3ff376b

Please sign in to comment.