From 8b4529c6523babac2b9e6094357e0f34a4de3626 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Wed, 28 Jun 2023 11:33:51 +0000 Subject: [PATCH 1/2] Warn when publishing breaking release with deprecated members --- lib/src/validator.dart | 2 + .../validator/breaking_with_deprecated.dart | 104 ++++++++++++++++++ .../breaking_with_deprecated_test.dart | 43 ++++++++ 3 files changed, 149 insertions(+) create mode 100644 lib/src/validator/breaking_with_deprecated.dart create mode 100644 test/validator/breaking_with_deprecated_test.dart diff --git a/lib/src/validator.dart b/lib/src/validator.dart index 07ef9b36c..f7ed3fdf2 100644 --- a/lib/src/validator.dart +++ b/lib/src/validator.dart @@ -12,6 +12,7 @@ import 'entrypoint.dart'; import 'log.dart' as log; import 'sdk.dart'; import 'validator/analyze.dart'; +import 'validator/breaking_with_deprecated.dart'; import 'validator/changelog.dart'; import 'validator/compiled_dartdoc.dart'; import 'validator/dependency.dart'; @@ -159,6 +160,7 @@ abstract class Validator { PubspecTypoValidator(), LeakDetectionValidator(), SizeValidator(), + RemoveDeprecatedOnBreakingReleaseValidator(), ]; final context = ValidationContext( diff --git a/lib/src/validator/breaking_with_deprecated.dart b/lib/src/validator/breaking_with_deprecated.dart new file mode 100644 index 000000000..7507c5d2f --- /dev/null +++ b/lib/src/validator/breaking_with_deprecated.dart @@ -0,0 +1,104 @@ +// Copyright (c) 2020, 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 'dart:async'; + +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:collection/collection.dart' show IterableExtension; +import 'package:path/path.dart' as p; +import 'package:pub_semver/pub_semver.dart'; +import 'package:source_span/source_span.dart'; + +import '../dart.dart'; +import '../exceptions.dart'; +import '../io.dart'; +import '../package_name.dart'; +import '../validator.dart'; + +/// Gives a warning when releasing a breaking version containing @Deprecated +/// annotations. +class RemoveDeprecatedOnBreakingReleaseValidator extends Validator { + @override + Future validate() async { + final hostedSource = entrypoint.cache.hosted; + List existingVersions; + try { + existingVersions = await entrypoint.cache.getVersions( + hostedSource.refFor(entrypoint.root.name, url: serverUrl.toString()), + ); + } on PackageNotFoundException { + existingVersions = []; + } + existingVersions.sort((a, b) => a.version.compareTo(b.version)); + + final currentVersion = entrypoint.root.pubspec.version; + + final previousRelease = existingVersions + .lastWhereOrNull((id) => id.version < entrypoint.root.version); + + if (previousRelease != null && + !VersionConstraint.compatibleWith(previousRelease.version) + .allows(currentVersion)) { + // A breaking release. + final packagePath = p.normalize(p.absolute(entrypoint.rootDir)); + final analysisContextManager = AnalysisContextManager(packagePath); + for (var file in filesBeneath('lib', recursive: true).where( + (file) => + p.extension(file) == '.dart' && + !p.isWithin(p.join(entrypoint.root.dir, 'lib', 'src'), file), + )) { + final unit = analysisContextManager.parse(file); + for (final declaration in unit.declarations) { + warnIfDeprecated(declaration, file); + if (declaration is ClassOrAugmentationDeclaration) { + for (final member in declaration.members) { + warnIfDeprecated(member, file); + } + } + if (declaration is MixinOrAugmentationDeclaration) { + for (final member in declaration.members) { + warnIfDeprecated(member, file); + } + } + if (declaration is EnumDeclaration) { + for (final member in declaration.members) { + warnIfDeprecated(member, file); + } + } + } + } + } + } + + /// Warn if [declaration] has a This is a syntactic check only, and therefore + /// imprecise but much faster than doing resolution. + /// + /// Cases where this will break down: + /// ``` + /// const d = Deprecated('Please don't use'); + /// @d class P {} // False negative. + /// ``` + /// + /// ``` + /// import 'dart:core as core'; + /// import 'mylib.dart' show Deprecated; + /// + /// @Deprecated() class A {} // False positive + /// ``` + void warnIfDeprecated(Declaration declaration, String file) { + for (final commentOrAnnotation in declaration.sortedCommentAndAnnotations) { + if (commentOrAnnotation + case Annotation(name: SimpleIdentifier(name: 'Deprecated'))) { + warnings.add( + SourceFile.fromString(readTextFile(file), url: file) + .span(commentOrAnnotation.offset, + commentOrAnnotation.offset + commentOrAnnotation.length) + .message( + 'You are about to publish a breaking release. Consider removing this deprecated declaration.', + ), + ); + } + } + } +} diff --git a/test/validator/breaking_with_deprecated_test.dart b/test/validator/breaking_with_deprecated_test.dart new file mode 100644 index 000000000..ed034d926 --- /dev/null +++ b/test/validator/breaking_with_deprecated_test.dart @@ -0,0 +1,43 @@ +// Copyright (c) 2022, 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 'package:test/test.dart'; + +import '../descriptor.dart' as d; +import '../test_pub.dart'; +import 'utils.dart'; + +void main() { + test('should only warn when publishing a breaking release ', () async { + final server = await servePackages(); + await d.dir(appPath, [ + d.validPubspec(extras: {'version': '2.0.0'}), + d.file('LICENSE', 'Eh, do what you want.'), + d.file('README.md', "This package isn't real."), + d.file('CHANGELOG.md', '# 2.0.0\nFirst version\n'), + d.dir('lib', [ + d.file( + 'test_pkg.dart', + "@Deprecated('Stop using this please') int i = 1;", + ), + d.dir('src', [ + d.file( + 'support.dart', + "@Deprecated('Stop using this please') class B {}", + ), + ]) + ]), + ]).create(); + // No earlier versions, so not a breaking release. + await expectValidation(); + server.serve('test_pkg', '1.0.0'); + await expectValidationWarning( + allOf( + contains('Consider removing this deprecated declaration.'), + contains('int i'), + isNot(contains('class B')), + ), + ); + }); +} From 1b2ec52e29bf2d70b077c619484ce47c11133d38 Mon Sep 17 00:00:00 2001 From: Sigurd Meldgaard Date: Wed, 28 Jun 2023 11:57:20 +0000 Subject: [PATCH 2/2] Trailing commas --- lib/src/validator/breaking_with_deprecated.dart | 6 ++++-- test/validator/breaking_with_deprecated_test.dart | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/src/validator/breaking_with_deprecated.dart b/lib/src/validator/breaking_with_deprecated.dart index 7507c5d2f..fa940731d 100644 --- a/lib/src/validator/breaking_with_deprecated.dart +++ b/lib/src/validator/breaking_with_deprecated.dart @@ -92,8 +92,10 @@ class RemoveDeprecatedOnBreakingReleaseValidator extends Validator { case Annotation(name: SimpleIdentifier(name: 'Deprecated'))) { warnings.add( SourceFile.fromString(readTextFile(file), url: file) - .span(commentOrAnnotation.offset, - commentOrAnnotation.offset + commentOrAnnotation.length) + .span( + commentOrAnnotation.offset, + commentOrAnnotation.offset + commentOrAnnotation.length, + ) .message( 'You are about to publish a breaking release. Consider removing this deprecated declaration.', ), diff --git a/test/validator/breaking_with_deprecated_test.dart b/test/validator/breaking_with_deprecated_test.dart index ed034d926..b25dd320c 100644 --- a/test/validator/breaking_with_deprecated_test.dart +++ b/test/validator/breaking_with_deprecated_test.dart @@ -26,7 +26,7 @@ void main() { 'support.dart', "@Deprecated('Stop using this please') class B {}", ), - ]) + ]), ]), ]).create(); // No earlier versions, so not a breaking release.