Skip to content

Commit

Permalink
Allow configuring project-wide page width using a surrounding analysi…
Browse files Browse the repository at this point in the history
…s_options.yaml file (#1571)

When using the dart format CLI (and not the library API), if the user doesn't specify a page width using --line-length, then the formatter will walk the directories surrounding each formatted file looking for an analysis_options.yaml file. If one is found, then it looks for a configured page width like:

```
formatter:
  page_width: 123
```

If found, then the file is formatted at that page width. If any sort of failure occurs, the default page width is used instead.

This is hidden behind the "tall-style" experiment flag and the intent is to ship this when the rest of the new tall style ships.

This is a fairly large change. To try to make it easier to review, I broke it into a series of hopefully more digestible commits. You might want to review those separately.

Fix #833.
  • Loading branch information
munificent authored Oct 7, 2024
1 parent fb00aab commit 9c17d3b
Show file tree
Hide file tree
Showing 27 changed files with 2,431 additions and 948 deletions.
27 changes: 27 additions & 0 deletions analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,30 @@ include: package:dart_flutter_team_lints/analysis_options.yaml
analyzer:
errors:
comment_references: ignore
linter:
rules:
# Either "unnecessary_final" or "prefer_final_locals" should be used so
# that the codebase consistently uses either "var" or "final" for local
# variables. Choosing the former because the latter also requires "final"
# even on local variables and pattern variables that have type annotations,
# as in:
#
# final Object upcast = 123;
# //^^^ Unnecessarily verbose.
#
# switch (json) {
# case final List list: ...
# // ^^^^^ Unnecessarily verbose.
# }
#
# Using "unnecessary_final" allows those to be:
#
# Object upcast = 123;
#
# switch (json) {
# case List list: ...
# }
#
# Also, making local variables non-final is consistent with parameters,
# which are also non-final.
- unnecessary_final
115 changes: 115 additions & 0 deletions lib/src/analysis_options/analysis_options_file.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:yaml/yaml.dart';

import 'file_system.dart';
import 'merge_options.dart';

/// The analysis options configuration is a dynamically-typed JSON-like data
/// structure.
///
/// (It's JSON-*like* and not JSON because maps in it may have non-string keys.)
typedef AnalysisOptions = Map<Object?, Object?>;

/// Interface for taking a "package:" URI that may appear in an analysis
/// options file's "include" key and resolving it to a file path which can be
/// passed to [FileSystem.join()].
typedef ResolvePackageUri = Future<String?> Function(Uri packageUri);

/// Reads an `analysis_options.yaml` file in [directory] or in the nearest
/// surrounding folder that contains that file using [fileSystem].
///
/// Stops walking parent directories as soon as it finds one that contains an
/// `analysis_options.yaml` file. If it reaches the root directory without
/// finding one, returns an empty [YamlMap].
///
/// If an `analysis_options.yaml` file is found, reads it and parses it to a
/// [YamlMap]. If the map contains an `include` key whose value is a list, then
/// reads any of the other referenced YAML files and merges them into this one.
/// Returns the resulting map with the `include` key removed.
///
/// If there any "package:" includes, then they are resolved to file paths
/// using [resolvePackageUri]. If [resolvePackageUri] is omitted, an exception
/// is thrown if any "package:" includes are found.
Future<AnalysisOptions> findAnalysisOptions(
FileSystem fileSystem, FileSystemPath directory,
{ResolvePackageUri? resolvePackageUri}) async {
while (true) {
var optionsPath = await fileSystem.join(directory, 'analysis_options.yaml');
if (await fileSystem.fileExists(optionsPath)) {
return readAnalysisOptions(fileSystem, optionsPath,
resolvePackageUri: resolvePackageUri);
}

var parent = await fileSystem.parentDirectory(directory);
if (parent == null) break;
directory = parent;
}

// If we get here, we didn't find an analysis_options.yaml.
return const {};
}

/// Uses [fileSystem] to read the analysis options file at [optionsPath].
///
/// If there any "package:" includes, then they are resolved to file paths
/// using [resolvePackageUri]. If [resolvePackageUri] is omitted, an exception
/// is thrown if any "package:" includes are found.
Future<AnalysisOptions> readAnalysisOptions(
FileSystem fileSystem, FileSystemPath optionsPath,
{ResolvePackageUri? resolvePackageUri}) async {
var yaml = loadYamlNode(await fileSystem.readFile(optionsPath));

// If for some reason the YAML isn't a map, consider it malformed and yield
// a default empty map.
if (yaml is! YamlMap) return const {};

// Lower the YAML to a regular map.
var options = {...yaml};

// If there is an `include:` key, then load that and merge it with these
// options.
if (options['include'] case String include) {
options.remove('include');

// If the include path is "package:", resolve it to a file path first.
var includeUri = Uri.tryParse(include);
if (includeUri != null && includeUri.scheme == 'package') {
if (resolvePackageUri != null) {
var filePath = await resolvePackageUri(includeUri);
if (filePath != null) {
include = filePath;
} else {
throw PackageResolutionException(
'Failed to resolve package URI "$include" in include.');
}
} else {
throw PackageResolutionException(
'Couldn\'t resolve package URI "$include" in include because '
'no package resolver was provided.');
}
}

// The include path may be relative to the directory containing the current
// options file.
var includePath = await fileSystem.join(
(await fileSystem.parentDirectory(optionsPath))!, include);
var includeFile = await readAnalysisOptions(fileSystem, includePath,
resolvePackageUri: resolvePackageUri);
options = merge(includeFile, options) as AnalysisOptions;
}

return options;
}

/// Exception thrown when an analysis options file contains a "package:" URI in
/// an include and resolving the URI to a file path failed.
final class PackageResolutionException implements Exception {
final String _message;

PackageResolutionException(this._message);

@override
String toString() => _message;
}
44 changes: 44 additions & 0 deletions lib/src/analysis_options/file_system.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

/// Abstraction over a file system.
///
/// Implement this if you want to control how this package locates and reads
/// files.
abstract interface class FileSystem {
/// Returns `true` if there is a file at [path].
Future<bool> fileExists(covariant FileSystemPath path);

/// Joins [from] and [to] into a single path with appropriate path separators.
///
/// Note that [to] may be an absolute path implementation of [join()] should
/// be prepared to handle that by ignoring [from].
Future<FileSystemPath> join(covariant FileSystemPath from, String to);

/// Returns a path for the directory containing [path].
///
/// If [path] is a root path, then returns `null`.
Future<FileSystemPath?> parentDirectory(covariant FileSystemPath path);

/// Returns the series of directories surrounding [path], from innermost out.
///
/// If [path] is itself a directory, then it should be the first directory
/// yielded by this. Otherwise, the stream should begin with the directory
/// containing that file.
// Stream<FileSystemPath> parentDirectories(FileSystemPath path);

/// Reads the contents of the file as [path], which should exist and contain
/// UTF-8 encoded text.
Future<String> readFile(covariant FileSystemPath path);
}

/// Abstraction over a file or directory in a [FileSystem].
///
/// An implementation of [FileSystem] should have a corresponding implementation
/// of this class. It can safely assume that any instances of this passed in to
/// the class were either directly created as instances of the implementation
/// class by the host application, or were returned by methods on that same
/// [FileSystem] object. Thus it is safe for an implementation of [FileSystem]
/// to downcast instances of this to its expected implementation type.
abstract interface class FileSystemPath {}
50 changes: 50 additions & 0 deletions lib/src/analysis_options/io_file_system.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:io';

import 'package:path/path.dart' as p;

import 'file_system.dart';

/// An implementation of [FileSystem] using `dart:io`.
final class IOFileSystem implements FileSystem {
Future<IOFileSystemPath> makePath(String path) async =>
IOFileSystemPath._(path);

@override
Future<bool> fileExists(covariant IOFileSystemPath path) =>
File(path.path).exists();

@override
Future<FileSystemPath> join(covariant IOFileSystemPath from, String to) =>
makePath(p.join(from.path, to));

@override
Future<FileSystemPath?> parentDirectory(
covariant IOFileSystemPath path) async {
// Make [path] absolute (if not already) so that we can walk outside of the
// literal path string passed.
var result = p.dirname(p.absolute(path.path));

// If the parent directory is the same as [path], we must be at the root.
if (result == path.path) return null;

return makePath(result);
}

@override
Future<String> readFile(covariant IOFileSystemPath path) =>
File(path.path).readAsString();
}

/// An abstraction over a file path string, used by [IOFileSystem].
///
/// To create an instance of this, use [IOFileSystem.makePath()].
final class IOFileSystemPath implements FileSystemPath {
/// The underlying physical file system path.
final String path;

IOFileSystemPath._(this.path);
}
65 changes: 65 additions & 0 deletions lib/src/analysis_options/merge_options.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

/// Merges a [defaults] options set with an [overrides] options set using
/// simple override semantics, suitable for merging two configurations where
/// one defines default values that are added to (and possibly overridden) by an
/// overriding one.
///
/// The merge rules are:
///
/// * Lists are concatenated without duplicates.
/// * A list of strings is promoted to a map of strings to `true` when merged
/// with another map of strings to booleans. For example `['opt1', 'opt2']`
/// is promoted to `{'opt1': true, 'opt2': true}`.
/// * Maps unioned. When both have the same key, the corresponding values are
/// merged, recursively.
/// * Otherwise, a non-`null` override replaces a default value.
Object? merge(Object? defaults, Object? overrides) {
return switch ((defaults, overrides)) {
(List(isAllStrings: true) && var list, Map(isToBools: true)) =>
merge(_promoteList(list), overrides),
(Map(isToBools: true), List(isAllStrings: true) && var list) =>
merge(defaults, _promoteList(list)),
(Map defaultsMap, Map overridesMap) => _mergeMap(defaultsMap, overridesMap),
(List defaultsList, List overridesList) =>
_mergeList(defaultsList, overridesList),
(_, null) =>
// Default to override, unless the overriding value is `null`.
defaults,
_ => overrides,
};
}

/// Promote a list of strings to a map of those strings to `true`.
Map<Object?, Object?> _promoteList(List<Object?> list) {
return {for (var element in list) element: true};
}

/// Merge lists, avoiding duplicates.
List<Object?> _mergeList(List<Object?> defaults, List<Object?> overrides) {
// Add them both to a set so that the overrides replace the defaults.
return {...defaults, ...overrides}.toList();
}

/// Merge maps (recursively).
Map<Object?, Object?> _mergeMap(
Map<Object?, Object?> defaults, Map<Object?, Object?> overrides) {
var merged = {...defaults};

overrides.forEach((key, value) {
merged.update(key, (defaultValue) => merge(defaultValue, value),
ifAbsent: () => value);
});

return merged;
}

extension<T> on List<T> {
bool get isAllStrings => every((e) => e is String);
}

extension<K, V> on Map<K, V> {
bool get isToBools => values.every((v) => v is bool);
}
9 changes: 6 additions & 3 deletions lib/src/cli/format_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,12 @@ final class FormatCommand extends Command<int> {
}
}

