Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
## Unreleased

- Add config file support: any option can be set in a `ciach.yaml` in the
package root, and the command line overrides it. `--config <path>` reads one
from elsewhere, `--no-config` ignores it.
- Add `-v`, `--verbose` to narrate a run on stderr: config used, every setting
and the layer it came from, scan phases, what `--remove` touches. Supersedes
`--progress`.
- Resolve options with `package:config`, so each one is declared once for the

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mentioning this in changelog is unnecesary

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I know; there's a lot more of unnecessary writing in this PR – I'll make sure it's better ❤️

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@claude fix this

command line, the config file and its default. Invalid values are now all
reported at once, and `--concurrency 0` and a stray second path argument word
their errors slightly differently.
- Document the `--unused-union-members`, `--report-tojson`,
`--generated-suffix`, and `--help` options in the README, which existed in
the CLI but were missing from the options table.
Expand Down
304 changes: 138 additions & 166 deletions README.md

Large diffs are not rendered by default.

176 changes: 112 additions & 64 deletions bin/ciach.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ import 'dart:io';
import 'package:args/args.dart';
import 'package:ciach/ciach.dart';
import 'package:ciach/src/cli/args.dart';
import 'package:ciach/src/cli/config.dart';
import 'package:ciach/src/cli/options.dart';
import 'package:ciach/src/cli/verbose.dart';
import 'package:ciach/src/reporter.dart';
import 'package:collection/collection.dart';
import 'package:config/config.dart';
import 'package:path/path.dart' as p;

