diff --git a/lib/src/command/lish.dart b/lib/src/command/lish.dart index 5a8e4a7e6..892dc3861 100644 --- a/lib/src/command/lish.dart +++ b/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); } diff --git a/lib/src/validator.dart b/lib/src/validator.dart index 80c41a079..ee69ee651 100644 --- a/lib/src/validator.dart +++ b/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 diagnostics) => diagnostics + .map((diagnostic) => "* ${diagnostic.split('\n').join('\n ')}\n") + .join('\n'); + final sections = []; + + 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')); }); } diff --git a/lib/src/validator/dependency_override.dart b/lib/src/validator/dependency_override.dart index edefa8cfc..feeea6e57 100644 --- a/lib/src/validator/dependency_override.dart +++ b/lib/src/validator/dependency_override.dart @@ -12,7 +12,7 @@ import '../validator.dart'; /// absence thereof). class DependencyOverrideValidator extends Validator { @override - Future validate() { + Future 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(); } } diff --git a/pubspec.lock b/pubspec.lock index 03806cc6f..dff072b5a 100644 --- a/pubspec.lock +++ b/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" 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 b8fa47677..c748a9156 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 @@ void main() { 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 08c84e915..fabef4193 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 @@ 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.')); }); } 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 a09b75b1a..49fca21eb 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 @@ 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!'), - ); }); } 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 f773bf642..80c3a95ae 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 @@ 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 --------------------------------- 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 000000000..c85af45e9 --- /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 02addbaee..6d6223091 100644 --- a/test/validator/analyze_test.dart +++ b/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 -'), diff --git a/test/validator/dependency_test.dart b/test/validator/dependency_test.dart index 858df1686..7823d4585 100644 --- a/test/validator/dependency_test.dart +++ b/test/validator/dependency_test.dart @@ -33,12 +33,12 @@ d.DirectoryDescriptor package({ } Future expectValidation({ - Object? error, + Object? message, int exitCode = 0, Map 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 expectValidation({ } Future expectValidationWarning( - Object? error, { + Object? message, { int count = 1, Map 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 expectValidationError( - String text, { + String message, { Map 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 cc98a32a6..1762b3a10 100644 --- a/test/validator/file_case_test.dart +++ b/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 expectValidation(Matcher error, int exitCode) async { +Future 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.', ), diff --git a/test/validator/flutter_constraint_test.dart b/test/validator/flutter_constraint_test.dart index 95217f488..c2c63f5ca 100644 --- a/test/validator/flutter_constraint_test.dart +++ b/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 expectValidation(Matcher error, int exitCode) async { +Future 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 f92653a21..0ca1875b3 100644 --- a/test/validator/git_status_test.dart +++ b/test/validator/git_status_test.dart @@ -10,14 +10,14 @@ import '../descriptor.dart' as d; import '../test_pub.dart'; Future expectValidation( - Matcher error, + Matcher output, int exitCode, { List extraArgs = const [], Map 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 58c431fbd..ea73c6d24 100644 --- a/test/validator/gitignore_test.dart +++ b/test/validator/gitignore_test.dart @@ -12,13 +12,13 @@ import '../descriptor.dart' as d; import '../test_pub.dart'; Future expectValidation( - Matcher error, + Matcher output, int exitCode, { Map 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 3b3016cf4..f47013448 100644 --- a/test/validator/utils.dart +++ b/test/validator/utils.dart @@ -25,14 +25,14 @@ Future expectValidationDeprecated( } Future expectValidation({ - Object? error, + Object? message, int exitCode = 0, Map environment = const {}, List? 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 @@ Future expectValidationWarning( }) async { final s = count == 1 ? '' : 's'; await expectValidation( - error: allOf([ + message: allOf([ contains(error), contains('Package has $count warning$s'), ]), @@ -64,7 +64,7 @@ Future expectValidationHint( }) async { final s = count == 1 ? '' : 's'; await expectValidation( - error: allOf([ + message: allOf([ contains(hint), contains('and $count hint$s'), ]), @@ -77,7 +77,7 @@ Future expectValidationError( Map 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 000000000..352c8eb1a --- /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 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', + }, + ); + }); +}