From f380255255ce156ded14ffb1844c3bbb755f8701 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Mon, 23 Dec 2024 13:56:16 -0500 Subject: [PATCH 01/21] Removed check for .dart_tool --- lib/src/dependency_validator.dart | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 926fe85..8047ac9 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -33,11 +33,6 @@ Future run() async { logger.shout(red.wrap('pubspec.yaml not found')); exit(1); } - if (!File('.dart_tool/package_config.json').existsSync()) { - logger.shout(red.wrap( - 'No .dart_tool/package_config.json file found, please run "pub get" first.')); - exit(1); - } DepValidatorConfig config; final configFile = File('dart_dependency_validator.yaml'); @@ -45,7 +40,8 @@ Future run() async { config = DepValidatorConfig.fromYaml(configFile.readAsStringSync()); } else { final pubspecConfig = PubspecDepValidatorConfig.fromYaml( - File('pubspec.yaml').readAsStringSync()); + File('pubspec.yaml').readAsStringSync(), + ); if (pubspecConfig.isNotEmpty) { logger.warning(yellow.wrap( 'Configuring dependency_validator in pubspec.yaml is deprecated.\n' @@ -53,6 +49,7 @@ Future run() async { } config = pubspecConfig.dependencyValidator; } + final excludes = config.exclude .map((s) { try { From 78a5ebe019a7ab679ff5345e06cb00f6fbc712f8 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Mon, 23 Dec 2024 14:40:57 -0500 Subject: [PATCH 02/21] Support Pub Workspaces --- lib/src/dependency_validator.dart | 29 +++++++++++++++++++---------- lib/src/utils.dart | 9 +++++++++ pubspec.yaml | 7 +++++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 8047ac9..f6b65c0 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -28,19 +28,19 @@ import 'pubspec_config.dart'; import 'utils.dart'; /// Check for missing, under-promoted, over-promoted, and unused dependencies. -Future run() async { - if (!File('pubspec.yaml').existsSync()) { +Future run({String root = "."}) async { + if (!File('$root/pubspec.yaml').existsSync()) { logger.shout(red.wrap('pubspec.yaml not found')); exit(1); } DepValidatorConfig config; - final configFile = File('dart_dependency_validator.yaml'); + final configFile = File('$root/dart_dependency_validator.yaml'); if (configFile.existsSync()) { config = DepValidatorConfig.fromYaml(configFile.readAsStringSync()); } else { final pubspecConfig = PubspecDepValidatorConfig.fromYaml( - File('pubspec.yaml').readAsStringSync(), + File('$root/pubspec.yaml').readAsStringSync(), ); if (pubspecConfig.isNotEmpty) { logger.warning(yellow.wrap( @@ -67,13 +67,22 @@ Future run() async { logger.fine('ignored packages:\n${bulletItems(ignoredPackages)}\n'); // Read and parse the analysis_options.yaml in the current working directory. - final optionsIncludePackage = getAnalysisOptionsIncludePackage(); + final optionsIncludePackage = getAnalysisOptionsIncludePackage(path: root); // Read and parse the pubspec.yaml in the current working directory. - final pubspecFile = File('pubspec.yaml'); + final pubspecFile = File('$root/pubspec.yaml'); final pubspec = Pubspec.parse(pubspecFile.readAsStringSync(), sourceUrl: pubspecFile.uri); + if (pubspec.isWorkspaceRoot) { + logger.fine('In a workspace. Recursing through sub-packages...'); + for (final package in pubspec.workspace ?? []) { + await run(root: package); + logger.info(''); + } + return; + } + logger.info('Validating dependencies for ${pubspec.name}...'); if (!config.allowPins) { @@ -89,7 +98,7 @@ Future run() async { logger.fine('dev_dependencies:\n' '${bulletItems(devDeps)}\n'); - final publicDirs = ['bin/', 'lib/']; + final publicDirs = ['$root/bin/', '$root/lib/']; final publicDartFiles = [ for (final dir in publicDirs) ...listDartFilesIn(dir, excludes), ]; @@ -132,11 +141,11 @@ Future run() async { final publicDirGlobs = [for (final dir in publicDirs) Glob('$dir**')]; final nonPublicDartFiles = - listDartFilesIn('./', [...excludes, ...publicDirGlobs]); + listDartFilesIn('$root/', [...excludes, ...publicDirGlobs]); final nonPublicScssFiles = - listScssFilesIn('./', [...excludes, ...publicDirGlobs]); + listScssFilesIn('$root/', [...excludes, ...publicDirGlobs]); final nonPublicLessFiles = - listLessFilesIn('./', [...excludes, ...publicDirGlobs]); + listLessFilesIn('$root/', [...excludes, ...publicDirGlobs]); logger ..fine('non-public dart files:\n' diff --git a/lib/src/utils.dart b/lib/src/utils.dart index b83ee4a..d2b3ef7 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -183,3 +183,12 @@ DependencyPinEvaluation inspectVersionForPins(VersionConstraint constraint) { return DependencyPinEvaluation.emptyPin; } + +/// Utilities for Pubspec objects. +extension PubspecUtils on Pubspec { + /// Whether this package is the root of a Pub Workspace. + bool get isWorkspaceRoot => workspace != null; + + /// Whether this package is a sub-package in a Pub Workspace. + bool get isInWorkspace => resolution == 'workspace'; +} diff --git a/pubspec.yaml b/pubspec.yaml index 9d6916e..285179c 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -28,5 +28,12 @@ dev_dependencies: test_descriptor: ^2.0.0 workiva_analysis_options: ^1.2.2 +dependency_overrides: + pubspec_parse: + git: + url: https://github.com/Levi-Lesches/dart-lang-tools + ref: pubspec-workspaces-1814 + path: pkgs/pubspec_parse + executables: dependency_validator: From 1e2fa91d37c401efe1c5ecd9f3a3bd63a349b9d7 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 31 Dec 2024 23:34:24 -0500 Subject: [PATCH 03/21] Allow processing sub-packages AND root package --- lib/src/dependency_validator.dart | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index f6b65c0..6322180 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -80,7 +80,6 @@ Future run({String root = "."}) async { await run(root: package); logger.info(''); } - return; } logger.info('Validating dependencies for ${pubspec.name}...'); @@ -139,13 +138,14 @@ Future run({String root = "."}) async { '${bulletItems(packagesUsedInPublicFiles)}\n'); final publicDirGlobs = [for (final dir in publicDirs) Glob('$dir**')]; + final subpackageGlobs = [for (final subpackage in pubspec.workspace ?? []) Glob('$subpackage**')]; final nonPublicDartFiles = - listDartFilesIn('$root/', [...excludes, ...publicDirGlobs]); + listDartFilesIn('$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs]); final nonPublicScssFiles = - listScssFilesIn('$root/', [...excludes, ...publicDirGlobs]); + listScssFilesIn('$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs]); final nonPublicLessFiles = - listLessFilesIn('$root/', [...excludes, ...publicDirGlobs]); + listLessFilesIn('$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs]); logger ..fine('non-public dart files:\n' From 53e430c399d7692331af616063b4b12d2c5ea202 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 31 Dec 2024 23:41:12 -0500 Subject: [PATCH 04/21] Return status separately from each package --- bin/dependency_validator.dart | 2 +- lib/src/dependency_validator.dart | 32 +++++++++++++++---------------- lib/src/utils.dart | 2 +- pubspec.yaml | 2 +- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/bin/dependency_validator.dart b/bin/dependency_validator.dart index bc762f8..6aab908 100644 --- a/bin/dependency_validator.dart +++ b/bin/dependency_validator.dart @@ -78,5 +78,5 @@ void main(List args) async { Logger.root.level = Level.ALL; } - await run(); + await checkPackage(); } diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 6322180..a8e84ef 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -28,10 +28,11 @@ import 'pubspec_config.dart'; import 'utils.dart'; /// Check for missing, under-promoted, over-promoted, and unused dependencies. -Future run({String root = "."}) async { +Future checkPackage({String root = "."}) async { + var result = true; if (!File('$root/pubspec.yaml').existsSync()) { logger.shout(red.wrap('pubspec.yaml not found')); - exit(1); + return false; } DepValidatorConfig config; @@ -59,8 +60,7 @@ Future run({String root = "."}) async { return null; } }) - .where((g) => g != null) - .cast() + .nonNulls .toList(); logger.fine('excludes:\n${bulletItems(excludes.map((g) => g.pattern))}\n'); final ignoredPackages = config.ignore; @@ -72,12 +72,12 @@ Future run({String root = "."}) async { // Read and parse the pubspec.yaml in the current working directory. final pubspecFile = File('$root/pubspec.yaml'); final pubspec = - Pubspec.parse(pubspecFile.readAsStringSync(), sourceUrl: pubspecFile.uri); + Pubspec.parse(pubspecFile.readAsStringSync(), sourceUrl: pubspecFile.uri,); if (pubspec.isWorkspaceRoot) { logger.fine('In a workspace. Recursing through sub-packages...'); for (final package in pubspec.workspace ?? []) { - await run(root: package); + await checkPackage(root: package); logger.info(''); } } @@ -199,7 +199,7 @@ Future run({String root = "."}) async { 'These packages are used in lib/ but are not dependencies:', missingDependencies, ); - exitCode = 1; + result = false; } // Packages that are used outside lib/ but are not dev_dependencies. @@ -221,7 +221,7 @@ Future run({String root = "."}) async { 'These packages are used outside lib/ but are not dev_dependencies:', missingDevDependencies, ); - exitCode = 1; + result = false; } // Packages that are not used in lib/, but are used elsewhere, that are @@ -241,7 +241,7 @@ Future run({String root = "."}) async { 'These packages are only used outside lib/ and should be downgraded to dev_dependencies:', overPromotedDependencies, ); - exitCode = 1; + result = false; } // Packages that are used in lib/, but are dev_dependencies. @@ -257,7 +257,7 @@ Future run({String root = "."}) async { 'These packages are used in lib/ and should be promoted to actual dependencies:', underPromotedDependencies, ); - exitCode = 1; + result = false; } // Packages that are not used anywhere but are dependencies. @@ -275,8 +275,7 @@ Future run({String root = "."}) async { if (packageConfig == null) { logger.severe(red.wrap( 'Could not find package config. Make sure you run `dart pub get` first.')); - exitCode = 1; - return; + return false; } // Remove deps that provide builders that will be applied @@ -303,7 +302,7 @@ Future run({String root = "."}) async { Level.FINE, 'The following packages contain builders that are auto-applied or referenced in "build.yaml"', unusedDependencies, - packagesWithConsumedBuilders); + packagesWithConsumedBuilders,); unusedDependencies.removeAll(packagesWithConsumedBuilders); // Remove deps that provide executables, those are assumed to be used @@ -325,7 +324,7 @@ Future run({String root = "."}) async { unusedDependencies, nonDevPackagesWithExecutables, ); - exitCode = 1; + result = false; } logIntersection( @@ -353,12 +352,13 @@ Future run({String root = "."}) async { 'These packages may be unused, or you may be using assets from these packages:', unusedDependencies, ); - exitCode = 1; + result = false; } - if (exitCode == 0) { + if (result) { logger.info(green.wrap('✓ No dependency issues found!')); } + return result; } /// Whether a dependency at [path] defines an auto applied builder. diff --git a/lib/src/utils.dart b/lib/src/utils.dart index d2b3ef7..5347296 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -93,7 +93,7 @@ Iterable listFilesWithExtensionIn( /// Logs the given [message] at [level] and lists all of the given [dependencies]. void log(Level level, String message, Iterable dependencies) { final sortedDependencies = dependencies.toList()..sort(); - var combined = [message, bulletItems(sortedDependencies), ''].join('\n'); + var combined = [message, bulletItems(sortedDependencies)].join('\n'); if (level >= Level.SEVERE) { combined = red.wrap(combined)!; } else if (level >= Level.WARNING) { diff --git a/pubspec.yaml b/pubspec.yaml index 285179c..eb710e4 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -4,7 +4,7 @@ description: Checks for missing, under-promoted, over-promoted, and unused depen homepage: https://github.com/Workiva/dependency_validator environment: - sdk: '>=2.19.0 <3.0.0' + sdk: '^3.0.0' dependencies: analyzer: '>=5.4.0 <7.0.0' From e04e1d3b7218c8e9be504235b722e3381f8a5c59 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 31 Dec 2024 23:47:52 -0500 Subject: [PATCH 05/21] Added -C option --- bin/dependency_validator.dart | 10 +++++++++- lib/src/dependency_validator.dart | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/bin/dependency_validator.dart b/bin/dependency_validator.dart index 6aab908..a6556cf 100644 --- a/bin/dependency_validator.dart +++ b/bin/dependency_validator.dart @@ -21,6 +21,7 @@ import 'package:logging/logging.dart'; const String helpArg = 'help'; const String verboseArg = 'verbose'; +const String rootDirArg = 'directory'; const String helpMessage = '''Dependency Validator 2.0 is configured statically via the pubspec.yaml example: @@ -45,6 +46,12 @@ final ArgParser argParser = ArgParser() verboseArg, defaultsTo: false, help: 'Display extra information for debugging.', + ) + ..addOption( + rootDirArg, + abbr: "C", + help: 'Validate dependencies in a subdirectory', + defaultsTo: '.', ); void showHelpAndExit({ExitCode exitCode = ExitCode.success}) { @@ -78,5 +85,6 @@ void main(List args) async { Logger.root.level = Level.ALL; } - await checkPackage(); + final rootDir = argResults.option(rootDirArg) ?? '.'; + await checkPackage(root: rootDir); } diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index a8e84ef..bd193fa 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -28,7 +28,7 @@ import 'pubspec_config.dart'; import 'utils.dart'; /// Check for missing, under-promoted, over-promoted, and unused dependencies. -Future checkPackage({String root = "."}) async { +Future checkPackage({required String root}) async { var result = true; if (!File('$root/pubspec.yaml').existsSync()) { logger.shout(red.wrap('pubspec.yaml not found')); From 42608bebdb959b784fcabbf7606126c56cddef80 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 1 Jan 2025 00:03:00 -0500 Subject: [PATCH 06/21] Added -C option --- bin/dependency_validator.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/dependency_validator.dart b/bin/dependency_validator.dart index a6556cf..a997f44 100644 --- a/bin/dependency_validator.dart +++ b/bin/dependency_validator.dart @@ -85,6 +85,7 @@ void main(List args) async { Logger.root.level = Level.ALL; } + Logger.root.info(''); final rootDir = argResults.option(rootDirArg) ?? '.'; await checkPackage(root: rootDir); } From dfcb42a4195f9ea6e2432c9cbc1e15fd3f11db55 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 1 Jan 2025 00:11:03 -0500 Subject: [PATCH 07/21] Handle exit codes properly --- bin/dependency_validator.dart | 3 ++- lib/src/dependency_validator.dart | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/bin/dependency_validator.dart b/bin/dependency_validator.dart index a997f44..f90b97a 100644 --- a/bin/dependency_validator.dart +++ b/bin/dependency_validator.dart @@ -87,5 +87,6 @@ void main(List args) async { Logger.root.info(''); final rootDir = argResults.option(rootDirArg) ?? '.'; - await checkPackage(root: rootDir); + final result = await checkPackage(root: rootDir); + exit(result ? 0 : 1); } diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index bd193fa..f2f3b54 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -74,10 +74,11 @@ Future checkPackage({required String root}) async { final pubspec = Pubspec.parse(pubspecFile.readAsStringSync(), sourceUrl: pubspecFile.uri,); + var subResult = true; if (pubspec.isWorkspaceRoot) { logger.fine('In a workspace. Recursing through sub-packages...'); for (final package in pubspec.workspace ?? []) { - await checkPackage(root: package); + subResult &= await checkPackage(root: package); logger.info(''); } } @@ -358,7 +359,7 @@ Future checkPackage({required String root}) async { if (result) { logger.info(green.wrap('✓ No dependency issues found!')); } - return result; + return result && subResult; } /// Whether a dependency at [path] defines an auto applied builder. From e82fb02d22e8a7358d10ff31da4061b80ce108f8 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Thu, 2 Jan 2025 20:03:23 -0500 Subject: [PATCH 08/21] Fix path normalization --- lib/src/dependency_validator.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index f2f3b54..02f8985 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -54,7 +54,9 @@ Future checkPackage({required String root}) async { final excludes = config.exclude .map((s) { try { - return Glob(s); + // Globs don't support './' paths. Use posix to avoid '\' paths + final globPath = p.posix.normalize('$root/$s'); + return Glob(globPath); } catch (_, __) { logger.shout(yellow.wrap('invalid glob syntax: "$s"')); return null; From 728b62f19e2bcbbf4aeb50568da37e84cd152459 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Thu, 2 Jan 2025 20:23:52 -0500 Subject: [PATCH 09/21] Formatted --- lib/src/dependency_validator.dart | 37 ++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 02f8985..d37b7e9 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -73,8 +73,10 @@ Future checkPackage({required String root}) async { // Read and parse the pubspec.yaml in the current working directory. final pubspecFile = File('$root/pubspec.yaml'); - final pubspec = - Pubspec.parse(pubspecFile.readAsStringSync(), sourceUrl: pubspecFile.uri,); + final pubspec = Pubspec.parse( + pubspecFile.readAsStringSync(), + sourceUrl: pubspecFile.uri, + ); var subResult = true; if (pubspec.isWorkspaceRoot) { @@ -141,14 +143,22 @@ Future checkPackage({required String root}) async { '${bulletItems(packagesUsedInPublicFiles)}\n'); final publicDirGlobs = [for (final dir in publicDirs) Glob('$dir**')]; - final subpackageGlobs = [for (final subpackage in pubspec.workspace ?? []) Glob('$subpackage**')]; + final subpackageGlobs = [ + for (final subpackage in pubspec.workspace ?? []) Glob('$subpackage**') + ]; - final nonPublicDartFiles = - listDartFilesIn('$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs]); - final nonPublicScssFiles = - listScssFilesIn('$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs]); - final nonPublicLessFiles = - listLessFilesIn('$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs]); + final nonPublicDartFiles = listDartFilesIn( + '$root/', + [...excludes, ...publicDirGlobs, ...subpackageGlobs], + ); + final nonPublicScssFiles = listScssFilesIn( + '$root/', + [...excludes, ...publicDirGlobs, ...subpackageGlobs], + ); + final nonPublicLessFiles = listLessFilesIn( + '$root/', + [...excludes, ...publicDirGlobs, ...subpackageGlobs], + ); logger ..fine('non-public dart files:\n' @@ -302,10 +312,11 @@ Future checkPackage({required String root}) async { } logIntersection( - Level.FINE, - 'The following packages contain builders that are auto-applied or referenced in "build.yaml"', - unusedDependencies, - packagesWithConsumedBuilders,); + Level.FINE, + 'The following packages contain builders that are auto-applied or referenced in "build.yaml"', + unusedDependencies, + packagesWithConsumedBuilders, + ); unusedDependencies.removeAll(packagesWithConsumedBuilders); // Remove deps that provide executables, those are assumed to be used From ccaf5097bcf8dd7aefd119d539c096f41663f3dd Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 7 Jan 2025 19:00:05 -0500 Subject: [PATCH 10/21] Updated pubspec_parse to v1.5.0 --- pubspec.yaml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index eb710e4..6904bed 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -18,7 +18,7 @@ dependencies: package_config: ^2.0.0 path: ^1.8.0 pub_semver: ^2.0.0 - pubspec_parse: ^1.0.0 + pubspec_parse: ^1.5.0 yaml: ^3.1.0 dev_dependencies: @@ -28,12 +28,5 @@ dev_dependencies: test_descriptor: ^2.0.0 workiva_analysis_options: ^1.2.2 -dependency_overrides: - pubspec_parse: - git: - url: https://github.com/Levi-Lesches/dart-lang-tools - ref: pubspec-workspaces-1814 - path: pkgs/pubspec_parse - executables: dependency_validator: From dcfdf834c4d0a9b15fd7e379c57ab64c81b98a78 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 7 Jan 2025 19:01:01 -0500 Subject: [PATCH 11/21] Moved common test functions to test/utils.dart --- test/executable_test.dart | 28 ++-------------- test/utils.dart | 69 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 26 deletions(-) create mode 100644 test/utils.dart diff --git a/test/executable_test.dart b/test/executable_test.dart index e9f8093..23148fa 100644 --- a/test/executable_test.dart +++ b/test/executable_test.dart @@ -19,6 +19,8 @@ import 'package:io/io.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; +import 'utils.dart'; + /// `master` on build_config has a min sdk bound of dart 3.0.0. /// Since dependency_validator is still designed to be used on dart 2 /// code, we still want to run unit tests using this older version @@ -27,32 +29,6 @@ import 'package:test_descriptor/test_descriptor.dart' as d; /// dart 2 as a dependency const buildConfigRef = 'e2c837b48bd3c4428cb40e2bc1a6cf47d45df8cc'; -ProcessResult checkProject(String projectPath, - {List optionalArgs = const []}) { - final pubGetResult = - Process.runSync('dart', ['pub', 'get'], workingDirectory: projectPath); - if (pubGetResult.exitCode != 0) { - return pubGetResult; - } - - final args = [ - 'run', - 'dependency_validator', - // This makes it easier to print(result.stdout) for debugging tests - '--verbose', - ...optionalArgs, - ]; - - return Process.runSync('dart', args, workingDirectory: projectPath); -} - -/// Removes indentation from `'''` string blocks. -String unindent(String multilineString) { - var indent = RegExp(r'^( *)').firstMatch(multilineString)![1]; - assert(indent != null && indent.isNotEmpty); - return multilineString.replaceAll('$indent', ''); -} - void main() { group('dependency_validator', () { late ProcessResult result; diff --git a/test/utils.dart b/test/utils.dart new file mode 100644 index 0000000..bf75076 --- /dev/null +++ b/test/utils.dart @@ -0,0 +1,69 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:dependency_validator/src/dependency_validator.dart'; +import 'package:dependency_validator/src/pubspec_config.dart'; +import 'package:logging/logging.dart'; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +import 'pubspec_to_json.dart'; + +ProcessResult checkProject( + String projectPath, { + List optionalArgs = const [], +}) { + final pubGetResult = Process.runSync( + 'dart', + ['pub', 'get'], + workingDirectory: projectPath, + ); + if (pubGetResult.exitCode != 0) { + return pubGetResult; + } + + final args = [ + 'run', + 'dependency_validator', + // This makes it easier to print(result.stdout) for debugging tests + '--verbose', + ...optionalArgs, + ]; + + return Process.runSync('dart', args, workingDirectory: projectPath); +} + +/// Removes indentation from `'''` string blocks. +String unindent(String multilineString) { + var indent = RegExp(r'^( *)').firstMatch(multilineString)![1]; + assert(indent != null && indent.isNotEmpty); + return multilineString.replaceAll('$indent', ''); +} + +Future checkWorkspace({ + required Pubspec workspacePubspec, + required List workspace, + required Pubspec subpackagePubspec, + required List subpackage, + DepValidatorConfig? workspaceConfig, + DepValidatorConfig? subpackageConfig, + bool checkSubpackage = false, + bool verbose = false, +}) async { + final dir = d.dir('workspace', [ + ...workspace, + d.file('pubspec.yaml', jsonEncode(workspacePubspec.toJson())), + if (workspaceConfig != null) + d.file('dart_dependency_validator.yaml', jsonEncode(workspaceConfig.toJson())), + d.dir('subpackage', [ + ...subpackage, + d.file('pubspec.yaml', jsonEncode(subpackagePubspec.toJson())), + if (subpackageConfig != null) + d.file('dart_dependency_validator.yaml', jsonEncode(subpackageConfig.toJson())), + ]), + ],); + await dir.create(); + final root = checkSubpackage ? "subpackage" : "workspace"; + Logger.root.level = verbose ? Level.ALL : Level.OFF; + return await checkPackage(root: "${d.sandbox}/$root"); +} From 8d65128caccb5b40e49fbdebea5025198f5ba12a Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 7 Jan 2025 19:02:16 -0500 Subject: [PATCH 12/21] Added Config.toJson() and Pubspec.toJson() --- lib/src/pubspec_config.dart | 4 ++- lib/src/pubspec_config.g.dart | 7 +++++ test/pubspec_to_json.dart | 55 +++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 test/pubspec_to_json.dart diff --git a/lib/src/pubspec_config.dart b/lib/src/pubspec_config.dart index 3b0aad1..f89ead5 100644 --- a/lib/src/pubspec_config.dart +++ b/lib/src/pubspec_config.dart @@ -30,7 +30,7 @@ class PubspecDepValidatorConfig { @JsonSerializable( anyMap: true, checked: true, - createToJson: false, + createToJson: true, fieldRename: FieldRename.snake) class DepValidatorConfig { @JsonKey(defaultValue: []) @@ -55,4 +55,6 @@ class DepValidatorConfig { checkedYamlDecode( yamlContent, (m) => DepValidatorConfig.fromJson(m ?? {}), allowNull: true, sourceUrl: sourceUrl); + + Map toJson() => _$DepValidatorConfigToJson(this); } diff --git a/lib/src/pubspec_config.g.dart b/lib/src/pubspec_config.g.dart index d2b7949..3ea887d 100644 --- a/lib/src/pubspec_config.g.dart +++ b/lib/src/pubspec_config.g.dart @@ -41,3 +41,10 @@ DepValidatorConfig _$DepValidatorConfigFromJson(Map json) => $checkedCreate( }, fieldKeyMap: const {'allowPins': 'allow_pins'}, ); + +Map _$DepValidatorConfigToJson(DepValidatorConfig instance) => + { + 'exclude': instance.exclude, + 'ignore': instance.ignore, + 'allow_pins': instance.allowPins, + }; diff --git a/test/pubspec_to_json.dart b/test/pubspec_to_json.dart new file mode 100644 index 0000000..a698457 --- /dev/null +++ b/test/pubspec_to_json.dart @@ -0,0 +1,55 @@ + +import "package:pubspec_parse/pubspec_parse.dart"; + +extension on Map { + Iterable<(K, V)> get records sync* { + for (final entry in entries) { + yield (entry.key, entry.value); + } + } +} + +typedef Json = Map; + +extension on Dependency { + Json toJson() => switch (this) { + SdkDependency(:final sdk, :final version) => { + "sdk": sdk, + "version": version.toString(), + }, + HostedDependency(:final hosted, :final version) => { + if (hosted != null) "hosted": hosted.url.toString(), + "version": version.toString(), + }, + GitDependency(:final url, :final ref, :final path) => { + "git": { + "url": url.toString(), + if (path != null) "ref": ref, + if (path != null) "path": path, + }, + }, + PathDependency(:final path) => { + "path": path.replaceAll(r'\', '/'), + }, + }; +} + +/// An as-needed implementation of `Pubspec.toJson` for testing. +/// +/// See: https://github.com/dart-lang/tools/issues/1801 +extension PubspecToJson on Pubspec { + Json toJson() => { + "name": name, + "environment": { + for (final (sdk, version) in environment.records) + sdk: version.toString(), + }, + if (resolution != null) "resolution": resolution, + if (workspace != null) "workspace": workspace, + "dependencies": { + for (final (name, dependency) in dependencies.records) + name: dependency.toJson(), + }, + // ... + }; +} From c8d107be5a4ab3d86ce9d3d0b71e9925188012c4 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 7 Jan 2025 20:21:24 -0500 Subject: [PATCH 13/21] Added 11 workspace tests --- lib/src/dependency_validator.dart | 35 ++++++-- test/utils.dart | 42 +++++++-- test/workspace_test.dart | 138 ++++++++++++++++++++++++++++++ 3 files changed, 199 insertions(+), 16 deletions(-) create mode 100644 test/workspace_test.dart diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index d37b7e9..db78807 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -32,6 +32,7 @@ Future checkPackage({required String root}) async { var result = true; if (!File('$root/pubspec.yaml').existsSync()) { logger.shout(red.wrap('pubspec.yaml not found')); + logger.fine('Path: $root/pubspec.yaml'); return false; } @@ -55,7 +56,7 @@ Future checkPackage({required String root}) async { .map((s) { try { // Globs don't support './' paths. Use posix to avoid '\' paths - final globPath = p.posix.normalize('$root/$s'); + final globPath = '$root/$s'.replaceAll(r'\', '/'); return Glob(globPath); } catch (_, __) { logger.shout(yellow.wrap('invalid glob syntax: "$s"')); @@ -82,7 +83,7 @@ Future checkPackage({required String root}) async { if (pubspec.isWorkspaceRoot) { logger.fine('In a workspace. Recursing through sub-packages...'); for (final package in pubspec.workspace ?? []) { - subResult &= await checkPackage(root: package); + subResult &= await checkPackage(root: '$root/$package'); logger.info(''); } } @@ -142,11 +143,18 @@ Future checkPackage({required String root}) async { logger.fine('packages used in public facing files:\n' '${bulletItems(packagesUsedInPublicFiles)}\n'); - final publicDirGlobs = [for (final dir in publicDirs) Glob('$dir**')]; + final publicDirGlobs = [ + for (final dir in publicDirs) + Glob('$dir**'.replaceAll(r'\', '/')), + ]; + final subpackageGlobs = [ - for (final subpackage in pubspec.workspace ?? []) Glob('$subpackage**') + for (final subpackage in pubspec.workspace ?? []) + Glob('$root/$subpackage**'.replaceAll(r'\', '/')), ]; + logger.fine('subpackage globs: $subpackageGlobs'); + final nonPublicDartFiles = listDartFilesIn( '$root/', [...excludes, ...publicDirGlobs, ...subpackageGlobs], @@ -303,9 +311,11 @@ Future checkPackage({required String root}) async { .any((key) => key.startsWith('$dependencyName:')); final packagesWithConsumedBuilders = Set(); - for (final package in unusedDependencies.map((name) => packageConfig[name])) { + for (final name in unusedDependencies) { + final package = packageConfig[name]; + if (package == null) continue; // Check if a builder is used from this package - if (rootPackageReferencesDependencyInBuildYaml(package!.name) || + if (rootPackageReferencesDependencyInBuildYaml(package.name) || await dependencyDefinesAutoAppliedBuilder(p.fromUri(package.root))) { packagesWithConsumedBuilders.add(package.name); } @@ -320,14 +330,21 @@ Future checkPackage({required String root}) async { unusedDependencies.removeAll(packagesWithConsumedBuilders); // Remove deps that provide executables, those are assumed to be used - final packagesWithExecutables = unusedDependencies.where((name) { + bool providesExecutable(String name) { final package = packageConfig[name]; - final binDir = Directory(p.join(p.fromUri(package!.root), 'bin')); + if (package == null) return false; + final binDir = Directory(p.join(p.fromUri(package.root), 'bin')); if (!binDir.existsSync()) return false; // Search for executables, if found we assume they are used return binDir.listSync().any((entity) => entity.path.endsWith('.dart')); - }).toSet(); + } + + final packagesWithExecutables = { + for (final package in unusedDependencies) + if (providesExecutable(package)) + package, + }; final nonDevPackagesWithExecutables = packagesWithExecutables.where(pubspec.dependencies.containsKey).toSet(); diff --git a/test/utils.dart b/test/utils.dart index bf75076..9a9967b 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -4,9 +4,13 @@ import 'dart:io'; import 'package:dependency_validator/src/dependency_validator.dart'; import 'package:dependency_validator/src/pubspec_config.dart'; import 'package:logging/logging.dart'; +import 'package:pub_semver/pub_semver.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; +export 'package:logging/logging.dart' show Level; + import 'pubspec_to_json.dart'; ProcessResult checkProject( @@ -40,16 +44,38 @@ String unindent(String multilineString) { return multilineString.replaceAll('$indent', ''); } -Future checkWorkspace({ - required Pubspec workspacePubspec, +void initLogs() => Logger.root.onRecord + .map((record) => record.message) + .listen(print); + +final requireDart36 = { + "sdk": VersionConstraint.compatibleWith(Version.parse('3.6.0')), +}; + +Future checkWorkspace({ + required Map workspaceDeps, + required Map subpackageDeps, required List workspace, - required Pubspec subpackagePubspec, required List subpackage, DepValidatorConfig? workspaceConfig, DepValidatorConfig? subpackageConfig, bool checkSubpackage = false, - bool verbose = false, + Level logLevel = Level.OFF, + Matcher matcher = isTrue, }) async { + + final workspacePubspec = Pubspec( + 'workspace', + environment: requireDart36, + workspace: ['subpackage'], + dependencies: workspaceDeps, + ); + final subpackagePubspec = Pubspec( + 'subpackage', + environment: requireDart36, + resolution: 'workspace', + dependencies: subpackageDeps, + ); final dir = d.dir('workspace', [ ...workspace, d.file('pubspec.yaml', jsonEncode(workspacePubspec.toJson())), @@ -63,7 +89,9 @@ Future checkWorkspace({ ]), ],); await dir.create(); - final root = checkSubpackage ? "subpackage" : "workspace"; - Logger.root.level = verbose ? Level.ALL : Level.OFF; - return await checkPackage(root: "${d.sandbox}/$root"); + final root = checkSubpackage ? "workspace/subpackage" : "workspace"; + + Logger.root.level = logLevel; + final result = await checkPackage(root: "${d.sandbox}/$root"); + expect(result, matcher); } diff --git a/test/workspace_test.dart b/test/workspace_test.dart new file mode 100644 index 0000000..7a96471 --- /dev/null +++ b/test/workspace_test.dart @@ -0,0 +1,138 @@ +import 'package:dependency_validator/src/pubspec_config.dart'; +import 'package:pub_semver/pub_semver.dart'; +import 'package:pubspec_parse/pubspec_parse.dart'; +import 'package:test/test.dart'; +import 'package:test_descriptor/test_descriptor.dart' as d; + +import 'utils.dart'; + +final usesHttp = [ + d.dir('lib', [ + d.file('main.dart', 'import "package:http/http.dart";'), + ]), +]; + +final dependsOnHttp = { + 'http': HostedDependency( + version: VersionConstraint.any, + ), +}; + +final usesMeta = [ + d.dir('lib', [ + d.file('main.dart', 'import "package:meta/meta.dart";'), + ]), +]; + +final dependsOnMeta = { + "meta": HostedDependency(version: VersionConstraint.any), +}; + +final excludeMain = DepValidatorConfig( + exclude: ['lib/main.dart'], +); + +void main() => group('Workspaces', () { + initLogs(); + test('works in the trivial case', () => checkWorkspace( + workspaceDeps: {}, + workspace: [], + subpackage: [], + subpackageDeps: {}, + )); + + test('works in a basic case', () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: usesHttp, + subpackageDeps: dependsOnHttp, + )); + + test('works when the packages have different dependencies', () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: usesMeta, + subpackageDeps: dependsOnMeta, + )); + + group('fails when the root has an issue', () { + test('(sub-package is okay)', () => checkWorkspace( + workspace: [], + workspaceDeps: {}, + subpackage: usesHttp, + subpackageDeps: dependsOnHttp, + checkSubpackage: true, + )); + + test('even when it shares a dependency with the subpackage', () => checkWorkspace( + workspaceDeps: dependsOnHttp, + workspace: [], + subpackageDeps: dependsOnHttp, + subpackage: usesHttp, + matcher: isFalse, + )); + }); + + group('fails when the subpackage has an issue', () { + test('(root is okay)', () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: [], + subpackageDeps: {}, + )); + + test('even when it shares a dependency with the subpackage', () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: usesHttp, + subpackageDeps: {}, + matcher: isFalse, + )); + }); + + group('handles configs', () { + test('at the root', () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: {}, + workspaceConfig: excludeMain, + subpackage: [], + subpackageDeps: {}, + )); + + test('and fails at root when config is in subpackage', () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: {}, + subpackage: [], + subpackageDeps: {}, + subpackageConfig: excludeMain, + matcher: isFalse, + )); + + test('in a subpackage', () => checkWorkspace( + workspace: [], + workspaceDeps: {}, + subpackage: usesHttp, + subpackageDeps: {}, + subpackageConfig: excludeMain, + )); + + test('and fails in subpackage when config is in root', () => checkWorkspace( + workspace: [], + workspaceDeps: {}, + workspaceConfig: excludeMain, + subpackage: usesHttp, + subpackageDeps: {}, + matcher: isFalse, + )); + }); + + group('Workspaces', () { + group('fail when packages are missing from the pubspec', () { + + }); + + group('fails when there are over-promoted packages', () { + + }); + }); +}); From 5e98109a9fe2b75dc2dfd50f2afe61bf8d66e31a Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 7 Jan 2025 20:22:40 -0500 Subject: [PATCH 14/21] Formatted --- lib/src/dependency_validator.dart | 6 +- test/pubspec_to_json.dart | 67 +++++---- test/utils.dart | 35 ++--- test/workspace_test.dart | 216 ++++++++++++++++-------------- 4 files changed, 168 insertions(+), 156 deletions(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index db78807..0973748 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -144,8 +144,7 @@ Future checkPackage({required String root}) async { '${bulletItems(packagesUsedInPublicFiles)}\n'); final publicDirGlobs = [ - for (final dir in publicDirs) - Glob('$dir**'.replaceAll(r'\', '/')), + for (final dir in publicDirs) Glob('$dir**'.replaceAll(r'\', '/')), ]; final subpackageGlobs = [ @@ -342,8 +341,7 @@ Future checkPackage({required String root}) async { final packagesWithExecutables = { for (final package in unusedDependencies) - if (providesExecutable(package)) - package, + if (providesExecutable(package)) package, }; final nonDevPackagesWithExecutables = diff --git a/test/pubspec_to_json.dart b/test/pubspec_to_json.dart index a698457..1e96f95 100644 --- a/test/pubspec_to_json.dart +++ b/test/pubspec_to_json.dart @@ -1,7 +1,6 @@ - import "package:pubspec_parse/pubspec_parse.dart"; -extension on Map { +extension on Map { Iterable<(K, V)> get records sync* { for (final entry in entries) { yield (entry.key, entry.value); @@ -13,25 +12,25 @@ typedef Json = Map; extension on Dependency { Json toJson() => switch (this) { - SdkDependency(:final sdk, :final version) => { - "sdk": sdk, - "version": version.toString(), - }, - HostedDependency(:final hosted, :final version) => { - if (hosted != null) "hosted": hosted.url.toString(), - "version": version.toString(), - }, - GitDependency(:final url, :final ref, :final path) => { - "git": { - "url": url.toString(), - if (path != null) "ref": ref, - if (path != null) "path": path, - }, - }, - PathDependency(:final path) => { - "path": path.replaceAll(r'\', '/'), - }, - }; + SdkDependency(:final sdk, :final version) => { + "sdk": sdk, + "version": version.toString(), + }, + HostedDependency(:final hosted, :final version) => { + if (hosted != null) "hosted": hosted.url.toString(), + "version": version.toString(), + }, + GitDependency(:final url, :final ref, :final path) => { + "git": { + "url": url.toString(), + if (path != null) "ref": ref, + if (path != null) "path": path, + }, + }, + PathDependency(:final path) => { + "path": path.replaceAll(r'\', '/'), + }, + }; } /// An as-needed implementation of `Pubspec.toJson` for testing. @@ -39,17 +38,17 @@ extension on Dependency { /// See: https://github.com/dart-lang/tools/issues/1801 extension PubspecToJson on Pubspec { Json toJson() => { - "name": name, - "environment": { - for (final (sdk, version) in environment.records) - sdk: version.toString(), - }, - if (resolution != null) "resolution": resolution, - if (workspace != null) "workspace": workspace, - "dependencies": { - for (final (name, dependency) in dependencies.records) - name: dependency.toJson(), - }, - // ... - }; + "name": name, + "environment": { + for (final (sdk, version) in environment.records) + sdk: version.toString(), + }, + if (resolution != null) "resolution": resolution, + if (workspace != null) "workspace": workspace, + "dependencies": { + for (final (name, dependency) in dependencies.records) + name: dependency.toJson(), + }, + // ... + }; } diff --git a/test/utils.dart b/test/utils.dart index 9a9967b..a944bda 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -44,9 +44,8 @@ String unindent(String multilineString) { return multilineString.replaceAll('$indent', ''); } -void initLogs() => Logger.root.onRecord - .map((record) => record.message) - .listen(print); +void initLogs() => + Logger.root.onRecord.map((record) => record.message).listen(print); final requireDart36 = { "sdk": VersionConstraint.compatibleWith(Version.parse('3.6.0')), @@ -63,7 +62,6 @@ Future checkWorkspace({ Level logLevel = Level.OFF, Matcher matcher = isTrue, }) async { - final workspacePubspec = Pubspec( 'workspace', environment: requireDart36, @@ -76,18 +74,23 @@ Future checkWorkspace({ resolution: 'workspace', dependencies: subpackageDeps, ); - final dir = d.dir('workspace', [ - ...workspace, - d.file('pubspec.yaml', jsonEncode(workspacePubspec.toJson())), - if (workspaceConfig != null) - d.file('dart_dependency_validator.yaml', jsonEncode(workspaceConfig.toJson())), - d.dir('subpackage', [ - ...subpackage, - d.file('pubspec.yaml', jsonEncode(subpackagePubspec.toJson())), - if (subpackageConfig != null) - d.file('dart_dependency_validator.yaml', jsonEncode(subpackageConfig.toJson())), - ]), - ],); + final dir = d.dir( + 'workspace', + [ + ...workspace, + d.file('pubspec.yaml', jsonEncode(workspacePubspec.toJson())), + if (workspaceConfig != null) + d.file('dart_dependency_validator.yaml', + jsonEncode(workspaceConfig.toJson())), + d.dir('subpackage', [ + ...subpackage, + d.file('pubspec.yaml', jsonEncode(subpackagePubspec.toJson())), + if (subpackageConfig != null) + d.file('dart_dependency_validator.yaml', + jsonEncode(subpackageConfig.toJson())), + ]), + ], + ); await dir.create(); final root = checkSubpackage ? "workspace/subpackage" : "workspace"; diff --git a/test/workspace_test.dart b/test/workspace_test.dart index 7a96471..94c166a 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -33,106 +33,118 @@ final excludeMain = DepValidatorConfig( ); void main() => group('Workspaces', () { - initLogs(); - test('works in the trivial case', () => checkWorkspace( - workspaceDeps: {}, - workspace: [], - subpackage: [], - subpackageDeps: {}, - )); - - test('works in a basic case', () => checkWorkspace( - workspace: usesHttp, - workspaceDeps: dependsOnHttp, - subpackage: usesHttp, - subpackageDeps: dependsOnHttp, - )); - - test('works when the packages have different dependencies', () => checkWorkspace( - workspace: usesHttp, - workspaceDeps: dependsOnHttp, - subpackage: usesMeta, - subpackageDeps: dependsOnMeta, - )); - - group('fails when the root has an issue', () { - test('(sub-package is okay)', () => checkWorkspace( - workspace: [], - workspaceDeps: {}, - subpackage: usesHttp, - subpackageDeps: dependsOnHttp, - checkSubpackage: true, - )); - - test('even when it shares a dependency with the subpackage', () => checkWorkspace( - workspaceDeps: dependsOnHttp, - workspace: [], - subpackageDeps: dependsOnHttp, - subpackage: usesHttp, - matcher: isFalse, - )); - }); - - group('fails when the subpackage has an issue', () { - test('(root is okay)', () => checkWorkspace( - workspace: usesHttp, - workspaceDeps: dependsOnHttp, - subpackage: [], - subpackageDeps: {}, - )); - - test('even when it shares a dependency with the subpackage', () => checkWorkspace( - workspace: usesHttp, - workspaceDeps: dependsOnHttp, - subpackage: usesHttp, - subpackageDeps: {}, - matcher: isFalse, - )); - }); - - group('handles configs', () { - test('at the root', () => checkWorkspace( - workspace: usesHttp, - workspaceDeps: {}, - workspaceConfig: excludeMain, - subpackage: [], - subpackageDeps: {}, - )); - - test('and fails at root when config is in subpackage', () => checkWorkspace( - workspace: usesHttp, - workspaceDeps: {}, - subpackage: [], - subpackageDeps: {}, - subpackageConfig: excludeMain, - matcher: isFalse, - )); - - test('in a subpackage', () => checkWorkspace( - workspace: [], - workspaceDeps: {}, - subpackage: usesHttp, - subpackageDeps: {}, - subpackageConfig: excludeMain, - )); - - test('and fails in subpackage when config is in root', () => checkWorkspace( - workspace: [], - workspaceDeps: {}, - workspaceConfig: excludeMain, - subpackage: usesHttp, - subpackageDeps: {}, - matcher: isFalse, - )); - }); - - group('Workspaces', () { - group('fail when packages are missing from the pubspec', () { - - }); - - group('fails when there are over-promoted packages', () { - + initLogs(); + test( + 'works in the trivial case', + () => checkWorkspace( + workspaceDeps: {}, + workspace: [], + subpackage: [], + subpackageDeps: {}, + )); + + test( + 'works in a basic case', + () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: usesHttp, + subpackageDeps: dependsOnHttp, + )); + + test( + 'works when the packages have different dependencies', + () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: usesMeta, + subpackageDeps: dependsOnMeta, + )); + + group('fails when the root has an issue', () { + test( + '(sub-package is okay)', + () => checkWorkspace( + workspace: [], + workspaceDeps: {}, + subpackage: usesHttp, + subpackageDeps: dependsOnHttp, + checkSubpackage: true, + )); + + test( + 'even when it shares a dependency with the subpackage', + () => checkWorkspace( + workspaceDeps: dependsOnHttp, + workspace: [], + subpackageDeps: dependsOnHttp, + subpackage: usesHttp, + matcher: isFalse, + )); + }); + + group('fails when the subpackage has an issue', () { + test( + '(root is okay)', + () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: [], + subpackageDeps: {}, + )); + + test( + 'even when it shares a dependency with the subpackage', + () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: dependsOnHttp, + subpackage: usesHttp, + subpackageDeps: {}, + matcher: isFalse, + )); + }); + + group('handles configs', () { + test( + 'at the root', + () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: {}, + workspaceConfig: excludeMain, + subpackage: [], + subpackageDeps: {}, + )); + + test( + 'and fails at root when config is in subpackage', + () => checkWorkspace( + workspace: usesHttp, + workspaceDeps: {}, + subpackage: [], + subpackageDeps: {}, + subpackageConfig: excludeMain, + matcher: isFalse, + )); + + test( + 'in a subpackage', + () => checkWorkspace( + workspace: [], + workspaceDeps: {}, + subpackage: usesHttp, + subpackageDeps: {}, + subpackageConfig: excludeMain, + )); + + test( + 'and fails in subpackage when config is in root', + () => checkWorkspace( + workspace: [], + workspaceDeps: {}, + workspaceConfig: excludeMain, + subpackage: usesHttp, + subpackageDeps: {}, + matcher: isFalse, + )); + }); }); - }); -}); From 9aa3b9d331759e2ad9a7ceb5f6cfb1b49fa0ea8c Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Tue, 7 Jan 2025 20:31:45 -0500 Subject: [PATCH 15/21] Removed unused option in checkWorkspace --- test/utils.dart | 9 +++------ test/workspace_test.dart | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/test/utils.dart b/test/utils.dart index a944bda..56ebaef 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -58,21 +58,20 @@ Future checkWorkspace({ required List subpackage, DepValidatorConfig? workspaceConfig, DepValidatorConfig? subpackageConfig, - bool checkSubpackage = false, Level logLevel = Level.OFF, Matcher matcher = isTrue, }) async { final workspacePubspec = Pubspec( 'workspace', environment: requireDart36, - workspace: ['subpackage'], dependencies: workspaceDeps, + workspace: ['subpackage'], ); final subpackagePubspec = Pubspec( 'subpackage', environment: requireDart36, - resolution: 'workspace', dependencies: subpackageDeps, + resolution: 'workspace', ); final dir = d.dir( 'workspace', @@ -92,9 +91,7 @@ Future checkWorkspace({ ], ); await dir.create(); - final root = checkSubpackage ? "workspace/subpackage" : "workspace"; - Logger.root.level = logLevel; - final result = await checkPackage(root: "${d.sandbox}/$root"); + final result = await checkPackage(root: '${d.sandbox}/workspace'); expect(result, matcher); } diff --git a/test/workspace_test.dart b/test/workspace_test.dart index 94c166a..69acd34 100644 --- a/test/workspace_test.dart +++ b/test/workspace_test.dart @@ -69,7 +69,6 @@ void main() => group('Workspaces', () { workspaceDeps: {}, subpackage: usesHttp, subpackageDeps: dependsOnHttp, - checkSubpackage: true, )); test( From c0de3f7eaab1f5ab0836d800e1491c8312822aee Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 8 Jan 2025 10:35:59 -0500 Subject: [PATCH 16/21] Refactored executable_tests to use new checkProject() --- lib/src/dependency_validator.dart | 10 +- lib/src/utils.dart | 7 + test/executable_test.dart | 856 ++++++++++-------------------- test/pubspec_to_json.dart | 4 + test/utils.dart | 78 ++- 5 files changed, 359 insertions(+), 596 deletions(-) diff --git a/lib/src/dependency_validator.dart b/lib/src/dependency_validator.dart index 0973748..6a669d3 100644 --- a/lib/src/dependency_validator.dart +++ b/lib/src/dependency_validator.dart @@ -16,7 +16,6 @@ import 'dart:io'; import 'package:build_config/build_config.dart'; import 'package:dependency_validator/src/import_export_ast_visitor.dart'; -import 'package:glob/glob.dart'; import 'package:io/ansi.dart'; import 'package:logging/logging.dart'; import 'package:package_config/package_config.dart'; @@ -55,9 +54,7 @@ Future checkPackage({required String root}) async { final excludes = config.exclude .map((s) { try { - // Globs don't support './' paths. Use posix to avoid '\' paths - final globPath = '$root/$s'.replaceAll(r'\', '/'); - return Glob(globPath); + return makeGlob("$root/$s"); } catch (_, __) { logger.shout(yellow.wrap('invalid glob syntax: "$s"')); return null; @@ -104,6 +101,7 @@ Future checkPackage({required String root}) async { '${bulletItems(devDeps)}\n'); final publicDirs = ['$root/bin/', '$root/lib/']; + logger.fine("Excluding: $excludes"); final publicDartFiles = [ for (final dir in publicDirs) ...listDartFilesIn(dir, excludes), ]; @@ -144,12 +142,12 @@ Future checkPackage({required String root}) async { '${bulletItems(packagesUsedInPublicFiles)}\n'); final publicDirGlobs = [ - for (final dir in publicDirs) Glob('$dir**'.replaceAll(r'\', '/')), + for (final dir in publicDirs) makeGlob('$dir**'), ]; final subpackageGlobs = [ for (final subpackage in pubspec.workspace ?? []) - Glob('$root/$subpackage**'.replaceAll(r'\', '/')), + makeGlob('$root/$subpackage**'), ]; logger.fine('subpackage globs: $subpackageGlobs'); diff --git a/lib/src/utils.dart b/lib/src/utils.dart index 5347296..6610243 100644 --- a/lib/src/utils.dart +++ b/lib/src/utils.dart @@ -192,3 +192,10 @@ extension PubspecUtils on Pubspec { /// Whether this package is a sub-package in a Pub Workspace. bool get isInWorkspace => resolution == 'workspace'; } + +/// Makes a glob object for the given path. +/// +/// This function removes `./` paths and replaces all `\` with `/`. +Glob makeGlob(String path) => Glob( + p.posix.normalize(path.replaceAll(r'\', '/')), + ); diff --git a/test/executable_test.dart b/test/executable_test.dart index 23148fa..15ae353 100644 --- a/test/executable_test.dart +++ b/test/executable_test.dart @@ -15,419 +15,257 @@ @TestOn('vm') import 'dart:io'; +import 'package:dependency_validator/src/pubspec_config.dart'; import 'package:io/io.dart'; import 'package:test/test.dart'; import 'package:test_descriptor/test_descriptor.dart' as d; import 'utils.dart'; -/// `master` on build_config has a min sdk bound of dart 3.0.0. -/// Since dependency_validator is still designed to be used on dart 2 -/// code, we still want to run unit tests using this older version -/// -/// The following ref, is the last commit in build_config that allowed -/// dart 2 as a dependency -const buildConfigRef = 'e2c837b48bd3c4428cb40e2bc1a6cf47d45df8cc'; - void main() { group('dependency_validator', () { late ProcessResult result; - setUp(() async { - // Create fake project that any test may use - final fakeProjectPubspec = unindent(''' - name: fake_project - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - final fakeProjectBuild = unindent(''' - builders: - someBuilder: - import: "package:fake_project/builder.dart" - builder_factories: ["someBuilder"] - build_extensions: {".dart" : [".woot.g.dart"]} - auto_apply: none - build_to: cache - '''); - - await d.dir('fake_project', [ - d.dir('lib', [ - d.file('fake.dart', 'bool fake = true;'), - d.file('builder.dart', 'bool fake = true;'), - ]), - d.file('pubspec.yaml', fakeProjectPubspec), - d.file('build.yaml', fakeProjectBuild), - ]).create(); - }); - tearDown(() { printOnFailure(result.stdout); printOnFailure(result.stderr); }); test('fails with incorrect usage', () async { - final pubspec = unindent(''' - name: common_binaries - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - '''); - - await d.dir('common_binaries', [ - d.dir('lib', [ - d.file('fake.dart', 'bool fake = true;'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - - result = checkProject('${d.sandbox}/common_binaries', - optionalArgs: ['-x', 'tool/wdesk_sdk']); - + result = await checkProject( + args: ['-x', 'tool/wdesk_sdk'], + ); expect(result.exitCode, ExitCode.usage.code); }); group('fails when there are packages missing from the pubspec', () { - setUp(() async { - final pubspec = unindent(''' - name: missing - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('missing', [ - d.dir('lib', [ - d.file('missing.dart', 'import \'package:yaml/yaml.dart\';'), - d.file('missing.scss', '@import \'package:somescsspackage/foo\';'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - }); - - test('', () { - result = checkProject('${d.sandbox}/missing'); + final project = [ + d.dir('lib', [ + d.file('missing.dart', 'import "package:yaml/yaml.dart";'), + d.file('missing.scss', '@import "package:some_scss_package/foo";'), + ]), + ]; - expect(result.exitCode, equals(1)); + test('', () async { + result = await checkProject(project: project); + expect(result.exitCode, 1); expect( - result.stderr, - contains( - 'These packages are used in lib/ but are not dependencies:')); + result.stderr, + contains('These packages are used in lib/ but are not dependencies:'), + ); expect(result.stderr, contains('yaml')); - expect(result.stderr, contains('somescsspackage')); + expect(result.stderr, contains('some_scss_package')); }); - test('except when the lib directory is excluded', () async { - await d.dir('missing', [ - d.file('dart_dependency_validator.yaml', unindent(''' - exclude: - - 'lib/**' - ''')) - ]).create(); - result = checkProject('${d.sandbox}/missing'); - expect(result.exitCode, equals(0)); + final excludeLib = DepValidatorConfig(exclude: ['lib/**']); + final ignorePackages = DepValidatorConfig( + ignore: ['yaml', 'some_scss_package'], + ); + + test('except when lib is excluded', () async { + result = await checkProject( + project: project, + config: excludeLib, + ); + expect(result.exitCode, 0); expect(result.stderr, isEmpty); }); - test( - 'except when the lib directory is excluded (deprecated pubspec method)', - () { - final dependencyValidatorSection = unindent(''' - dependency_validator: - exclude: - - 'lib/**' - '''); - - File('${d.sandbox}/missing/pubspec.yaml').writeAsStringSync( - dependencyValidatorSection, - mode: FileMode.append, - flush: true); - - result = checkProject('${d.sandbox}/missing'); + test('except when lib is excluded (deprecated pubspec method)', () async { + result = await checkProject( + project: project, + config: excludeLib, + embedConfigInPubspec: true, + ); expect(result.exitCode, equals(0)); expect( - result.stderr, - contains( - 'Configuring dependency_validator in pubspec.yaml is deprecated')); + result.stderr, + contains( + 'Configuring dependency_validator in pubspec.yaml is deprecated', + ), + ); }); test('except when they are ignored', () async { - await d.dir('missing', [ - d.file('dart_dependency_validator.yaml', unindent(''' - ignore: - - yaml - - somescsspackage - ''')) - ]).create(); - result = checkProject('${d.sandbox}/missing'); + result = await checkProject( + project: project, + config: ignorePackages, + ); expect(result.exitCode, 0); }); - test('except when they are ignored (deprecated pubspec method)', () { - final dependencyValidatorSection = unindent(''' - dependency_validator: - ignore: - - yaml - - somescsspackage - '''); - - File('${d.sandbox}/missing/pubspec.yaml').writeAsStringSync( - dependencyValidatorSection, - mode: FileMode.append); - - result = checkProject('${d.sandbox}/missing'); + test('except when they are ignored (deprecated pubspec method)', + () async { + result = await checkProject( + project: project, + config: ignorePackages, + embedConfigInPubspec: true, + ); expect(result.exitCode, 0); }); }); group('fails when there are over promoted packages', () { - setUp(() async { - final pubspec = unindent(''' - name: over_promoted - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dependencies: - path: any - yaml: any - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('over_promoted', [ - d.dir('web', [ - d.file('main.dart', 'import \'package:path/path.dart\';'), - d.file('over_promoted.scss', '@import \'package:yaml/foo\';'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - }); - - test('', () { - result = checkProject('${d.sandbox}/over_promoted'); + final project = [ + d.dir('web', [ + d.file('main.dart', 'import "package:path/path.dart";'), + d.file('over_promoted.scss', '@import "package:yaml/foo";'), + ]), + ]; + final dependencies = { + "path": hostedAny, + "yaml": hostedAny, + }; + final config = DepValidatorConfig(ignore: ['path', 'yaml']); + + test('', () async { + result = await checkProject( + project: project, + dependencies: dependencies, + ); expect(result.exitCode, 1); expect( - result.stderr, - contains( - 'These packages are only used outside lib/ and should be downgraded to dev_dependencies:')); + result.stderr, + contains( + 'These packages are only used outside lib/ and should be downgraded to dev_dependencies:', + ), + ); expect(result.stderr, contains('path')); expect(result.stderr, contains('yaml')); }); test('except when they are ignored', () async { - await d.dir('over_promoted', [ - d.file('dart_dependency_validator.yaml', unindent(''' - ignore: - - path - - yaml - ''')) - ]).create(); - result = checkProject('${d.sandbox}/over_promoted'); + result = await checkProject( + project: project, + dependencies: dependencies, + config: config, + ); expect(result.exitCode, 0); }); - test('except when they are ignored (deprecated pubspec method)', () { - final dependencyValidatorSection = unindent(''' - dependency_validator: - ignore: - - path - - yaml - '''); - - File('${d.sandbox}/over_promoted/pubspec.yaml').writeAsStringSync( - dependencyValidatorSection, - mode: FileMode.append); - - result = checkProject('${d.sandbox}/over_promoted'); + test('except when they are ignored (deprecated pubspec method)', + () async { + result = await checkProject( + project: project, + dependencies: dependencies, + config: config, + embedConfigInPubspec: true, + ); expect(result.exitCode, 0); }); }); group('fails when there are under promoted packages', () { - setUp(() async { - final pubspec = unindent(''' - name: under_promoted - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - logging: any - yaml: any - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('under_promoted', [ - d.dir('lib', [ - d.file('under_promoted.dart', - 'import \'package:logging/logging.dart\';'), - d.file('under_promoted.scss', '@import \'package:yaml/foo\';'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - }); + final devDependencies = { + "logging": hostedAny, + "yaml": hostedAny, + }; + + final project = [ + d.dir('lib', [ + d.file('main.dart', 'import "package:logging/logging.dart";'), + d.file('main.scss', '@import "package:yaml/foo";'), + ]), + ]; - test('', () { - result = checkProject('${d.sandbox}/under_promoted'); + final config = DepValidatorConfig(ignore: ['logging', 'yaml']); + test('', () async { + result = await checkProject( + project: project, + devDependencies: devDependencies, + ); expect(result.exitCode, 1); expect( - result.stderr, - contains( - 'These packages are used in lib/ and should be promoted to actual dependencies:')); + result.stderr, + contains( + 'These packages are used in lib/ and should be promoted to actual dependencies:', + ), + ); expect(result.stderr, contains('logging')); expect(result.stderr, contains('yaml')); }); test('except when they are ignored', () async { - await d.dir('under_promoted', [ - d.file('dart_dependency_validator.yaml', unindent(''' - ignore: - - logging - - yaml - ''')) - ]).create(); - result = checkProject('${d.sandbox}/under_promoted'); + result = await checkProject( + project: project, + devDependencies: devDependencies, + config: config, + ); expect(result.exitCode, 0); }); - test('except when they are ignored (deprecated pubspec method)', () { - final dependencyValidatorSection = unindent(''' - dependency_validator: - ignore: - - logging - - yaml - '''); - - File('${d.sandbox}/under_promoted/pubspec.yaml').writeAsStringSync( - dependencyValidatorSection, - mode: FileMode.append); - - result = checkProject('${d.sandbox}/under_promoted'); + test('except when they are ignored (deprecated pubspec method)', + () async { + result = await checkProject( + project: project, + devDependencies: devDependencies, + config: config, + embedConfigInPubspec: true, + ); expect(result.exitCode, 0); }); }); group('fails when there are unused packages', () { - setUp(() async { - final unusedPubspec = unindent(''' - name: unused - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - fake_project: - path: ${d.sandbox}/fake_project - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('unused', [ - d.file('pubspec.yaml', unusedPubspec), - ]).create(); - }); + final devDependencies = { + 'yaml': hostedAny, + }; - test('', () { - result = checkProject('${d.sandbox}/unused'); + final config = DepValidatorConfig(ignore: ['yaml']); + + test('', () async { + result = await checkProject( + devDependencies: devDependencies, + ); expect(result.exitCode, 1); expect( - result.stderr, - contains( - 'These packages may be unused, or you may be using assets from these packages:')); - expect(result.stderr, contains('fake_project')); + result.stderr, + contains( + 'These packages may be unused, or you may be using assets from these packages:', + ), + ); + expect(result.stderr, contains('yaml')); }); test('and import is commented out', () async { - await d.dir('unused', [ - d.dir('lib', [ - d.file('commented_out.dart', - '// import \'package:other_project/other.dart\';'), // commented out import - ]), - d.dir('test', [ - d.file('valid.dart', 'import \'package:fake_project/fake.dart\';'), - ]) - ]).create(); - result = checkProject('${d.sandbox}/unused'); + result = await checkProject( + devDependencies: devDependencies, + project: [ + d.dir('lib', [ + d.file( + 'main.dart', + '// import "package:other_project/other.dart";', + ), + ]), + d.dir('test', [ + d.file('valid.dart', 'import "package:yaml/fake.dart";'), + ]) + ], + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); test('except when they are ignored', () async { - await d.dir('unused', [ - d.file('dart_dependency_validator.yaml', unindent(''' - ignore: - - fake_project - - yaml - ''')) - ]).create(); - result = checkProject('${d.sandbox}/unused'); + result = await checkProject( + devDependencies: devDependencies, + config: config, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); - test('except when they are ignored (deprecated pubspec method)', () { - final dependencyValidatorSection = unindent(''' - dependency_validator: - ignore: - - fake_project - - yaml - '''); - - File('${d.sandbox}/unused/pubspec.yaml').writeAsStringSync( - dependencyValidatorSection, - mode: FileMode.append); - - result = checkProject('${d.sandbox}/unused'); + test('except when they are ignored (deprecated pubspec method)', + () async { + result = await checkProject( + devDependencies: devDependencies, + config: config, + embedConfigInPubspec: true, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); @@ -435,119 +273,70 @@ void main() { test('warns when the analyzer package is depended on but not used', () async { - final pubspec = unindent(''' - name: analyzer_dep - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dependencies: - analyzer: any - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('project', [ - d.dir('lib', [ - d.file('analyzer_dep.dart', ''), - ]), - d.file('dart_dependency_validator.yaml', unindent(''' - ignore: - - analyzer - ''')), - d.file('pubspec.yaml', pubspec), - ]).create(); - - result = checkProject('${d.sandbox}/project'); - + result = await checkProject( + dependencies: { + "analyzer": hostedAny, + }, + project: [ + d.dir('lib', [ + d.file('main.dart', ''), + ]), + ], + config: DepValidatorConfig(ignore: ['analyzer']), + ); expect(result.exitCode, 0); expect( - result.stderr, - contains( - 'You do not need to depend on `analyzer` to run the Dart analyzer.')); + result.stderr, + contains( + 'You do not need to depend on `analyzer` to run the Dart analyzer.', + ), + ); }); test('passes when all dependencies are used and valid', () async { - final pubspec = unindent(''' - name: valid - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dependencies: - logging: any - yaml: any - fake_project: - path: ${d.sandbox}/fake_project - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - pedantic: any - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - final validDotDart = '' - 'import \'package:logging/logging.dart\';' - 'import \'package:fake_project/fake.dart\';' - '// import \'package:does_not_exist/fake.dart\''; // commented out and unused - - await d.dir('valid', [ - d.dir('lib', [ - d.file('valid.dart', validDotDart), - d.file('valid.scss', '@import \'package:yaml/foo\';'), - ]), - d.file('pubspec.yaml', pubspec), - d.file('analysis_options.yaml', - 'include: package:pedantic/analysis_options.1.8.0.yaml'), - ]).create(); - - result = checkProject('${d.sandbox}/valid'); + result = await checkProject( + dependencies: { + "logging": hostedAny, + "yaml": hostedAny, + }, + devDependencies: { + "pedantic": hostedAny, + }, + project: [ + d.dir('lib', [ + d.file('main.dart', unindent(''' + import 'package:logging/logging.dart'; + import 'package:yaml/yaml.dart'; + // import 'package:does_not_exist/fake.dart'; + ''')), + d.file('main.scss', '@import "package:yaml/foo";'), + ]), + d.file( + 'analysis_options.yaml', + 'include: package:pedantic/analysis_options.1.8.0.yaml', + ), + ], + runPubGet: true, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); test('passes when dependencies not used provide executables', () async { - final pubspec = unindent(''' - name: common_binaries - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - build_runner: ^2.3.3 - coverage: any - dart_style: ^2.3.2 - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('common_binaries', [ - d.dir('lib', [ - d.file('fake.dart', 'bool fake = true;'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - - result = checkProject('${d.sandbox}/common_binaries'); + result = await checkProject( + devDependencies: { + "build_runner": hostedCompatibleWith('2.3.3'), + 'coverage': hostedAny, + 'dart_style': hostedCompatibleWith('2.3.2'), + }, + project: [ + d.dir('lib', [ + d.file('main.dart', 'book fake = true;'), + ]), + ], + runPubGet: true, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); @@ -556,73 +345,45 @@ void main() { test( 'fails when dependencies not used provide executables, but are not dev_dependencies', () async { - final pubspec = unindent(''' - name: common_binaries - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dependencies: - build_runner: ^2.3.3 - coverage: any - dart_style: ^2.3.2 - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('common_binaries', [ - d.dir('lib', [ - d.file('fake.dart', 'bool fake = true;'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - - result = checkProject('${d.sandbox}/common_binaries'); + result = await checkProject( + dependencies: { + "build_runner": hostedCompatibleWith('2.3.3'), + "coverage": hostedAny, + "dart_style": hostedCompatibleWith('2.3.2'), + }, + project: [ + d.dir('lib', [ + d.file('main.dart', 'bool fake = true;'), + ]), + ], + runPubGet: true, + ); expect(result.exitCode, 1); expect( - result.stderr, - contains( - 'The following packages contain executables, and are only used outside of lib/. These should be downgraded to dev_dependencies')); + result.stderr, + contains( + 'The following packages contain executables, and are only used outside of lib/. These should be downgraded to dev_dependencies', + ), + ); }); test( 'passes when dependencies are not imported but provide auto applied builders', () async { - final pubspec = unindent(''' - name: common_binaries - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - build_test: ^2.0.1 - build_vm_compilers: ^1.0.3 - build_web_compilers: ^3.2.7 - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('common_binaries', [ - d.dir('lib', [ - d.file('fake.dart', 'bool fake = true;'), - ]), - d.file('pubspec.yaml', pubspec), - ]).create(); - - result = checkProject('${d.sandbox}/common_binaries'); + result = await checkProject( + devDependencies: { + 'build_test': hostedCompatibleWith('2.0.1'), + 'build_vm_compilers': hostedCompatibleWith('1.0.3'), + 'build_web_compilers': hostedCompatibleWith('3.2.7'), + }, + project: [ + d.dir('lib', [ + d.file('main.dart', 'book fake = true;'), + ]), + ], + runPubGet: true, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); @@ -630,108 +391,69 @@ void main() { test('passes when dependencies are not imported but provide used builders', () async { - final pubspec = unindent(''' - name: common_binaries - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dev_dependencies: - fake_project: - path: ${d.sandbox}/fake_project - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - final build = unindent(r''' - targets: - $default: - builders: - fake_project|someBuilder: - options: {} - '''); - - await d.dir('common_binaries', [ - d.dir('lib', [ - d.file('nope.dart', 'bool nope = true;'), - ]), - d.file('pubspec.yaml', pubspec), - d.file('build.yaml', build), - ]).create(); - - result = checkProject('${d.sandbox}/common_binaries'); + result = await checkProject( + devDependencies: { + 'yaml': hostedAny, + }, + project: [ + d.dir('lib', [ + d.file('main.dart', 'bool fake = true;'), + ]), + d.file( + 'build.yaml', + unindent(r''' + targets: + $default: + builders: + yaml|someBuilder: + options: {} + '''), + ), + ], + runPubGet: true, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); group('when a dependency is pinned', () { - setUp(() async { - final pubspec = unindent(''' - name: dependency_pins - version: 0.0.0 - private: true - environment: - sdk: '>=2.12.0 <4.0.0' - dependencies: - logging: 1.0.2 - dev_dependencies: - dependency_validator: - path: ${Directory.current.path} - dependency_overrides: - build_config: - git: - url: https://github.com/dart-lang/build.git - path: build_config - ref: $buildConfigRef - '''); - - await d.dir('dependency_pins', [ - d.file('pubspec.yaml', pubspec), - ]).create(); - }); + final dependencies = { + 'logging': hostedPinned('1.0.2'), + }; - test('fails if pins are not ignored', () { - result = checkProject('${d.sandbox}/dependency_pins'); + final allowPins = DepValidatorConfig(allowPins: true); + final ignorePackage = DepValidatorConfig(ignore: ['logging']); + + test('fails if pins are not ignored', () async { + result = await checkProject(dependencies: dependencies); expect(result.exitCode, 1); expect( - result.stderr, - contains( - 'These packages are pinned in pubspec.yaml:\n * logging')); + result.stderr, + contains('These packages are pinned in pubspec.yaml:\n * logging'), + ); }); test('should not fails if package is pinned but pins allowed', () async { - await d.dir('dependency_pins', [ - d.dir('lib', [ - d.file('test.dart', unindent(''' - import 'package:logging/logging.dart'; - final log = Logger('ExampleLogger'); - ''')), - ]), - d.file('dart_dependency_validator.yaml', unindent(''' - allow_pins: true - ''')) - ]).create(); - result = checkProject('${d.sandbox}/dependency_pins'); + result = await checkProject( + dependencies: dependencies, + config: allowPins, + project: [ + d.dir('lib', [ + d.file('main.dart', 'import "package:logging/logging.dart";'), + ]), + ], + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); test('ignores infractions if the package is ignored', () async { - await d.dir('dependency_pins', [ - d.file('dart_dependency_validator.yaml', unindent(''' - ignore: - - logging - ''')) - ]).create(); - result = checkProject('${d.sandbox}/dependency_pins'); + result = await checkProject( + dependencies: dependencies, + config: ignorePackage, + ); expect(result.exitCode, 0); expect(result.stdout, contains('No dependency issues found!')); }); diff --git a/test/pubspec_to_json.dart b/test/pubspec_to_json.dart index 1e96f95..3fdb7df 100644 --- a/test/pubspec_to_json.dart +++ b/test/pubspec_to_json.dart @@ -49,6 +49,10 @@ extension PubspecToJson on Pubspec { for (final (name, dependency) in dependencies.records) name: dependency.toJson(), }, + "dev_dependencies": { + for (final (name, dependency) in devDependencies.records) + name: dependency.toJson(), + } // ... }; } diff --git a/test/utils.dart b/test/utils.dart index 56ebaef..e1a2e1f 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -13,29 +13,57 @@ export 'package:logging/logging.dart' show Level; import 'pubspec_to_json.dart'; -ProcessResult checkProject( - String projectPath, { - List optionalArgs = const [], -}) { - final pubGetResult = Process.runSync( - 'dart', - ['pub', 'get'], - workingDirectory: projectPath, +Future checkProject({ + DepValidatorConfig? config, + Map dependencies = const {}, + Map devDependencies = const {}, + List project = const [], + List args = const [], + bool embedConfigInPubspec = false, + bool runPubGet = false, +}) async { + final pubspec = Pubspec( + 'project', + environment: requireDart36, + dependencies: dependencies, + devDependencies: { + ...devDependencies, + 'dependency_validator': PathDependency(Directory.current.absolute.path), + }, ); - if (pubGetResult.exitCode != 0) { - return pubGetResult; + final pubspecJson = pubspec.toJson(); + if (embedConfigInPubspec && config != null) { + pubspecJson['dependency_validator'] = config.toJson(); + } + final dir = d.dir('project', [ + ...project, + d.file('pubspec.yaml', jsonEncode(pubspecJson)), + if (config != null && !embedConfigInPubspec) + d.file('dart_dependency_validator.yaml', jsonEncode(config.toJson())), + ]); + await dir.create(); + final path = '${d.sandbox}/project'; + if (runPubGet) { + final pubGet = await Process.run( + 'dart', + ['pub', 'get'], + workingDirectory: path, + ); + expect(pubGet.exitCode, 0); } + final commandArgs = ['run', 'dependency_validator', '--verbose', ...args]; + return await Process.run('dart', commandArgs, workingDirectory: path); +} - final args = [ - 'run', - 'dependency_validator', - // This makes it easier to print(result.stdout) for debugging tests - '--verbose', - ...optionalArgs, - ]; +Dependency hostedCompatibleWith(String version) => HostedDependency( + version: VersionConstraint.compatibleWith(Version.parse(version)), + ); - return Process.runSync('dart', args, workingDirectory: projectPath); -} +Dependency hostedPinned(String version) => HostedDependency( + version: Version.parse(version), + ); + +final hostedAny = HostedDependency(version: VersionConstraint.any); /// Removes indentation from `'''` string blocks. String unindent(String multilineString) { @@ -79,14 +107,18 @@ Future checkWorkspace({ ...workspace, d.file('pubspec.yaml', jsonEncode(workspacePubspec.toJson())), if (workspaceConfig != null) - d.file('dart_dependency_validator.yaml', - jsonEncode(workspaceConfig.toJson())), + d.file( + 'dart_dependency_validator.yaml', + jsonEncode(workspaceConfig.toJson()), + ), d.dir('subpackage', [ ...subpackage, d.file('pubspec.yaml', jsonEncode(subpackagePubspec.toJson())), if (subpackageConfig != null) - d.file('dart_dependency_validator.yaml', - jsonEncode(subpackageConfig.toJson())), + d.file( + 'dart_dependency_validator.yaml', + jsonEncode(subpackageConfig.toJson()), + ), ]), ], ); From 00e960ef33ce4fe37dd326d630cf8cdb63061b90 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 8 Jan 2025 10:52:16 -0500 Subject: [PATCH 17/21] Removed unused option in checkProject() --- test/executable_test.dart | 5 ----- test/utils.dart | 9 --------- 2 files changed, 14 deletions(-) diff --git a/test/executable_test.dart b/test/executable_test.dart index 15ae353..22c0fca 100644 --- a/test/executable_test.dart +++ b/test/executable_test.dart @@ -316,7 +316,6 @@ void main() { 'include: package:pedantic/analysis_options.1.8.0.yaml', ), ], - runPubGet: true, ); expect(result.exitCode, 0); @@ -335,7 +334,6 @@ void main() { d.file('main.dart', 'book fake = true;'), ]), ], - runPubGet: true, ); expect(result.exitCode, 0); @@ -356,7 +354,6 @@ void main() { d.file('main.dart', 'bool fake = true;'), ]), ], - runPubGet: true, ); expect(result.exitCode, 1); @@ -382,7 +379,6 @@ void main() { d.file('main.dart', 'book fake = true;'), ]), ], - runPubGet: true, ); expect(result.exitCode, 0); @@ -410,7 +406,6 @@ void main() { '''), ), ], - runPubGet: true, ); expect(result.exitCode, 0); diff --git a/test/utils.dart b/test/utils.dart index e1a2e1f..7e287de 100644 --- a/test/utils.dart +++ b/test/utils.dart @@ -20,7 +20,6 @@ Future checkProject({ List project = const [], List args = const [], bool embedConfigInPubspec = false, - bool runPubGet = false, }) async { final pubspec = Pubspec( 'project', @@ -43,14 +42,6 @@ Future checkProject({ ]); await dir.create(); final path = '${d.sandbox}/project'; - if (runPubGet) { - final pubGet = await Process.run( - 'dart', - ['pub', 'get'], - workingDirectory: path, - ); - expect(pubGet.exitCode, 0); - } final commandArgs = ['run', 'dependency_validator', '--verbose', ...args]; return await Process.run('dart', commandArgs, workingDirectory: path); } From a7228c1e96bcb703580d57740820a8b83193b830 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 8 Jan 2025 17:47:53 -0500 Subject: [PATCH 18/21] Update CI to use latest Dart (^3.0) --- .github/workflows/ci.yaml | 9 +++++---- .github/workflows/publish.yaml | 4 +++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 40b52fd..dd3194c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,14 +9,15 @@ on: jobs: build: uses: Workiva/gha-dart-oss/.github/workflows/build.yaml@v0.1.7 + with: + sdk: stable checks: uses: Workiva/gha-dart-oss/.github/workflows/checks.yaml@v0.1.7 + with: + sdk: stable unit-tests: - strategy: - matrix: - sdk: [2.19.6, stable] uses: Workiva/gha-dart-oss/.github/workflows/test-unit.yaml@v0.1.7 with: - sdk: ${{ matrix.sdk }} \ No newline at end of file + sdk: stable diff --git a/.github/workflows/publish.yaml b/.github/workflows/publish.yaml index f978e2e..34a91e1 100644 --- a/.github/workflows/publish.yaml +++ b/.github/workflows/publish.yaml @@ -12,4 +12,6 @@ permissions: jobs: publish: - uses: Workiva/gha-dart-oss/.github/workflows/publish.yaml@v0.1.7 \ No newline at end of file + uses: Workiva/gha-dart-oss/.github/workflows/publish.yaml@v0.1.7 + with: + sdk: stable From adc2fd36a0c291d4c70c0e9722b24d5e0e548334 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 8 Jan 2025 17:50:56 -0500 Subject: [PATCH 19/21] Update example --- example/example.md | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/example/example.md b/example/example.md index 32be703..22ce7db 100644 --- a/example/example.md +++ b/example/example.md @@ -1,4 +1,13 @@ -Either globally activate this package or add it as a dev_dependency. Then run: +Either globally activate this package or add it as a dev_dependency: +```bash +# Install as a dev dependency on the project -- shared with all collaborators +$ dart pub add --dev dependency_validator + +# Install globally on your system -- does not impact the project +$ dart pub global activate dependency_validator +``` + +Then run: ```bash # If installed as a dependency: @@ -8,16 +17,17 @@ $ dart run dependency_validator $ dart pub global run dependency_validator ``` -If needed, configure dependency_validator in your `pubspec.yaml`: +If needed, add a configuration in `dart_dependency_validator.yaml`: ```yaml -# pubsec.yaml -dependency_validator: - # Exclude one or more paths from being scanned. - # Supports glob syntax. - exclude: - - "app/**" - # Ignore one or more packages. - ignore: - - analyzer +# Exclude one or more paths from being scanned. Supports glob syntax. +exclude: + - "app/**" + +# Ignore one or more packages. +ignore: + - analyzer + +# Allow dependencies to be pinned to a specific version instead of a range +allowPins: true ``` From 8423b55415e68e4d8ef50f67281f9c3f1cb4f652 Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 8 Jan 2025 17:52:18 -0500 Subject: [PATCH 20/21] Bumped and added to changelog --- CHANGELOG.md | 5 +++++ pubspec.yaml | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c065297..1a307d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +# 5.0.0 + +- **Breaking change**: Requires Dart 3.0 or above +- Added support for Pub Workspaces (monorepos) + # 4.1.1 - Update the output of parse failures to include the path to the file which failed to parse diff --git a/pubspec.yaml b/pubspec.yaml index 6904bed..bf815de 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -1,5 +1,5 @@ name: dependency_validator -version: 4.1.2 +version: 5.0.0 description: Checks for missing, under-promoted, over-promoted, and unused dependencies. homepage: https://github.com/Workiva/dependency_validator From 2c79086d68a1e558fdbcd8aa73438a3b69e67fec Mon Sep 17 00:00:00 2001 From: Levi Lesches Date: Wed, 8 Jan 2025 18:00:10 -0500 Subject: [PATCH 21/21] Updated README --- README.md | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 6f08707..dc4a9e1 100644 --- a/README.md +++ b/README.md @@ -46,5 +46,24 @@ ignore: - analyzer ``` -> Note: Previously this configuration lived in the `pubspec.yaml`, but that +> [!Note] +> Previously this configuration lived in the `pubspec.yaml`, but that > option was deprecated because `pub publish` warns about unrecognized keys. + +## Pub Workspaces (monorepos) + +This package supports [Pub Workspaces](https://dart.dev/tools/pub/workspaces), a collection of packages in one repository. Workspaces allow Pub to share dependencies between your packages. Your top-level package's `pubspec.yaml` should have a `workspace` field that indicates which sub-packages should be included, like this: + +```yaml +workspace: + - pkg1 + - pkg2 +``` + +and your sub-packages should have `resolution: workspace` in their `pubspec.yaml`s. For more information, see the linked documentation. + +**Running `dependency_validator` will always validate the package your terminal is in**. If you run the tool on the top-level workspace package, it will analyze the workspace package _and_ its sub-packages. To just analyze a sub-package, run the tool in its folder, or pass the `-C` argument: + +```bash +$ dart run dependency_validator -C pkg1 +```