diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fdf675..38b3ecf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Version 0.17.0 - bump dart SDK requirements to >=3.0.0 - Search downloaded packages in entire .pub-cache hosted directory in case path namining changes +- adds information about the package versions and the version check result to the reports (console, json, markdown) ## Version 0.16.3 - adds more diff result reporting options (cli, json, markdown) diff --git a/lib/src/cli/commands/diff_command.dart b/lib/src/cli/commands/diff_command.dart index 1127816..e874160 100644 --- a/lib/src/cli/commands/diff_command.dart +++ b/lib/src/cli/commands/diff_command.dart @@ -198,19 +198,23 @@ You may want to do this if you want to make sure } })(); + VersionCheckResult? versionCheckResult; + if (versionCheckMode != VersionCheckMode.none) { + versionCheckResult = VersionCheck.check( + diffResult: diffResult, + oldPackageApi: oldPackageApi, + newPackageApi: newPackageApi, + ignorePrerelease: ignorePrerelease, + versionCheckMode: versionCheckMode, + ); + } + stdout.writeln('-- Generating report using: ${reporter.reporterName} --'); - await reporter.generateReport(diffResult); - - if (versionCheckMode != VersionCheckMode.none && - !VersionCheck.versionChangeMatchesChanges( - diffResult: diffResult, - oldPackageApi: oldPackageApi, - newPackageApi: newPackageApi, - ignorePrerelease: ignorePrerelease, - versionCheckMode: versionCheckMode)) { + await reporter.generateReport(diffResult, versionCheckResult); + if (versionCheckResult?.success ?? true) { + return 0; + } else { return -1; } - - return 0; } } diff --git a/lib/src/cli/commands/version_check.dart b/lib/src/cli/commands/version_check.dart index 63ed2ab..6b1c432 100644 --- a/lib/src/cli/commands/version_check.dart +++ b/lib/src/cli/commands/version_check.dart @@ -1,23 +1,43 @@ import 'dart:io'; -import 'package:colorize/colorize.dart'; import 'package:pub_semver/pub_semver.dart'; import '../../../api_tool.dart'; +class VersionCheckResult { + final bool success; + final Version oldVersion; + final Version newVersion; + final Version? neededVersion; + final String explanation; + + VersionCheckResult.success({ + required this.oldVersion, + required this.newVersion, + Version? neededVersion, + required this.explanation, + }) : success = true, + neededVersion = neededVersion ?? newVersion; + + VersionCheckResult.failure({ + required this.oldVersion, + required this.newVersion, + required this.neededVersion, + required this.explanation, + }) : success = false; +} + /// helper class to check if the version change matches the changes abstract class VersionCheck { /// checks if the version change between [oldPackageApi] and [newPackageApi] matches the changes in [diffResult] - static bool versionChangeMatchesChanges({ + static VersionCheckResult check({ required PackageApiDiffResult diffResult, required PackageApi oldPackageApi, required PackageApi newPackageApi, required bool ignorePrerelease, required VersionCheckMode versionCheckMode, }) { - stdout.writeln(''); stdout.writeln('Checking Package version'); - stdout.writeln(''); if (oldPackageApi.packageVersion == null) { throw PackageApiDiffError( message: 'Old package doesn\'t contain a version]'); @@ -36,14 +56,21 @@ abstract class VersionCheck { diffResult.apiChanges.any((change) => !change.type.requiresMinorBump); if (versionCheckMode == VersionCheckMode.none) { - stdout.writeln('Skipping version check completely'); - return true; + return VersionCheckResult.success( + oldVersion: oldVersion, + newVersion: newVersion, + explanation: + 'Skipping version check completely as the version check mode is $versionCheckMode', + ); } if (versionCheckMode == VersionCheckMode.onlyBreakingChanges && !containsBreakingChanges) { - stdout.writeln( - 'Skipping version check because there are no breaking changes'); - return true; + return VersionCheckResult.success( + oldVersion: oldVersion, + newVersion: newVersion, + explanation: + 'Skipping version check because there are no breaking changes', + ); } if (ignorePrerelease) { @@ -57,28 +84,35 @@ abstract class VersionCheck { final oldVersionWithoutPreRelease = Version.parse(oldVersion.toString()); oldVersionWithoutPreRelease.preRelease.clear(); if (oldVersionWithoutPreRelease <= newVersion) { - stdout.writeln( - 'Skipping version check because the old version is a pre-release and the new version is the same or higher without the pre-release part'); - return true; + return VersionCheckResult.success( + oldVersion: oldVersion, + newVersion: newVersion, + explanation: + 'Skipping version check because the old version is a pre-release and the new version is the same or higher without the pre-release part', + ); } } if (newVersion.isPreRelease) { // pre-release. We don't look at differentiation between breaking and non-breaking changes - stdout.writeln( - 'We got a pre release. We only check if there are any changes'); + final prefix = + 'We got a pre release. We only check if there are any changes.'; if (containsAnyChanges && oldVersion >= newVersion) { - stdout.writeln( - 'Got "${Colorize(newVersion.toString()).bold()}" expected > "${Colorize(oldVersion.toString()).bold()}" (pre-release but changes)'); - return false; + return VersionCheckResult.failure( + oldVersion: oldVersion, + newVersion: newVersion, + neededVersion: null, + explanation: + '$prefix Got "$newVersion" expected > "$oldVersion" (pre-release but changes)', + ); } - stdout.writeln(Colorize('New version is OK!').green()); - final explaination = containsAnyChanges - ? 'which is > "${Colorize(oldVersion.toString()).bold()}" (pre-release but changes)' + final explanation = containsAnyChanges + ? 'which is > "$oldVersion" (pre-release but changes)' : 'and no changes'; - stdout.writeln( - 'Got "${Colorize(newVersion.toString()).bold()}" $explaination'); - return true; + return VersionCheckResult.success( + oldVersion: oldVersion, + newVersion: newVersion, + explanation: '$prefix Got "$newVersion" $explanation'); } Version expectedMinVersion = @@ -99,19 +133,22 @@ abstract class VersionCheck { } } - stdout.writeln('Old version: "$oldVersion"'); - stdout.writeln( - 'Expecting minimum version: "$expectedMinVersion" ($versionExplanation)'); if (newVersion < expectedMinVersion) { - stdout.writeln(Colorize('New Version is too low!').red()); - stdout.writeln( - 'Got "${Colorize(newVersion.toString()).bold()}" expected >= "${Colorize(expectedMinVersion.toString()).bold()}"'); - return false; + return VersionCheckResult.failure( + oldVersion: oldVersion, + newVersion: newVersion, + neededVersion: expectedMinVersion, + explanation: + 'Got "$newVersion" expected >= "$expectedMinVersion" ($versionExplanation)', + ); } else { - stdout.writeln(Colorize('New version is OK!').green()); - stdout.writeln( - 'Got "${Colorize(newVersion.toString()).bold()}" which is >= "${Colorize(expectedMinVersion.toString()).bold()}"'); - return true; + return VersionCheckResult.success( + oldVersion: oldVersion, + newVersion: newVersion, + neededVersion: expectedMinVersion, + explanation: + 'Got "$newVersion" which is >= "$expectedMinVersion" ($versionExplanation)', + ); } } } diff --git a/lib/src/diff/report/console_diff_reporter.dart b/lib/src/diff/report/console_diff_reporter.dart index 473992e..0cb0976 100644 --- a/lib/src/diff/report/console_diff_reporter.dart +++ b/lib/src/diff/report/console_diff_reporter.dart @@ -4,6 +4,7 @@ import 'package:colorize/colorize.dart'; import 'package:console/console.dart'; import '../../../api_tool.dart'; +import '../../cli/commands/version_check.dart'; import 'diff_reporter.dart'; class ConsoleDiffReporter extends DiffReporter { @@ -11,7 +12,10 @@ class ConsoleDiffReporter extends DiffReporter { final String reporterName = 'Console Reporter'; @override - Future generateReport(PackageApiDiffResult diffResult) async { + Future generateReport( + PackageApiDiffResult diffResult, + VersionCheckResult? versionCheckResult, + ) async { void printChanges(bool breaking) { final changes = _printApiChangeNode(diffResult.rootNode, breaking); if (changes == null) { @@ -32,6 +36,23 @@ class ConsoleDiffReporter extends DiffReporter { } else { stdout.writeln('No changes detected!'); } + + if (versionCheckResult != null) { + stdout.writeln('Version Check'); + if (versionCheckResult.success) { + stdout.writeln(Colorize('New version is OK!').green()); + } else { + stdout.writeln(Colorize('New Version is too low!').red()); + } + stdout.writeln(); + stdout.writeln('Old version: "${versionCheckResult.oldVersion}"'); + stdout.writeln('New version: "${versionCheckResult.newVersion}"'); + if (versionCheckResult.neededVersion != null) { + stdout.writeln('Needed version: "${versionCheckResult.neededVersion}"'); + } + stdout.writeln(); + stdout.writeln(versionCheckResult.explanation); + } } String? _printApiChangeNode(ApiChangeTreeNode node, bool breaking) { diff --git a/lib/src/diff/report/diff_reporter.dart b/lib/src/diff/report/diff_reporter.dart index 2e43820..dcbb19a 100644 --- a/lib/src/diff/report/diff_reporter.dart +++ b/lib/src/diff/report/diff_reporter.dart @@ -1,7 +1,12 @@ +import 'package:dart_apitool/src/cli/commands/version_check.dart'; + import '../../../api_tool.dart'; abstract class DiffReporter { String get reporterName; - Future generateReport(PackageApiDiffResult diffResult); + Future generateReport( + PackageApiDiffResult diffResult, + VersionCheckResult? versionCheckResult, + ); } diff --git a/lib/src/diff/report/json_diff_reporter.dart b/lib/src/diff/report/json_diff_reporter.dart index 80439cc..ede6988 100644 --- a/lib/src/diff/report/json_diff_reporter.dart +++ b/lib/src/diff/report/json_diff_reporter.dart @@ -2,6 +2,7 @@ import 'dart:convert'; import 'dart:io'; import '../../../api_tool_cli.dart'; +import '../../cli/commands/version_check.dart'; import 'diff_reporter.dart'; class JsonDiffReporter extends DiffReporter { @@ -17,8 +18,11 @@ class JsonDiffReporter extends DiffReporter { }); @override - Future generateReport(PackageApiDiffResult diffResult) async { - final jsonReport = { + Future generateReport( + PackageApiDiffResult diffResult, + VersionCheckResult? versionCheckResult, + ) async { + final report = { 'reportName': 'API Changes Report', 'apiToolInfo': { 'toolName': 'dart_apitool', @@ -27,15 +31,33 @@ class JsonDiffReporter extends DiffReporter { 'generatedAt': DateTime.now().toUtc().toLocal().toString(), 'oldRef': oldPackageRef.ref, 'newRef': newPackageRef.ref - }, - 'report': {}, + } }; + if (versionCheckResult != null) { + report['version'] = { + 'success': versionCheckResult.success, + 'old': versionCheckResult.oldVersion.toString(), + 'new': versionCheckResult.newVersion.toString(), + 'needed': versionCheckResult.neededVersion?.toString(), + 'explanation': versionCheckResult.explanation.toString(), + }; + } + + report['report'] = getChanges(diffResult); + // Write the JSON report to a file + await outputFile.writeAsString(jsonEncode(report)); + + print('JSON report generated at ${outputFile.path}'); + } + + Map getChanges(PackageApiDiffResult diffResult) { + final changeReport = {}; void addChanges(bool breaking) { final changes = _printApiChangeNode(diffResult.rootNode, breaking); if (changes != null) { - jsonReport['report'] - [breaking ? 'breakingChanges' : 'nonBreakingChanges'] = changes; + changeReport[breaking ? 'breakingChanges' : 'nonBreakingChanges'] = + changes; } } @@ -44,13 +66,9 @@ class JsonDiffReporter extends DiffReporter { addChanges(true); // Breaking changes addChanges(false); // Non-breaking changes } else { - jsonReport['report']['noChangesDetected'] = true; + changeReport['noChangesDetected'] = true; } - - // Write the JSON report to a file - await outputFile.writeAsString(jsonEncode(jsonReport)); - - print('JSON report generated at ${outputFile.path}'); + return changeReport; } Map? _printApiChangeNode( diff --git a/lib/src/diff/report/markdown_diff_reporter.dart b/lib/src/diff/report/markdown_diff_reporter.dart index 4b53401..11a6667 100644 --- a/lib/src/diff/report/markdown_diff_reporter.dart +++ b/lib/src/diff/report/markdown_diff_reporter.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:dart_apitool/api_tool_cli.dart'; +import '../../cli/commands/version_check.dart'; import 'diff_reporter.dart'; const changeCodesReadMe = @@ -20,7 +21,10 @@ class MarkdownDiffReporter extends DiffReporter { }); @override - Future generateReport(PackageApiDiffResult diffResult) async { + Future generateReport( + PackageApiDiffResult diffResult, + VersionCheckResult? versionCheckResult, + ) async { final markdownReport = StringBuffer(); markdownReport ..writeln('# API Changes Report') @@ -62,6 +66,23 @@ class MarkdownDiffReporter extends DiffReporter { ..writeln('No changes detected!'); } + if (versionCheckResult != null) { + stdout.writeln('### Version Check'); + if (versionCheckResult.success) { + stdout.writeln('New version is OK!'); + } else { + stdout.writeln('New Version is too low!'); + } + stdout.writeln(); + stdout.writeln('Old version: "${versionCheckResult.oldVersion}"'); + stdout.writeln('New version: "${versionCheckResult.newVersion}"'); + if (versionCheckResult.neededVersion != null) { + stdout.writeln('Needed version: "${versionCheckResult.neededVersion}"'); + } + stdout.writeln(); + stdout.writeln(versionCheckResult.explanation); + } + // Write the Markdown report to a file await outputFile.writeAsString(markdownReport.toString()); diff --git a/lib/src/tooling/pub_interaction.dart b/lib/src/tooling/pub_interaction.dart index 4df9251..3daa7f6 100644 --- a/lib/src/tooling/pub_interaction.dart +++ b/lib/src/tooling/pub_interaction.dart @@ -63,8 +63,6 @@ abstract class PubInteraction { final packagePath = path.join(hostedUrl, '$packageName-$version'); if (Directory(packagePath).existsSync()) { return packagePath; - } else { - return null; } } return null; diff --git a/test/unit_tests/cli/commands/version_check_test.dart b/test/unit_tests/cli/commands/version_check_test.dart index 9e42ba1..9301efc 100644 --- a/test/unit_tests/cli/commands/version_check_test.dart +++ b/test/unit_tests/cli/commands/version_check_test.dart @@ -40,27 +40,27 @@ void main() { group('Version Check', () { test('is fine with breaking change and major version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addBreaking]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), newPackageApi: createTestPackageApi(packageVersion: '2.0.0'), ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test('is NOT fine with breaking change and minor version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addBreaking]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), newPackageApi: createTestPackageApi(packageVersion: '1.1.0'), ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isFalse); + expect(versionChangeCheckResult.success, isFalse); }); test('is fine with non-breaking change and minor version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addCompatibleMinor]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), @@ -68,10 +68,10 @@ void main() { ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test('is NOT fine with non-breaking change and patch version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addCompatibleMinor]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), @@ -79,11 +79,11 @@ void main() { ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isFalse); + expect(versionChangeCheckResult.success, isFalse); }); test('is fine with non-breaking (patch) change and patch version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addCompatiblePatch]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), @@ -91,11 +91,11 @@ void main() { ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test('is fine with non-breaking (patch) change and minor version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addCompatiblePatch]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), @@ -103,11 +103,11 @@ void main() { ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test('is fine with non-breaking (patch) change and major version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addCompatiblePatch]), oldPackageApi: createTestPackageApi(packageVersion: '1.0.0'), @@ -115,51 +115,51 @@ void main() { ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test( 'is fine with breaking change and only prerelease tag in version change', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addBreaking]), oldPackageApi: createTestPackageApi(packageVersion: '2.0.0-dev01'), newPackageApi: createTestPackageApi(packageVersion: '2.0.1'), ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test('ignores prerelease tag if ignorePrerelease is set', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addBreaking]), oldPackageApi: createTestPackageApi(packageVersion: '2.0.0'), newPackageApi: createTestPackageApi(packageVersion: '2.1.0-dev01'), ignorePrerelease: true, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isFalse); + expect(versionChangeCheckResult.success, isFalse); }); test('if old and new version have prerelease set, nothing matters', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addBreaking]), oldPackageApi: createTestPackageApi(packageVersion: '2.0.0-dev00'), newPackageApi: createTestPackageApi(packageVersion: '2.1.0-dev01'), ignorePrerelease: false, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isTrue); + expect(versionChangeCheckResult.success, isTrue); }); test( 'if old and new version have prerelease set, the base version still has to be the same or higher for the new package', () { - bool versionChangeCheckResult = VersionCheck.versionChangeMatchesChanges( + final versionChangeCheckResult = VersionCheck.check( diffResult: createDiffResult(changeTypes: [ApiChangeType.addBreaking]), oldPackageApi: createTestPackageApi(packageVersion: '2.1.0-dev00'), newPackageApi: createTestPackageApi(packageVersion: '2.0.0-dev01'), ignorePrerelease: false, versionCheckMode: VersionCheckMode.fully, ); - expect(versionChangeCheckResult, isFalse); + expect(versionChangeCheckResult.success, isFalse); }); }); } diff --git a/test/unit_tests/diff/report/console_diff_reporter_test.dart b/test/unit_tests/diff/report/console_diff_reporter_test.dart index 01aba42..b590b65 100644 --- a/test/unit_tests/diff/report/console_diff_reporter_test.dart +++ b/test/unit_tests/diff/report/console_diff_reporter_test.dart @@ -1,10 +1,17 @@ import 'package:dart_apitool/api_tool.dart'; +import 'package:dart_apitool/src/cli/commands/version_check.dart'; import 'package:dart_apitool/src/diff/report/console_diff_reporter.dart'; +import 'package:pub_semver/pub_semver.dart'; import 'package:test/test.dart'; void main() { group('ConsoleDiffReporter', () { late ConsoleDiffReporter reporter; + final anyVersionCheckResult = VersionCheckResult.success( + oldVersion: Version(0, 0, 0), + newVersion: Version(0, 0, 0), + explanation: '', + ); setUp(() { reporter = ConsoleDiffReporter(); @@ -47,29 +54,34 @@ void main() { test('Can handle empty diff report', () { final diffResult = createEmptyDiffResult(); - expect(() => reporter.generateReport(diffResult), returnsNormally); + expect(() => reporter.generateReport(diffResult, anyVersionCheckResult), + returnsNormally); }); test('Can handle diff report with only one breaking change', () { final diffResult = createEmptyDiffResult(); addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01); - expect(() => reporter.generateReport(diffResult), returnsNormally); + expect(() => reporter.generateReport(diffResult, anyVersionCheckResult), + returnsNormally); }); test('Can handle diff report with multiple breaking changes', () { final diffResult = createEmptyDiffResult(); addBreakingChange(diffResult, changeCode: ApiChangeCode.ci01); addBreakingChange(diffResult, changeCode: ApiChangeCode.ci04); - expect(() => reporter.generateReport(diffResult), returnsNormally); + expect(() => reporter.generateReport(diffResult, anyVersionCheckResult), + returnsNormally); }); test('Can handle diff report with only one non-breaking change', () { final diffResult = createEmptyDiffResult(); addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02); - expect(() => reporter.generateReport(diffResult), returnsNormally); + expect(() => reporter.generateReport(diffResult, anyVersionCheckResult), + returnsNormally); }); test('Can handle diff report with multiple non-breaking changes', () { final diffResult = createEmptyDiffResult(); addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02); addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci05); - expect(() => reporter.generateReport(diffResult), returnsNormally); + expect(() => reporter.generateReport(diffResult, anyVersionCheckResult), + returnsNormally); }); test('Can handle diff report with breaking and non-breaking changes', () { final diffResult = createEmptyDiffResult(); @@ -77,7 +89,8 @@ void main() { addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci02); addBreakingChange(diffResult, changeCode: ApiChangeCode.ci04); addNonBreakingChange(diffResult, changeCode: ApiChangeCode.ci05); - expect(() => reporter.generateReport(diffResult), returnsNormally); + expect(() => reporter.generateReport(diffResult, anyVersionCheckResult), + returnsNormally); }); }); }