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',
+ },
+ );
+ });
+}