var pageWidth = int.tryParse(argResults['line-length'] as String) ??
usageException('--line-length must be an integer, was '
'"${argResults['line-length']}".');
int? pageWidth;
if (argResults.wasParsed('line-length')) {
pageWidth = int.tryParse(argResults['line-length'] as String) ??
usageException('--line-length must be an integer, was '
'"${argResults['line-length']}".');
}

var indent = int.tryParse(argResults['indent'] as String) ??
usageException('--indent must be an integer, was '
Expand Down
9 changes: 6 additions & 3 deletions lib/src/cli/formatter_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ final class FormatterOptions {
final int indent;

/// The number of columns that formatted output should be constrained to fit
/// within.
final int pageWidth;
/// within or `null` if not specified.
///
/// If omitted, the formatter defaults to a page width of
/// [DartFormatter.defaultPageWidth].
final int? pageWidth;

/// Whether symlinks should be traversed when formatting a directory.
final bool followLinks;
Expand All @@ -49,7 +52,7 @@ final class FormatterOptions {
FormatterOptions(
{this.languageVersion,
this.indent = 0,
this.pageWidth = 80,
this.pageWidth,
this.followLinks = false,
this.show = Show.changed,
this.output = Output.write,
Expand Down
Loading

0 comments on commit 9c17d3b

Please sign in to comment.