Add support for circular dependency detection

bin/src/assert_pubspec_yaml_consistency.dart

@@ -46,7 +46,7 @@ void assertPubspecYamlConsistency(Iterable<DartPackage> packages) {
   );
 
   if (inconsistentSpecList.isNotEmpty) {
-    printDependencyUsageReport(
+    printInconsistentDependencyUsageReport(
       report: inconsistentSpecList,
       formatDependency: _formatDependencySpec,
     );

bin/src/commands/probe_command_runner.dart

@@ -30,9 +30,9 @@ import 'dart:io';
 import 'package:args/args.dart';
 import 'package:borg/src/configuration/factory.dart';
 import 'package:borg/src/dart_package/dart_package.dart';
+import 'package:borg/src/find_circular_dependencies.dart';
 import 'package:borg/src/find_inconsistent_dependencies.dart';
 import 'package:meta/meta.dart';
-import 'package:path/path.dart' as path;
 import 'package:pubspec_lock/pubspec_lock.dart';
 
 import '../assert_pubspec_yaml_consistency.dart';
@@ -88,34 +88,53 @@ class ProbeCommandRunner {
   void _checkPubspecLockFiles(Iterable<DartPackage> packages) {
     final pubspecLocks = Map.fromEntries(
       packages
-          .map((p) => File(path.join(p.path, 'pubspec.lock')))
           .where(
-            (f) =>
-                f.existsSync() &&
-                [FileSystemEntityType.file].contains(f.statSync().type),
+            (package) =>
+                package.pubspecLockFile.existsSync() &&
+                [FileSystemEntityType.file]
+                    .contains(package.pubspecLockFile.statSync().type),
           )
           .map(
-            (f) => MapEntry(
-              f.path,
-              f.readAsStringSync().loadPubspecLockFromYaml(),
+            (package) => MapEntry(
+              package,
+              package.pubspecLockFile
+                  .readAsStringSync()
+                  .loadPubspecLockFromYaml(),
             ),
           ),
     );
 
     print('Analyzing dependencies...');
     final inconsistentUsageList = findInconsistentDependencies(pubspecLocks);
+    final circularDependenciesReport =
+        findCircularDependencies(pubspecLocks).toList();
 
     if (inconsistentUsageList.isNotEmpty && getCorrectFlag(argResults)) {
       correctPackageDependencyBasedOnReport(report: inconsistentUsageList);
-    } else if (inconsistentUsageList.isNotEmpty) {
-      printDependencyUsageReport(
-        report: inconsistentUsageList,
-        formatDependency: _formatDependencyInfo,
-      );
-      throw const BorgException(
-        'FAILURE: Inconsistent use of external dependencies detected!\n'
-        '         Consider to use the --correct option to fix issues.',
-      );
+    } else {
+      if (inconsistentUsageList.isNotEmpty) {
+        printInconsistentDependencyUsageReport(
+          report: inconsistentUsageList,
+          formatDependency: _formatDependencyInfo,
+        );
+      }
+
+      if (circularDependenciesReport.isNotEmpty) {
+        printCircularDependencyUsageReport(
+          report: circularDependenciesReport,
+          formatDependency: _formatDependencyInfo,
+        );
+      }
+
+      if (inconsistentUsageList.isNotEmpty ||
+          circularDependenciesReport.isNotEmpty) {
+        throw BorgException(
+          'FAILURE: Inconsistent use of (external) dependencies detected!',
+          supportMessage: inconsistentUsageList.isNotEmpty
+              ? 'Consider to use the --correct option to fix issues.'
+              : null,
+        );
+      }
     }
   }
 }

bin/src/utils/borg_exception.dart

@@ -31,9 +31,13 @@ import 'package:meta/meta.dart';
 
 @immutable
 class BorgException implements Exception {
-  const BorgException(this.message);
+  const BorgException(
+    this.message, {
+    this.supportMessage,
+  });
 
   final String message;
+  final String? supportMessage;
 }
 
 void exitWithMessageOnBorgException({
@@ -42,8 +46,12 @@ void exitWithMessageOnBorgException({
 }) {
   try {
     action();
-  } on BorgException catch (e) {
-    print(e.message);
+  } on BorgException catch (exception) {
+    print(exception.message);
+    if (exception.supportMessage != null) {
+      print(exception.supportMessage);
+    }
+
     exit(exitCode);
   }
 }

bin/src/utils/print_dependency_usage_report.dart

@@ -27,7 +27,7 @@ import 'package:borg/borg.dart';
 
 // ignore_for_file: avoid_print
 
-void printDependencyUsageReport<DependencyType>({
+void printInconsistentDependencyUsageReport<DependencyType>({
   required List<DependencyUsageReport<DependencyType>> report,
   required String Function(DependencyType dependency) formatDependency,
 }) {
@@ -39,11 +39,33 @@ void printDependencyUsageReport<DependencyType>({
       '\n${use.dependencyName}: '
       'inconsistent dependency specifications detected',
     );
+
     printDependencyUsage(
       dependencies: use.references,
       formatDependency: formatDependency,
     );
   }
+
+  print('');
+}
+
+void printCircularDependencyUsageReport<DependencyType>({
+  required List<DependencyUsageReport<DependencyType>> report,
+  required String Function(DependencyType dependency) formatDependency,
+}) {
+  for (final use in report) {
+    print(
+      '\n${use.dependencyName}: '
+      'circular dependency detected',
+    );
+
+    printDependencyUsage(
+      dependencies: use.references,
+      formatDependency: formatDependency,
+    );
+  }
+
+  print('');
 }
 
 void printDependencyUsage<DependencyType>({

lib/src/dart_package/dart_package.dart

@@ -23,6 +23,8 @@
  *
  */
 
+import 'dart:io';
+
 import 'package:meta/meta.dart';
 import 'package:path/path.dart';
 import 'package:plain_optional/plain_optional.dart';
@@ -47,11 +49,13 @@ class DartPackage {
         pubspecLock = tryToReadFileSync(join(path, 'pubspec.lock')).iif(
           some: (content) => Optional(content.loadPubspecLockFromYaml()),
           none: () => const Optional.none(),
-        );
+        ),
+        pubspecLockFile = File(join(path, 'pubspec.lock'));
 
   final String path;
   final PubspecYaml pubspecYaml;
   final Optional<PubspecLock> pubspecLock;
+  final File pubspecLockFile;
 
   bool get isFlutterPackage =>
       pubspecYaml.dependencies.any((d) => d.package() == 'flutter');

lib/src/find_circular_dependencies.dart

@@ -0,0 +1,61 @@
+/*
+ * MIT License
+ *
+ * Copyright (c) 2020 Alexei Sintotski
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ */
+
+import 'package:pubspec_lock/pubspec_lock.dart';
+
+import 'dart_package/dart_package.dart';
+import 'generic_dependency_usage_report.dart';
+
+/// Finds circular dependencies in the provided set of
+/// pubspec.lock content and generates report on inconsistent package usage.
+Iterable<DependencyUsageReport<PackageDependency>> findCircularDependencies(
+  Map<DartPackage, PubspecLock> pubspecLocks,
+) sync* {
+  for (final package in pubspecLocks.entries) {
+    final packagesDependingOnCurrentPackage = pubspecLocks.entries
+        .where((pubspecLock) => pubspecLock.key != package.key)
+        .where(
+          (pubspecLock) => pubspecLock.value.packages.any((dependency) =>
+              package.key.pubspecYaml.name == dependency.package()),
+        )
+        .map((pubspecLock) => pubspecLock.key.pubspecYaml.name)
+        .toList();
+
+    final circularDependencies = package.value.packages
+        .where((dependency) =>
+            packagesDependingOnCurrentPackage.contains(dependency.package()))
+        .toSet();
+
+    if (circularDependencies.isNotEmpty) {
+      yield DependencyUsageReport(
+        dependencyName: package.key.pubspecYaml.name,
+        references: {
+          for (final dependency in circularDependencies)
+            dependency: [package.key.pubspecYaml.name],
+        },
+      );
+    }
+  }
+}

lib/src/find_inconsistent_dependencies.dart

@@ -25,6 +25,7 @@
 
 import 'package:pubspec_lock/pubspec_lock.dart';
 
+import 'dart_package/dart_package.dart';
 import 'generic_dependency_usage_report.dart';
 
 /// Finds inconsistent set of external dependencies in the provided set of
@@ -38,7 +39,7 @@ import 'generic_dependency_usage_report.dart';
 /// compared as strings, not semantically.
 ///
 List<DependencyUsageReport<PackageDependency>> findInconsistentDependencies(
-  Map<String, PubspecLock> pubspecLocks,
+  Map<DartPackage, PubspecLock> pubspecLocks,
 ) {
   final dependencies = _collectAllDependencies(pubspecLocks).toSet();
   final externalDependencies =
@@ -56,7 +57,7 @@ List<DependencyUsageReport<PackageDependency>> findInconsistentDependencies(
 }
 
 Iterable<PackageDependency> _collectAllDependencies(
-  Map<String, PubspecLock> pubspecLocks,
+  Map<DartPackage, PubspecLock> pubspecLocks,
 ) =>
     pubspecLocks.values.expand((pubspecLock) => pubspecLock.packages);
 
@@ -106,7 +107,7 @@ Iterable<PackageDependency> _filterOutConsistentDependencies(
 List<DependencyUsageReport<PackageDependency>> _createReport(
   Iterable<PackageDependency> dependencies,
   Map<PackageDependency, PackageDependency> normalizedDependencyMap,
-  Map<String, PubspecLock> pubspecLocks,
+  Map<DartPackage, PubspecLock> pubspecLocks,
 ) {
   final names = dependencies.map((d) => d.package()).toSet();
   return names
@@ -115,13 +116,14 @@ List<DependencyUsageReport<PackageDependency>> _createReport(
           dependencyName: name,
           references: Map.fromEntries(
             dependencies
-                .where((d) => d.package() == name)
-                .where((d) => normalizedDependencyMap.containsKey(d))
+                .where((dependency) => dependency.package() == name)
+                .where((dependency) =>
+                    normalizedDependencyMap.containsKey(dependency))
                 .map(
-                  (d) => MapEntry(
-                    normalizedDependencyMap[d]!,
+                  (dependency) => MapEntry(
+                    normalizedDependencyMap[dependency]!,
                     _referencesToDependency(
-                      normalizedDependencyMap[d]!,
+                      normalizedDependencyMap[dependency]!,
                       pubspecLocks,
                     ),
                   ),
@@ -133,10 +135,12 @@ List<DependencyUsageReport<PackageDependency>> _createReport(
 }
 
 List<String> _referencesToDependency(
-  PackageDependency d,
-  Map<String, PubspecLock> pubspecLocks,
+  PackageDependency dependency,
+  Map<DartPackage, PubspecLock> pubspecLocks,
 ) =>
     [
       for (final pubspecLock in pubspecLocks.entries)
-        if (pubspecLock.value.packages.contains(d)) ...[pubspecLock.key]
+        if (pubspecLock.value.packages.contains(dependency)) ...[
+          pubspecLock.key.pubspecLockFile.path
+        ]
     ];

test/dart_package_utils.dart

@@ -0,0 +1,13 @@
+import 'package:borg/src/dart_package/dart_package.dart';
+import 'package:plain_optional/plain_optional.dart';
+
+DartPackage dartPackage(String name) => DartPackage(
+      path: '/$name',
+      tryToReadFileSync: (path) {
+        if (path.endsWith('pubspec.yaml')) {
+          return Optional('name: $name');
+        }
+
+        return const Optional.none();
+      },
+    );