Report major breaking change type in JSON report and support pub refs without version

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## Version 0.19.1
+- fix JSON report to specify "major" as change type for breaking changes
+- adapt pub ref mechanism to also support omitting the version (and using the latest version if no version has been specified)
+
 ## Version 0.19.0
 - introduces `force-use-flutter` option for all commands to force dart_apitool to use the `flutter` command.
 - extend type usage tracking and fix situations in which types that are used in @visibleForTesting contexts were detected as not exported

lib/src/cli/commands/command_mixin.dart

@@ -69,12 +69,12 @@ OBSOLETE: Has no effect anymore.
         isInCache: false,
       ));
     } else if (ref.isPubRef) {
-      await stdoutSession
-          .writeln('Preparing ${ref.pubPackage!}:${ref.pubVersion!}');
+      await stdoutSession.writeln(
+          'Preparing ${ref.pubPackage!}:${ref.pubVersion ?? 'latest'}');
       await stdoutSession.writeln('Downloading');
       String sourceDir = await PubInteraction.installPackageToCache(
         ref.pubPackage!,
-        ref.pubVersion!,
+        ref.pubVersion,
         stdoutSession: stdoutSession,
       );
       sources.add(SourceItem(
@@ -156,7 +156,7 @@ OBSOLETE: Has no effect anymore.
     if (preparedRef.packageRef.isPubRef) {
       path = PubInteraction.getPackagePathInCache(
           preparedRef.packageRef.pubPackage!,
-          preparedRef.packageRef.pubVersion!);
+          preparedRef.packageRef.pubVersion);
     }
     if (path == null) {
       throw ArgumentError(

lib/src/cli/package_ref.dart

@@ -41,7 +41,11 @@ class PackageRef {
       return null;
     }
     final uri = Uri.parse(ref);
-    return uri.path.replaceAll('/', '');
+    final path = uri.path.replaceAll('/', '');
+    if (path.isEmpty) {
+      return null;
+    }
+    return path;
   }
 
   @override

lib/src/diff/report/json_diff_reporter.dart

@@ -81,7 +81,11 @@ class JsonDiffReporter extends DiffReporter {
                 'changeDescription': c.changeDescription,
                 'changeCode': c.changeCode.code,
                 'isBreaking': c.isBreaking,
-                'type': c.type.requiresMinorBump ? 'minor' : 'patch',
+                'type': c.isBreaking
+                    ? 'major'
+                    : c.type.requiresMinorBump
+                        ? 'minor'
+                        : 'patch',
               })
           .toList();
       final childNodes = n.children.values

lib/src/tooling/pub_interaction.dart

@@ -3,15 +3,17 @@ import 'dart:io';
 import 'package:dart_apitool/src/tooling/dart_interaction.dart';
 import 'package:freezed_annotation/freezed_annotation.dart';
 import 'package:path/path.dart' as path;
+import 'package:pub_semver/pub_semver.dart';
 import '../errors/run_dart_error.dart';
 import '../utils/utils.dart';
 
 /// helper class for interactions with pub
 abstract class PubInteraction {
   /// installs a package to the pub cache and returns the path to it throws [RunDartError] on failure
+  /// if no [version] is provided dart will decide what version to use ("the best of all known versions")
   static Future<String> installPackageToCache(
     String packageName,
-    String version, {
+    String? version, {
     StdoutSession? stdoutSession,
   }) async {
     await DartInteraction.runDartCommand(
@@ -20,10 +22,17 @@ abstract class PubInteraction {
         'cache',
         'add',
         packageName,
-        '-v $version',
+        if (version != null) '-v $version',
       ],
       stdoutSession: stdoutSession,
     );
+    if (version == null) {
+      await stdoutSession?.writeln(
+          'No version for $packageName specified, using latest version');
+      final latestVersion = getLatestVersionInCacheFor(packageName);
+      version = latestVersion.toString();
+      await stdoutSession?.writeln('Using version $version');
+    }
     return getPackagePathInCache(packageName, version);
   }
 
@@ -56,8 +65,52 @@ abstract class PubInteraction {
     return cacheDir;
   }
 
+  static Version getLatestVersionInCacheFor(String packageName) {
+    final allVersions = getAllPackageVersionsForPackageInCache(packageName);
+    final versions = allVersions.keys.toList(growable: false);
+    if (versions.isEmpty) {
+      throw RunDartError('No version of $packageName found');
+    }
+
+    versions.sort();
+    return versions.last;
+  }
+
+  static Map<Version, String> getAllPackageVersionsForPackageInCache(
+      String packageName) {
+    final cacheDir = pubCacheDir;
+    final hostedDir = path.join(cacheDir, 'hosted');
+
+    final versions = <Version, String>{};
+
+    final repositoryDirs =
+        Directory(hostedDir).listSync().whereType<Directory>().toList();
+
+    for (final repositoryDir in repositoryDirs) {
+      for (final potentialPackageDir
+          in repositoryDir.listSync().whereType<Directory>()) {
+        final packageDirName = path.basename(potentialPackageDir.path);
+        final dirParts = packageDirName.split('-').toList();
+        final versionPart = dirParts.last;
+        dirParts.removeLast();
+        final packageNamePart = dirParts.join('-');
+
+        if (packageNamePart != packageName) {
+          continue;
+        }
+
+        final version = Version.parse(versionPart);
+        versions[version] = potentialPackageDir.path;
+      }
+    }
+    return versions;
+  }
+
   /// returns the cache path of a package with the given [packageName] and [version]
-  static String getPackagePathInCache(String packageName, String version) {
+  /// if [version] is null the latest version is used
+  static String getPackagePathInCache(String packageName, String? version) {
+    version ??= getLatestVersionInCacheFor(packageName).toString();
+
     String? findHostedDirectory(List<String> hostedUrls) {
       for (final hostedUrl in hostedUrls) {
         final packagePath = path.join(hostedUrl, '$packageName-$version');

pubspec.yaml

@@ -2,7 +2,7 @@ name: dart_apitool
 description: A tool to analyze the public API of a package, create a model of it and diff it against another version to check semver.
 repository: https://github.com/bmw-tech/dart_apitool
 
-version: 0.19.0-dev
+version: 0.19.1-dev
 
 environment:
   sdk: ">=3.0.0 <4.0.0"
@@ -31,6 +31,7 @@ dev_dependencies:
   freezed: ^2.2.0
   json_serializable: ^6.3.1
   lints: ^3.0.0
+  mocktail: ^1.0.4
   test: ^1.21.4
 
 executables:

test/integration_tests/cli/diff_command_test.dart

@@ -143,5 +143,25 @@ void main() {
       },
       timeout: integrationTestTimeout,
     );
+
+    test(
+      'diffing cloud_firestore 4.3.1 to latest works',
+      () async {
+        // just some random package for testing the diff command for pub refs without version
+        final diffCommand = DiffCommand();
+        final runner =
+            CommandRunner<int>('dart_apitool_tests', 'Test for dart_apitool')
+              ..addCommand(diffCommand);
+        final exitCode = await runner.run([
+          'diff',
+          '--old',
+          'pub://cloud_firestore/4.3.1',
+          '--new',
+          'pub://cloud_firestore',
+        ]);
+        expect(exitCode, 0);
+      },
+      timeout: integrationTestTimeout,
+    );
   });
 }

test/unit_tests/diff/report/json_diff_reporter_test.dart

@@ -0,0 +1,181 @@
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:dart_apitool/api_tool_cli.dart';
+import 'package:dart_apitool/src/cli/commands/version_check.dart';
+import 'package:dart_apitool/src/diff/report/json_diff_reporter.dart';
+import 'package:pub_semver/pub_semver.dart';
+import 'package:test/test.dart';
+
+import 'package:mocktail/mocktail.dart';
+
+class MockFile extends Mock implements File {}
+
+void main() {
+  group('JsonDiffReporter', () {
+    late JsonDiffReporter reporter;
+    final anyVersionCheckResult = VersionCheckResult.success(
+      oldVersion: Version(1, 0, 0),
+      newVersion: Version(2, 0, 0),
+      explanation: '',
+    );
+    final oldPackageRef = PackageRef('pub://package/1.0.0');
+    final newPackageRef = PackageRef('pub://package/2.0.0');
+    late MockFile mockFile;
+    late StringBuffer collectedFileContent;
+
+    setUp(() {
+      collectedFileContent = StringBuffer();
+      mockFile = MockFile();
+      reporter = JsonDiffReporter(
+        oldPackageRef: oldPackageRef,
+        newPackageRef: newPackageRef,
+        outputFile: mockFile,
+      );
+      when(() => mockFile.writeAsString(any())).thenAnswer((invocation) async {
+        collectedFileContent.write(invocation.positionalArguments.first);
+        return mockFile;
+      });
+      when(() => mockFile.path).thenReturn('mocked_file_path.json');
+    });
+
+    PackageApiDiffResult createEmptyDiffResult() {
+      return PackageApiDiffResult();
+    }
+
+    void addBreakingChange(
+      PackageApiDiffResult diffResult, {
+      ApiChangeCode changeCode = ApiChangeCode.ci01,
+    }) {
+      diffResult.addApiChange(ApiChange(
+        changeCode: changeCode,
+        changeDescription: 'Test breaking change: ${changeCode.name}',
+        contextTrace: [],
+        isExperimental: false,
+        type: ApiChangeType.remove,
+      ));
+    }
+
+    void addNonBreakingChange(
+      PackageApiDiffResult diffResult, {
+      ApiChangeCode changeCode = ApiChangeCode.ci02,
+      ApiChangeType changeType = ApiChangeType.addCompatiblePatch,
+    }) {
+      diffResult.addApiChange(ApiChange(
+        changeCode: changeCode,
+        changeDescription: 'Test non-breaking change: ${changeCode.name}',
+        contextTrace: [],
+        isExperimental: false,
+        type: changeType,
+      ));
+    }
+
+    test('Can be instantiated', () {
+      // the setup would have failed already
+      expect(reporter, isA<JsonDiffReporter>());
+    });
+
+    test('Can handle empty diff report', () async {
+      final diffResult = createEmptyDiffResult();
+      await reporter.generateReport(diffResult, anyVersionCheckResult);
+      final jsonReport = jsonDecode(collectedFileContent.toString());
+      expect(jsonReport['report']['noChangesDetected'], isTrue);
+    });
+    test('Can handle diff report with only one breaking change', () async {
+      final diffResult = createEmptyDiffResult();
+      addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01);
+      await reporter.generateReport(diffResult, anyVersionCheckResult);
+      final jsonReport = jsonDecode(collectedFileContent.toString());
+      expect(jsonReport['report']['breakingChanges'], isNotNull);
+      expect(jsonReport['report']['breakingChanges']['children'].length, 1);
+      expect(
+          jsonReport['report']['breakingChanges']['children']
+              .single['changeCode'],
+          ApiChangeCode.ci01.code);
+      expect(jsonReport['report']['breakingChanges']['children'].single['type'],
+          'major');
+    });
+    test('Can handle diff report with multiple breaking changes', () async {
+      final diffResult = createEmptyDiffResult();
+      addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01);
+      addBreakingChange(diffResult, changeCode: ApiChangeCode.ci04);
+      await reporter.generateReport(diffResult, anyVersionCheckResult);
+      final jsonReport = jsonDecode(collectedFileContent.toString());
+      expect(jsonReport['report']['breakingChanges'], isNotNull);
+      expect(jsonReport['report']['breakingChanges']['children'].length, 2);
+      expect(
+          jsonReport['report']['breakingChanges']['children'][0]['changeCode'],
+          ApiChangeCode.ci01.code);
+      expect(
+          jsonReport['report']['breakingChanges']['children'][1]['changeCode'],
+          ApiChangeCode.ci04.code);
+    });
+    test('Can handle diff report with only one non-breaking change', () async {
+      final diffResult = createEmptyDiffResult();
+      addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02);
+      await reporter.generateReport(diffResult, anyVersionCheckResult);
+      final jsonReport = jsonDecode(collectedFileContent.toString());
+      expect(jsonReport['report']['nonBreakingChanges'], isNotNull);
+      expect(jsonReport['report']['nonBreakingChanges']['children'].length, 1);
+      expect(
+          jsonReport['report']['nonBreakingChanges']['children']
+              .single['changeCode'],
+          ApiChangeCode.ci02.code);
+      expect(
+          jsonReport['report']['nonBreakingChanges']['children'].single['type'],
+          'patch');
+    });
+    test('Can handle diff report with multiple non-breaking changes', () async {
+      final diffResult = createEmptyDiffResult();
+      addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02);
+      addNonBreakingChange(diffResult,
+          changeCode: ApiChangeCode.ci05,
+          changeType: ApiChangeType.addCompatibleMinor);
+      await reporter.generateReport(diffResult, anyVersionCheckResult);
+      final jsonReport = jsonDecode(collectedFileContent.toString());
+      expect(jsonReport['report']['nonBreakingChanges'], isNotNull);
+      expect(jsonReport['report']['nonBreakingChanges']['children'].length, 2);
+      expect(
+          jsonReport['report']['nonBreakingChanges']['children'][0]
+              ['changeCode'],
+          ApiChangeCode.ci02.code);
+      expect(
+          jsonReport['report']['nonBreakingChanges']['children'][1]
+              ['changeCode'],
+          ApiChangeCode.ci05.code);
+      expect(jsonReport['report']['nonBreakingChanges']['children'][0]['type'],
+          'patch');
+      expect(jsonReport['report']['nonBreakingChanges']['children'][1]['type'],
+          'minor');
+    });
+    test('Can handle diff report with breaking and non-breaking changes',
+        () async {
+      final diffResult = createEmptyDiffResult();
+      addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01);
+      addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02);
+      addBreakingChange(diffResult, changeCode: ApiChangeCode.ci04);
+      addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci05);
+
+      await reporter.generateReport(diffResult, anyVersionCheckResult);
+      final jsonReport = jsonDecode(collectedFileContent.toString());
+      expect(jsonReport['report']['breakingChanges'], isNotNull);
+      expect(jsonReport['report']['breakingChanges']['children'].length, 2);
+      expect(
+          jsonReport['report']['breakingChanges']['children'][0]['changeCode'],
+          ApiChangeCode.ci01.code);
+      expect(
+          jsonReport['report']['breakingChanges']['children'][1]['changeCode'],
+          ApiChangeCode.ci04.code);
+      expect(jsonReport['report']['nonBreakingChanges'], isNotNull);
+      expect(jsonReport['report']['nonBreakingChanges']['children'].length, 2);
+      expect(
+          jsonReport['report']['nonBreakingChanges']['children'][0]
+              ['changeCode'],
+          ApiChangeCode.ci02.code);
+      expect(
+          jsonReport['report']['nonBreakingChanges']['children'][1]
+              ['changeCode'],
+          ApiChangeCode.ci05.code);
+    });
+  });
+}