diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b77328b..9fc298d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -13,6 +13,11 @@ jobs: runs-on: ubuntu-latest + permissions: + contents: read + id-token: write + statuses: write + steps: - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 @@ -39,6 +44,79 @@ jobs: with: dry-run: true + # Informational: keep the `pana` status out of the required checks. It + # carries the verdict, so nothing below fails this job. + - name: Set up the latest stable Dart for pana + uses: dart-lang/setup-dart@65eb853c7ba17dde3be364c3d2858773e7144260 # v1.7.2 + with: + sdk: stable # the score has to be the one pub.dev will give + + - name: Install pana + run: dart pub global activate pana + + - name: Score the package + id: score + continue-on-error: true + run: | + pana --json . > "$RUNNER_TEMP/pana.json" + + granted=$(jq '.scores.grantedPoints' "$RUNNER_TEMP/pana.json") + max=$(jq '.scores.maxPoints' "$RUNNER_TEMP/pana.json") + + # Two nulls would compare equal and read as a full score. + case "$granted$max" in + '' | *[!0-9]*) + echo "::error title=pana score::no score in pana's JSON report" + exit 1 + ;; + esac + + echo "score=$granted/$max" >> "$GITHUB_OUTPUT" + if [ "$granted" = "$max" ]; then + echo 'state=success' >> "$GITHUB_OUTPUT" + else + echo 'state=failure' >> "$GITHUB_OUTPUT" + fi + + { + echo "## pana score: $granted/$max" + echo + jq -r '.report.sections[] + | "- \(if .grantedPoints == .maxPoints then ":white_check_mark:" else ":x:" end) \(.title): \(.grantedPoints)/\(.maxPoints)"' \ + "$RUNNER_TEMP/pana.json" + + if [ "$granted" != "$max" ]; then + echo + echo '
Deductions' + echo + jq -r '.report.sections[] + | select(.grantedPoints < .maxPoints) + | .summary' "$RUNNER_TEMP/pana.json" + echo + echo '
' + fi + } >> "$GITHUB_STEP_SUMMARY" + + - name: Report the score + continue-on-error: true + # A fork's token can't write statuses. + if: | + always() && (github.event_name != 'pull_request' || + github.event.pull_request.head.repo.full_name == github.repository) + env: + GH_TOKEN: ${{ github.token }} + # The pull request head, not the merge commit `github.sha` names. + SHA: ${{ github.event.pull_request.head.sha || github.sha }} + STATE: ${{ steps.score.outputs.state || 'error' }} + SCORE: ${{ steps.score.outputs.score || 'pana produced no score' }} + run: | + gh api "repos/$GITHUB_REPOSITORY/statuses/$SHA" \ + --silent \ + -f state="$STATE" \ + -f context='pana' \ + -f description="$SCORE" \ + -f target_url="$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" + verify-sdk-floor: name: Verify Dart 3.10 floor diff --git a/CHANGELOG.md b/CHANGELOG.md index 633c99c..b4cb678 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ ## 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 ` 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`. - Fix a comment mentioning `@override` or `vm:entry-point` skipping the declaration below it. ([#29](https://github.com/leancodepl/ciach/pull/29)) - Add a secondary `textDocument/definition` check for zero-reference declarations, so valid uses the reference search misses no longer produce diff --git a/README.md b/README.md index 756e969..cf94fcb 100644 --- a/README.md +++ b/README.md @@ -20,64 +20,40 @@ AI-Provenance: [![Test status][test-badge]][test-badge-link] [![License: Apache 2.0][license-badge]][license-badge-link] -**Dead code detector and unused code finder for Dart and Flutter.** Finds -**unused (never-referenced) declarations** — classes, functions, methods, -fields, constants, enum values, and so on — in a Dart or Flutter package, and +**Dead code detector for Dart and Flutter.** Finds declarations that are never +referenced — classes, functions, methods, fields, constants, enum values — and can remove them for you. -### About the name - *"Ciach!"* — pronounced **/t͡ɕax/** — is Polish for the sound of a clean chop, -the noise a knife makes right before something falls off. Fitting, since -that's exactly what this tool finds for you: dead code, waiting to be cut. +the noise a knife makes right before something falls off. ## Installation -There are two ways to get the `ciach` command, depending on how you want to -run it: - -- **Global activation** — a single `ciach` command available everywhere, - independent of any particular project: +Install it globally for a `ciach` command everywhere, in `~/.pub-cache/bin`: - ```bash - dart pub global activate ciach - ciach - ``` - - This puts `ciach` in `~/.pub-cache/bin`; add that to your `PATH` if - `dart pub global activate` warns that it isn't there already. +```bash +dart pub global activate ciach +``` -- **As a dev dependency** — pinned per-project, so everyone on the team (and - CI) uses the same version: +Or add it as a dev dependency, which pins the version for the team and CI: - ```bash - dart pub add --dev ciach - dart run ciach - ``` +```bash +dart pub add --dev ciach +dart run ciach +``` -The rest of this README shows bare `ciach …` commands; prefix them with -`dart run` if you installed it as a dev dependency instead of globally. +Examples below show bare `ciach …`; prefix them with `dart run` for the second. ## Usage ```bash -# Scan the current package -ciach - -# Scan a specific package -ciach path/to/package - -# Only the highest-confidence dead code (private, never-referenced), as JSON -ciach --no-public -f json - -# GitHub Actions annotations; fail the job if anything is found -ciach -f github --set-exit-if-changed - -# Remove what's found, after confirming -ciach --remove - -# Remove without asking (e.g. from a script) -ciach --remove --force +ciach # scan the current package +ciach path/to/package # scan another package +ciach --no-public -f json # private-only (highest confidence), as JSON +ciach -f github --set-exit-if-changed # CI: annotations, non-zero if anything is found +ciach --remove # delete the findings, after confirming +ciach --remove --force # …without asking +ciach --verbose # explain what's happening ``` ### Options @@ -86,6 +62,8 @@ ciach --remove --force | --- | --- | --- | | `[path]` | `.` | Package root to analyze. | | `-h, --help` | — | Print usage information. | +| `--config ` | auto | Read settings from this YAML file instead of the auto-discovered one. See [Configuration file](#configuration-file). | +| `--no-config` | off | Ignore the config file, even if one is found. | | `--[no-]public` | on | Report unused public declarations too. Disable to report only private (`_`-prefixed) ones. | | `--[no-]generated` | off | Scan generated files (`*.g.dart`, `*.freezed.dart`, `*.mocks.dart`, …). | | `--[no-]overrides` | off | Report `@override` members too. Off by default — see limitations. | @@ -103,175 +81,169 @@ ciach --remove --force | `-j, --concurrency ` | `16` | Reference queries kept in flight against the analysis server. | | `--[no-]color` | auto | Colorize text output. | | `--[no-]progress` | auto | Show scan progress on stderr. | +| `-v, --verbose` | off | Explain what's happening on stderr. See [Verbose mode](#verbose-mode). | | `--dart ` | current SDK | Path to the `dart` executable to launch the server with. | Exit codes: `0` success, `1` unused found with `--set-exit-if-changed`, `2` usage or analysis error. +### Configuration file + +Every option above can live in a `ciach.yaml` in the package root, keyed by its +long name minus the `--`, plus `path` for the positional argument: + +```yaml +public: false # --no-public +exclude: ['test/**', 'tool/**'] # repeatable options take a list, or a bare string +kinds: [class, function, method] +format: github +set-exit-if-changed: true +``` + +Command line beats config file beats default, even when the flag matches the +default (`ciach --public` overrides `public: false`), and a repeatable option on +the command line replaces the config's list rather than adding to it. Unknown +keys and wrong-typed values are usage errors naming the file and the key. + +Discovery looks for that one file name in the analyzed package root, never in a +parent, so each package in a monorepo owns its config. `--config ` reads +one from elsewhere; `--no-config` ignores a discovered one; the two can't be +combined. + +### Verbose mode + +`-v` narrates the run on stderr, with elapsed times: the config file read and +what it set, every setting and the layer it came from, each scan phase, anything +the definition check rescued, and what `--remove` touches. + +```console +$ ciach -v +[ 0.0s] Read config from ciach.yaml. +[ 0.0s] It sets 2 options: +[ 0.0s] public: false +[ 0.0s] exclude: test/** +[ 0.0s] Settings for this run: +[ 0.0s] path: /home/me/pkg (command line) +[ 0.0s] public: false (config file) +[ 0.0s] exclude: test/** (config file) +[ 0.0s] concurrency: 16 (default) +[ 0.0s] color: true (auto-detected) +… +[ 0.1s] Starting Dart analysis server… +[ 0.3s] Collecting declarations from 13 file(s)… +[ 0.5s] Scanned 13 file(s) and checked 44 declaration(s) in 478ms: 4 unused, 1 referenced only from doc comments. +``` + +It all goes to stderr, so `ciach -v -f json | jq` still works. Reach for it when +a config file seems not to apply, or to find the phase eating the time. It +supersedes `--progress`, whose self-overwriting line would fight with it. + ### Doc-only findings -A dartdoc `[Xxx]` comment link resolves to a real declaration, so the -analysis server counts it as a reference — but "someone linked to this in a -comment" isn't the same confidence level as "something actually calls this". -Declarations with no *code* references, only a comment link, are reported -separately as **doc-only**, in every format: +A dartdoc `[Xxx]` link resolves to a real declaration, so the analysis server +counts it as a reference — but a comment mentioning something isn't the same as +code calling it. Declarations with no *code* references are reported separately, +in every format: ``` -$ ciach lib/greeting.dart 15:6 function danglingFunction (public) Referenced only from doc comments — not counted as unused, never removed: lib/greeting.dart 40:6 function docOnlyMentioned (public) - -Found 1 unused declaration in 1 file (scanned 6 files, 44 declarations, 0.5s). 1 more referenced only from doc comments. ``` -Doc-only findings are informational: they never count toward -`--set-exit-if-changed`, are never touched by `--remove`, and get a `::notice` -(not `::warning`) in `-f github` output. If one really is dead code, remove -its doc comment link (or the comment itself) and re-run to have it reported -as properly unused. +These never count toward `--set-exit-if-changed`, are never touched by +`--remove`, and get a `::notice` rather than a `::warning` in `-f github`. Drop +the doc link to have one reported as properly unused. ### GitHub Actions -Add `ciach` as a dev dependency (see [Installation](#installation)) so the -version is pinned and `dart pub get` is all the setup CI needs: - ```yaml - run: dart pub get - run: dart run ciach -f github --set-exit-if-changed ``` -Each finding becomes a `::warning` annotation shown inline on the PR diff. Run -it from the repository root so annotation paths resolve; when scanning a -sub-package (e.g. `ciach -f github app`), the scan path is -prepended automatically so annotations still point at the right files. +Each finding becomes a `::warning` annotation inline on the PR diff. Run it from +the repository root so paths resolve; when scanning a sub-package (`ciach -f +github app`), the scan path is prepended automatically. ### Removing declarations -`--remove` deletes every reported declaration from source — its doc comment -and annotations included — after showing what it's about to remove and asking -for confirmation: +`--remove` deletes every reported declaration — doc comment and annotations +included — after showing what it is about to remove and asking: ``` -$ ciach --remove -lib/greeting.dart - 15:6 function danglingFunction (public) -... Found 4 unused declarations in 2 files (scanned 6 files, 44 declarations, 0.5s). Remove 4 unused declarations? [y/N] y Removed 4 unused declarations from 2 files. ``` -`--remove --force` skips the prompt; use it in a script once you're confident -in the results (`--force` on its own, without `--remove`, is a usage error). -Without a terminal to confirm on (e.g. piped into another program) and -without `--force`, nothing is removed. - -Run `dart format` afterward — removal is conservative about *what* to delete -(it leaves ambiguous multi-variable statements like `int a = 1, b = 2;` -alone unless every declarator in them is unused) but not about spacing, so -expect the odd extra blank line. - -Because removal acts on whatever the finder reports, it inherits the same -false-positive risk as the report itself (see [What it skips by -default](#what-it-skips-by-default) and [Limitations](#limitations) below) — -enabling `--overrides` or `--operators` widens that risk considerably. -[Doc-only findings](#doc-only-findings) are never included, regardless of -those flags. Review the diff (or your test suite) after removing, the same as +`--force` skips the prompt (and is a usage error on its own); with no terminal to +confirm on and no `--force`, nothing is removed. Run `dart format` afterward: +removal is conservative about *what* it deletes — an ambiguous `int a = 1, b = 2;` +is left alone unless every declarator is unused — but not about spacing. + +Removal acts on whatever the finder reports, so it inherits the same +false-positive risk, which `--overrides` and `--operators` widen considerably. +[Doc-only findings](#doc-only-findings) are never included. Review the diff, as you would after any automated refactor. ## What it skips by default -These are all off by default because they're known sources of false -positives — a used declaration reported as unused. Each has a flag to opt -back in, at the cost of reintroducing that risk: - -- **`main`** — the program entry point. Always skipped; there's no flag for - this one. -- **`@override` members** — they are frequently reached polymorphically or by a - framework (Flutter's `build`, `initState`, `dispose`, `toString`, `==`, …), - which a name-based reference search can miss. Use `--overrides` to include - them. -- **Operator overloads** (`operator +`, `operator ==`, …) — the analysis - server's reference search does not resolve infix operator syntax (`a + b`) - back to the operator's declaration, so a used operator is reported as - unused every time. See `example/lib/extensions.dart`. Use `--operators` to - include them. -- **`call` methods** — a `call` method makes its object callable via - implicit-call syntax (`obj(...)`), which the reference search can't resolve - back to the declaration, the same way it can't resolve infix operators. A - used `call` method would be reported as unused every time. Always skipped; - there's no flag for this one. See `example/lib/callables.dart`. -- **`@pragma('vm:entry-point')`** — reachable from native code / reflection. -- **Generated files** — by filename convention and the - `GENERATED CODE - DO NOT MODIFY BY HAND` banner. Use `--generated` to include. - Even when excluded from the scan, they are still opened while analyzing, so a - declaration referenced *only* from generated code (e.g. a `toJson` called - from a `.g.dart` part) is not misreported as unused. -- **`type parameters`** and non-declaration symbols. - -**Private constructors are *not* skipped.** An unused `ClassName._` is dead code -like any other and is reported (and removed by `--remove`). When it's the sole, -zero-parameter `ClassName._()` — the classic prevent-instantiation marker — the -finding carries a hint suggesting `abstract final class`, the idiomatic way to -make a static-only class non-instantiable. See `example/lib/private_ctors.dart`. - -**dartdoc `[Xxx]` reference links** are a related wrinkle, handled a bit -differently: a link resolves to a real declaration, so the analysis server -counts it as a reference, which would otherwise hide genuinely dead code -(e.g. `/// See [Dog.sound]` would keep `Dog.sound` looking used). Rather than -a flag, these get their own always-on category — see [Doc-only -findings](#doc-only-findings). +Each of these is a known source of false positives; the flag opts back in at +that cost. + +| Skipped | Why | Flag | +| --- | --- | --- | +| `main` | the entry point is never unused | — | +| `@override` members | often reached polymorphically or by a framework (`build`, `initState`, `==`, …), which a name-based search misses | `--overrides` | +| Operator overloads | the server doesn't resolve `a + b` back to the declaration, so a used operator is flagged every time | `--operators` | +| `call` methods | implicit-call syntax (`obj(…)`) is unresolvable the same way | — | +| `@pragma('vm:entry-point')` | reachable from native code or reflection | — | +| Generated files | by filename convention and the `GENERATED CODE - DO NOT MODIFY BY HAND` banner. Still opened during analysis, so a declaration used only from a `.g.dart` isn't misreported | `--generated` | +| `toJson()` | `jsonEncode(obj)` calls it by dynamic dispatch, leaving no source-level reference | `--report-tojson` | +| Type parameters | always "used" within their scope | — | +| dartdoc `[Xxx]` links | not a code reference; reported as [doc-only](#doc-only-findings) instead of hidden | — | + +Private constructors are **not** skipped: an unused `ClassName._` is dead code +like any other. A sole zero-parameter `ClassName._()` — the classic +prevent-instantiation marker — is reported with a hint suggesting `abstract final +class` instead. See [example/](example) for a runnable demonstration of each case. ## Limitations -This is a static, reference-based heuristic. Expect to review its output rather -than delete blindly: +This is a static, reference-based heuristic, so review its output rather than +deleting blindly: -- **Public API of a library package** is legitimately "unused" from the - package's own perspective. Prefer `--no-public` for library packages, or treat - public findings as advisory. -- **Reflection / dynamic invocation / serialization** (e.g. `dart:mirrors`, - code that is only referenced by name in generated code you excluded) is not - visible to a reference search. -- **Entry points beyond `main`** (isolate entry points, plugin registrants) may - need excluding or annotating with `@pragma('vm:entry-point')`. -- Results are only as good as the analysis: a package that does not analyze - cleanly (missing `pub get`, errors) may yield incomplete references. +- **A library package's public API** is legitimately unused from inside the + package. Prefer `--no-public` there, or treat public findings as advisory. +- **Reflection, dynamic invocation, and names referenced only from generated + code you excluded** are invisible to a reference search. +- **Entry points other than `main`** (isolate entry points, plugin registrants) + need excluding or `@pragma('vm:entry-point')`. +- A package that doesn't analyze cleanly (missing `pub get`, errors) yields + incomplete references. ## Performance -Runtime is dominated by the analysis server, not the tool. Two phases matter: - -1. **Initial analysis** — the server analyzes the whole package (and, for a - Flutter app, the SDK/dependencies) once before any query. This is a fixed - per-run cost (tens of seconds for a large app) and cannot be skipped: - incomplete analysis would produce wrong reference counts. -2. **Reference queries** — one `textDocument/references` per declaration. - Requests run through a global pool (`-j/--concurrency`, default 16) and the - scanned files are kept open so the server's resolved-unit cache stays warm. - -The biggest lever is **how much you ask**: - -- **`--no-public`** is by far the cheapest mode. Private declarations are - library-scoped, so the server only searches one library per query instead of - the whole workspace — often several times faster, and it surfaces the - highest-confidence dead code. -- **`--include` / `--exclude`** to scan only the part of the tree you care - about — references are still counted from everywhere, so results stay correct. -- **`-j`** to tune concurrency; the default (16) is near the point of - diminishing returns for the analysis server's internal parallelism. +Runtime is the analysis server's, not the tool's. It analyzes the whole package +once per run — tens of seconds for a large Flutter app, and unskippable, since +incomplete analysis means wrong reference counts — then answers one +`textDocument/references` per declaration through a pool of `-j` (default 16), +with scanned files kept open so its resolved-unit cache stays warm. -For repeated runs, compile once to skip the JIT warmup: -`dart compile exe bin/ciach.dart -o ciach` — `dart pub global activate` already -does this for you. +The lever is how much you ask for. `--no-public` is by far the cheapest mode: +private declarations are library-scoped, so each query searches one library +instead of the whole workspace, and it surfaces the highest-confidence dead code +anyway. `--include`/`--exclude` narrow the scan while still counting references +from everywhere. `dart pub global activate` compiles ahead of time, so there's no +JIT warmup per run. ## Library usage -The tool also exposes a public API for running the finder programmatically: +The finder is also available programmatically: ```dart import 'package:ciach/ciach.dart'; @@ -292,8 +264,7 @@ dart analyze dart test # spins up a real analysis server against the example/ package ``` -The implementation lives under `lib/src/`; the CLI entry point is `bin/`. See -[example/](example) for a runnable demonstration. +The implementation lives under `lib/src/`; the CLI entry point is `bin/`. ## License @@ -339,4 +310,4 @@ We are **top-tier experts** focused on Flutter Enterprise solutions. [leancode-estimate]: https://leancode.co/get-estimate?utm_source=github.com&utm_medium=referral&utm_campaign=ciach [leancode-packages]: https://pub.dev/packages?q=publisher%3Aleancode.co&sort=downloads [patrol-landing]: https://patrol.leancode.co/?utm_source=github.com&utm_medium=referral&utm_campaign=ciach -[banner-img]: https://raw.githubusercontent.com/leancodepl/ciach/refs/heads/main/doc/imgs/banner.png \ No newline at end of file +[banner-img]: https://raw.githubusercontent.com/leancodepl/ciach/refs/heads/main/doc/imgs/banner.png diff --git a/bin/ciach.dart b/bin/ciach.dart index a1e6f88..afa13ad 100644 --- a/bin/ciach.dart +++ b/bin/ciach.dart @@ -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 main(List arguments) async { @@ -41,75 +46,79 @@ Future _run(List 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; - } + // The root named on the command line: a config file's own `path` can't decide + // where that file is read from. + final projectDir = args.rest.isEmpty ? '.' : args.rest.first; - final Set kinds; + final ResolvedOptions resolved; + final ConfigFile config; + final CiachConfiguration configuration; try { - kinds = parseKinds(args.multiOption('kinds')); + config = .load( + projectDir: projectDir, + explicitPath: explicitConfig, + ignore: ignoreConfig, + ); + configuration = resolveConfiguration(args, config); + resolved = resolveOptions( + configuration, + colorDefault: stdout.supportsAnsiEscapes, + // Progress goes to stderr, so default it on only for a terminal. + 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( + // Verbose keeps every phase line; progress overwrites one in place. + onProgress: + log?.write ?? (showProgress ? _ProgressPrinter().update : null), + ), + ).run(); } on Object catch (e, st) { if (showProgress) { stderr.writeln(); @@ -124,17 +133,25 @@ Future _run(List 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.', + ); + if (result.recoveredReferences.isNotEmpty) { + log?.write( + 'Kept ${result.recoveredReferences.length} declaration(s) the reference search called unused: the definition check found a use for each. Reported as warnings.', + ); + } + 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. + // GitHub resolves annotation paths from the repo root, so prepend 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)); @@ -143,30 +160,41 @@ Future _run(List arguments) async { stderr.write(Reporter.warningsText(result)); } - 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], and +/// deletes the declarations from disk. Future _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') { @@ -174,8 +202,7 @@ Future _removeUnused( } 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; } @@ -191,14 +218,21 @@ Future _removeUnused( return; } + if (log != null) { + final byFile = result.unused + .whereNot((d) => d.removalBlocked) + .groupFoldBy((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 - // reported line that carried the hint. + // Repeat any advisory hints: removing a declaration takes the reported line + // that carried its hint with it. final removedHints = result.unused .where((d) => !d.removalBlocked && d.hint != null) .map((d) => '${d.qualifiedName}: ${d.hint}') @@ -208,6 +242,21 @@ Future _removeUnused( } } +/// Prints `--verbose` narration to stderr — not stdout, so `-f json` stays +/// machine-readable — one line per message, stamped with the elapsed time. +class _VerboseLog { + final _stopwatch = Stopwatch()..start(); + + /// Writes one stamped line. + void write(String message) { + final seconds = (_stopwatch.elapsedMilliseconds / 1000).toStringAsFixed(1); + stderr.writeln('[${seconds.padLeft(5)}s] $message'); + } + + /// Writes a line per message. + void writeAll(Iterable messages) => messages.forEach(write); +} + /// Prints single-line, overwriting progress to stderr. class _ProgressPrinter { int _lastLength = 0; diff --git a/lib/src/cli/args.dart b/lib/src/cli/args.dart index 603aa4f..08fd794 100644 --- a/lib/src/cli/args.dart +++ b/lib/src/cli/args.dart @@ -11,6 +11,7 @@ import 'package:args/args.dart'; import 'package:ciach/ciach.dart'; import 'package:collection/collection.dart'; +import 'package:config/config.dart'; /// Friendly `--kinds` names mapped to LSP symbol kinds. const kindAliases = { @@ -34,6 +35,9 @@ const kindAliases = { /// The `--kinds` alias names, sorted, for help text and error messages. String get kindNames => kindAliases.keys.sorted().join(', '); +/// The accepted `--format` values; the first one is the default. +const formatNames = ['text', 'json', 'github']; + /// Parses the `--kinds` values (comma-separated, repeatable) into symbol kinds, /// falling back to [FinderOptions.defaultKinds] when none are given. /// @@ -54,145 +58,291 @@ Set parseKinds(List raw) { }; } -/// Builds the CLI argument parser. Every flag and option ciach accepts is -/// declared here, so adding one is a single-file change. -ArgParser buildParser() => .new() - ..addFlag( - 'help', - abbr: 'h', - negatable: false, - help: 'Print this usage information.', - ) - ..addFlag( - 'public', - defaultsTo: true, - help: - 'Report unused public declarations too. Disable to report only\n' - 'private (underscore-prefixed) declarations, which are the\n' - 'highest-confidence dead code.', - ) - ..addFlag( - 'generated', - help: 'Scan generated files (*.g.dart, *.freezed.dart, …). Off by default.', - ) - ..addFlag( - 'overrides', - help: - 'Report members annotated with @override too. Off by default,\n' - 'since overrides are often reached polymorphically and a plain\n' - 'reference search can miss those uses.', - ) - ..addFlag( - 'operators', - help: - 'Report operator overloads (operator +, operator ==, …) too. Off\n' - 'by default: the analysis server never resolves infix operator\n' - "syntax (a + b) back to the operator's declaration, so a used\n" - 'operator is reported as unused every time.', - ) - ..addFlag( - 'unused-union-members', - help: - 'Also flag a class whose only references are type patterns over its\n' - '(sealed) supertype — matched but never constructed. Off by default:\n' - 'a `case Foo():` arm otherwise counts as a use. Report-only: these\n' - 'findings are surfaced but --remove never deletes them or their\n' - 'pattern arms (removing a sealed member and rewriting its switches\n' - 'is left to a human). Conservative: any reference that is not clearly\n' - 'a type pattern keeps the class alive.', - ) - ..addFlag( - 'report-tojson', - help: - 'Report a `toJson()` serialization hook as unused too. Off by\n' - 'default: `jsonEncode(obj)` calls `obj.toJson()` by dynamic dispatch\n' - 'with no source-level `.toJson()` reference for the search to see, so\n' - 'a live serializer would be flagged. Enable to audit dead `toJson`s.', - ) - ..addFlag( - 'set-exit-if-changed', - help: - 'Exit with a non-zero status when any unused declaration is found\n' - '(useful in CI).', - ) - ..addFlag( - 'remove', - help: - 'Remove unused declarations from source after reporting them.\n' - 'Prompts for confirmation first, unless --force is also given.', - ) - ..addFlag( - 'force', - help: 'Skip the confirmation prompt for --remove. Requires --remove.', - ) - ..addMultiOption( - 'exclude', - abbr: 'e', - help: 'Glob(s), relative to the root, of files to skip. Repeatable.', - valueHelp: 'glob', - ) - ..addMultiOption( - 'include', - abbr: 'i', - help: 'If given, only scan files matching these glob(s). Repeatable.', - valueHelp: 'glob', - ) - ..addMultiOption( - 'generated-suffix', - help: - 'Additional filename suffix to treat as generated (and so\n' - 'exclude from the scan), on top of the built-in set (*.g.dart,\n' - '*.freezed.dart, …). Use for custom code generators, e.g.\n' - '--generated-suffix .gc.dart. Include the leading dot. Repeatable.\n' - 'Ignored when --generated is set.', - valueHelp: 'suffix', - ) - ..addMultiOption( - 'kinds', - abbr: 'k', - help: - 'Restrict to these declaration kinds (comma-separated).\n' - 'Valid: $kindNames.', - valueHelp: 'kind,kind', - ) - ..addOption( - 'format', - abbr: 'f', - allowed: ['text', 'json', 'github'], - defaultsTo: 'text', - help: 'Output format.', - allowedHelp: { - 'text': 'Human-readable, grouped by file.', - 'json': 'Machine-readable JSON.', - 'github': 'GitHub Actions `::warning` annotations.', - }, - ) - ..addFlag( - 'color', - help: 'Colorize text output. Defaults to auto-detecting the terminal.', - ) - ..addFlag( - 'progress', - help: 'Show scan progress on stderr. Defaults to on for a terminal.', - ) - ..addOption( - 'concurrency', - abbr: 'j', - defaultsTo: '16', - help: - 'How many reference queries to run against the analysis server at\n' - 'once. Higher can be faster on large projects, up to the limit of\n' - 'the analysis server parallelism.', - valueHelp: 'n', - ) - ..addOption( - 'dart', - help: - 'Path to the dart executable used to launch the analysis server.\n' - 'Defaults to the SDK running this tool.', - valueHelp: 'path', +/// Every setting ciach accepts, declared once for the command line, the config +/// file (`configKey`) and its default. +/// +/// [help], [config] and [noConfig] have no `configKey`: a config file doesn't +/// get to decide whether it is read. +enum CiachOption implements OptionDefinition { + help( + FlagOption( + argName: 'help', + argAbbrev: 'h', + negatable: false, + defaultsTo: false, + helpText: 'Print this usage information.', + ), + ), + config( + StringOption( + argName: 'config', + valueHelp: 'path', + helpText: + 'Path to a YAML config file. Defaults to $configFileName in the\n' + 'analyzed package root, when present.', + ), + ), + // A flag of its own, not a negated `config` — that name is the option above. + noConfig( + FlagOption( + argName: 'no-config', + negatable: false, + defaultsTo: false, + helpText: + 'Ignore any config file, including one that would be discovered\n' + 'automatically. Cannot be combined with --config.', + ), + ), + path( + StringOption( + argPos: 0, + configKey: '/path', + defaultsTo: '.', + helpText: 'Package root to analyze.', + ), + ), + public( + FlagOption( + argName: 'public', + configKey: '/public', + defaultsTo: true, + helpText: + 'Report unused public declarations too. Disable to report only\n' + 'private (underscore-prefixed) declarations, which are the\n' + 'highest-confidence dead code.', + ), + ), + generated( + FlagOption( + argName: 'generated', + configKey: '/generated', + defaultsTo: false, + helpText: + 'Scan generated files (*.g.dart, *.freezed.dart, …). Off by default.', + ), + ), + overrides( + FlagOption( + argName: 'overrides', + configKey: '/overrides', + defaultsTo: false, + helpText: + 'Report members annotated with @override too. Off by default,\n' + 'since overrides are often reached polymorphically and a plain\n' + 'reference search can miss those uses.', + ), + ), + operators( + FlagOption( + argName: 'operators', + configKey: '/operators', + defaultsTo: false, + helpText: + 'Report operator overloads (operator +, operator ==, …) too. Off\n' + 'by default: the analysis server never resolves infix operator\n' + "syntax (a + b) back to the operator's declaration, so a used\n" + 'operator is reported as unused every time.', + ), + ), + unusedUnionMembers( + FlagOption( + argName: 'unused-union-members', + configKey: '/unused-union-members', + defaultsTo: false, + helpText: + 'Also flag a class whose only references are type patterns over its\n' + '(sealed) supertype — matched but never constructed. Off by default:\n' + 'a `case Foo():` arm otherwise counts as a use. Report-only: these\n' + 'findings are surfaced but --remove never deletes them or their\n' + 'pattern arms (removing a sealed member and rewriting its switches\n' + 'is left to a human). Conservative: any reference that is not clearly\n' + 'a type pattern keeps the class alive.', + ), + ), + reportToJson( + FlagOption( + argName: 'report-tojson', + configKey: '/report-tojson', + defaultsTo: false, + helpText: + 'Report a `toJson()` serialization hook as unused too. Off by\n' + 'default: `jsonEncode(obj)` calls `obj.toJson()` by dynamic dispatch\n' + 'with no source-level `.toJson()` reference for the search to see, so\n' + 'a live serializer would be flagged. Enable to audit dead `toJson`s.', + ), + ), + setExitIfChanged( + FlagOption( + argName: 'set-exit-if-changed', + configKey: '/set-exit-if-changed', + negatable: false, + defaultsTo: false, + helpText: + 'Exit with a non-zero status when any unused declaration is found\n' + '(useful in CI).', + ), + ), + remove( + FlagOption( + argName: 'remove', + configKey: '/remove', + negatable: false, + defaultsTo: false, + helpText: + 'Remove unused declarations from source after reporting them.\n' + 'Prompts for confirmation first, unless --force is also given.', + ), + ), + force( + FlagOption( + argName: 'force', + configKey: '/force', + negatable: false, + defaultsTo: false, + helpText: 'Skip the confirmation prompt for --remove. Requires --remove.', + ), + ), + exclude( + MultiStringOption.noSplit( + argName: 'exclude', + argAbbrev: 'e', + configKey: '/exclude', + defaultsTo: [], + valueHelp: 'glob', + helpText: 'Glob(s), relative to the root, of files to skip. Repeatable.', + ), + ), + include( + MultiStringOption.noSplit( + argName: 'include', + argAbbrev: 'i', + configKey: '/include', + defaultsTo: [], + valueHelp: 'glob', + helpText: 'If given, only scan files matching these glob(s). Repeatable.', + ), + ), + generatedSuffix( + MultiStringOption.noSplit( + argName: 'generated-suffix', + configKey: '/generated-suffix', + defaultsTo: [], + valueHelp: 'suffix', + helpText: + 'Additional filename suffix to treat as generated (and so\n' + 'exclude from the scan), on top of the built-in set (*.g.dart,\n' + '*.freezed.dart, …). Use for custom code generators, e.g.\n' + '--generated-suffix .gc.dart. Include the leading dot. Repeatable.\n' + 'Ignored when --generated is set.', + ), + ), + kinds( + MultiStringOption( + argName: 'kinds', + argAbbrev: 'k', + configKey: '/kinds', + defaultsTo: [], + valueHelp: 'kind,kind', + // Rejects an unknown kind wherever it came from; the conversion to + // symbol kinds happens later. + customValidator: parseKinds, + // Listed in `usage`, which can read them off kindAliases. + helpText: + 'Restrict to these declaration kinds (comma-separated).\n' + 'The kinds are listed at the end of this help.', + ), + ), + format( + StringOption( + argName: 'format', + argAbbrev: 'f', + configKey: '/format', + allowedValues: formatNames, + defaultsTo: 'text', + helpText: 'Output format.', + allowedHelp: { + 'text': 'Human-readable, grouped by file.', + 'json': 'Machine-readable JSON.', + 'github': 'GitHub Actions `::warning` annotations.', + }, + ), + ), + color( + FlagOption( + argName: 'color', + configKey: '/color', + helpText: + 'Colorize text output. Defaults to auto-detecting the terminal.', + ), + ), + progress( + FlagOption( + argName: 'progress', + configKey: '/progress', + helpText: 'Show scan progress on stderr. Defaults to on for a terminal.', + ), + ), + verbose( + FlagOption( + argName: 'verbose', + argAbbrev: 'v', + configKey: '/verbose', + defaultsTo: false, + helpText: + 'Explain what is happening on stderr: which config file was used and\n' + 'what it set, the settings the run ended up with, each scan phase as\n' + 'it starts, and what --remove touches. Supersedes --progress, whose\n' + 'single overwriting line would fight with it.', + ), + ), + concurrency( + IntOption( + argName: 'concurrency', + argAbbrev: 'j', + configKey: '/concurrency', + valueHelp: 'n', + defaultsTo: 16, + min: 1, + helpText: + 'How many reference queries to run against the analysis server at\n' + 'once. Higher can be faster on large projects, up to the limit of\n' + 'the analysis server parallelism.', + ), + ), + dart( + StringOption( + argName: 'dart', + configKey: '/dart', + valueHelp: 'path', + helpText: + 'Path to the dart executable used to launch the analysis server.\n' + 'Defaults to the SDK running this tool.', + ), ); + const CiachOption(this.option); + + @override + final ConfigOptionBase option; + + /// The `ciach.yaml` key for this option, its JSON pointer without the `/`. + String? get configKey => option.configKey?.substring(1); +} + +/// Ciach's resolved options, named so the enum's `dynamic` argument is written +/// once. +typedef CiachConfiguration = Configuration>; + +/// The file name config discovery looks for in the project directory. +const configFileName = 'ciach.yaml'; + +/// The argument parser for [CiachOption.values]. +ArgParser buildParser() { + final parser = ArgParser(); + prepareOptionsForParsing(CiachOption.values, parser); + return parser; +} + /// The full `--help` text, wrapping [parser]'s generated option list. String usage(ArgParser parser) => ''' @@ -204,6 +354,22 @@ Usage: ciach [options] [path] ${parser.usage} +Declaration kinds (-k, --kinds): +${_wrapped(kindNames, indent: ' ')} + +Config file: + Every option above can also be set in $configFileName in the package root, + keyed by its long name, plus `path` for the positional argument. The command + line wins over the file; --no-config ignores the file; --verbose says which + file was read and what it set. + + # $configFileName + public: false + exclude: + - 'test/**' + kinds: [class, function] + format: json + Examples: # Scan the current package ciach @@ -211,6 +377,12 @@ Examples: # Only private declarations, excluding tests, as JSON ciach --no-public -e 'test/**' -f json lib/ + # Read settings from a config file elsewhere + ciach --config tool/ciach.yaml + + # Ignore the package's config file for one run + ciach --no-config + # GitHub Actions annotations, fail the job if anything is found ciach -f github --set-exit-if-changed @@ -219,3 +391,16 @@ Examples: # Remove without asking (e.g. in a script) ciach --remove --force'''; + +/// [text] as [indent]-prefixed lines of at most [width] characters. +String _wrapped(String text, {String indent = '', int width = 76}) { + final lines = []; + for (final word in text.split(' ')) { + if (lines.isEmpty || '${lines.last} $word'.length > width) { + lines.add('$indent$word'); + } else { + lines[lines.length - 1] = '${lines.last} $word'; + } + } + return lines.join('\n'); +} diff --git a/lib/src/cli/config.dart b/lib/src/cli/config.dart new file mode 100644 index 0000000..cf1d8e1 --- /dev/null +++ b/lib/src/cli/config.dart @@ -0,0 +1,227 @@ +import 'dart:io'; + +import 'package:ciach/src/cli/args.dart'; +import 'package:collection/collection.dart'; +import 'package:config/config.dart'; +import 'package:path/path.dart' as p; +import 'package:yaml/yaml.dart'; + +/// Every key a config file may contain, following [CiachOption]. +final configKeys = {for (final option in CiachOption.values) ?option.configKey}; + +/// The config-file layer of option resolution: a [ConfigurationBroker] over the +/// settings of a `ciach.yaml`. +/// +/// Where to look for that file, whether to read it at all, and reporting a +/// malformed one against its path stay ciach's own business; the command line +/// beating it, and its defaults, are `package:config`'s. +class ConfigFile implements ConfigurationBroker> { + const ConfigFile._({ + required this.settings, + required this.path, + required this.ignored, + }); + + /// Creates a config that sets nothing, from nowhere. + const ConfigFile.empty() : settings = const {}, path = null, ignored = false; + + /// Parses [source], read from the file [origin], which names it in errors. + /// + /// Throws a [FormatException] on anything but a map of known keys with values + /// their options accept. + factory ConfigFile.parse(String source, {required String origin}) { + final Object? document; + try { + document = loadYaml(source, sourceUrl: Uri.file(origin)); + } on YamlException catch (e) { + throw FormatException('$origin: not valid YAML: ${e.message}'); + } + + // An empty file parses to null; that is no settings, not an error. + if (document == null) { + return ._(settings: const {}, path: origin, ignored: false); + } + if (document is! Map) { + throw FormatException('$origin: the top level must be a map of options.'); + } + + final unknown = document.keys + .map((key) => '$key') + .whereNot(configKeys.contains) + .toList(); + if (unknown.isNotEmpty) { + final valid = (configKeys.toList()..sort()).join(', '); + throw FormatException( + '$origin: unknown option${unknown.length == 1 ? '' : 's'} ${unknown.map((key) => "'$key'").join(', ')}. Valid options: $valid.', + ); + } + + final file = ConfigFile._( + settings: { + for (final entry in document.entries) + // A valueless key (`public:`) counts as unset, and YAML collections + // are unwrapped to plain Dart ones. + if (entry.value case final value?) + '${entry.key}': value is Iterable ? value.toList() : value, + }, + path: origin, + ignored: false, + ); + + // Resolution only asks for the keys it needs, so without checking the whole + // file here, a bad value under an overridden option would go unreported. + file.settings.keys.forEach(file._typedValue); + return file; + } + + /// Finds and reads the config file for a run: [explicitPath] from `--config` + /// if given, else [configFileName] in [projectDir]. + /// + /// With [ignore] the file is located but never read, so `--verbose` can name + /// what `--no-config` skipped and an unparseable file is no error. + /// + /// Throws a [FormatException] if [explicitPath] is missing or the file cannot + /// be read or parsed. + static ConfigFile load({ + required String projectDir, + String? explicitPath, + bool ignore = false, + }) { + final discovered = File(p.join(projectDir, configFileName)); + if (ignore) { + return ._( + settings: const {}, + path: discovered.existsSync() ? discovered.path : null, + ignored: true, + ); + } + + final File file; + if (explicitPath != null) { + file = File(explicitPath); + if (!file.existsSync()) { + throw FormatException('Config file does not exist: $explicitPath'); + } + } else { + if (!discovered.existsSync()) { + return const .empty(); + } + file = discovered; + } + + try { + return .parse(file.readAsStringSync(), origin: file.path); + } on FileSystemException catch (e) { + throw FormatException( + 'Cannot read config file ${file.path}: ${e.message}', + ); + } + } + + /// What the file sets, keyed by config key, in the order it set them. + final Map settings; + + /// The file the [settings] came from, or that [ignored] skipped; `null` when + /// there was no config file at all. + final String? path; + + /// Whether `--no-config` skipped a file, leaving it unread. + final bool ignored; + + /// The value for [key], a JSON pointer such as `/public`. + @override + Object? valueOrNull(String key, CiachConfiguration cfg) => + _typedValue(key.startsWith('/') ? key.substring(1) : key); + + /// The value under [key], as the type its option takes. + /// + /// The type comes from the option, not the value, so that `exclude: ['a']` + /// arrives as the `List` the option needs rather than a + /// `List`, and a mismatch names the file and the key. + /// + /// The first three accept less than their type allows, and `package:config` + /// keeps its own validators internal, so they reuse what the options declare. + Object? _typedValue(String key) => switch (_optionFor(key)) { + .format => _oneOf(key, formatNames), + .kinds => _kinds(key), + .concurrency => _positiveInt(key), + final option => switch (option.option) { + FlagOption() => _boolean(key), + IntOption() => _positiveInt(key), + MultiOption() => _strings(key), + _ => _string(key), + }, + }; + + CiachOption _optionFor(String key) => + .values.firstWhere((option) => option.configKey == key); + + bool? _boolean(String key) => switch (settings[key]) { + null => null, + final bool value => value, + final other => _wrong(key, 'true or false', other), + }; + + String? _string(String key) => switch (settings[key]) { + null => null, + final String value => value, + final other => _wrong(key, 'a string', other), + }; + + /// The strings under [key], accepting a bare string as a one-element list. + List? _strings(String key) => switch (settings[key]) { + null => null, + final String value => [value], + final Iterable values => [ + for (final value in values) + if (value is String) value else _wrong(key, 'a list of strings', value), + ], + final other => _wrong(key, 'a list of strings', other), + }; + + /// The string under [key], which has to be one of [allowed]. + String? _oneOf(String key, List allowed) { + final value = _string(key); + if (value != null && !allowed.contains(value)) { + throw FormatException( + "$path: '$key' must be one of ${allowed.join(', ')}, got '$value'.", + ); + } + return value; + } + + /// The kind names under [key], validated but not yet converted. + List? _kinds(String key) { + final values = _strings(key); + if (values != null) { + try { + parseKinds(values); + } on FormatException catch (e) { + throw FormatException("$path: '$key': ${e.message}"); + } + } + return values; + } + + int? _positiveInt(String key) => switch (settings[key]) { + null => null, + final int value && > 0 => value, + final other => _wrong(key, 'a positive integer', other), + }; + + /// Reports the wrong type, naming the file and the key. + Never _wrong(String key, String expected, Object? value) => + throw FormatException( + "$path: '$key' must be $expected, got ${_describe(value)}.", + ); + + static String _describe(Object? value) => switch (value) { + null => 'null', + String() => 'a string', + bool() => 'a boolean', + num() => 'a number', + Iterable() => 'a list', + Map() => 'a map', + _ => '$value', + }; +} diff --git a/lib/src/cli/options.dart b/lib/src/cli/options.dart new file mode 100644 index 0000000..e689bb8 --- /dev/null +++ b/lib/src/cli/options.dart @@ -0,0 +1,133 @@ +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:config/config.dart'; +import 'package:path/path.dart' as p; + +/// A resolved [Configuration] in the types the rest of the tool works in: kind +/// names converted, the inverted flags flipped, the auto-detected ones settled. +class ResolvedOptions { + const ResolvedOptions({ + required this.rootPath, + required this.includeGlobs, + required this.excludeGlobs, + required this.additionalGeneratedSuffixes, + required this.kinds, + required this.includePublic, + required this.includeGenerated, + required this.overrides, + required this.operators, + required this.unusedUnionMembers, + required this.reportToJson, + required this.setExitIfChanged, + required this.remove, + required this.force, + required this.format, + required this.useColor, + required this.showProgress, + required this.verbose, + required this.concurrency, + required this.dartExecutable, + }); + + /// Package root to analyze, as written; see [absoluteRootPath]. + final String rootPath; + final List includeGlobs; + final List excludeGlobs; + final List additionalGeneratedSuffixes; + final Set kinds; + final bool includePublic; + final bool includeGenerated; + + /// Whether to report `@override` members — inverted for the finder. + final bool overrides; + + /// Whether to report operator overloads — inverted for the finder. + final bool operators; + final bool unusedUnionMembers; + final bool reportToJson; + final bool setExitIfChanged; + final bool remove; + final bool force; + final String format; + final bool useColor; + + /// Whether to show scan progress. Always `false` when [verbose] is set, whose + /// durable lines the overwriting progress line would fight with. + final bool showProgress; + final bool verbose; + final int concurrency; + final String? dartExecutable; + + /// [rootPath] resolved against the current directory. + String get absoluteRootPath => p.normalize(p.absolute(rootPath)); + + /// The finder's share of these settings, reporting progress to [onProgress]. + FinderOptions finderOptions({void Function(String message)? onProgress}) => + .new( + rootPath: absoluteRootPath, + includeGlobs: includeGlobs, + excludeGlobs: excludeGlobs, + additionalGeneratedSuffixes: additionalGeneratedSuffixes, + kinds: kinds, + includePublic: includePublic, + includeGenerated: includeGenerated, + skipOverrides: !overrides, + skipOperators: !operators, + unusedUnionMembers: unusedUnionMembers, + reportToJson: reportToJson, + concurrency: concurrency, + dartExecutable: dartExecutable, + onProgress: onProgress, + ); +} + +/// Resolves every [CiachOption] from [args] and [config], the command line +/// winning over the file and the file over the default. +/// +/// Throws a [UsageException] listing every malformed value, from either layer. +CiachConfiguration resolveConfiguration(ArgResults args, ConfigFile config) => + .resolve( + options: CiachOption.values, + argResults: args, + configBroker: config, + ); + +/// The settings of [configuration], with [colorDefault] and [progressDefault] +/// standing in for the two nobody asked for either way. +ResolvedOptions resolveOptions( + CiachConfiguration configuration, { + required bool colorDefault, + required bool progressDefault, +}) { + final verbose = configuration.value(CiachOption.verbose); + final progress = + configuration.optionalValue(CiachOption.progress) ?? progressDefault; + + return .new( + rootPath: configuration.value(CiachOption.path), + includeGlobs: configuration.value(CiachOption.include), + excludeGlobs: configuration.value(CiachOption.exclude), + additionalGeneratedSuffixes: configuration.value( + CiachOption.generatedSuffix, + ), + // Already validated by the option; this only converts the names. + kinds: parseKinds(configuration.value(CiachOption.kinds)), + includePublic: configuration.value(CiachOption.public), + includeGenerated: configuration.value(CiachOption.generated), + overrides: configuration.value(CiachOption.overrides), + operators: configuration.value(CiachOption.operators), + unusedUnionMembers: configuration.value(CiachOption.unusedUnionMembers), + reportToJson: configuration.value(CiachOption.reportToJson), + setExitIfChanged: configuration.value(CiachOption.setExitIfChanged), + remove: configuration.value(CiachOption.remove), + force: configuration.value(CiachOption.force), + format: configuration.value(CiachOption.format), + useColor: configuration.optionalValue(CiachOption.color) ?? colorDefault, + showProgress: progress && !verbose, + verbose: verbose, + concurrency: configuration.value(CiachOption.concurrency), + dartExecutable: configuration.optionalValue(CiachOption.dart), + ); +} diff --git a/lib/src/cli/verbose.dart b/lib/src/cli/verbose.dart new file mode 100644 index 0000000..48f6e36 --- /dev/null +++ b/lib/src/cli/verbose.dart @@ -0,0 +1,113 @@ +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/models.dart'; +import 'package:collection/collection.dart'; +import 'package:config/config.dart'; +import 'package:pro_lsp/pro_lsp.dart' show SymbolKind; + +/// The `--verbose` account of the config file: read (and what it set), skipped, +/// or missing from [projectDir], which is named so a file that sits elsewhere +/// and silently didn't apply is obvious. +List describeConfigSource( + ConfigFile config, { + required String projectDir, +}) { + final path = config.path; + + if (config.ignored) { + return [ + if (path != null) + 'Ignoring the config file $path (--no-config).' + else + 'Ignoring any config file (--no-config); there is no $configFileName in $projectDir anyway.', + ]; + } + + if (path == null) { + return [ + 'No $configFileName in $projectDir; using command-line arguments and built-in defaults.', + ]; + } + + final settings = config.settings; + return [ + 'Read config from $path.', + if (settings.isEmpty) + ' It sets nothing; using command-line arguments and built-in defaults.' + else ...[ + ' It sets ${settings.length} option${settings.length == 1 ? '' : 's'}:', + for (final entry in settings.entries) + ' ${entry.key}: ${_value(entry.value)}', + ], + ]; +} + +/// The `--verbose` rundown of the run's settings: one line per config key, each +/// naming the layer its value came from. +/// +/// [resolved] supplies the values, [configuration] their layers, and +/// [dartExecutable] the `dart` only the caller can resolve. +List describeSettings( + CiachConfiguration configuration, + ResolvedOptions resolved, { + required String dartExecutable, +}) => [ + 'Settings for this run:', + for (final option in CiachOption.values) + if (option.configKey case final key?) + ' $key: ${_setting(option, resolved, dartExecutable)} (${_source(configuration.valueSourceType(option))})', +]; + +/// The value of [option] as the run uses it, with the root made absolute, the +/// kinds as labels, and the auto-detected flags as they settled. +String _setting( + CiachOption option, + ResolvedOptions resolved, + String dartExecutable, +) => switch (option) { + .path => resolved.absoluteRootPath, + .public => '${resolved.includePublic}', + .generated => '${resolved.includeGenerated}', + .overrides => '${resolved.overrides}', + .operators => '${resolved.operators}', + .unusedUnionMembers => '${resolved.unusedUnionMembers}', + .reportToJson => '${resolved.reportToJson}', + .setExitIfChanged => '${resolved.setExitIfChanged}', + .remove => '${resolved.remove}', + .force => '${resolved.force}', + .exclude => _value(resolved.excludeGlobs), + .include => _value(resolved.includeGlobs), + .generatedSuffix => _value(resolved.additionalGeneratedSuffixes), + .kinds => _kinds(resolved.kinds), + .format => resolved.format, + .color => '${resolved.useColor}', + .progress => '${resolved.showProgress}', + .verbose => '${resolved.verbose}', + .concurrency => '${resolved.concurrency}', + .dart => dartExecutable, + // No config key, so never listed; spelled out so a new option must be too. + .help || .config || .noConfig => '', +}; + +/// Where a value came from, in the user's words. +String _source(ValueSourceType source) => switch (source) { + .arg => 'command line', + .config => 'config file', + .envVar => 'environment', + .preset || .custom => 'preset', + .defaultValue => 'default', + .noValue => 'auto-detected', +}; + +/// A value as the log reads it: an empty list is `(none)`, a full one is +/// comma-separated. +String _value(Object? value) => switch (value) { + [] => '(none)', + List() => value.join(', '), + _ => '$value', +}; + +/// The kind labels, sorted. +String _kinds(Set kinds) => + kinds.map((kind) => kind.label).sorted().join(', '); diff --git a/pubspec.yaml b/pubspec.yaml index e031ad7..079d23f 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -19,10 +19,12 @@ environment: dependencies: args: ^2.7.0 collection: ^1.19.0 + config: ^0.9.0 glob: ^2.1.3 path: ^1.9.1 pro_lsp: ^0.3.0 stream_channel: ^2.1.4 + yaml: ^3.1.3 dev_dependencies: leancode_lint: ^24.0.0 diff --git a/test/config_test.dart b/test/config_test.dart new file mode 100644 index 0000000..523c1d8 --- /dev/null +++ b/test/config_test.dart @@ -0,0 +1,625 @@ +import 'dart:io'; + +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:config/config.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +void main() { + final parser = buildParser(); + + /// Resolves [arguments] against [config], the terminal-probed defaults given + /// explicitly so no test depends on the terminal it runs in. + ResolvedOptions resolveWith( + List arguments, + ConfigFile config, { + required bool colorDefault, + required bool progressDefault, + }) => resolveOptions( + resolveConfiguration(parser.parse(arguments), config), + colorDefault: colorDefault, + progressDefault: progressDefault, + ); + + /// As above, with both auto-detected settings off. + ResolvedOptions resolve(List arguments, [ConfigFile? config]) => + resolveWith( + arguments, + config ?? const .empty(), + colorDefault: false, + progressDefault: false, + ); + + /// What a config [source] alone resolves to. + ResolvedOptions resolveFile(String source) => + resolve(const [], .parse(source, origin: 'ciach.yaml')); + + group('ConfigFile.parse', () { + test('reads every option', () { + // Checked through the merge, which is what the CLI does with them. + final resolved = resolveFile(''' +path: packages/app +public: false +generated: true +overrides: true +operators: true +unused-union-members: true +report-tojson: true +set-exit-if-changed: true +remove: true +force: true +exclude: + - 'test/**' + - 'tool/**' +include: + - 'lib/**' +generated-suffix: + - .gc.dart +kinds: [class, function] +format: github +color: true +progress: true +concurrency: 4 +dart: /sdk/bin/dart +'''); + + expect(resolved.rootPath, 'packages/app'); + expect(resolved.includePublic, isFalse); + expect(resolved.includeGenerated, isTrue); + expect(resolved.overrides, isTrue); + expect(resolved.operators, isTrue); + expect(resolved.unusedUnionMembers, isTrue); + expect(resolved.reportToJson, isTrue); + expect(resolved.setExitIfChanged, isTrue); + expect(resolved.remove, isTrue); + expect(resolved.force, isTrue); + expect(resolved.excludeGlobs, ['test/**', 'tool/**']); + expect(resolved.includeGlobs, ['lib/**']); + expect(resolved.additionalGeneratedSuffixes, ['.gc.dart']); + expect(resolved.kinds, {.class$, .function}); + expect(resolved.format, 'github'); + expect(resolved.useColor, isTrue); + expect(resolved.showProgress, isTrue); + expect(resolved.concurrency, 4); + expect(resolved.dartExecutable, '/sdk/bin/dart'); + // On its own, since it forces `progress` off. + expect(resolveFile('verbose: true').verbose, isTrue); + }); + + test('covers every command-line option', () { + // Anything settable on the command line is settable in the file. + final cliOnly = {'help', 'config', 'no-config'}; + final optionNames = parser.options.keys.toSet().difference(cliOnly); + + expect(configKeys.difference({'path'}), optionNames); + }); + + test('settings lists what the file sets, and only that', () { + final config = ConfigFile.parse(''' +public: false +exclude: ['test/**'] +concurrency: 4 +''', origin: 'ciach.yaml'); + + expect(config.settings, { + 'public': false, + 'exclude': ['test/**'], + 'concurrency': 4, + }); + expect(config.path, 'ciach.yaml'); + expect(config.ignored, isFalse); + }); + + test('treats an empty or comment-only file as no settings', () { + for (final source in ['', '\n', '# nothing here\n']) { + expect( + ConfigFile.parse(source, origin: 'ciach.yaml').settings, + isEmpty, + reason: 'for ${source.trim()}', + ); + } + }); + + test('treats a valueless key as unset', () { + final config = ConfigFile.parse('public:\n', origin: 'ciach.yaml'); + + expect(config.settings, isEmpty); + expect(resolve(const [], config).includePublic, isTrue); + }); + + test('accepts a bare string where a list is expected', () { + expect(resolveFile("exclude: 'test/**'").excludeGlobs, ['test/**']); + }); + + test('accepts comma-separated kinds in one string', () { + expect(resolveFile('kinds: class,method').kinds, { + .class$, + .method, + }); + }); + + test('rejects an unknown option, naming the file and valid keys', () { + expect( + () => ConfigFile.parse('publik: false', origin: 'ciach.yaml'), + throwsA( + isFormatException('ciach.yaml', contains("unknown option 'publik'")), + ), + ); + }); + + test('rejects a top-level document that is not a map', () { + expect( + () => ConfigFile.parse('- public', origin: 'ciach.yaml'), + throwsA( + isFormatException('ciach.yaml', contains('must be a map of options')), + ), + ); + }); + + test('rejects malformed YAML', () { + expect( + () => ConfigFile.parse('public: [unclosed', origin: 'ciach.yaml'), + throwsA(isFormatException('ciach.yaml', contains('not valid YAML'))), + ); + }); + + test('rejects a non-boolean flag', () { + expect( + () => resolveFile('public: sometimes'), + throwsA( + isFormatException( + 'ciach.yaml', + contains("'public' must be true or false, got a string"), + ), + ), + ); + }); + + test('rejects a non-string in a list', () { + expect( + () => resolveFile('exclude: [1]'), + throwsA(isFormatException('ciach.yaml', contains('a list of strings'))), + ); + }); + + test('rejects an unknown format', () { + expect( + () => resolveFile('format: xml'), + throwsA( + isFormatException('ciach.yaml', contains('text, json, github')), + ), + ); + }); + + test('rejects an unknown kind', () { + expect( + () => resolveFile('kinds: [klass]'), + throwsA( + isFormatException('ciach.yaml', contains("Unknown kind 'klass'")), + ), + ); + }); + + test('rejects a non-positive concurrency', () { + for (final value in ['0', '-2', 'many', '1.5']) { + expect( + () => resolveFile('concurrency: $value'), + throwsA( + isFormatException('ciach.yaml', contains('positive integer')), + ), + reason: 'for concurrency: $value', + ); + } + }); + + test('checks the type of every key, even one the argv overrides', () { + // Every option is given on the command line below, so only the eager + // check when the file is parsed can still catch the bad value. A map fits + // no setting, so it is the one wrong value that works for every key. + const everyOption = [ + '--public', + '--generated', + '--overrides', + '--operators', + '--unused-union-members', + '--report-tojson', + '--set-exit-if-changed', + '--remove', + '--force', + '-e', + 'test/**', + '-i', + 'lib/**', + '--generated-suffix', + '.gc.dart', + '-k', + 'class', + '-f', + 'json', + '--color', + '--progress', + '--verbose', + '-j', + '2', + '--dart', + '/sdk/bin/dart', + 'lib', + ]; + + for (final key in configKeys) { + expect( + () => resolve( + everyOption, + .parse('$key: {a: b}', origin: 'ciach.yaml'), + ), + throwsA(isFormatException('ciach.yaml', contains("'$key'"))), + reason: 'for a map under $key', + ); + } + }); + }); + + group('ConfigFile.load', () { + late Directory tempDir; + + setUp(() { + tempDir = Directory.systemTemp.createTempSync('ciach_config_test_'); + }); + + tearDown(() { + tempDir.deleteSync(recursive: true); + }); + + void write(String name, String content) => + File(p.join(tempDir.path, name)).writeAsStringSync(content); + + test('discovers ciach.yaml in the project directory', () { + write('ciach.yaml', 'public: false'); + + final config = ConfigFile.load(projectDir: tempDir.path); + + expect(config.path, p.join(tempDir.path, 'ciach.yaml')); + expect(config.settings['public'], isFalse); + }); + + test('discovers ciach.yaml only, not other spellings', () { + write('ciach.yml', 'format: json'); + write('.ciach.yaml', 'format: json'); + + expect(ConfigFile.load(projectDir: tempDir.path).path, isNull); + + write('ciach.yaml', 'format: github'); + + expect( + ConfigFile.load(projectDir: tempDir.path).settings['format'], + 'github', + ); + }); + + test('returns an empty config when the directory has none', () { + final config = ConfigFile.load(projectDir: tempDir.path); + + expect(config.path, isNull); + expect(config.settings, isEmpty); + }); + + test('does not look outside the project directory', () { + write('ciach.yaml', 'public: false'); + final nested = Directory(p.join(tempDir.path, 'nested'))..createSync(); + + expect(ConfigFile.load(projectDir: nested.path).path, isNull); + }); + + test('loads an explicit path from outside the project directory', () { + write('elsewhere.yaml', 'format: json'); + final nested = Directory(p.join(tempDir.path, 'nested'))..createSync(); + + final config = ConfigFile.load( + projectDir: nested.path, + explicitPath: p.join(tempDir.path, 'elsewhere.yaml'), + ); + + expect(config.settings['format'], 'json'); + }); + + test('an explicit path wins over a discoverable file', () { + write('ciach.yaml', 'format: github'); + write('other.yaml', 'format: json'); + + final config = ConfigFile.load( + projectDir: tempDir.path, + explicitPath: p.join(tempDir.path, 'other.yaml'), + ); + + expect(config.settings['format'], 'json'); + }); + + test('reports a missing explicit path', () { + expect( + () => ConfigFile.load( + projectDir: tempDir.path, + explicitPath: p.join(tempDir.path, 'nope.yaml'), + ), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('Config file does not exist'), + ), + ), + ); + }); + + test('ignore reads nothing, but still names the file it skipped', () { + // Invalid on purpose: --no-config never parses it, so it can't fail. + write('ciach.yaml', 'publik: [unclosed'); + + final config = ConfigFile.load(projectDir: tempDir.path, ignore: true); + + expect(config.ignored, isTrue); + expect(config.path, p.join(tempDir.path, 'ciach.yaml')); + expect(config.settings, isEmpty); + }); + + test('ignore reports no path when there was no config anyway', () { + final config = ConfigFile.load(projectDir: tempDir.path, ignore: true); + + expect(config.ignored, isTrue); + expect(config.path, isNull); + }); + + test('ignore skips an explicit path, and its would-be errors', () { + final config = ConfigFile.load( + projectDir: tempDir.path, + explicitPath: p.join(tempDir.path, 'nope.yaml'), + ignore: true, + ); + + expect(config.path, isNull); + }); + }); + + group('resolveOptions', () { + test('falls back to the built-in defaults with no config and no args', () { + final resolved = resolve(const []); + + expect(resolved.rootPath, '.'); + expect(resolved.includePublic, isTrue); + expect(resolved.includeGenerated, isFalse); + expect(resolved.overrides, isFalse); + expect(resolved.operators, isFalse); + expect(resolved.unusedUnionMembers, isFalse); + expect(resolved.reportToJson, isFalse); + expect(resolved.setExitIfChanged, isFalse); + expect(resolved.remove, isFalse); + expect(resolved.force, isFalse); + expect(resolved.includeGlobs, isEmpty); + expect(resolved.excludeGlobs, isEmpty); + expect(resolved.additionalGeneratedSuffixes, isEmpty); + expect(resolved.kinds, FinderOptions.defaultKinds); + expect(resolved.format, 'text'); + expect(resolved.verbose, isFalse); + expect(resolved.concurrency, 16); + expect(resolved.dartExecutable, isNull); + }); + + test('command-line flags override the config', () { + final config = ConfigFile.parse(''' +path: packages/app +public: false +generated: true +format: json +color: false +progress: false +concurrency: 4 +dart: /sdk/bin/dart +''', origin: 'ciach.yaml'); + + final resolved = resolve(const [ + '--public', + '--no-generated', + '--format', + 'github', + '--color', + '--progress', + '--concurrency', + '2', + '--dart', + '/other/dart', + 'packages/other', + ], config); + + expect(resolved.rootPath, 'packages/other'); + expect(resolved.includePublic, isTrue); + expect(resolved.includeGenerated, isFalse); + expect(resolved.format, 'github'); + expect(resolved.useColor, isTrue); + expect(resolved.showProgress, isTrue); + expect(resolved.concurrency, 2); + expect(resolved.dartExecutable, '/other/dart'); + }); + + test('a command-line list replaces the config list, not appends', () { + final config = ConfigFile.parse( + "exclude: ['test/**']\nkinds: [class]", + origin: 'ciach.yaml', + ); + + final resolved = resolve(const ['-e', 'tool/**', '-k', 'method'], config); + + expect(resolved.excludeGlobs, ['tool/**']); + expect(resolved.kinds, {.method}); + }); + + test('repeated command-line values all survive', () { + final resolved = resolve(const [ + '-e', + 'test/**', + '-e', + 'tool/**', + '--generated-suffix', + '.gc.dart', + '--generated-suffix', + '.pb.dart', + ]); + + expect(resolved.excludeGlobs, ['test/**', 'tool/**']); + expect(resolved.additionalGeneratedSuffixes, ['.gc.dart', '.pb.dart']); + }); + + test('explicitly passing a flag at its default value still wins', () { + // --public matches the default, so only the layer it came from tells it + // apart from an absent flag — and it has to, to beat `public: false`. + final resolved = resolve(const [ + '--public', + ], .parse('public: false', origin: 'ciach.yaml')); + + expect(resolved.includePublic, isTrue); + }); + + test('auto-detected color and progress are the last resort', () { + final auto = resolveWith( + const [], + const .empty(), + colorDefault: true, + progressDefault: true, + ); + expect(auto.useColor, isTrue); + expect(auto.showProgress, isTrue); + + final fromConfig = resolveWith( + const [], + .parse('color: false\nprogress: false', origin: 'c.yaml'), + colorDefault: true, + progressDefault: true, + ); + expect(fromConfig.useColor, isFalse); + expect(fromConfig.showProgress, isFalse); + + final fromArgs = resolveWith( + const ['--no-color', '--no-progress'], + .parse('color: true\nprogress: true', origin: 'c.yaml'), + colorDefault: true, + progressDefault: true, + ); + expect(fromArgs.useColor, isFalse); + expect(fromArgs.showProgress, isFalse); + }); + + test('verbose comes from the command line or the config', () { + expect(resolve(const ['--verbose']).verbose, isTrue); + expect(resolve(const ['-v']).verbose, isTrue); + expect(resolveFile('verbose: true').verbose, isTrue); + expect( + resolve(const [ + '--no-verbose', + ], .parse('verbose: true', origin: 'c.yaml')).verbose, + isFalse, + ); + }); + + test('verbose supersedes the progress line', () { + // Both write to stderr, so verbose wins however progress was asked for. + expect( + resolveWith( + const ['--verbose', '--progress'], + const .empty(), + colorDefault: false, + progressDefault: true, + ).showProgress, + isFalse, + ); + expect( + resolveWith( + const [], + .parse('verbose: true\nprogress: true', origin: 'c.yaml'), + colorDefault: false, + progressDefault: true, + ).showProgress, + isFalse, + ); + expect( + resolveWith( + const ['--progress'], + .parse('verbose: true', origin: 'c.yaml'), + colorDefault: false, + progressDefault: false, + ).showProgress, + isFalse, + ); + // …but progress still works when verbose is off. + expect(resolve(const ['--progress']).showProgress, isTrue); + }); + + test('rejects a non-positive --concurrency', () { + for (final value in ['0', '-1']) { + expect( + () => resolve(['--concurrency', value]), + throwsA(isUsageException(contains('below the minimum (1)'))), + reason: 'for --concurrency $value', + ); + } + expect( + () => resolve(const ['--concurrency', 'lots']), + throwsA(isUsageException(contains('lots'))), + ); + }); + + test('rejects an unknown --kinds value', () { + expect( + () => resolve(const ['-k', 'klass']), + throwsA(isUsageException(contains("Unknown kind 'klass'"))), + ); + }); + + test('reports every problem at once, not just the first', () { + // An out-of-range `--format` is not among them: an option's `allowed` + // list is the arg parser's business, so that fails before resolution. + expect( + () => resolve(const ['-k', 'klass', '-j', '0']), + throwsA( + isUsageException( + allOf( + contains("Unknown kind 'klass'"), + contains('below the minimum (1)'), + ), + ), + ), + ); + }); + + test('hands the finder its share of the settings', () { + final resolved = resolve(const [ + '--no-public', + '--overrides', + '-e', + 'test/**', + '-j', + '4', + ]); + final options = resolved.finderOptions(); + + expect(options.rootPath, p.normalize(p.absolute('.'))); + expect(options.includePublic, isFalse); + // Inverted for the finder. + expect(options.skipOverrides, isFalse); + expect(options.skipOperators, isTrue); + expect(options.excludeGlobs, ['test/**']); + expect(options.concurrency, 4); + expect(options.onProgress, isNull); + }); + }); +} + +/// A [UsageException] whose message matches [message]. +Matcher isUsageException(Matcher message) => + isA().having((e) => e.message, 'message', message); + +/// A [FormatException] whose message names [origin] and matches [message]. +Matcher isFormatException(String origin, Matcher message) => + isA() + .having((e) => e.message, 'message', startsWith('$origin:')) + .having((e) => e.message, 'message', message); diff --git a/test/verbose_test.dart b/test/verbose_test.dart new file mode 100644 index 0000000..71c5498 --- /dev/null +++ b/test/verbose_test.dart @@ -0,0 +1,174 @@ +import 'dart:io'; + +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:path/path.dart' as p; +import 'package:test/test.dart'; + +void main() { + final parser = buildParser(); + + group('describeConfigSource', () { + test('names the file it read and every option it set', () { + final lines = describeConfigSource( + .parse("public: false\nexclude: ['test/**']", origin: '/c.yaml'), + projectDir: '/pkg', + ); + + expect(lines, [ + 'Read config from /c.yaml.', + ' It sets 2 options:', + ' public: false', + ' exclude: test/**', + ]); + }); + + test('says so when the file it read is empty', () { + final lines = describeConfigSource( + .parse('# nothing\n', origin: '/pkg/ciach.yaml'), + projectDir: '/pkg', + ); + + expect(lines, [ + 'Read config from /pkg/ciach.yaml.', + contains('It sets nothing'), + ]); + }); + + test('names the directory it searched when nothing was found', () { + final lines = describeConfigSource( + const .empty(), + projectDir: 'packages/app', + ); + + expect(lines, [ + allOf(contains('No ciach.yaml in packages/app'), contains('defaults')), + ]); + }); + + test('names the file it skipped for --no-config', () { + final dir = Directory.systemTemp.createTempSync('ciach_verbose_test_'); + addTearDown(() => dir.deleteSync(recursive: true)); + File(p.join(dir.path, configFileName)).writeAsStringSync('public: false'); + + final lines = describeConfigSource( + .load(projectDir: dir.path, ignore: true), + projectDir: dir.path, + ); + + expect(lines, [ + 'Ignoring the config file ${p.join(dir.path, 'ciach.yaml')} (--no-config).', + ]); + }); + + test('says --no-config changed nothing when there was no file', () { + final dir = Directory.systemTemp.createTempSync('ciach_verbose_test_'); + addTearDown(() => dir.deleteSync(recursive: true)); + + final lines = describeConfigSource( + .load(projectDir: dir.path, ignore: true), + projectDir: dir.path, + ); + + expect(lines, [ + allOf( + contains('--no-config'), + contains('no ciach.yaml in ${dir.path}'), + ), + ]); + }); + }); + + group('describeSettings', () { + List describe([List arguments = const []]) { + final configuration = resolveConfiguration( + parser.parse(arguments), + const .empty(), + ); + return describeSettings( + configuration, + resolveOptions( + configuration, + colorDefault: false, + progressDefault: false, + ), + dartExecutable: '/sdk/bin/dart', + ); + } + + test('lists every setting the run uses, under one key per option', () { + final lines = describe(); + + expect(lines.first, 'Settings for this run:'); + final keys = lines.skip(1).map((l) => l.trim().split(':').first).toSet(); + // `path` stands in for the positional argument, as in the config file. + expect(keys, configKeys); + }); + + test('reports the resolved values, not the raw arguments', () { + final lines = describe(const [ + '--no-public', + '-e', + 'test/**', + '-j', + '4', + '/pkg', + ]); + + expect(lines, containsAll([' path: /pkg (command line)'])); + expect(lines, contains(' public: false (command line)')); + expect(lines, contains(' exclude: test/** (command line)')); + expect(lines, contains(' concurrency: 4 (command line)')); + expect(lines, contains(' dart: /sdk/bin/dart (auto-detected)')); + }); + + test('names the layer each value came from', () { + final configuration = resolveConfiguration( + parser.parse(const ['--no-public']), + .parse('format: json\nremove: true', origin: 'c.yaml'), + ); + final lines = describeSettings( + configuration, + resolveOptions( + configuration, + colorDefault: false, + progressDefault: false, + ), + dartExecutable: '/sdk/bin/dart', + ); + + expect(lines, contains(' public: false (command line)')); + expect(lines, contains(' format: json (config file)')); + expect(lines, contains(' remove: true (config file)')); + expect(lines, contains(' concurrency: 16 (default)')); + expect(lines, contains(' color: false (auto-detected)')); + }); + + test('marks an empty list rather than printing nothing', () { + expect(describe(), contains(' exclude: (none) (default)')); + expect(describe(), contains(' include: (none) (default)')); + expect(describe(), contains(' generated-suffix: (none) (default)')); + }); + + test('lists the kinds, all of them by default', () { + final kinds = describe() + .singleWhere((l) => l.startsWith(' kinds:')) + .replaceFirst(' kinds: ', '') + .replaceFirst(' (default)', '') + .split(', '); + + // Two aliases can map to one kind, so labels can be fewer. + expect(kinds, hasLength(FinderOptions.defaultKinds.length)); + expect( + describe(const [ + '-k', + 'class,method', + ]).singleWhere((l) => l.startsWith(' kinds:')), + ' kinds: class, method (command line)', + ); + }); + }); +}