Output validation errors on stdout, slightly prettier formatted (#4424)

lib/src/command/lish.dart

@@ -466,7 +466,7 @@ the \$PUB_HOSTED_URL environment variable.''',
         ? _publicationFromEntrypoint()
         : _publicationFromArchive(_fromArchive));
     if (dryRun) {
-      log.warning(publication.warningsCountMessage);
+      log.message(publication.warningsCountMessage);
       if (publication.warningCount != 0) {
         overrideExitCode(DATA);
       }

lib/src/validator.dart

@@ -185,36 +185,26 @@ abstract class Validator {
           .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  ')}");
+      String presentDiagnostics(List<String> diagnostics) => diagnostics
+          .map((diagnostic) => "* ${diagnostic.split('\n').join('\n  ')}\n")
+          .join('\n');
+      final sections = <String>[];
+
+      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.error('');
-      }
-
-      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  ')}");
-        }
-        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'));
     });
   }
 

lib/src/validator/dependency_override.dart

@@ -12,7 +12,7 @@ import '../validator.dart';
 /// 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 @@ This might be necessary for packages with cyclic dependencies.
 
 Please be extra careful when publishing.''');
     }
-    return Future.value();
   }
 }

pubspec.lock

@@ -5,23 +5,23 @@ packages:
     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 @@ packages:
     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 @@ packages:
     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 @@ packages:
     source: hosted
     version: "2.2.1"
 sdks:
-  dart: ">=3.6.0-0 <4.0.0"
+  dart: ">=3.5.0 <4.0.0"

test/lish/dry_run_package_validation_has_a_warning_test.dart

@@ -28,7 +28,7 @@ void main() {
 
     await pub.shouldExit(exit_codes.DATA);
     expect(
-      pub.stderr,
+      pub.stdout,
       emitsThrough('Package has 1 warning.'),
     );
   });

test/lish/dry_run_package_validation_has_no_warnings_test.dart

@@ -18,6 +18,6 @@ void main() {
     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.'));
   });
 }

test/lish/force_publishes_if_there_are_warnings_test.dart

@@ -42,20 +42,17 @@ void main() {
     });
 
     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!'),
-    );
   });
 }

test/testdata/goldens/directory_option_test/commands taking a --directory~-C parameter work.txt

@@ -138,8 +138,8 @@ Publishing test_pkg 1.0.0 to http://localhost:$PORT:
 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 ---------------------------------
 

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
+

test/validator/analyze_test.dart

@@ -80,7 +80,7 @@ analyzer:
     ]).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 @@ void main() {
     ]).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 -'),

test/validator/dependency_test.dart

@@ -33,12 +33,12 @@ d.DirectoryDescriptor package({
 }
 
 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> expectValidation({
 }
 
 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,

test/validator/file_case_test.dart

@@ -15,9 +15,9 @@ import 'package:test/test.dart';
 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 @@ void main() {
     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.',
         ),

test/validator/flutter_constraint_test.dart

@@ -7,9 +7,9 @@ import 'package:test/test.dart';
 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,

test/validator/git_status_test.dart

@@ -10,14 +10,14 @@ import '../descriptor.dart' as d;
 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),

test/validator/gitignore_test.dart

@@ -12,13 +12,13 @@ import '../descriptor.dart' as d;
 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),