From c5fccae54fa305d60302f453f8b192f7b51977ea Mon Sep 17 00:00:00 2001 From: Morgan Hunt <39742020+mrgnhnt96@users.noreply.github.com> Date: Sat, 27 Jan 2024 10:51:06 -0700 Subject: [PATCH] feat: Support nested analysis options files (#212) --- packages/custom_lint/CHANGELOG.md | 4 + .../custom_lint/lib/src/package_utils.dart | 50 +++++++++++ .../lib/src/v2/server_to_client_channel.dart | 2 +- packages/custom_lint/lib/src/workspace.dart | 43 ++++++--- .../custom_lint/test/cli_process_test.dart | 5 +- .../custom_lint/test/src/workspace_test.dart | 88 ++++++++++++++++--- .../custom_lint_builder/lib/src/client.dart | 17 ++-- 7 files changed, 177 insertions(+), 32 deletions(-) diff --git a/packages/custom_lint/CHANGELOG.md b/packages/custom_lint/CHANGELOG.md index 745dd070..ffbf4e49 100644 --- a/packages/custom_lint/CHANGELOG.md +++ b/packages/custom_lint/CHANGELOG.md @@ -1,3 +1,7 @@ +## Unreleased minor + +- Added support for `analysis_options.yaml` that are nt at the root of the project (thanks to @mrgnhnt96) + ## 0.5.8 - 2024-01-09 - `// ignore` comments now correctly respect indentation when they are inserted (thanks to @PiotrRogulski) diff --git a/packages/custom_lint/lib/src/package_utils.dart b/packages/custom_lint/lib/src/package_utils.dart index fe69656d..8e184871 100644 --- a/packages/custom_lint/lib/src/package_utils.dart +++ b/packages/custom_lint/lib/src/package_utils.dart @@ -1,5 +1,6 @@ import 'dart:io'; +import 'package:meta/meta.dart'; import 'package:package_config/package_config.dart'; import 'package:path/path.dart'; import 'package:pubspec_parse/pubspec_parse.dart'; @@ -155,8 +156,57 @@ Future tryParsePackageConfig(Directory directory) async { /// does not exists. Future parsePackageConfig(Directory directory) async { final packageConfigFile = directory.packageConfig; + return PackageConfig.parseBytes( await packageConfigFile.readAsBytes(), packageConfigFile.uri, ); } + +/// Finds the project directory associated with an analysis context root, or null if it is not found +/// +/// This is a folder that contains both a `pubspec.yaml` and a `.dart_tool/package_config.json` file. +/// It is either alongside the analysis_options.yaml file, or in a parent directory. +Directory? tryFindProjectDirectory( + Directory directory, { + Directory? original, +}) { + try { + return findProjectDirectory( + directory, + original: original, + ); + } catch (_) { + return null; + } +} + +/// Finds the project directory associated with an analysis context root +/// +/// This is a folder that contains a `pubspec.yaml` file. +/// It is either alongside the analysis_options.yaml file, or in a parent directory. +Directory findProjectDirectory( + Directory directory, { + Directory? original, +}) { + if (directory.pubspec.existsSync()) { + return directory; + } + + if (directory.parent.uri == directory.uri) { + throw FindProjectError._(original?.path ?? directory.path); + } + + return findProjectDirectory(directory.parent, original: directory); +} + +/// No pubspec.yaml file was found for a plugin. +@internal +class FindProjectError extends FileSystemException { + /// An error that represents the folder [path] where the search for the pubspec started. + FindProjectError._(String path) + : super('Failed to find dart project at $path:\n', path); + + @override + String toString() => message; +} diff --git a/packages/custom_lint/lib/src/v2/server_to_client_channel.dart b/packages/custom_lint/lib/src/v2/server_to_client_channel.dart index e287c225..83549726 100644 --- a/packages/custom_lint/lib/src/v2/server_to_client_channel.dart +++ b/packages/custom_lint/lib/src/v2/server_to_client_channel.dart @@ -325,6 +325,6 @@ class CustomLintRequestFailure implements Exception { @override String toString() { - return 'A request throw the exception:$message\n$stackTrace'; + return 'A request threw the exception:$message\n$stackTrace'; } } diff --git a/packages/custom_lint/lib/src/workspace.dart b/packages/custom_lint/lib/src/workspace.dart index 9c8c2aee..c4de8607 100644 --- a/packages/custom_lint/lib/src/workspace.dart +++ b/packages/custom_lint/lib/src/workspace.dart @@ -432,10 +432,13 @@ class CustomLintWorkspace { final contextRootsWithCustomLint = await Future.wait( allContextRoots.map((contextRoot) async { - final pubspecFile = Directory(contextRoot.root.path).pubspec; - if (!pubspecFile.existsSync()) { - return null; - } + final projectDir = tryFindProjectDirectory( + Directory(contextRoot.root.path), + ); + if (projectDir == null) return null; + + final pubspec = await tryParsePubspec(projectDir); + if (pubspec == null) return null; final optionFile = contextRoot.optionsFile; if (optionFile == null) { @@ -492,8 +495,10 @@ class CustomLintWorkspace { final uniquePluginNames = projects.expand((e) => e.plugins).map((e) => e.name).toSet(); + final realProjects = projects.where((e) => e.isProjectRoot).toList(); + return CustomLintWorkspace._( - projects, + realProjects, contextRoots, uniquePluginNames, workingDirectory: workingDirectory, @@ -857,7 +862,8 @@ class CustomLintPluginCheckerCache { if (cached != null) return cached; return _cache[directory] = Future(() async { - final pubspec = await parsePubspec(directory); + final pubspec = await tryParsePubspec(directory); + if (pubspec == null) return false; // TODO test that dependency_overrides & dev_dependencies aren't checked. return pubspec.dependencies.containsKey('custom_lint_builder'); @@ -968,6 +974,7 @@ class CustomLintProject { required this.packageConfig, required this.pubspec, required this.pubspecOverrides, + required this.analysisDirectory, }); /// Decode a [CustomLintProject] from a directory. @@ -976,18 +983,18 @@ class CustomLintProject { CustomLintPluginCheckerCache cache, ) async { final directory = Directory(contextRoot.root); - - final projectPubspec = await parsePubspec(directory) + final projectDirectory = findProjectDirectory(directory); + final projectPubspec = await parsePubspec(projectDirectory).catchError( // ignore: avoid_types_on_closure_parameters - .catchError((Object err, StackTrace stack) { + (Object err, StackTrace stack) { throw PubspecParseError._( directory.path, error: err, errorStackTrace: stack, ); }); - final pubspecOverrides = await tryParsePubspecOverrides(directory); - final projectPackageConfig = await parsePackageConfig(directory) + final pubspecOverrides = await tryParsePubspecOverrides(projectDirectory); + final projectPackageConfig = await parsePackageConfig(projectDirectory) // ignore: avoid_types_on_closure_parameters .catchError((Object err, StackTrace stack) { throw PackageConfigParseError._( @@ -1027,7 +1034,8 @@ class CustomLintProject { return CustomLintProject._( plugins: plugins.whereNotNull().toList(), - directory: directory, + directory: projectDirectory, + analysisDirectory: directory, packageConfig: projectPackageConfig, pubspec: projectPubspec, pubspecOverrides: pubspecOverrides, @@ -1044,10 +1052,21 @@ class CustomLintProject { final Map? pubspecOverrides; /// The folder of the project being analyzed. + /// Generally, where the pubspec.yaml is located final Directory directory; /// The enabled plugins for this project. final List plugins; + + /// Where the analysis options file is located + /// It could be null if the project doesn't have an analysis options file. + /// + /// The analysis options file doesn't not have to be in [directory] + final Directory? analysisDirectory; + + bool get isProjectRoot { + return analysisDirectory == directory; + } } class _PackageAndPubspec { diff --git a/packages/custom_lint/test/cli_process_test.dart b/packages/custom_lint/test/cli_process_test.dart index 6d9722bf..1dd52922 100644 --- a/packages/custom_lint/test/cli_process_test.dart +++ b/packages/custom_lint/test/cli_process_test.dart @@ -278,7 +278,10 @@ Analyzing... 'dependency conflict', () async { // Create two packages with the same name but different paths - final workspace = await createSimpleWorkspace(['dep', 'dep']); + final workspace = await createSimpleWorkspace( + ['dep', 'dep'], + local: true, + ); final plugin = createPlugin( parent: workspace, diff --git a/packages/custom_lint/test/src/workspace_test.dart b/packages/custom_lint/test/src/workspace_test.dart index 5505a5a5..8a3e265e 100644 --- a/packages/custom_lint/test/src/workspace_test.dart +++ b/packages/custom_lint/test/src/workspace_test.dart @@ -231,6 +231,7 @@ void writeSimplePackageConfig( Future createSimpleWorkspace( List projectEntry, { bool withPackageConfig = true, + bool local = false, }) async { /// The number of time we've created a package with a given name. final packageCount = {}; @@ -249,7 +250,7 @@ Future createSimpleWorkspace( return folderName; } - return createWorkspace(withPackageConfig: withPackageConfig, { + return createWorkspace(local: local, withPackageConfig: withPackageConfig, { for (final projectEntry in projectEntry) if (projectEntry is Pubspec) getFolderName(projectEntry.name): projectEntry @@ -275,8 +276,9 @@ Future createSimpleWorkspace( Future createWorkspace( Map pubspecs, { bool withPackageConfig = true, + bool local = false, }) async { - final dir = createTemporaryDirectory(); + final dir = createTemporaryDirectory(local: local); String packagePathOf(Dependency dependency, String name) { switch (dependency) { @@ -339,10 +341,16 @@ Future createWorkspace( return dir; } -Directory createTemporaryDirectory() { - final dir = Directory.current // - .dir('.dart_tool') - .createTempSync('custom_lint_test'); +Directory createTemporaryDirectory({bool local = false}) { + final Directory dir; + if (local) { + // The cli_process_test needs it to be local in order for the relative paths to match + dir = + Directory.current.dir('.dart_tool').createTempSync('custom_lint_test'); + } else { + // Others need global directory in order to not pick up this project's package_config.json + dir = Directory.systemTemp.createTempSync('custom_lint_test'); + } addTearDown(() => dir.deleteSync(recursive: true)); // Watches process kill to delete the temporary directory. @@ -2453,25 +2461,64 @@ dependency_overrides: }); group('fromContextRoots', () { - test('throws MissingPubspecError if package does not contain a pubspec', + /// Shorthand for calling [CustomLintWorkspace.fromContextRoots] from + /// a list of path. + Future fromContextRootsFromPaths( + List paths, { + required Directory workingDirectory, + }) { + return CustomLintWorkspace.fromContextRoots( + paths.map((path) => ContextRoot(path, [])).toList(), + workingDirectory: workingDirectory, + ); + } + + test( + 'finds pubspecs above analysis options file if there exists one', + () async { + final workspace = await createSimpleWorkspace(['package']); + + final analysisFile = workspace.dir('package').analysisOptions; + analysisFile.createSync(); + analysisFile.writeAsStringSync(analysisOptionsWithCustomLintEnabled); + final nestedAnalysisFile = + workspace.dir('package', 'test').analysisOptions; + nestedAnalysisFile.createSync(recursive: true); + nestedAnalysisFile + .writeAsStringSync(analysisOptionsWithCustomLintEnabled); + + final customLintWorkspace = await CustomLintWorkspace.fromPaths( + [p.join(workspace.path, 'package')], + workingDirectory: workspace, + ); + // Expect one context root for the workspace and one for the test folder + expect(customLintWorkspace.contextRoots.length, equals(2)); + }, + ); + + test( + 'throws PackageConfigParseError if package has a pubspec but no .dart_tool/package_config.json', () async { - final workspace = await createSimpleWorkspace([]); - workspace.dir('package').createSync(recursive: true); + final workspace = await createSimpleWorkspace(['package']); + workspace.dir('package', '.dart_tool').deleteSync(recursive: true); expect( () => fromContextRootsFromPaths( [p.join(workspace.path, 'package')], workingDirectory: workspace, ), - throwsA(isA()), + throwsA(isA()), ); }); test( - 'throws MissingPackageConfigError if package has a pubspec but no .dart_tool/package_config.json', + 'throws PackageConfigParseError if package has a malformed .dart_tool/package_config.json', () async { final workspace = await createSimpleWorkspace(['package']); - workspace.dir('package', '.dart_tool').deleteSync(recursive: true); + workspace + .dir('package', '.dart_tool') + .file('package_config.json') + .writeAsStringSync('malformed'); expect( () => fromContextRootsFromPaths( @@ -2482,6 +2529,23 @@ dependency_overrides: ); }); + test('throws PubspecParseError if package has a malformed pubspec.yaml', + () async { + final workspace = await createSimpleWorkspace(['package']); + workspace + .dir('package') + .file('pubspec.yaml') + .writeAsStringSync('malformed'); + + expect( + () => fromContextRootsFromPaths( + [p.join(workspace.path, 'package')], + workingDirectory: workspace, + ), + throwsA(isA()), + ); + }); + test('Supports empty workspace', () async { final customLintWorkspace = await fromContextRootsFromPaths( [], diff --git a/packages/custom_lint_builder/lib/src/client.dart b/packages/custom_lint_builder/lib/src/client.dart index 736350a8..d9cde641 100644 --- a/packages/custom_lint_builder/lib/src/client.dart +++ b/packages/custom_lint_builder/lib/src/client.dart @@ -375,12 +375,17 @@ class _ClientAnalyzerPlugin extends analyzer_plugin.ServerPlugin { // Otherwise tests may miss the first hot reload. await _client._hotReloader; - final pubspecs = { - for (final analysisContext in contextCollection.contexts) - analysisContext: parsePubspec( - io.Directory(analysisContext.contextRoot.root.path), - ), - }; + final pubspecs = >{}; + + for (final context in contextCollection.contexts) { + final pubspec = tryFindProjectDirectory( + io.Directory(context.contextRoot.root.path), + ); + + if (pubspec != null) { + pubspecs[context] = parsePubspec(pubspec); + } + } // Running before updating the configs as the config parsing depends // on this operation.