Future<void> main(List<String> arguments) async {
Expand Down Expand Up @@ -41,75 +46,80 @@ Future<int> _run(List<String> arguments) async {
return 0;
}

final rest = args.rest;
if (rest.length > 1) {
stderr.writeln(
'Expected at most one path argument, got: ${rest.join(', ')}',
);
return 2;
}
final rootPath = rest.isEmpty ? '.' : rest.first;
final rootDir = Directory(rootPath);
if (!rootDir.existsSync()) {
stderr.writeln('Path does not exist: $rootPath');
final ignoreConfig = args.flag('no-config');
final explicitConfig = args.option('config');
if (ignoreConfig && explicitConfig != null) {
stderr.writeln('--config cannot be combined with --no-config.');
return 2;
}

if (args.flag('force') && !args.flag('remove')) {
stderr.writeln('--force requires --remove.');
return 2;
}
// Discovery looks in the package root named on the command line — a config
// file's own `path` cannot decide where that config file is read from.
final projectDir = args.rest.isEmpty ? '.' : args.rest.first;

final Set<SymbolKind> kinds;
final ResolvedOptions resolved;
final ConfigFile config;
final CiachConfiguration configuration;
try {
kinds = parseKinds(args.multiOption('kinds'));
config = ConfigFile.load(
projectDir: projectDir,
explicitPath: explicitConfig,
ignore: ignoreConfig,
);
configuration = resolveConfiguration(args, config);
resolved = resolveOptions(
configuration,
colorDefault: stdout.supportsAnsiEscapes,
// Progress goes to stderr; default on only when it won't clutter a pipe.
progressDefault: stderr.hasTerminal,
);
} on UsageException catch (e) {
stderr.writeln(e.message);
return 2;
} on FormatException catch (e) {
stderr.writeln(e.message);
return 2;
}

// `allowed` on the option already rejects unknown values during parsing.
final format = args.option('format')!;
final log = resolved.verbose ? _VerboseLog() : null;
log?.writeAll(describeConfigSource(config, projectDir: projectDir));

final useColor = args.wasParsed('color')
? args.flag('color')
: stdout.supportsAnsiEscapes;
// Progress goes to stderr; default on only when it won't clutter a pipe.
final showProgress = args.wasParsed('progress')
? args.flag('progress')
: stderr.hasTerminal;
final rootDir = Directory(resolved.rootPath);
if (!rootDir.existsSync()) {
stderr.writeln('Path does not exist: ${resolved.rootPath}');
return 2;
}

final int concurrency;
try {
concurrency = int.parse(args.option('concurrency')!);
if (concurrency < 1) {
throw const FormatException();
}
} on FormatException {
stderr.writeln('--concurrency must be a positive integer.');
if (resolved.force && !resolved.remove) {
stderr.writeln(
'Skipping the removal prompt only makes sense when removing: --force (or `force: true`) requires --remove (or `remove: true`).',
);
return 2;
}

final options = FinderOptions(
rootPath: rootDir.absolute.path,
includeGlobs: args.multiOption('include'),
excludeGlobs: args.multiOption('exclude'),
kinds: kinds,
includePublic: args.flag('public'),
includeGenerated: args.flag('generated'),
additionalGeneratedSuffixes: args.multiOption('generated-suffix'),
skipOverrides: !args.flag('overrides'),
skipOperators: !args.flag('operators'),
unusedUnionMembers: args.flag('unused-union-members'),
reportToJson: args.flag('report-tojson'),
concurrency: concurrency,
dartExecutable: args.option('dart'),
onProgress: showProgress ? _ProgressPrinter().update : null,
final format = resolved.format;
final useColor = resolved.useColor;
final showProgress = resolved.showProgress;
final rootPath = resolved.absoluteRootPath;

log?.writeAll(
describeSettings(
configuration,
resolved,
dartExecutable: resolved.dartExecutable ?? Platform.resolvedExecutable,
),
);

final FinderResult result;
try {
result = await Ciach(options).run();
result = await Ciach(
resolved.finderOptions(
// The finder narrates its phases through one callback; verbose keeps
// every line, plain progress overwrites a single one in place.
onProgress:
log?.write ?? (showProgress ? _ProgressPrinter().update : null),
),
).run();
} on Object catch (e, st) {
if (showProgress) {
stderr.writeln();
Expand All @@ -124,55 +134,68 @@ Future<int> _run(List<String> arguments) async {
stderr.writeln();
}

log?.write(
'Scanned ${result.filesScanned} file(s) and checked ${result.declarationsChecked} declaration(s) in ${result.elapsed.inMilliseconds}ms: ${result.unused.length} unused, ${result.docOnly.length} referenced only from doc comments.',
);

switch (format) {
case 'json':
stdout.writeln(Reporter.json(result));
case 'github':
// GitHub resolves annotation paths from the repo root; make the finding
// paths root-relative by prepending the scan root's path from here.
final prefix = p
.split(
p.relative(rootDir.absolute.path, from: Directory.current.path),
)
.split(p.relative(rootPath, from: Directory.current.path))
.join('/');
log?.write("Prefixing annotation paths with '$prefix/'.");
stdout.write(Reporter.github(result, pathPrefix: prefix));
case _:
stdout.writeln(Reporter.text(result, useColor: useColor));
}

if (result.unused.isNotEmpty && args.flag('remove')) {
await _removeUnused(result, rootDir.absolute.path, args, format, useColor);
if (result.unused.isNotEmpty && resolved.remove) {
await _removeUnused(result, rootPath, resolved, format, useColor, log);
} else if (result.unused.isNotEmpty) {
log?.write('Leaving the findings in place; --remove was not given.');
}

if (result.unused.isNotEmpty && args.flag('set-exit-if-changed')) {
if (result.unused.isNotEmpty && resolved.setExitIfChanged) {
return 1;
}
return 0;
}

/// Reports what would be removed, confirms unless [ArgResults.flag]
/// `'force'` is set, and deletes the unused declarations from disk.
/// Reports what would be removed, confirms unless [ResolvedOptions.force] is
/// set, and deletes the unused declarations from disk.
Future<void> _removeUnused(
FinderResult result,
String rootPath,
ArgResults args,
ResolvedOptions resolved,
String format,
bool useColor,
_VerboseLog? log,
) async {
final count = result.unused.length;
final plural = count == 1 ? '' : 's';

var proceed = args.flag('force');
final blocked = result.unused.where((d) => d.removalBlocked).length;
if (blocked > 0) {
log?.write(
'Skipping $blocked of $count finding$plural: removing them safely would need a source rewrite (see --unused-union-members and remove safety).',
);
}

var proceed = resolved.force;
if (!proceed) {
log?.write('Asking for confirmation; pass --force to skip the prompt.');
// The chosen --format may not be human-readable; show the findings
// again so the confirmation prompt is never a shot in the dark.
if (format != 'text') {
stderr.writeln(Reporter.text(result, useColor: useColor));
}
if (!stdin.hasTerminal) {
stdout.writeln(
'Refusing to remove declarations without a terminal to confirm on; '
'pass --force to remove without asking.',
'Refusing to remove declarations without a terminal to confirm on; pass --force to remove without asking.',
);
return;
}
Expand All @@ -188,10 +211,18 @@ Future<void> _removeUnused(
return;
}

if (log != null) {
final byFile = result.unused
.whereNot((d) => d.removalBlocked)
.groupFoldBy<String, int>((d) => d.filePath, (n, _) => (n ?? 0) + 1);
for (final entry in byFile.entries) {
log.write('Rewriting ${entry.key} (${entry.value} declaration(s)).');
}
}

final filesChanged = removeDeclarations(result.unused, rootPath);
stdout.writeln(
'Removed $count unused declaration$plural from $filesChanged '
"file${filesChanged == 1 ? '' : 's'}. Run 'dart format' to tidy up spacing.",
"Removed $count unused declaration$plural from $filesChanged file${filesChanged == 1 ? '' : 's'}. Run 'dart format' to tidy up spacing.",
);
// Surface any advisory hints (e.g. a removed prevent-instantiation
// constructor) once more, since removing the declaration also removes the
Expand All @@ -205,6 +236,23 @@ Future<void> _removeUnused(
}
}

/// Prints `--verbose` narration to stderr, one durable line per message,
/// stamped with the elapsed time so slow phases stand out.
///
/// stderr, not stdout, so `-f json` output stays machine-readable.
class _VerboseLog {
final _stopwatch = Stopwatch()..start();

/// Writes one line, prefixed with the elapsed time.
void write(String message) {
final seconds = (_stopwatch.elapsedMilliseconds / 1000).toStringAsFixed(1);
stderr.writeln('[${seconds.padLeft(5)}s] $message');
}

/// Writes a line per message in [messages].
void writeAll(Iterable<String> messages) => messages.forEach(write);
}

/// Prints single-line, overwriting progress to stderr.
class _ProgressPrinter {
int _lastLength = 0;
Expand Down
Loading