Skip to content

Commit

Permalink
feat: Support nested analysis options files (#212)
Browse files Browse the repository at this point in the history
  • Loading branch information
mrgnhnt96 authored Jan 27, 2024
1 parent f934981 commit c5fccae
Show file tree
Hide file tree
Showing 7 changed files with 177 additions and 32 deletions.
4 changes: 4 additions & 0 deletions packages/custom_lint/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
Expand Down
50 changes: 50 additions & 0 deletions packages/custom_lint/lib/src/package_utils.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -155,8 +156,57 @@ Future<PackageConfig?> tryParsePackageConfig(Directory directory) async {
/// does not exists.
Future<PackageConfig> 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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
}
43 changes: 31 additions & 12 deletions packages/custom_lint/lib/src/workspace.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -968,6 +974,7 @@ class CustomLintProject {
required this.packageConfig,
required this.pubspec,
required this.pubspecOverrides,
required this.analysisDirectory,
});

/// Decode a [CustomLintProject] from a directory.
Expand All @@ -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._(
Expand Down Expand Up @@ -1027,7 +1034,8 @@ class CustomLintProject {

return CustomLintProject._(
plugins: plugins.whereNotNull().toList(),
directory: directory,
directory: projectDirectory,
analysisDirectory: directory,
packageConfig: projectPackageConfig,
pubspec: projectPubspec,
pubspecOverrides: pubspecOverrides,
Expand All @@ -1044,10 +1052,21 @@ class CustomLintProject {
final Map<String, Dependency>? 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<CustomLintPlugin> 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 {
Expand Down
5 changes: 4 additions & 1 deletion packages/custom_lint/test/cli_process_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
88 changes: 76 additions & 12 deletions packages/custom_lint/test/src/workspace_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ void writeSimplePackageConfig(
Future<Directory> createSimpleWorkspace(
List<Object> projectEntry, {
bool withPackageConfig = true,
bool local = false,
}) async {
/// The number of time we've created a package with a given name.
final packageCount = <String, int>{};
Expand All @@ -249,7 +250,7 @@ Future<Directory> 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
Expand All @@ -275,8 +276,9 @@ Future<Directory> createSimpleWorkspace(
Future<Directory> createWorkspace(
Map<String, Pubspec> pubspecs, {
bool withPackageConfig = true,
bool local = false,
}) async {
final dir = createTemporaryDirectory();
final dir = createTemporaryDirectory(local: local);

String packagePathOf(Dependency dependency, String name) {
switch (dependency) {
Expand Down Expand Up @@ -339,10 +341,16 @@ Future<Directory> 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.
Expand Down Expand Up @@ -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<CustomLintWorkspace> fromContextRootsFromPaths(
List<String> 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<PubspecParseError>()),
throwsA(isA<PackageConfigParseError>()),
);
});

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(
Expand All @@ -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<PubspecParseError>()),
);
});

test('Supports empty workspace', () async {
final customLintWorkspace = await fromContextRootsFromPaths(
[],
Expand Down
17 changes: 11 additions & 6 deletions packages/custom_lint_builder/lib/src/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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 = <AnalysisContext, Future<Pubspec>>{};

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.
Expand Down

1 comment on commit c5fccae

@vercel
Copy link

@vercel vercel bot commented on c5fccae Jan 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.