Output validation errors on stdout, slightly prettier formatted (#4424)
diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index 5a8e4a7..892dc38 100644 --- a/lib/src/command/lish.dart +++ b/lib/src/command/lish.dart
@@ -466,7 +466,7 @@ ? _publicationFromEntrypoint() : _publicationFromArchive(_fromArchive)); if (dryRun) { - log.warning(publication.warningsCountMessage); + log.message(publication.warningsCountMessage); if (publication.warningCount != 0) { overrideExitCode(DATA); }
diff --git a/lib/src/validator.dart b/lib/src/validator.dart index 80c41a0..ee69ee6 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart
@@ -185,36 +185,26 @@ .addAll([for (final validator in validators) ...validator.warnings]); errors.addAll([for (final validator in validators) ...validator.errors]); - if (errors.isNotEmpty) { - final s = errors.length > 1 ? 's' : ''; - log.error('Package validation found the following error$s:'); - for (var error in errors) { - log.error("* ${error.split('\n').join('\n ')}"); - } - log.error(''); - } + String presentDiagnostics(List<String> diagnostics) => diagnostics + .map((diagnostic) => "* ${diagnostic.split('\n').join('\n ')}\n") + .join('\n'); + final sections = <String>[]; - if (warnings.isNotEmpty) { - final s = warnings.length > 1 ? 's' : ''; - log.warning( - 'Package validation found the following potential issue$s:', - ); - for (var warning in warnings) { - log.warning("* ${warning.split('\n').join('\n ')}"); + for (final (kind, diagnostics) in [ + ('error', errors), + ('potential issue', warnings), + ('hint', hints), + ]) { + if (diagnostics.isNotEmpty) { + final s = diagnostics.length > 1 ? 's' : ''; + final count = diagnostics.length > 1 ? '${diagnostics.length} ' : ''; + sections.add( + 'Package validation found the following $count$kind$s:\n' + '${presentDiagnostics(diagnostics)}', + ); } - log.warning(''); } - - if (hints.isNotEmpty) { - final s = hints.length > 1 ? 's' : ''; - log.warning( - 'Package validation found the following hint$s:', - ); - for (var hint in hints) { - log.warning("* ${hint.split('\n').join('\n ')}"); - } - log.warning(''); - } + log.message(sections.join('\n')); }); }
diff --git a/lib/src/validator/dependency_override.dart b/lib/src/validator/dependency_override.dart index edefa8c..feeea6e 100644 --- a/lib/src/validator/dependency_override.dart +++ b/lib/src/validator/dependency_override.dart
@@ -12,7 +12,7 @@ /// absence thereof). class DependencyOverrideValidator extends Validator { @override - Future validate() { + Future<void> validate() async { final overridden = MapKeySet(context.entrypoint.workspaceRoot.allOverridesInWorkspace); final dev = MapKeySet(package.devDependencies); @@ -31,6 +31,5 @@ Please be extra careful when publishing.'''); } - return Future.value(); } }
diff --git a/pubspec.lock b/pubspec.lock index 03806cc..dff072b 100644 --- a/pubspec.lock +++ b/pubspec.lock
@@ -5,23 +5,23 @@ dependency: transitive description: name: _fe_analyzer_shared - sha256: c57b02f47e021c9d7ced6d2e28824b315e0fd585578274bc4c2a5db0626f154a + sha256: "45cfa8471b89fb6643fe9bf51bd7931a76b8f5ec2d65de4fb176dba8d4f22c77" url: "https://pub.dev" source: hosted - version: "75.0.0" + version: "73.0.0" _macros: dependency: transitive description: dart source: sdk - version: "0.3.3" + version: "0.3.2" analyzer: dependency: "direct main" description: name: analyzer - sha256: ef226c581b7cd875f734125b1b9928df3db08cc85ff87ce7d9be89a677aaee23 + sha256: "4959fec185fe70cce007c57e9ab6983101dbe593d2bf8bbfb4453aaec0cf470a" url: "https://pub.dev" source: hosted - version: "6.10.0" + version: "6.8.0" args: dependency: "direct main" description: @@ -178,10 +178,10 @@ dependency: transitive description: name: lints - sha256: "4a16b3f03741e1252fda5de3ce712666d010ba2122f8e912c94f9f7b90e1a4c3" + sha256: "3315600f3fb3b135be672bf4a178c55f274bebe368325ae18462c89ac1e3b413" url: "https://pub.dev" source: hosted - version: "5.1.0" + version: "5.0.0" logging: dependency: transitive description: @@ -194,10 +194,10 @@ dependency: transitive description: name: macros - sha256: "1d9e801cd66f7ea3663c45fc708450db1fa57f988142c64289142c9b7ee80656" + sha256: "0acaed5d6b7eab89f63350bccd82119e6c602df0f391260d0e32b5e23db79536" url: "https://pub.dev" source: hosted - version: "0.1.3-main.0" + version: "0.1.2-main.4" matcher: dependency: transitive description: @@ -479,4 +479,4 @@ source: hosted version: "2.2.1" sdks: - dart: ">=3.6.0-0 <4.0.0" + dart: ">=3.5.0 <4.0.0"
diff --git a/test/lish/dry_run_package_validation_has_a_warning_test.dart b/test/lish/dry_run_package_validation_has_a_warning_test.dart index b8fa476..c748a91 100644 --- a/test/lish/dry_run_package_validation_has_a_warning_test.dart +++ b/test/lish/dry_run_package_validation_has_a_warning_test.dart
@@ -28,7 +28,7 @@ await pub.shouldExit(exit_codes.DATA); expect( - pub.stderr, + pub.stdout, emitsThrough('Package has 1 warning.'), ); });
diff --git a/test/lish/dry_run_package_validation_has_no_warnings_test.dart b/test/lish/dry_run_package_validation_has_no_warnings_test.dart index 08c84e9..fabef41 100644 --- a/test/lish/dry_run_package_validation_has_no_warnings_test.dart +++ b/test/lish/dry_run_package_validation_has_no_warnings_test.dart
@@ -18,6 +18,6 @@ final pub = await startPublish(globalServer, args: ['--dry-run']); await pub.shouldExit(exit_codes.SUCCESS); - expect(pub.stderr, emitsThrough('Package has 0 warnings.')); + expect(pub.stdout, emitsThrough('Package has 0 warnings.')); }); }
diff --git a/test/lish/force_publishes_if_there_are_warnings_test.dart b/test/lish/force_publishes_if_there_are_warnings_test.dart index a09b75b..49fca21 100644 --- a/test/lish/force_publishes_if_there_are_warnings_test.dart +++ b/test/lish/force_publishes_if_there_are_warnings_test.dart
@@ -42,20 +42,17 @@ }); await pub.shouldExit(exit_codes.SUCCESS); - final stderrLines = await pub.stderr.rest.toList(); + final stdoutLines = await pub.stdout.rest.toList(); expect( - stderrLines, + stdoutLines, allOf([ contains('Package validation found the following potential issue:'), contains( '* Your dependency on "foo" should have a version constraint. ' 'For example:', ), + contains('Message from server: Package test_pkg 1.0.0 uploaded!'), ]), ); - expect( - pub.stdout, - emitsThrough('Message from server: Package test_pkg 1.0.0 uploaded!'), - ); }); }
diff --git a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt index f773bf6..80c3a95 100644 --- a/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt +++ b/test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt
@@ -138,8 +138,8 @@ Total compressed archive size: <1 KB. Validating package... The server may enforce additional checks. -[STDERR] -[STDERR] Package has 0 warnings. + +Package has 0 warnings. -------------------------------- END OF OUTPUT ---------------------------------
diff --git a/test/testdata/goldens/validator/validations_output_test/Layout of publication warnings.txt b/test/testdata/goldens/validator/validations_output_test/Layout of publication warnings.txt new file mode 100644 index 0000000..c85af45 --- /dev/null +++ b/test/testdata/goldens/validator/validations_output_test/Layout of publication warnings.txt
@@ -0,0 +1,65 @@ +# GENERATED BY: test/validator/validations_output_test.dart + +## Section 0 +$ pub publish --dry-run +Resolving dependencies... +Downloading packages... +! bar 1.0.0 (overridden) ++ foo 1.0.0 +Changed 1 dependency! +Publishing myapp 0.0.0 to http://localhost:$PORT: +├── bin +│ └── main.dart (<1 KB) +└── pubspec.yaml (<1 KB) + +Total compressed archive size: <1 KB. +Validating package... +Package validation found the following 4 errors: +* You must have a LICENSE file in the root directory. + An open-source license helps ensure people can legally use your code. + +* Your pubspec.yaml is missing a "description" field. + +* Your pubspec.yaml is missing a "version" field. + +* line 1, column 1 of bin/main.dart: foo is in the `dev_dependencies` section of `pubspec.yaml`. Packages used in bin/ must be declared in the `dependencies` section. + ╷ + 1 │ import 'package:foo/foo.dart'; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + ╵ + +Package validation found the following 5 potential issues: +* `dart analyze` found the following issue(s): + Analyzing bin, pubspec.yaml... + + warning - bin/main.dart:1:8 - Unused import: 'package:foo/foo.dart'. Try removing the import directive. - unused_import + + 1 issue found. + + +* It's strongly recommended to include a "homepage" or "repository" field in your pubspec.yaml + +* Your dependency on "bar" should have a version constraint. For example: + + dependencies: + bar: ^1.0.0 + + Without a constraint, you're promising to support all future versions of "bar". + +* Please add a README.md file that describes your package. + +* Please add a `CHANGELOG.md` to your package. See https://dart.dev/tools/pub/publishing#important-files. + +Package validation found the following hint: +* Non-dev dependencies are overridden in pubspec.yaml. + + This indicates you are not testing your package against the same versions of its + dependencies that users will have when they use it. + + This might be necessary for packages with cyclic dependencies. + + Please be extra careful when publishing. +[STDERR] Sorry, your package is missing some requirements and can't be published yet. +[STDERR] For more information, see: https://dart.dev/tools/pub/cmd/pub-lish. +[EXIT CODE] 65 +
diff --git a/test/validator/analyze_test.dart b/test/validator/analyze_test.dart index 02addba..6d62230 100644 --- a/test/validator/analyze_test.dart +++ b/test/validator/analyze_test.dart
@@ -80,7 +80,7 @@ ]).create(); await expectValidation( - error: allOf([ + message: allOf([ contains( "The 'http' protocol shouldn't be used because it isn't secure. " "Try using a secure protocol, such as 'https'.", @@ -132,7 +132,7 @@ ]).create(); await expectValidation( - error: allOf([ + message: allOf([ contains('`dart analyze` found the following issue(s):'), contains('Analyzing bin, lib, build.dart, link.dart, pubspec.yaml...'), contains('error -'),
diff --git a/test/validator/dependency_test.dart b/test/validator/dependency_test.dart index 858df16..7823d45 100644 --- a/test/validator/dependency_test.dart +++ b/test/validator/dependency_test.dart
@@ -33,12 +33,12 @@ } Future<void> expectValidation({ - Object? error, + Object? message, int exitCode = 0, Map<String, String> environment = const {}, }) async { await runPub( - error: error ?? contains('Package has 0 warnings.'), + output: message ?? contains('Package has 0 warnings.'), args: ['publish', '--dry-run'], // workingDirectory: d.path(appPath), exitCode: exitCode, @@ -47,25 +47,25 @@ } Future<void> expectValidationWarning( - Object? error, { + Object? message, { int count = 1, Map<String, String> environment = const {}, }) async { - if (error is String) error = contains(error); + if (message is String) message = contains(message); await expectValidation( - error: allOf([error, contains('Package has $count warning')]), + message: allOf([message, contains('Package has $count warning')]), exitCode: DATA, environment: environment, ); } Future<void> expectValidationError( - String text, { + String message, { Map<String, String> environment = const {}, }) async { await expectValidation( - error: allOf([ - contains(text), + message: allOf([ + contains(message), contains('Package validation found the following error:'), ]), exitCode: DATA,
diff --git a/test/validator/file_case_test.dart b/test/validator/file_case_test.dart index cc98a32..1762b3a 100644 --- a/test/validator/file_case_test.dart +++ b/test/validator/file_case_test.dart
@@ -15,9 +15,9 @@ import '../descriptor.dart' as d; import '../test_pub.dart'; -Future<void> expectValidation(Matcher error, int exitCode) async { +Future<void> expectValidation(Matcher output, int exitCode) async { await runPub( - error: error, + output: output, args: ['publish', '--dry-run'], workingDirectory: d.path(appPath), exitCode: exitCode, @@ -30,7 +30,7 @@ await d.dir(appPath, [d.file('Pubspec.yaml')]).create(); await expectValidation( allOf( - contains('Package validation found the following error:'), + matches(r'Package validation found the following \d* ?errors?:'), contains( 'The file ./pubspec.yaml and ./Pubspec.yaml only differ in capitalization.', ),
diff --git a/test/validator/flutter_constraint_test.dart b/test/validator/flutter_constraint_test.dart index 95217f4..c2c63f5 100644 --- a/test/validator/flutter_constraint_test.dart +++ b/test/validator/flutter_constraint_test.dart
@@ -7,9 +7,9 @@ import '../descriptor.dart' as d; import '../test_pub.dart'; -Future<void> expectValidation(Matcher error, int exitCode) async { +Future<void> expectValidation(Matcher output, int exitCode) async { await runPub( - error: error, + output: output, args: ['publish', '--dry-run'], environment: { 'FLUTTER_ROOT': fakeFlutterRoot.io.path,
diff --git a/test/validator/git_status_test.dart b/test/validator/git_status_test.dart index f92653a..0ca1875 100644 --- a/test/validator/git_status_test.dart +++ b/test/validator/git_status_test.dart
@@ -10,14 +10,14 @@ import '../test_pub.dart'; Future<void> expectValidation( - Matcher error, + Matcher output, int exitCode, { List<String> extraArgs = const [], Map<String, String> environment = const {}, String? workingDirectory, }) async { await runPub( - error: error, + output: output, args: ['publish', '--dry-run', ...extraArgs], environment: environment, workingDirectory: workingDirectory ?? d.path(appPath),
diff --git a/test/validator/gitignore_test.dart b/test/validator/gitignore_test.dart index 58c431f..ea73c6d 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart
@@ -12,13 +12,13 @@ import '../test_pub.dart'; Future<void> expectValidation( - Matcher error, + Matcher output, int exitCode, { Map<String, String> environment = const {}, String? workingDirectory, }) async { await runPub( - error: error, + output: output, args: ['publish', '--dry-run'], environment: environment, workingDirectory: workingDirectory ?? d.path(appPath),
diff --git a/test/validator/utils.dart b/test/validator/utils.dart index 3b3016c..f470134 100644 --- a/test/validator/utils.dart +++ b/test/validator/utils.dart
@@ -25,14 +25,14 @@ } Future<void> expectValidation({ - Object? error, + Object? message, int exitCode = 0, Map<String, String> environment = const {}, List<String>? extraArgs, String? workingDirectory, }) async { await runPub( - error: error ?? contains('Package has 0 warnings.'), + output: message ?? contains('Package has 0 warnings.'), args: ['publish', '--dry-run', ...?extraArgs], // workingDirectory: d.path(appPath), exitCode: exitCode, @@ -48,7 +48,7 @@ }) async { final s = count == 1 ? '' : 's'; await expectValidation( - error: allOf([ + message: allOf([ contains(error), contains('Package has $count warning$s'), ]), @@ -64,7 +64,7 @@ }) async { final s = count == 1 ? '' : 's'; await expectValidation( - error: allOf([ + message: allOf([ contains(hint), contains('and $count hint$s'), ]), @@ -77,7 +77,7 @@ Map<String, String> environment = const {}, }) async { await expectValidation( - error: allOf([ + message: allOf([ contains(text), contains('Package validation found the following error:'), ]),
diff --git a/test/validator/validations_output_test.dart b/test/validator/validations_output_test.dart new file mode 100644 index 0000000..352c8eb --- /dev/null +++ b/test/validator/validations_output_test.dart
@@ -0,0 +1,36 @@ +// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import '../descriptor.dart'; +import '../golden_file.dart'; +import '../test_pub.dart'; + +Future<void> main() async { + testWithGolden('Layout of publication warnings', (ctx) async { + final server = await servePackages(); + server.serve('foo', '1.0.0'); + server.serve('bar', '1.0.0'); + + await dir(appPath, [ + pubspec({ + 'name': 'myapp', + 'dev_dependencies': {'foo': '1.0.0'}, + 'dependencies': {'bar': null}, + 'dependency_overrides': {'bar': '1.0.0'}, + }), + dir('bin', [ + file('main.dart', ''' +import 'package:foo/foo.dart'; +'''), + ]), + ]).create(); + await ctx.run( + ['publish', '--dry-run'], + environment: { + // Use more columns to avoid unintended line breaking. + '_PUB_TEST_TERMINAL_COLUMNS': '200', + }, + ); + }); +}