Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.0.2 return non zero for bad usage #79

Merged
merged 4 commits into from
Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# 2.0.2

- Return non-zero exit code from executable when incorrect args are used

# 2.0.1

- Fix a path issue on Windows.
Expand Down
10 changes: 6 additions & 4 deletions bin/dependency_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ import 'dart:io' show exit, stderr, stdout;

import 'package:args/args.dart';
import 'package:dependency_validator/dependency_validator.dart';
import 'package:io/io.dart';
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't a dependency in v2, so we need to add it:

  io: ^0.3.5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

import 'package:logging/logging.dart';

const String helpArg = 'help';
const String verboseArg = 'verbose';
const String helpMessage = '''Dependency Validator 2.0 is configured statically via the pubspec.yaml
const String helpMessage =
'''Dependency Validator 2.0 is configured statically via the pubspec.yaml
example:
# in pubspec.yaml
dependency_validator:
Expand All @@ -45,10 +47,10 @@ final ArgParser argParser = ArgParser()
help: 'Display extra information for debugging.',
);

void showHelpAndExit() {
void showHelpAndExit({ExitCode exitCode = ExitCode.success}) {
Logger.root.shout(helpMessage);
Logger.root.shout(argParser.usage);
exit(0);
exit(exitCode.code);
}

void main(List<String> args) async {
Expand All @@ -65,7 +67,7 @@ void main(List<String> args) async {
try {
argResults = argParser.parse(args);
} on FormatException catch (_) {
showHelpAndExit();
showHelpAndExit(exitCode: ExitCode.usage);
}

if (argResults.wasParsed(helpArg) && argResults[helpArg]) {
Expand Down
79 changes: 51 additions & 28 deletions lib/dependency_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ Future<Null> run() async {
exit(1);
}
if (!File('.dart_tool/package_config.json').existsSync()) {
logger.shout('No .dart_tool/package_config.json file found, please run "pub get" first.');
logger.shout(
'No .dart_tool/package_config.json file found, please run "pub get" first.');
exit(1);
}

final config = PubspecDepValidatorConfig.fromYaml(File('pubspec.yaml').readAsStringSync()).dependencyValidator;
final config = PubspecDepValidatorConfig.fromYaml(
File('pubspec.yaml').readAsStringSync())
.dependencyValidator;
final configExcludes = config?.exclude
?.map((s) {
try {
Expand All @@ -57,7 +60,8 @@ Future<Null> run() async {
final optionsIncludePackage = getAnalysisOptionsIncludePackage();

// Read and parse the pubspec.yaml in the current working directory.
final pubspec = Pubspec.parse(File('pubspec.yaml').readAsStringSync(), sourceUrl: 'pubspec.yaml');
final pubspec = Pubspec.parse(File('pubspec.yaml').readAsStringSync(),
sourceUrl: 'pubspec.yaml');

logger.info('Validating dependencies for ${pubspec.name}\n');

Expand Down Expand Up @@ -95,7 +99,8 @@ Future<Null> run() async {
// export directive.
final packagesUsedInPublicFiles = <String>{};
for (final file in publicDartFiles) {
final matches = importExportDartPackageRegex.allMatches(file.readAsStringSync());
final matches =
importExportDartPackageRegex.allMatches(file.readAsStringSync());
for (final match in matches) {
packagesUsedInPublicFiles.add(match.group(2));
}
Expand All @@ -117,9 +122,12 @@ Future<Null> run() async {

final publicDirGlobs = [for (final dir in publicDirs) Glob('$dir**')];

final nonPublicDartFiles = listDartFilesIn('./', [...excludes, ...publicDirGlobs]);
final nonPublicScssFiles = listScssFilesIn('./', [...excludes, ...publicDirGlobs]);
final nonPublicLessFiles = listLessFilesIn('./', [...excludes, ...publicDirGlobs]);
final nonPublicDartFiles =
listDartFilesIn('./', [...excludes, ...publicDirGlobs]);
final nonPublicScssFiles =
listScssFilesIn('./', [...excludes, ...publicDirGlobs]);
final nonPublicLessFiles =
listLessFilesIn('./', [...excludes, ...publicDirGlobs]);

logger
..fine('non-public dart files:\n'
Expand All @@ -137,7 +145,8 @@ Future<Null> run() async {
if (optionsIncludePackage != null) optionsIncludePackage,
};
for (final file in nonPublicDartFiles) {
final matches = importExportDartPackageRegex.allMatches(file.readAsStringSync());
final matches =
importExportDartPackageRegex.allMatches(file.readAsStringSync());
for (final match in matches) {
packagesUsedOutsidePublicDirs.add(match.group(2));
}
Expand Down Expand Up @@ -165,10 +174,10 @@ Future<Null> run() async {
// Remove all explicitly declared dependencies
.difference(deps)
.difference(devDeps)
// Ignore self-imports - packages have implicit access to themselves.
..remove(pubspec.name)
// Ignore known missing packages.
..removeAll(ignoredPackages);
// Ignore self-imports - packages have implicit access to themselves.
..remove(pubspec.name)
// Ignore known missing packages.
..removeAll(ignoredPackages);

if (missingDependencies.isNotEmpty) {
log(
Expand All @@ -187,10 +196,10 @@ Future<Null> run() async {
// Remove all explicitly declared dependencies
.difference(devDeps)
.difference(deps)
// Ignore self-imports - packages have implicit access to themselves.
..remove(pubspec.name)
// Ignore known missing packages.
..removeAll(ignoredPackages);
// Ignore self-imports - packages have implicit access to themselves.
..remove(pubspec.name)
// Ignore known missing packages.
..removeAll(ignoredPackages);

if (missingDevDependencies.isNotEmpty) {
log(
Expand Down Expand Up @@ -245,16 +254,20 @@ Future<Null> run() async {
// Remove all deps that were used in Dart code somewhere in this package
.difference(packagesUsedInPublicFiles)
.difference(packagesUsedOutsidePublicDirs)
// Remove this package, since we know they're using our executable
..remove(dependencyValidatorPackageName);
// Remove this package, since we know they're using our executable
..remove(dependencyValidatorPackageName);

// Remove deps that provide builders that will be applied
final packageConfig = await findPackageConfig(Directory.current);
final rootBuildConfig = await BuildConfig.fromBuildConfigDir(pubspec.name, pubspec.dependencies.keys, '.');
final rootBuildConfig = await BuildConfig.fromBuildConfigDir(
pubspec.name, pubspec.dependencies.keys, '.');
bool rootPackageReferencesDependencyInBuildYaml(String dependencyName) => [
...rootBuildConfig.globalOptions.keys,
for (final target in rootBuildConfig.buildTargets.values) ...target.builders.keys,
].map((key) => normalizeBuilderKeyUsage(key, pubspec.name)).any((key) => key.startsWith('$dependencyName:'));
for (final target in rootBuildConfig.buildTargets.values)
...target.builders.keys,
]
.map((key) => normalizeBuilderKeyUsage(key, pubspec.name))
.any((key) => key.startsWith('$dependencyName:'));

final packagesWithConsumedBuilders = Set<String>();
for (final package in unusedDependencies.map((name) => packageConfig[name])) {
Expand All @@ -277,14 +290,18 @@ Future<Null> run() async {
for (final package in unusedDependencies.map((name) => packageConfig[name])) {
// Search for executables, if found we assume they are used
final binDir = Directory(p.join(p.fromUri(package.root), 'bin'));
hasDartFiles() => binDir.listSync().any((entity) => entity.path.endsWith('.dart'));
hasDartFiles() =>
binDir.listSync().any((entity) => entity.path.endsWith('.dart'));
if (binDir.existsSync() && hasDartFiles()) {
packagesWithExecutables.add(package.name);
}
}

logIntersection(Level.INFO, 'The following packages contain executables, they are assumed to be used:',
unusedDependencies, packagesWithExecutables);
logIntersection(
Level.INFO,
'The following packages contain executables, they are assumed to be used:',
unusedDependencies,
packagesWithExecutables);
unusedDependencies.removeAll(packagesWithExecutables);

if (unusedDependencies.contains('analyzer')) {
Expand Down Expand Up @@ -313,7 +330,10 @@ Future<Null> run() async {

/// Whether a dependency at [path] defines an auto applied builder.
Future<bool> dependencyDefinesAutoAppliedBuilder(String path) async =>
(await BuildConfig.fromPackageDir(path)).builderDefinitions.values.any((def) => def.autoApply != AutoApply.none);
(await BuildConfig.fromPackageDir(path))
.builderDefinitions
.values
.any((def) => def.autoApply != AutoApply.none);

/// Checks for dependency pins.
///
Expand All @@ -338,12 +358,15 @@ void checkPubspecForPins(
List<String> ignoredPackages = const [],
}) {
final List<String> infractions = [];
infractions.addAll(getDependenciesWithPins(pubspec.dependencies, ignoredPackages: ignoredPackages));
infractions.addAll(getDependenciesWithPins(pubspec.dependencies,
ignoredPackages: ignoredPackages));

infractions.addAll(getDependenciesWithPins(pubspec.devDependencies, ignoredPackages: ignoredPackages));
infractions.addAll(getDependenciesWithPins(pubspec.devDependencies,
ignoredPackages: ignoredPackages));

if (infractions.isNotEmpty) {
log(Level.WARNING, 'These packages are pinned in pubspec.yaml:', infractions);
log(Level.WARNING, 'These packages are pinned in pubspec.yaml:',
infractions);
exitCode = 1;
}
}
28 changes: 17 additions & 11 deletions lib/src/constants.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
/// Regex used to detect all Dart import and export directives.
final RegExp importExportDartPackageRegex =
RegExp(r'''\b(import|export)\s+['"]{1,3}package:([a-zA-Z0-9_]+)\/[^;]+''', multiLine: true);
final RegExp importExportDartPackageRegex = RegExp(
r'''\b(import|export)\s+['"]{1,3}package:([a-zA-Z0-9_]+)\/[^;]+''',
multiLine: true);

/// Regex used to detect all Sass import directives.
final RegExp importScssPackageRegex = RegExp(r'''\@import\s+['"]{1,3}package:\s*([a-zA-Z0-9_]+)\/[^;]+''');
final RegExp importScssPackageRegex =
RegExp(r'''\@import\s+['"]{1,3}package:\s*([a-zA-Z0-9_]+)\/[^;]+''');

/// Regex used to detect all Less import directives.
final RegExp importLessPackageRegex = RegExp(r'@import\s+(?:\(.*\)\s+)?"(?:packages\/|package:\/\/)([a-zA-Z1-9_-]+)\/');
final RegExp importLessPackageRegex = RegExp(
r'@import\s+(?:\(.*\)\s+)?"(?:packages\/|package:\/\/)([a-zA-Z1-9_-]+)\/');

/// String key in pubspec.yaml for the dependencies map.
const String dependenciesKey = 'dependencies';
Expand Down Expand Up @@ -34,7 +37,8 @@ class DependencyPinEvaluation {
String toString() => message;

/// <1.2.0
static const DependencyPinEvaluation blocksMinorBumps = DependencyPinEvaluation._('This pin blocks minor bumps.');
static const DependencyPinEvaluation blocksMinorBumps =
DependencyPinEvaluation._('This pin blocks minor bumps.');

/// <1.2.3
static const DependencyPinEvaluation blocksPatchReleases =
Expand All @@ -45,18 +49,20 @@ class DependencyPinEvaluation {
/// Note that <1.0.0-0 is legal because the exclusive bounds ignore the first
/// possible prerelease.
static const DependencyPinEvaluation buildOrPrerelease =
DependencyPinEvaluation._('Builds or preleases as max bounds block minor bumps and patches.');
DependencyPinEvaluation._(
'Builds or preleases as max bounds block minor bumps and patches.');

/// 1.2.3
static const DependencyPinEvaluation directPin = DependencyPinEvaluation._('This is a direct pin.');
static const DependencyPinEvaluation directPin =
DependencyPinEvaluation._('This is a direct pin.');

/// >1.2.3 <1.2.3
static const DependencyPinEvaluation emptyPin =
DependencyPinEvaluation._('Empty dependency versions cannot be resolved.');
static const DependencyPinEvaluation emptyPin = DependencyPinEvaluation._(
'Empty dependency versions cannot be resolved.');

/// <=1.2.3
static const DependencyPinEvaluation inclusiveMax =
DependencyPinEvaluation._('Inclusive max bounds restrict minor bumps and patches.');
static const DependencyPinEvaluation inclusiveMax = DependencyPinEvaluation._(
'Inclusive max bounds restrict minor bumps and patches.');

/// :)
static const DependencyPinEvaluation notAPin =
Expand Down
22 changes: 17 additions & 5 deletions lib/src/pubspec_config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,37 @@ import 'package:json_annotation/json_annotation.dart';

part 'pubspec_config.g.dart';

@JsonSerializable(anyMap: true, checked: true, createToJson: false, fieldRename: FieldRename.snake)
@JsonSerializable(
anyMap: true,
checked: true,
createToJson: false,
fieldRename: FieldRename.snake)
class PubspecDepValidatorConfig {
final DepValidatorConfig dependencyValidator;

PubspecDepValidatorConfig({this.dependencyValidator});

factory PubspecDepValidatorConfig.fromJson(Map json) => _$PubspecDepValidatorConfigFromJson(json);
factory PubspecDepValidatorConfig.fromJson(Map json) =>
_$PubspecDepValidatorConfigFromJson(json);

factory PubspecDepValidatorConfig.fromYaml(String yamlContent, {sourceUrl}) =>
checkedYamlDecode(yamlContent, (m) => PubspecDepValidatorConfig.fromJson(m), sourceUrl: sourceUrl);
checkedYamlDecode(
yamlContent, (m) => PubspecDepValidatorConfig.fromJson(m),
sourceUrl: sourceUrl);
}

@JsonSerializable(anyMap: true, checked: true, createToJson: false, fieldRename: FieldRename.snake)
@JsonSerializable(
anyMap: true,
checked: true,
createToJson: false,
fieldRename: FieldRename.snake)
class DepValidatorConfig {
final List<String> exclude;

final List<String> ignore;

DepValidatorConfig({this.exclude, this.ignore});

factory DepValidatorConfig.fromJson(Map json) => _$DepValidatorConfigFromJson(json);
factory DepValidatorConfig.fromJson(Map json) =>
_$DepValidatorConfigFromJson(json);
}
10 changes: 6 additions & 4 deletions lib/src/pubspec_config.g.dart

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 12 additions & 6 deletions lib/src/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import 'constants.dart';
final Logger logger = Logger('dependency_validator');

/// Returns a multi-line string with all [items] in a bulleted list format.
String bulletItems(Iterable<String> items) => items.map((l) => ' * $l').join('\n');
String bulletItems(Iterable<String> items) =>
items.map((l) => ' * $l').join('\n');

/// Returns the name of the package referenced in the `include:` directive in an
/// analysis_options.yaml file, or null if there is not one.
Expand Down Expand Up @@ -72,14 +73,16 @@ Iterable<File> listLessFilesIn(String dirPath, List<Glob> excludedDirs) =>
///
/// This also excludes Dart files that are in a hidden directory, like
/// `.dart_tool`.
Iterable<File> listFilesWithExtensionIn(String dirPath, List<Glob> excludes, String ext) {
Iterable<File> listFilesWithExtensionIn(
String dirPath, List<Glob> excludes, String ext) {
if (!FileSystemEntity.isDirectorySync(dirPath)) return [];

return Directory(dirPath)
.listSync(recursive: true)
.whereType<File>()
// Skip files in hidden directories (e.g. `.dart_tool/`)
.where((file) => !p.split(file.path).any((d) => d != '.' && d.startsWith('.')))
.where((file) =>
!p.split(file.path).any((d) => d != '.' && d.startsWith('.')))
// Filter by the given file extension
.where((file) => p.extension(file.path) == '.$ext')
// Skip any files that match one of the given exclude globs
Expand All @@ -94,15 +97,17 @@ void log(Level level, String message, Iterable<String> dependencies) {

/// Logs the given [message] at [level] and lists the intersection of [dependenciesA]
/// and [dependenciesB] if there is one.
void logIntersection(Level level, String message, Set<String> dependenciesA, Set<String> dependenciesB) {
void logIntersection(Level level, String message, Set<String> dependenciesA,
Set<String> dependenciesB) {
final intersection = dependenciesA.intersection(dependenciesB);
if (intersection.isNotEmpty) {
log(level, message, intersection);
}
}

/// Lists the packages with infractions
List<String> getDependenciesWithPins(Map<String, Dependency> dependencies, {List<String> ignoredPackages = const []}) {
List<String> getDependenciesWithPins(Map<String, Dependency> dependencies,
{List<String> ignoredPackages = const []}) {
final List<String> infractions = [];
for (String packageName in dependencies.keys) {
if (ignoredPackages.contains(packageName)) {
Expand All @@ -113,7 +118,8 @@ List<String> getDependenciesWithPins(Map<String, Dependency> dependencies, {List
final packageMeta = dependencies[packageName];

if (packageMeta is HostedDependency) {
final DependencyPinEvaluation evaluation = inspectVersionForPins(packageMeta.version);
final DependencyPinEvaluation evaluation =
inspectVersionForPins(packageMeta.version);

if (evaluation.isPin) {
infractions.add('$packageName: $version -- ${evaluation.message}');
Expand Down
Loading