Merge pull request #79 from Workiva/2.0.2_return_non_zero_for_bad_usage

2.0.2 return non zero for bad usage

CHANGELOG.md

@@ -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.

bin/dependency_validator.dart

@@ -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';
 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:
@@ -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 {
@@ -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]) {

lib/dependency_validator.dart

@@ -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 {
@@ -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');
 
@@ -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));
     }
@@ -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'
@@ -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));
     }
@@ -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(
@@ -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(
@@ -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])) {
@@ -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')) {
@@ -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.
 ///
@@ -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;
   }
 }

lib/src/constants.dart

@@ -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';
@@ -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 =
@@ -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 =

lib/src/pubspec_config.dart

@@ -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);
 }

lib/src/utils.dart

@@ -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.
@@ -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
@@ -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)) {
@@ -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}');