From 9e46c80e2b84d424b7ab5082475b507701322f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Baudet?= Date: Wed, 14 Oct 2026 18:10:23 -0600 Subject: [PATCH 1/3] feat(validator): check imports and barrels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read the import and export directives of every Dart file under a package's lib/ and bin/, and add five rules: - missing-primary-barrel: a feature package has no lib/{package}.dart. - deep-import: an import or export reaches past the barrels at the root of another workspace package's lib/, src/ or not. - cross-feature-primary-barrel: a feature imports another feature's primary presentation barrel instead of a dedicated one. - data-barrel-exports-internals: a data barrel exports from data_sources/, dtos/, or mappers/. - unused-dependency (warning): a workspace dependency under dependencies is never imported. Findings in a Dart file carry its path and line, and can be suppressed in that file with `// ffca-ignore: — `. The hook now also runs on .dart edits. For a Dart file the validator reports only the findings located in that file, so a Dart edit never blocks on a violation elsewhere in the package. Every fixture package gains a barrel that imports its declared dependencies, and analysis_options.yaml keeps the fixtures out of dart analyze. Closes #25 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq --- AGENTS.md | 3 +- CLAUDE.md | 2 +- README.md | 22 +- agents/ffca-layer-auditor.md | 23 +- hooks/hooks.json | 2 +- hooks/validate_layers.sh | 13 +- hooks/validate_layers_test.sh | 15 +- scripts/analysis_options.yaml | 4 + .../apps/mobile_app/lib/mobile_app.dart | 1 + .../alpha/alpha_data/lib/alpha_data.dart | 1 + .../alpha/alpha_domain/lib/alpha_domain.dart | 1 + .../beta/beta_data/lib/beta_data.dart | 2 + .../beta/beta_domain/lib/beta_domain.dart | 1 + .../lib/beta_presentation.dart | 1 + .../features/chi/chi_domain/pubspec.yaml | 4 + .../delta/delta_data/lib/delta_data.dart | 1 + .../delta/delta_domain/lib/delta_domain.dart | 1 + .../epsilon_domain/lib/epsilon_domain.dart | 1 + .../features/eta/eta_data/lib/eta_data.dart | 2 + .../eta/eta_domain/lib/eta_domain.dart | 1 + .../gamma/gamma_domain/lib/gamma_domain.dart | 1 + .../lib/gamma_presentation.dart | 2 + .../iota/iota_data/lib/iota_data.dart | 1 + .../iota/iota_domain/lib/iota_domain.dart | 1 + .../kappa/kappa_data/lib/kappa_data.dart | 1 + .../lambda_domain/lib/lambda_domain.dart | 1 + .../features/mu/mu_data/lib/mu_data.dart | 1 + .../features/mu/mu_domain/lib/mu_domain.dart | 1 + .../omicron_data/lib/omicron_data.dart | 1 + .../omicron_domain/lib/omicron_domain.dart | 1 + .../phi/phi_domain/lib/models/phi.dart | 1 + .../phi/phi_domain/lib/phi_domain.dart | 1 + .../features/phi/phi_domain/pubspec.yaml | 4 + .../lib/phi_presentation.dart | 2 + .../phi/phi_presentation/pubspec.yaml | 6 + .../phi_presentation/test/phi_helpers.dart | 2 + .../features/pi/pi_domain/lib/pi_domain.dart | 1 + .../lib/sigma_presentation.dart | 2 + .../sigma/sigma_presentation/pubspec.yaml | 6 + .../tau_remote_data_source/dtos/tau_dto.dart | 1 + .../tau/tau_data/lib/mappers/tau_mapper.dart | 1 + .../lib/repositories/tau_repository.dart | 1 + .../features/tau/tau_data/lib/tau_data.dart | 6 + .../features/tau/tau_data/pubspec.yaml | 6 + .../tau/tau_domain/lib/tau_domain.dart | 1 + .../features/tau/tau_domain/pubspec.yaml | 4 + .../theta/theta_data/lib/theta_data.dart | 2 + .../theta/theta_domain/lib/theta_domain.dart | 1 + .../lib/upsilon_presentation.dart | 1 + .../upsilon/upsilon_presentation/pubspec.yaml | 4 + .../zeta/zeta_domain/lib/zeta_domain.dart | 1 + .../fixtures/invalid_workspace/pubspec.yaml | 7 + .../shared/bad_shared/lib/bad_shared.dart | 1 + .../apps/mobile_app/lib/mobile_app.dart | 5 + .../lib/auth_data_firebase.dart | 1 + .../auth/auth_domain/lib/auth_domain.dart | 1 + .../cart/cart_data/lib/cart_data.dart | 1 + .../cart/cart_domain/lib/cart_domain.dart | 1 + .../cart_presentation/lib/cart_badge.dart | 1 + .../lib/cart_presentation.dart | 3 + .../lib/ideas_presentation.dart | 5 + .../product_data/lib/product_data.dart | 4 + .../lib/repositories/products_repository.dart | 1 + .../product_domain/lib/product_domain.dart | 1 + .../lib/product_presentation.dart | 2 + .../shared/api_client/lib/api_client.dart | 1 + .../shared/ui_kit/lib/ui_kit.dart | 1 + .../features/nu/nu_domain/lib/nu_domain.dart | 1 + .../nu_presentation/lib/nu_presentation.dart | 1 + .../omega/omega_data/lib/omega_data.dart | 1 + .../features/omega/omega_data/pubspec.yaml | 6 + .../omega/omega_domain/lib/models/omega.dart | 1 + .../omega/omega_domain/lib/omega_domain.dart | 1 + .../features/omega/omega_domain/pubspec.yaml | 4 + .../lib/omega_presentation.dart | 2 + .../omega/omega_presentation/pubspec.yaml | 6 + .../rho/rho_domain/lib/rho_domain.dart | 1 + .../features/xi/xi_data/lib/xi_data.dart | 1 + .../features/xi/xi_domain/lib/xi_domain.dart | 1 + .../fixtures/warning_workspace/pubspec.yaml | 3 + scripts/test/validate_layers_test.dart | 145 ++++++- scripts/validate_layers.dart | 401 ++++++++++++++---- skills/ffca-audit/SKILL.md | 2 +- 83 files changed, 677 insertions(+), 102 deletions(-) create mode 100644 scripts/analysis_options.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/apps/mobile_app/lib/mobile_app.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/alpha/alpha_data/lib/alpha_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/beta/beta_data/lib/beta_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/beta/beta_domain/lib/beta_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/beta/beta_presentation/lib/beta_presentation.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/chi/chi_domain/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/delta/delta_data/lib/delta_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/delta/delta_domain/lib/delta_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/epsilon/epsilon_domain/lib/epsilon_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/eta/eta_data/lib/eta_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/eta/eta_domain/lib/eta_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/gamma/gamma_domain/lib/gamma_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/gamma/gamma_presentation/lib/gamma_presentation.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/iota/iota_data/lib/iota_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/iota/iota_domain/lib/iota_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/kappa/kappa_data/lib/kappa_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/lambda/lambda_domain/lib/lambda_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/mu/mu_data/lib/mu_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/mu/mu_domain/lib/mu_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/omicron/omicron_data/lib/omicron_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/omicron/omicron_domain/lib/omicron_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/models/phi.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/phi_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/test/phi_helpers.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/pi/pi_domain/lib/pi_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/lib/sigma_presentation.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/dtos/tau_dto.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/mappers/tau_mapper.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/repositories/tau_repository.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/lib/tau_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/theta/theta_data/lib/theta_data.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/theta/theta_domain/lib/theta_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/zeta/zeta_domain/lib/zeta_domain.dart create mode 100644 scripts/test/fixtures/invalid_workspace/shared/bad_shared/lib/bad_shared.dart create mode 100644 scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/auth/auth_data_firebase/lib/auth_data_firebase.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/auth/auth_domain/lib/auth_domain.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/cart/cart_data/lib/cart_data.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_badge.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_presentation.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/ideas/ideas_presentation/lib/ideas_presentation.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/product/product_data/lib/product_data.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/product/product_data/lib/repositories/products_repository.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/product/product_domain/lib/product_domain.dart create mode 100644 scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart create mode 100644 scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart create mode 100644 scripts/test/fixtures/valid_workspace/shared/ui_kit/lib/ui_kit.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/nu/nu_domain/lib/nu_domain.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/nu/nu_presentation/lib/nu_presentation.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_data/lib/omega_data.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_data/pubspec.yaml create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/models/omega.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/omega_domain.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_domain/pubspec.yaml create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/lib/omega_presentation.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/pubspec.yaml create mode 100644 scripts/test/fixtures/warning_workspace/features/rho/rho_domain/lib/rho_domain.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/xi/xi_data/lib/xi_data.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/xi/xi_domain/lib/xi_domain.dart diff --git a/AGENTS.md b/AGENTS.md index f2c33c3..6e1d0c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -40,9 +40,10 @@ references/ domain_templates.md # Model, repository interface, Command and Query shapes presentation_templates.md # Cubit, Module, route shapes scripts/ + analysis_options.yaml # Excludes the test fixtures from analysis pubspec.yaml # Dev dependency on package:test for the validator tests sync_reference.dart # Regenerates references/ffca/ from VGV Engineering, --check for drift - validate_layers.dart # FFCA layer, naming, and cycle validator + validate_layers.dart # FFCA layer, naming, cycle, import, and barrel validator test/ validate_layers_test.dart # Validator tests fixtures/ diff --git a/CLAUDE.md b/CLAUDE.md index 077946f..8f140a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,7 +7,7 @@ `hooks/hooks.json` defines one PostToolUse hook and one PreToolUse hook. -- `Edit|MultiEdit|Write` matcher → `validate_layers.sh`. The matcher is a filter, not a guarantee, so the hook reads `tool_name` from the payload and exits 0 unless it is `Edit`, `MultiEdit`, or `Write`. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, meaning a `features/` folder exists above it, the hook runs `dart run scripts/validate_layers.dart --file `. The validator checks the edited package and its direct dependents. Exit 2 means at least one error-severity finding. The hook prints the findings to stderr and exits 2, which blocks Claude until the dependency is fixed. When the validator exits 0 with output, every finding is a warning: the hook returns it as `additionalContext` JSON and exits 0, so Claude sees it without being blocked. A `# ffca-ignore: — ` comment in a pubspec suppresses that rule for that package. +- `Edit|MultiEdit|Write` matcher → `validate_layers.sh`. The matcher is a filter, not a guarantee, so the hook reads `tool_name` from the payload and exits 0 unless it is `Edit`, `MultiEdit`, or `Write`. When the edited file is a `pubspec.yaml` or a Dart file inside an FFCA-shaped repo, meaning a `features/` folder exists above it, the hook runs `dart run scripts/validate_layers.dart --file `. For a pubspec, the validator checks the edited package and its direct dependents. For a Dart file under a package's `lib/` or `bin/`, it reports only the import and barrel findings located in that file, so a Dart edit never blocks on a violation elsewhere. Exit 2 means at least one error-severity finding. The hook prints the findings to stderr and exits 2, which blocks Claude until the dependency is fixed. When the validator exits 0 with output, every finding is a warning: the hook returns it as `additionalContext` JSON and exits 0, so Claude sees it without being blocked. A `# ffca-ignore: — ` comment in a pubspec suppresses that rule for that package, and `// ffca-ignore:` in a Dart file suppresses it for that file. - Any other file, or a repo with no `features/` folder, exits 0 silently. - The hook skips with exit 0 when `jq` or `dart` is missing from `PATH`. The `dart` check runs only for FFCA pubspec edits. A validator failure other than exit 2 is reported to stderr but never blocks. diff --git a/README.md b/README.md index 2081d68..8d9fc43 100644 --- a/README.md +++ b/README.md @@ -114,14 +114,14 @@ Skills activate automatically when Claude detects an FFCA repo or an FFCA-shaped ## Hooks -A PostToolUse hook runs on every `Edit`, `MultiEdit`, or `Write` and ignores any other tool call. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, it validates the package's layer dependencies. A PreToolUse hook gates every Very Good CLI MCP tool call. +A PostToolUse hook runs on every `Edit`, `MultiEdit`, or `Write` and ignores any other tool call. When the edited file is a `pubspec.yaml` inside an FFCA-shaped repo, it validates the package's layer dependencies. When it is a Dart file, it validates that file's imports and exports. A PreToolUse hook gates every Very Good CLI MCP tool call. | Hook | Event | Behavior | | --- | --- | --- | -| **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`MultiEdit`/`Write`) | Runs the FFCA validator incrementally on the edited package and its direct dependents. Exits 2 on an error (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. Passes warnings to Claude as non-blocking context. Passes silently otherwise | +| **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`MultiEdit`/`Write`) | Runs the FFCA validator incrementally: on the edited package and its direct dependents for a pubspec edit, and on the edited file alone for a Dart edit. Exits 2 on an error (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. Passes warnings to Claude as non-blocking context. Passes silently otherwise | | **Check VGV CLI** (`check_vgv_cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Auto-approves Very Good CLI MCP tool calls when the CLI is installed at 1.3.0 or newer, so they work in every run mode. Denies with an install or upgrade message when the CLI is missing or outdated. Stands aside for any other tool, or when the CLI version cannot be read | -The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK. +The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK. In `lib/` and `bin/`, it checks that each feature package has a primary barrel `lib/{package}.dart`, that imports of other workspace packages go through a barrel at the root of their `lib/`, that another feature's presentation is imported through a dedicated barrel rather than its primary one, that a data barrel never exports data sources, DTOs, or mappers, and that every declared workspace dependency is imported. The validator is also runnable directly for CI and audits, across the whole workspace: @@ -149,6 +149,11 @@ Every finding carries a rule ID in brackets. An **error** exits 2 and blocks the | `data-without-domain` | error | A data package's feature has no `_domain` package | | `domain-uses-flutter` | error | A domain package depends on the Flutter SDK | | `presentation-missing-own-domain` | warning | A presentation package does not depend on its feature's existing `_domain` | +| `missing-primary-barrel` | error | A feature package has no `lib/{package}.dart` | +| `deep-import` | error | A Dart file imports or exports another workspace package past the barrels at the root of its `lib/` | +| `cross-feature-primary-barrel` | error | A feature imports another feature's primary presentation barrel instead of a dedicated one | +| `data-barrel-exports-internals` | error | A data package's primary barrel exports from `data_sources/`, `dtos/`, or `mappers/` | +| `unused-dependency` | warning | A package declares a workspace dependency under `dependencies` but never imports it in `lib/` or `bin/` | | `suppression-missing-reason` | error | An `ffca-ignore` comment has no reason | | `suppression-unknown-rule` | error | An `ffca-ignore` comment names a rule that does not exist or cannot be suppressed | | `unused-suppression` | warning | An `ffca-ignore` comment matches no finding (`--all` only) | @@ -160,7 +165,14 @@ To accept a finding as a deliberate exception, add a comment with the rule ID an name: orders_data ``` -A suppression covers every finding with that rule ID in that package. The reason is required, so each exception explains itself in review. The last three rules cannot be suppressed. `--all` reports how many findings were suppressed, and flags suppressions that no longer match anything so they can be removed. +A finding in a Dart file can also be suppressed in that file, with `//`: + +```dart +// ffca-ignore: deep-import — the model is not exported from orders_domain yet +import 'package:orders_domain/models/order.dart'; +``` + +A suppression in a pubspec covers every finding with that rule ID in that package, and one in a Dart file covers that file. The reason is required, so each exception explains itself in review. The last three rules cannot be suppressed. `--all` reports how many findings were suppressed, and flags suppressions that no longer match anything so they can be removed. ### Prerequisites @@ -195,7 +207,7 @@ The server needs **Very Good CLI** 1.3.0 or newer. Install it with `dart pub glo | Agent | Behavior | | --- | --- | -| [**ffca-layer-auditor**](agents/ffca-layer-auditor.md) | Read-only architecture auditor. Runs the validator in `--all` mode, then adds source-level checks (declared-but-unused dependencies, barrel hygiene, DTO leakage, Command/Query necessity, module entry, split routing tables, deferred-loading reachability, misplaced packages, high fan-in) and returns a per-package verdict table. Reports violations, never auto-fixes | +| [**ffca-layer-auditor**](agents/ffca-layer-auditor.md) | Read-only architecture auditor. Runs the validator in `--all` mode, then adds source-level checks (barrel scope, DTO leakage, Command/Query necessity, module entry, split routing tables, deferred-loading reachability, misplaced packages, high fan-in) and returns a per-package verdict table. Reports violations, never auto-fixes | The auditor runs in its own context, so the same architecture review can be dispatched from the `ffca-audit` skill, a refactor, or a pre-PR flow without crowding the main conversation. The per-edit hook prevents bad pubspec dependencies as they are written; the agent answers whether the whole repo is healthy on demand. diff --git a/agents/ffca-layer-auditor.md b/agents/ffca-layer-auditor.md index e3eee6b..e3b7c07 100644 --- a/agents/ffca-layer-auditor.md +++ b/agents/ffca-layer-auditor.md @@ -39,23 +39,22 @@ From the repo root, run the validator in full-graph mode and treat every line it dart run ${CLAUDE_PLUGIN_ROOT}/scripts/validate_layers.dart --all ``` -This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, and feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding. +This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK), and imports and barrels (each feature package has a primary barrel, no import reaches past another package's barrels, another feature's presentation is imported through a dedicated barrel, a data barrel never exports data sources, DTOs, or mappers, and every declared workspace dependency is imported). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding. ## Step 2: source-level checks (qualitative) -The script validates the declared pubspec graph. These checks need the source, so do them by reading and grepping the tree. Cite the reference section for each. +The script validates the pubspec graph and the import and export directives in `lib/` and `bin/`. These checks need judgment or a deeper read of the source, so do them by reading and grepping the tree. Cite the reference section for each. 1. **Package inventory and classification.** Enumerate every package under `apps/`, `features/`, `shared/`. Classify each by folder, name, and inferred type: full feature, headless feature (domain plus data, no presentation), presentation-only feature (a `_presentation` with no domain or data sibling), or shared package. All four are legitimate; a missing sibling is a signal, not a violation. See `references/ffca/overview.md`, sections *Features*, *Headless features*, *Presentation-only features*, *Shared libraries*. -2. **Declared-but-unused dependencies.** For each package, every dependency on another workspace package in its pubspec, by `path:` or by name, should be imported somewhere in its source. A declared dependency that is never imported is a finding. Pay special attention to presentation packages. -3. **Barrel hygiene.** Each package has a primary barrel `lib/{package}.dart` re-exporting `src/` (or the layer's public files). Features with multiple independent entry points have subfeature barrels. No barrel re-exports a private symbol. Detect private re-exports with `rg -n "^export '.*/_" features shared apps`. A widget exported for another feature has its own narrow barrel whose transitive imports do not reach the feature's modules or screens; a cross-feature import that targets the primary barrel is a finding. See `references/ffca/presentation.md`, sections *Subfeature barrel files* and *Sharing a widget across features*, and `references/ffca/project_structure.md`, section *Layer subfolders*. -4. **DTO leakage.** Generated or transport types (`*.g.dart`, `*.freezed.dart`, anything under a `dtos/` folder) must not be imported outside the data package that owns them. Detect with `rg -n "import .*\.(g|freezed)\.dart'" features shared apps` and check the importing package owns the source. See `references/ffca/data.md`, section *DTOs and mappers*. -5. **Command and Query necessity.** A `Query` or `Command` class in a `*_domain` package is justified only when it combines two or more repositories or removes duplication across Blocs. A single-repository pass-through is an anti-pattern. Open each `*_query.dart` / `*_command.dart` and count injected repositories. Also check the verbs: commands expose `execute`, queries expose `get` or `watch`, and neither is a callable class. Classes named `*UseCase` are a naming finding. See `references/ffca/domain.md`, section *Business rules*, and `references/ffca/faq.md`, section *Should we use callable classes for Commands and Queries?* -6. **Module entry points.** App route bindings should instantiate a feature `*Module`, not a screen or page widget directly, so dependencies stay explicit. Inspect the app's router and verify each route builds a `*Module`. See `references/ffca/presentation.md`, section *The module*, and `references/ffca/navigation.md`. -7. **Misplaced packages.** Volatile or app-specific business logic sitting in `shared/`, or a generic, pub-publishable utility sitting in `features/`. Apply the decision rule and component table in `references/ffca/overview.md`, section *Shared libraries*. -8. **High fan-in domains.** A domain many features depend on may be doing too much. Note it for review against `references/ffca/domain.md`, section *Composing features*. - -9. **Split routing tables.** If the app's routes live in more than one file, every file but the entry library must begin with `part of`. A per-feature route file that is a separate library is silently dropped by `go_router_builder`, and the build still succeeds. Detect with `rg -L "^part of" apps/*/lib/**/[a-z_]*routes.dart`. See `references/ffca/navigation.md`, section *Splitting the routing table across files*. -10. **Deferred loading reachability.** For each feature the app imports with a `deferred as` prefix, check that no non-deferred import path from the app reaches the same package. An eager edge, typically one presentation package importing another, silently cancels the code splitting. Advisory on apps that ship only to iOS and Android. See `references/ffca/project_structure.md`, section *Deferred loading*. +2. **Barrel scope.** The validator checks that barrels exist and that imports go through them. Check what they expose: a feature with several independent entry points has a subfeature barrel for each, no barrel re-exports a private symbol (`rg -n "^export '.*/_" features shared apps`), and a barrel made for another feature exports only that widget, with transitive imports that do not reach the feature's modules or screens. See `references/ffca/presentation.md`, sections *Subfeature barrel files* and *Sharing a widget across features*. +3. **DTO leakage.** Generated or transport types (`*.g.dart`, `*.freezed.dart`, anything under a `dtos/` folder) must not be imported outside the data package that owns them. The validator already flags a data barrel that exports them and any import that reaches past a barrel, so look for the remaining path: a DTO or generated type in a public signature, such as a repository method. Detect with `rg -n "import .*\.(g|freezed)\.dart'" features shared apps` and check the importing package owns the source. See `references/ffca/data.md`, section *DTOs and mappers*. +4. **Command and Query necessity.** A `Query` or `Command` class in a `*_domain` package is justified only when it combines two or more repositories or removes duplication across Blocs. A single-repository pass-through is an anti-pattern. Open each `*_query.dart` / `*_command.dart` and count injected repositories. Also check the verbs: commands expose `execute`, queries expose `get` or `watch`, and neither is a callable class. Classes named `*UseCase` are a naming finding. See `references/ffca/domain.md`, section *Business rules*, and `references/ffca/faq.md`, section *Should we use callable classes for Commands and Queries?* +5. **Module entry points.** App route bindings should instantiate a feature `*Module`, not a screen or page widget directly, so dependencies stay explicit. Inspect the app's router and verify each route builds a `*Module`. See `references/ffca/presentation.md`, section *The module*, and `references/ffca/navigation.md`. +6. **Misplaced packages.** Volatile or app-specific business logic sitting in `shared/`, or a generic, pub-publishable utility sitting in `features/`. Apply the decision rule and component table in `references/ffca/overview.md`, section *Shared libraries*. +7. **High fan-in domains.** A domain many features depend on may be doing too much. Note it for review against `references/ffca/domain.md`, section *Composing features*. + +8. **Split routing tables.** If the app's routes live in more than one file, every file but the entry library must begin with `part of`. A per-feature route file that is a separate library is silently dropped by `go_router_builder`, and the build still succeeds. Detect with `rg -L "^part of" apps/*/lib/**/[a-z_]*routes.dart`. See `references/ffca/navigation.md`, section *Splitting the routing table across files*. +9. **Deferred loading reachability.** For each feature the app imports with a `deferred as` prefix, check that no non-deferred import path from the app reaches the same package. An eager edge, typically one presentation package importing another, silently cancels the code splitting. Advisory on apps that ship only to iOS and Android. See `references/ffca/project_structure.md`, section *Deferred loading*. ## Step 3: report diff --git a/hooks/hooks.json b/hooks/hooks.json index 8499b5c..c54908c 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -1,5 +1,5 @@ { - "description": "VGV FFCA plugin hooks: enforces Feature-First Clean Architecture layer rules on pubspec.yaml edits and gates Very Good CLI MCP tool calls", + "description": "VGV FFCA plugin hooks: enforces Feature-First Clean Architecture layer rules on pubspec.yaml and Dart edits and gates Very Good CLI MCP tool calls", "hooks": { "PreToolUse": [ { diff --git a/hooks/validate_layers.sh b/hooks/validate_layers.sh index fa4cd9b..a4e6d83 100755 --- a/hooks/validate_layers.sh +++ b/hooks/validate_layers.sh @@ -1,7 +1,8 @@ #!/bin/bash set -euo pipefail -# PostToolUse hook: enforce FFCA layer dependency rules on pubspec.yaml edits. +# PostToolUse hook: enforce FFCA layer dependency rules on pubspec.yaml edits, +# and the import and barrel rules on Dart edits. # Mirrors the analyze hook in vgv-ai-flutter-plugin: parse the payload with jq, # filter to the files we care about, skip gracefully when prerequisites are # missing, then run the validator and propagate its exit code (2 blocks Claude). @@ -27,10 +28,12 @@ esac # Extract the edited file path. file_path=$(jq -r '.tool_input.file_path // empty' <<<"$input") -# Skip unless this is a pubspec.yaml. -if [[ -z "$file_path" || "$(basename "$file_path")" != "pubspec.yaml" ]]; then - exit 0 -fi +# Skip unless this is a pubspec.yaml or a Dart file. For a Dart file the +# validator reports only the findings located in that file. +case "$(basename "$file_path")" in + pubspec.yaml | *.dart) ;; + *) exit 0 ;; +esac # Skip silently if the repo is not FFCA-shaped (no features/ folder above the # edited file). Only walks absolute paths; relative paths fall through to the diff --git a/hooks/validate_layers_test.sh b/hooks/validate_layers_test.sh index 1dd1b27..fbda1fa 100755 --- a/hooks/validate_layers_test.sh +++ b/hooks/validate_layers_test.sh @@ -45,7 +45,12 @@ VIOLATING="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/pubspec.yaml" NOT_FFCA="$FIXTURES/not_ffca/packages/some_pkg/pubspec.yaml" WARNING="$FIXTURES/warning_workspace/features/nu/nu_presentation/pubspec.yaml" SUPPRESSED="$FIXTURES/warning_workspace/features/xi/xi_data/pubspec.yaml" -NOT_PUBSPEC="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart" +NOT_PUBSPEC="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/README.md" +# alpha_domain's pubspec is violating, its barrel is not: a Dart edit only reports +# findings located in the edited file. +CLEAN_DART="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart" +VIOLATING_DART="$FIXTURES/invalid_workspace/features/sigma/sigma_presentation/lib/sigma_presentation.dart" +SUPPRESSED_DART="$FIXTURES/warning_workspace/features/omega/omega_presentation/lib/omega_presentation.dart" payload() { printf '{"tool_name":"%s","tool_input":{"file_path":"%s"}}' "$1" "$2"; } @@ -106,7 +111,7 @@ assert_outcome pass "null tool_name" "{\"tool_name\":null, echo "" echo "--- Files the hook does not validate (pass) ---" -assert_outcome pass "non-pubspec file" "$(payload Edit "$NOT_PUBSPEC")" +assert_outcome pass "neither pubspec nor Dart file" "$(payload Edit "$NOT_PUBSPEC")" assert_outcome pass "missing file_path" '{"tool_name":"Write","tool_input":{}}' assert_outcome pass "pubspec in a non-FFCA repo" "$(payload Edit "$NOT_FFCA")" @@ -120,6 +125,12 @@ assert_outcome block "MultiEdit on a violating pubspec" "$(payload MultiEdit assert_outcome warn "Edit on a pubspec with a warning" "$(payload Edit "$WARNING")" assert_outcome pass "Edit on a suppressed finding" "$(payload Edit "$SUPPRESSED")" +echo "" +echo "--- FFCA Dart edits ---" +assert_outcome block "Edit on a violating import" "$(payload Edit "$VIOLATING_DART")" +assert_outcome pass "Edit on a clean file in a violating package" "$(payload Edit "$CLEAN_DART")" +assert_outcome pass "Edit on an import suppressed in the file" "$(payload Edit "$SUPPRESSED_DART")" + echo "" echo "--- Missing prerequisites (skip) ---" assert_outcome skip "jq not installed" "$(payload Edit "$VIOLATING")" "$STUB_DIR/core" diff --git a/scripts/analysis_options.yaml b/scripts/analysis_options.yaml new file mode 100644 index 0000000..edfb184 --- /dev/null +++ b/scripts/analysis_options.yaml @@ -0,0 +1,4 @@ +analyzer: + exclude: + # Fixture packages import workspace packages that are never resolved. + - test/fixtures/** diff --git a/scripts/test/fixtures/invalid_workspace/apps/mobile_app/lib/mobile_app.dart b/scripts/test/fixtures/invalid_workspace/apps/mobile_app/lib/mobile_app.dart new file mode 100644 index 0000000..6f192fd --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/apps/mobile_app/lib/mobile_app.dart @@ -0,0 +1 @@ +// Barrel of mobile_app. diff --git a/scripts/test/fixtures/invalid_workspace/features/alpha/alpha_data/lib/alpha_data.dart b/scripts/test/fixtures/invalid_workspace/features/alpha/alpha_data/lib/alpha_data.dart new file mode 100644 index 0000000..7391c87 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/alpha/alpha_data/lib/alpha_data.dart @@ -0,0 +1 @@ +// Barrel of alpha_data. diff --git a/scripts/test/fixtures/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart b/scripts/test/fixtures/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart new file mode 100644 index 0000000..55c67ce --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart @@ -0,0 +1 @@ +import 'package:alpha_data/alpha_data.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/beta/beta_data/lib/beta_data.dart b/scripts/test/fixtures/invalid_workspace/features/beta/beta_data/lib/beta_data.dart new file mode 100644 index 0000000..b0ebc66 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/beta/beta_data/lib/beta_data.dart @@ -0,0 +1,2 @@ +import 'package:beta_domain/beta_domain.dart'; +import 'package:beta_presentation/beta_presentation.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/beta/beta_domain/lib/beta_domain.dart b/scripts/test/fixtures/invalid_workspace/features/beta/beta_domain/lib/beta_domain.dart new file mode 100644 index 0000000..2fcde69 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/beta/beta_domain/lib/beta_domain.dart @@ -0,0 +1 @@ +// Barrel of beta_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/beta/beta_presentation/lib/beta_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/beta/beta_presentation/lib/beta_presentation.dart new file mode 100644 index 0000000..84cff8e --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/beta/beta_presentation/lib/beta_presentation.dart @@ -0,0 +1 @@ +import 'package:beta_domain/beta_domain.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/chi/chi_domain/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/chi/chi_domain/pubspec.yaml new file mode 100644 index 0000000..3ea3a2a --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/chi/chi_domain/pubspec.yaml @@ -0,0 +1,4 @@ +name: chi_domain +resolution: workspace +environment: + sdk: ^3.11.0 diff --git a/scripts/test/fixtures/invalid_workspace/features/delta/delta_data/lib/delta_data.dart b/scripts/test/fixtures/invalid_workspace/features/delta/delta_data/lib/delta_data.dart new file mode 100644 index 0000000..0437200 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/delta/delta_data/lib/delta_data.dart @@ -0,0 +1 @@ +import 'package:delta_domain/delta_domain.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/delta/delta_domain/lib/delta_domain.dart b/scripts/test/fixtures/invalid_workspace/features/delta/delta_domain/lib/delta_domain.dart new file mode 100644 index 0000000..c71da08 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/delta/delta_domain/lib/delta_domain.dart @@ -0,0 +1 @@ +// Barrel of delta_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/epsilon/epsilon_domain/lib/epsilon_domain.dart b/scripts/test/fixtures/invalid_workspace/features/epsilon/epsilon_domain/lib/epsilon_domain.dart new file mode 100644 index 0000000..da4dd18 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/epsilon/epsilon_domain/lib/epsilon_domain.dart @@ -0,0 +1 @@ +import 'package:zeta_domain/zeta_domain.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/eta/eta_data/lib/eta_data.dart b/scripts/test/fixtures/invalid_workspace/features/eta/eta_data/lib/eta_data.dart new file mode 100644 index 0000000..a75d5d8 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/eta/eta_data/lib/eta_data.dart @@ -0,0 +1,2 @@ +import 'package:eta_domain/eta_domain.dart'; +import 'package:mobile_app/mobile_app.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/eta/eta_domain/lib/eta_domain.dart b/scripts/test/fixtures/invalid_workspace/features/eta/eta_domain/lib/eta_domain.dart new file mode 100644 index 0000000..e556e4c --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/eta/eta_domain/lib/eta_domain.dart @@ -0,0 +1 @@ +// Barrel of eta_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/gamma/gamma_domain/lib/gamma_domain.dart b/scripts/test/fixtures/invalid_workspace/features/gamma/gamma_domain/lib/gamma_domain.dart new file mode 100644 index 0000000..da9b1ec --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/gamma/gamma_domain/lib/gamma_domain.dart @@ -0,0 +1 @@ +// Barrel of gamma_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/gamma/gamma_presentation/lib/gamma_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/gamma/gamma_presentation/lib/gamma_presentation.dart new file mode 100644 index 0000000..62127fc --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/gamma/gamma_presentation/lib/gamma_presentation.dart @@ -0,0 +1,2 @@ +import 'package:gamma_domain/gamma_domain.dart'; +import 'package:delta_data/delta_data.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/iota/iota_data/lib/iota_data.dart b/scripts/test/fixtures/invalid_workspace/features/iota/iota_data/lib/iota_data.dart new file mode 100644 index 0000000..1c6e938 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/iota/iota_data/lib/iota_data.dart @@ -0,0 +1 @@ +// Barrel of iota_data. diff --git a/scripts/test/fixtures/invalid_workspace/features/iota/iota_domain/lib/iota_domain.dart b/scripts/test/fixtures/invalid_workspace/features/iota/iota_domain/lib/iota_domain.dart new file mode 100644 index 0000000..778970c --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/iota/iota_domain/lib/iota_domain.dart @@ -0,0 +1 @@ +// Barrel of iota_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/kappa/kappa_data/lib/kappa_data.dart b/scripts/test/fixtures/invalid_workspace/features/kappa/kappa_data/lib/kappa_data.dart new file mode 100644 index 0000000..828e23b --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/kappa/kappa_data/lib/kappa_data.dart @@ -0,0 +1 @@ +// Barrel of kappa_data. diff --git a/scripts/test/fixtures/invalid_workspace/features/lambda/lambda_domain/lib/lambda_domain.dart b/scripts/test/fixtures/invalid_workspace/features/lambda/lambda_domain/lib/lambda_domain.dart new file mode 100644 index 0000000..d1d6046 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/lambda/lambda_domain/lib/lambda_domain.dart @@ -0,0 +1 @@ +import 'package:flutter/flutter.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/mu/mu_data/lib/mu_data.dart b/scripts/test/fixtures/invalid_workspace/features/mu/mu_data/lib/mu_data.dart new file mode 100644 index 0000000..2b4b587 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/mu/mu_data/lib/mu_data.dart @@ -0,0 +1 @@ +// Barrel of mu_data. diff --git a/scripts/test/fixtures/invalid_workspace/features/mu/mu_domain/lib/mu_domain.dart b/scripts/test/fixtures/invalid_workspace/features/mu/mu_domain/lib/mu_domain.dart new file mode 100644 index 0000000..b1f70dd --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/mu/mu_domain/lib/mu_domain.dart @@ -0,0 +1 @@ +// Barrel of mu_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/omicron/omicron_data/lib/omicron_data.dart b/scripts/test/fixtures/invalid_workspace/features/omicron/omicron_data/lib/omicron_data.dart new file mode 100644 index 0000000..f060f65 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/omicron/omicron_data/lib/omicron_data.dart @@ -0,0 +1 @@ +// Barrel of omicron_data. diff --git a/scripts/test/fixtures/invalid_workspace/features/omicron/omicron_domain/lib/omicron_domain.dart b/scripts/test/fixtures/invalid_workspace/features/omicron/omicron_domain/lib/omicron_domain.dart new file mode 100644 index 0000000..02b983e --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/omicron/omicron_domain/lib/omicron_domain.dart @@ -0,0 +1 @@ +// Barrel of omicron_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/models/phi.dart b/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/models/phi.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/models/phi.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/phi_domain.dart b/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/phi_domain.dart new file mode 100644 index 0000000..a7c741f --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/lib/phi_domain.dart @@ -0,0 +1 @@ +// Barrel of phi_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/pubspec.yaml new file mode 100644 index 0000000..3e7306d --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_domain/pubspec.yaml @@ -0,0 +1,4 @@ +name: phi_domain +resolution: workspace +environment: + sdk: ^3.11.0 diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart new file mode 100644 index 0000000..ce75e06 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart @@ -0,0 +1,2 @@ +// Reaches past the phi_domain barrel. +import 'package:phi_domain/models/phi.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/pubspec.yaml new file mode 100644 index 0000000..ce686e9 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/pubspec.yaml @@ -0,0 +1,6 @@ +name: phi_presentation +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + phi_domain: any diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/test/phi_helpers.dart b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/test/phi_helpers.dart new file mode 100644 index 0000000..ab048b8 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/test/phi_helpers.dart @@ -0,0 +1,2 @@ +// Tests may reach into a package; only lib/ and bin/ are checked. +import 'package:phi_domain/models/phi.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/pi/pi_domain/lib/pi_domain.dart b/scripts/test/fixtures/invalid_workspace/features/pi/pi_domain/lib/pi_domain.dart new file mode 100644 index 0000000..3de538b --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/pi/pi_domain/lib/pi_domain.dart @@ -0,0 +1 @@ +// Barrel of pi_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/lib/sigma_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/lib/sigma_presentation.dart new file mode 100644 index 0000000..b37e481 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/lib/sigma_presentation.dart @@ -0,0 +1,2 @@ +// Imports another feature's primary barrel instead of a dedicated one. +import 'package:upsilon_presentation/upsilon_presentation.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/pubspec.yaml new file mode 100644 index 0000000..108d158 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/sigma/sigma_presentation/pubspec.yaml @@ -0,0 +1,6 @@ +name: sigma_presentation +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + upsilon_presentation: any diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/dtos/tau_dto.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/dtos/tau_dto.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/dtos/tau_dto.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/mappers/tau_mapper.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/mappers/tau_mapper.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/mappers/tau_mapper.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/repositories/tau_repository.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/repositories/tau_repository.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/repositories/tau_repository.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart new file mode 100644 index 0000000..0d46029 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart @@ -0,0 +1,6 @@ +import 'package:tau_domain/tau_domain.dart'; + +// Exports its DTOs and mappers along with the repository. +export 'data_sources/tau_remote_data_source/dtos/tau_dto.dart'; +export 'mappers/tau_mapper.dart'; +export 'repositories/tau_repository.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/pubspec.yaml new file mode 100644 index 0000000..ef9b357 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/pubspec.yaml @@ -0,0 +1,6 @@ +name: tau_data +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + tau_domain: any diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/lib/tau_domain.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/lib/tau_domain.dart new file mode 100644 index 0000000..82f8ad4 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/lib/tau_domain.dart @@ -0,0 +1 @@ +// Barrel of tau_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/pubspec.yaml new file mode 100644 index 0000000..88fcffa --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_domain/pubspec.yaml @@ -0,0 +1,4 @@ +name: tau_domain +resolution: workspace +environment: + sdk: ^3.11.0 diff --git a/scripts/test/fixtures/invalid_workspace/features/theta/theta_data/lib/theta_data.dart b/scripts/test/fixtures/invalid_workspace/features/theta/theta_data/lib/theta_data.dart new file mode 100644 index 0000000..e77395e --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/theta/theta_data/lib/theta_data.dart @@ -0,0 +1,2 @@ +import 'package:theta_domain/theta_domain.dart'; +import 'package:delta_data/delta_data.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/theta/theta_domain/lib/theta_domain.dart b/scripts/test/fixtures/invalid_workspace/features/theta/theta_domain/lib/theta_domain.dart new file mode 100644 index 0000000..5e72892 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/theta/theta_domain/lib/theta_domain.dart @@ -0,0 +1 @@ +// Barrel of theta_domain. diff --git a/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart new file mode 100644 index 0000000..ff5d578 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart @@ -0,0 +1 @@ +// Barrel of upsilon_presentation. diff --git a/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/pubspec.yaml new file mode 100644 index 0000000..fc9b6b6 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/pubspec.yaml @@ -0,0 +1,4 @@ +name: upsilon_presentation +resolution: workspace +environment: + sdk: ^3.11.0 diff --git a/scripts/test/fixtures/invalid_workspace/features/zeta/zeta_domain/lib/zeta_domain.dart b/scripts/test/fixtures/invalid_workspace/features/zeta/zeta_domain/lib/zeta_domain.dart new file mode 100644 index 0000000..b4dd3f9 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/zeta/zeta_domain/lib/zeta_domain.dart @@ -0,0 +1 @@ +import 'package:epsilon_domain/epsilon_domain.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/pubspec.yaml index 41f6d77..4999f44 100644 --- a/scripts/test/fixtures/invalid_workspace/pubspec.yaml +++ b/scripts/test/fixtures/invalid_workspace/pubspec.yaml @@ -8,6 +8,7 @@ workspace: - features/beta/beta_data - features/beta/beta_domain - features/beta/beta_presentation + - features/chi/chi_domain - features/delta/delta_data - features/delta/delta_domain - features/epsilon/epsilon_domain @@ -24,8 +25,14 @@ workspace: - features/omicron/omicron_data - features/omicron/omicron_domain - features/orders/orders_service + - features/phi/phi_domain + - features/phi/phi_presentation - features/pi/pi_domain + - features/sigma/sigma_presentation + - features/tau/tau_data + - features/tau/tau_domain - features/theta/theta_data - features/theta/theta_domain + - features/upsilon/upsilon_presentation - features/zeta/zeta_domain - shared/bad_shared diff --git a/scripts/test/fixtures/invalid_workspace/shared/bad_shared/lib/bad_shared.dart b/scripts/test/fixtures/invalid_workspace/shared/bad_shared/lib/bad_shared.dart new file mode 100644 index 0000000..2181ccc --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/shared/bad_shared/lib/bad_shared.dart @@ -0,0 +1 @@ +import 'package:alpha_domain/alpha_domain.dart'; diff --git a/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart b/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart new file mode 100644 index 0000000..4744db2 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart @@ -0,0 +1,5 @@ +import 'package:product_presentation/product_presentation.dart'; +import 'package:cart_presentation/cart_presentation.dart'; +import 'package:ideas_presentation/ideas_presentation.dart'; +import 'package:ui_kit/ui_kit.dart'; +import 'package:api_client/api_client.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/auth/auth_data_firebase/lib/auth_data_firebase.dart b/scripts/test/fixtures/valid_workspace/features/auth/auth_data_firebase/lib/auth_data_firebase.dart new file mode 100644 index 0000000..34b8a35 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/auth/auth_data_firebase/lib/auth_data_firebase.dart @@ -0,0 +1 @@ +import 'package:auth_domain/auth_domain.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/auth/auth_domain/lib/auth_domain.dart b/scripts/test/fixtures/valid_workspace/features/auth/auth_domain/lib/auth_domain.dart new file mode 100644 index 0000000..40e7820 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/auth/auth_domain/lib/auth_domain.dart @@ -0,0 +1 @@ +// Barrel of auth_domain. diff --git a/scripts/test/fixtures/valid_workspace/features/cart/cart_data/lib/cart_data.dart b/scripts/test/fixtures/valid_workspace/features/cart/cart_data/lib/cart_data.dart new file mode 100644 index 0000000..bb1c498 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/cart/cart_data/lib/cart_data.dart @@ -0,0 +1 @@ +import 'package:cart_domain/cart_domain.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart b/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart new file mode 100644 index 0000000..bb4f683 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart @@ -0,0 +1 @@ +import 'package:product_domain/product_domain.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_badge.dart b/scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_badge.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_badge.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_presentation.dart b/scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_presentation.dart new file mode 100644 index 0000000..2d9d1b3 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/cart/cart_presentation/lib/cart_presentation.dart @@ -0,0 +1,3 @@ +import 'package:cart_domain/cart_domain.dart'; +import 'package:product_domain/product_domain.dart'; +import 'package:ui_kit/ui_kit.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/ideas/ideas_presentation/lib/ideas_presentation.dart b/scripts/test/fixtures/valid_workspace/features/ideas/ideas_presentation/lib/ideas_presentation.dart new file mode 100644 index 0000000..ef5720d --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/ideas/ideas_presentation/lib/ideas_presentation.dart @@ -0,0 +1,5 @@ +import 'package:cart_domain/cart_domain.dart'; +// Another feature's widget comes through its dedicated subfeature barrel. +import 'package:cart_presentation/cart_badge.dart'; +import 'package:product_domain/product_domain.dart'; +import 'package:ui_kit/ui_kit.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/product/product_data/lib/product_data.dart b/scripts/test/fixtures/valid_workspace/features/product/product_data/lib/product_data.dart new file mode 100644 index 0000000..08a45c7 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/product/product_data/lib/product_data.dart @@ -0,0 +1,4 @@ +import 'package:product_domain/product_domain.dart'; + +// The repository implementation is public, its data sources are not. +export 'repositories/products_repository.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/product/product_data/lib/repositories/products_repository.dart b/scripts/test/fixtures/valid_workspace/features/product/product_data/lib/repositories/products_repository.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/product/product_data/lib/repositories/products_repository.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/valid_workspace/features/product/product_domain/lib/product_domain.dart b/scripts/test/fixtures/valid_workspace/features/product/product_domain/lib/product_domain.dart new file mode 100644 index 0000000..bc3595d --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/product/product_domain/lib/product_domain.dart @@ -0,0 +1 @@ +// Barrel of product_domain. diff --git a/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart b/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart new file mode 100644 index 0000000..5a5d1ba --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart @@ -0,0 +1,2 @@ +import 'package:product_domain/product_domain.dart'; +import 'package:ui_kit/ui_kit.dart'; diff --git a/scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart b/scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart new file mode 100644 index 0000000..642c57b --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart @@ -0,0 +1 @@ +// Barrel of api_client. diff --git a/scripts/test/fixtures/valid_workspace/shared/ui_kit/lib/ui_kit.dart b/scripts/test/fixtures/valid_workspace/shared/ui_kit/lib/ui_kit.dart new file mode 100644 index 0000000..3677a22 --- /dev/null +++ b/scripts/test/fixtures/valid_workspace/shared/ui_kit/lib/ui_kit.dart @@ -0,0 +1 @@ +// Barrel of ui_kit. diff --git a/scripts/test/fixtures/warning_workspace/features/nu/nu_domain/lib/nu_domain.dart b/scripts/test/fixtures/warning_workspace/features/nu/nu_domain/lib/nu_domain.dart new file mode 100644 index 0000000..7a2cf1f --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/nu/nu_domain/lib/nu_domain.dart @@ -0,0 +1 @@ +// Barrel of nu_domain. diff --git a/scripts/test/fixtures/warning_workspace/features/nu/nu_presentation/lib/nu_presentation.dart b/scripts/test/fixtures/warning_workspace/features/nu/nu_presentation/lib/nu_presentation.dart new file mode 100644 index 0000000..e73b69a --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/nu/nu_presentation/lib/nu_presentation.dart @@ -0,0 +1 @@ +// Barrel of nu_presentation. diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_data/lib/omega_data.dart b/scripts/test/fixtures/warning_workspace/features/omega/omega_data/lib/omega_data.dart new file mode 100644 index 0000000..84b1a71 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_data/lib/omega_data.dart @@ -0,0 +1 @@ +// Declares omega_domain but never imports it. diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_data/pubspec.yaml b/scripts/test/fixtures/warning_workspace/features/omega/omega_data/pubspec.yaml new file mode 100644 index 0000000..78552f3 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_data/pubspec.yaml @@ -0,0 +1,6 @@ +name: omega_data +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + omega_domain: any diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/models/omega.dart b/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/models/omega.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/models/omega.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/omega_domain.dart b/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/omega_domain.dart new file mode 100644 index 0000000..b4cc1cb --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/lib/omega_domain.dart @@ -0,0 +1 @@ +// Barrel of omega_domain. diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/pubspec.yaml b/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/pubspec.yaml new file mode 100644 index 0000000..15f429f --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_domain/pubspec.yaml @@ -0,0 +1,4 @@ +name: omega_domain +resolution: workspace +environment: + sdk: ^3.11.0 diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/lib/omega_presentation.dart b/scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/lib/omega_presentation.dart new file mode 100644 index 0000000..6d41b75 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/lib/omega_presentation.dart @@ -0,0 +1,2 @@ +// ffca-ignore: deep-import — the model is not exported from omega_domain yet +import 'package:omega_domain/models/omega.dart'; diff --git a/scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/pubspec.yaml b/scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/pubspec.yaml new file mode 100644 index 0000000..58dde0c --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/omega/omega_presentation/pubspec.yaml @@ -0,0 +1,6 @@ +name: omega_presentation +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + omega_domain: any diff --git a/scripts/test/fixtures/warning_workspace/features/rho/rho_domain/lib/rho_domain.dart b/scripts/test/fixtures/warning_workspace/features/rho/rho_domain/lib/rho_domain.dart new file mode 100644 index 0000000..55a2c20 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/rho/rho_domain/lib/rho_domain.dart @@ -0,0 +1 @@ +// Barrel of rho_domain. diff --git a/scripts/test/fixtures/warning_workspace/features/xi/xi_data/lib/xi_data.dart b/scripts/test/fixtures/warning_workspace/features/xi/xi_data/lib/xi_data.dart new file mode 100644 index 0000000..98354ef --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/xi/xi_data/lib/xi_data.dart @@ -0,0 +1 @@ +// Barrel of xi_data. diff --git a/scripts/test/fixtures/warning_workspace/features/xi/xi_domain/lib/xi_domain.dart b/scripts/test/fixtures/warning_workspace/features/xi/xi_domain/lib/xi_domain.dart new file mode 100644 index 0000000..d44f7fc --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/xi/xi_domain/lib/xi_domain.dart @@ -0,0 +1 @@ +// Barrel of xi_domain. diff --git a/scripts/test/fixtures/warning_workspace/pubspec.yaml b/scripts/test/fixtures/warning_workspace/pubspec.yaml index b210053..7ab7427 100644 --- a/scripts/test/fixtures/warning_workspace/pubspec.yaml +++ b/scripts/test/fixtures/warning_workspace/pubspec.yaml @@ -5,6 +5,9 @@ environment: workspace: - features/nu/nu_domain - features/nu/nu_presentation + - features/omega/omega_data + - features/omega/omega_domain + - features/omega/omega_presentation - features/rho/rho_domain - features/xi/xi_data - features/xi/xi_domain diff --git a/scripts/test/validate_layers_test.dart b/scripts/test/validate_layers_test.dart index 63dd0bc..7eb0add 100644 --- a/scripts/test/validate_layers_test.dart +++ b/scripts/test/validate_layers_test.dart @@ -187,9 +187,10 @@ void main() { }); test('every violation includes a fix line', () { - // One arrow per violation; there are at least the thirteen planted rules. + // One arrow per violation; there are at least the seventeen planted + // rules. final arrows = '→'.allMatches(err).length; - expect(arrows, greaterThanOrEqualTo(13)); + expect(arrows, greaterThanOrEqualTo(17)); }); test('every finding carries a rule ID', () { @@ -204,6 +205,53 @@ void main() { expect(err, contains(RegExp(r'FFCA: \d+ errors, 0 warnings\.'))); }); + test('feature package without a primary barrel', () { + expect( + err, + contains('[missing-primary-barrel] chi_domain has no primary barrel ' + 'lib/chi_domain.dart'), + ); + }); + + test('deep import past a barrel, reported in the Dart file', () { + expect( + err, + contains('FFCA findings in features/phi/phi_presentation/lib/' + 'phi_presentation.dart:'), + ); + expect( + err, + contains('[deep-import] line 2 imports ' + 'package:phi_domain/models/phi.dart'), + ); + }); + + test("another feature's primary presentation barrel", () { + expect( + err, + contains('[cross-feature-primary-barrel] line 2 imports the primary ' + 'barrel of upsilon_presentation'), + ); + }); + + test('data barrel exporting its DTOs and mappers', () { + expect( + err, + contains('[data-barrel-exports-internals] line 4 exports ' + 'data_sources/tau_remote_data_source/dtos/tau_dto.dart'), + ); + expect( + err, + contains('[data-barrel-exports-internals] line 5 exports ' + 'mappers/tau_mapper.dart'), + ); + expect(err, isNot(contains('repositories/tau_repository.dart'))); + }); + + test('ignores Dart files outside lib/ and bin/', () { + expect(err, isNot(contains('phi_presentation/test/'))); + }); + test('a suppression without a reason suppresses nothing', () { expect(err, contains('[suppression-missing-reason]')); expect(err, contains('omicron_data does not depend on omicron_domain')); @@ -308,6 +356,15 @@ void main() { expect(r.err, contains('features/kappa/ has no kappa_domain')); }); + test('a pubspec edit reports Dart-file findings in the package', () { + final r = _run([ + '--file', + _pubspec(_invalid, 'features/phi/phi_presentation'), + ]); + expect(r.code, 2); + expect(r.err, contains('[deep-import]')); + }); + test('flags a domain depending on the Flutter SDK', () { final r = _run([ '--file', @@ -339,7 +396,7 @@ void main() { test('--all exits 0 when every finding is a warning or suppressed', () { final r = _run(['--all', _warning]); expect(r.code, 0, reason: r.err); - expect(r.err, contains('FFCA: 0 errors, 2 warnings, 1 suppressed.')); + expect(r.err, contains('FFCA: 0 errors, 3 warnings, 2 suppressed.')); }); test('reports a warning with its rule ID', () { @@ -356,6 +413,20 @@ void main() { expect(r.err, isNot(contains('xi_data'))); }); + test('a Dart-file suppression covers a finding in that file', () { + final r = _run(['--all', _warning]); + expect(r.err, isNot(contains('omega_presentation'))); + }); + + test('warns about a dependency that is never imported', () { + final r = _run(['--all', _warning]); + expect( + r.err, + contains('⚠ [unused-dependency] omega_data declares omega_domain but ' + 'never imports it'), + ); + }); + test('reports an unused suppression in --all mode', () { final r = _run(['--all', _warning]); expect( @@ -390,6 +461,74 @@ void main() { }); }); + group('Dart file mode', () { + String dart(String workspace, String pkg, String file) => + '$workspace/$pkg/$file'; + + test('blocks on a finding in the edited file', () { + final r = _run([ + '--file', + dart( + _invalid, + 'features/sigma/sigma_presentation', + 'lib/sigma_presentation.dart', + ), + ]); + expect(r.code, 2); + expect(r.err, contains('[cross-feature-primary-barrel]')); + }); + + test('ignores findings elsewhere in the package', () { + // alpha_domain's pubspec has a domain -> data violation, but its barrel + // imports nothing wrong. + final r = _run([ + '--file', + dart(_invalid, 'features/alpha/alpha_domain', 'lib/alpha_domain.dart'), + ]); + expect(r.code, 0); + expect(r.err, isEmpty); + }); + + test('is silent on a finding suppressed in the file', () { + final r = _run([ + '--file', + dart( + _warning, + 'features/omega/omega_presentation', + 'lib/omega_presentation.dart', + ), + ]); + expect(r.code, 0); + expect(r.err, isEmpty); + }); + + test('passes a cross-feature import of a dedicated barrel', () { + final r = _run([ + '--file', + dart( + _valid, + 'features/ideas/ideas_presentation', + 'lib/ideas_presentation.dart', + ), + ]); + expect(r.code, 0); + expect(r.err, isEmpty); + }); + + test('skips a Dart file outside lib/ and bin/', () { + final r = _run([ + '--file', + dart( + _invalid, + 'features/phi/phi_presentation', + 'test/phi_helpers.dart', + ), + ]); + expect(r.code, 0); + expect(r.err, isEmpty); + }); + }); + group('graceful skips (exit 0)', () { test('non-pubspec file', () { final r = _run(['--file', '$_valid/README.md']); diff --git a/scripts/validate_layers.dart b/scripts/validate_layers.dart index 0c063b2..0eb4e33 100644 --- a/scripts/validate_layers.dart +++ b/scripts/validate_layers.dart @@ -2,8 +2,9 @@ // // Enforces the Feature-First Clean Architecture dependency rules on a Dart // monorepo's pubspec dependencies between its own packages. Used two ways: -// - by the plugin hook, incrementally, on every pubspec.yaml edit: +// - by the plugin hook, incrementally, on every pubspec.yaml or Dart edit: // dart run validate_layers.dart --file +// dart run validate_layers.dart --file // - by CI and the ffca-audit skill, across the whole workspace: // dart run validate_layers.dart --all // @@ -54,8 +55,10 @@ const Map> rules = { // but leaves the exit code at 0. // // A finding is suppressed with a comment in the pubspec of the package it is -// reported on. The reason is required: -// # ffca-ignore: — +// reported on, or, for a finding in a Dart file, in that file. The reason is +// required: +// # ffca-ignore: — (pubspec.yaml) +// // ffca-ignore: — (Dart file) // The suppression-* rules and unused-suppression cannot be suppressed. // --------------------------------------------------------------------------- enum Severity { error, warning } @@ -75,6 +78,11 @@ const Map ruleSeverities = { 'data-without-domain': Severity.error, 'domain-uses-flutter': Severity.error, 'presentation-missing-own-domain': Severity.warning, + 'missing-primary-barrel': Severity.error, + 'deep-import': Severity.error, + 'cross-feature-primary-barrel': Severity.error, + 'data-barrel-exports-internals': Severity.error, + 'unused-dependency': Severity.warning, 'suppression-missing-reason': Severity.error, 'suppression-unknown-rule': Severity.error, 'unused-suppression': Severity.warning, @@ -97,7 +105,8 @@ void main(List args) { } stderr.writeln( - 'Usage: validate_layers.dart --file | --all [rootDir]', + 'Usage: validate_layers.dart --file | ' + '--all [rootDir]', ); exit(64); } @@ -107,9 +116,11 @@ void main(List args) { // --------------------------------------------------------------------------- /// Incremental mode (the hook default): validate the edited package plus its -/// direct dependents. Graceful-skips (exit 0) when the target is not a -/// pubspec.yaml, does not exist, or is not inside an FFCA-shaped workspace. +/// direct dependents, or, for a Dart file, only that file's imports and +/// exports. Graceful-skips (exit 0) when the target is neither a pubspec.yaml +/// nor a Dart file, does not exist, or is not inside an FFCA-shaped workspace. int _runIncremental(String rawPath) { + if (rawPath.endsWith('.dart')) return _runDartFile(rawPath); if (_basename(rawPath) != 'pubspec.yaml') return 0; final file = File(rawPath); @@ -134,12 +145,54 @@ int _runIncremental(String rawPath) { } violations.addAll(_cycleViolations(workspace, from: edited)); - final result = _applySuppressions(violations, scope, reportUnused: false); + final result = _applySuppressions( + violations, + _declaredSuppressions(scope), + reportUnused: false, + ); if (result.findings.isEmpty) return 0; // silent pass _report(result, root); return _exitCode(result.findings); } +/// Dart-file mode: report only the findings located in the edited file, so a +/// Dart edit never blocks on a violation elsewhere in the package. Package-wide +/// findings, such as a missing barrel or an unused dependency, are reported on +/// pubspec edits and in full mode. +int _runDartFile(String rawPath) { + final file = File(rawPath); + if (!file.existsSync()) return 0; + + final root = _findWorkspaceRoot(file.parent.absolute.path); + if (root == null) return 0; // not FFCA-shaped + + final workspace = _discover(root); + final path = _normalize(file.absolute.path); + final dartFile = workspace.packages + .expand((p) => p.dartFiles) + .where((f) => f.path == path) + .firstOrNull; + if (dartFile == null) return 0; // outside lib/ and bin/ of a known package + + final package = workspace.packages.firstWhere( + (p) => p.dartFiles.contains(dartFile), + ); + final violations = _checkImports( + package, + workspace, + ).where((v) => v.file == dartFile).toList(); + + final result = _applySuppressions( + violations, + [ + for (final s in dartFile.suppressions) (package, s), + ], + reportUnused: false); + if (result.findings.isEmpty) return 0; + _report(result, root); + return _exitCode(result.findings); +} + /// Full-graph mode (CI and ffca-audit): validate every package and run a /// workspace-wide cycle check. int _runAll(List args) { @@ -164,7 +217,7 @@ int _runAll(List args) { final result = _applySuppressions( violations, - workspace.packages, + _declaredSuppressions(workspace.packages), reportUnused: true, ); if (result.findings.isEmpty) { @@ -187,12 +240,25 @@ int _exitCode(List<_Violation> findings) => // Suppressions // --------------------------------------------------------------------------- -/// Drops the findings suppressed in their package's pubspec and adds findings -/// for invalid suppressions in [scope]. Unused suppressions are only reported -/// with [reportUnused], in full mode, where every finding has been computed. +/// Every suppression declared in [packages], in their pubspecs and Dart files. +List<(Package, _Suppression)> _declaredSuppressions( + Iterable packages, +) => + [ + for (final p in packages) ...[ + for (final s in p.suppressions) (p, s), + for (final f in p.dartFiles) + for (final s in f.suppressions) (p, s), + ], + ]; + +/// Drops the findings suppressed in their package's pubspec, or in the Dart +/// file they are located in, and adds findings for the invalid suppressions in +/// [declared]. Unused suppressions are only reported with [reportUnused], in +/// full mode, where every finding has been computed. ({List<_Violation> findings, int suppressed}) _applySuppressions( List<_Violation> violations, - Iterable scope, { + List<(Package, _Suppression)> declared, { required bool reportUnused, }) { bool valid(_Suppression s) => @@ -204,7 +270,7 @@ int _exitCode(List<_Violation> findings) => final findings = <_Violation>[]; var suppressed = 0; for (final v in violations) { - final match = v.package.suppressions + final match = [...?v.file?.suppressions, ...v.package.suppressions] .where((s) => valid(s) && s.ruleId == v.id) .firstOrNull; if (match == null) { @@ -215,42 +281,41 @@ int _exitCode(List<_Violation> findings) => } } - for (final p in scope) { - for (final s in p.suppressions) { - final where = 'line ${s.line} of the ${p.name} pubspec'; - if (!ruleSeverities.containsKey(s.ruleId) || - _unsuppressible.contains(s.ruleId)) { - findings.add( - _Violation( - id: 'suppression-unknown-rule', - package: p, - rule: 'ffca-ignore on $where names "${s.ruleId}", which is not a ' - 'rule that can be suppressed', - fix: 'Use one of the rule IDs shown in brackets in the validator ' - 'output, or remove the comment.', - ), - ); - } else if (s.reason.isEmpty) { - findings.add( - _Violation( - id: 'suppression-missing-reason', - package: p, - rule: 'ffca-ignore: ${s.ruleId} on $where has no reason, so it ' - 'suppresses nothing', - fix: 'Add the reason after the rule ID: ' - '# ffca-ignore: ${s.ruleId} — ', - ), - ); - } else if (reportUnused && !used.contains(s)) { - findings.add( - _Violation( - id: 'unused-suppression', - package: p, - rule: 'ffca-ignore: ${s.ruleId} on $where matches no finding', - fix: 'Remove the comment; the exception is no longer needed.', - ), - ); - } + for (final (p, s) in declared) { + final where = s.where; + if (!ruleSeverities.containsKey(s.ruleId) || + _unsuppressible.contains(s.ruleId)) { + findings.add( + _Violation( + id: 'suppression-unknown-rule', + package: p, + rule: 'ffca-ignore on $where names "${s.ruleId}", which is not a ' + 'rule that can be suppressed', + fix: 'Use one of the rule IDs shown in brackets in the validator ' + 'output, or remove the comment.', + ), + ); + } else if (s.reason.isEmpty) { + findings.add( + _Violation( + id: 'suppression-missing-reason', + package: p, + rule: 'ffca-ignore: ${s.ruleId} on $where has no reason, so it ' + 'suppresses nothing', + fix: 'Add the reason after the rule ID: ' + '${s.marker} ffca-ignore: ${s.ruleId} — ', + ), + ); + } else if (reportUnused && !used.contains(s)) { + findings.add( + _Violation( + id: 'unused-suppression', + package: p, + rule: 'ffca-ignore: ${s.ruleId} on $where matches no finding', + fix: 'Remove the comment; the exception is no longer needed.', + ), + ); } } return (findings: findings, suppressed: suppressed); @@ -280,6 +345,7 @@ List<_Violation> _checkPackage(Package p, Workspace ws) { } out.addAll(_checkOwnership(p, ws)); + out.addAll(_checkImports(p, ws)); final allowed = rules[p.layer]; if (allowed == null) return out; // unknown source layer (e.g. app-less): skip @@ -372,6 +438,110 @@ List<_Violation> _checkOwnership(Package p, Workspace ws) { return out; } +/// Checks the package's barrels and its imports and exports of other +/// workspace packages. Implements "Layer subfolders" and the barrel rule in +/// references/ffca/project_structure.md, the dedicated-barrel condition in its +/// "Dependency rules", and "Widgets that own state" in +/// references/ffca/presentation.md. +List<_Violation> _checkImports(Package p, Workspace ws) { + final out = <_Violation>[]; + final barrel = 'lib/${p.name}.dart'; + + if (p.top == 'features' && !p.dartFiles.any((f) => f.libRel == barrel)) { + out.add( + _Violation( + id: 'missing-primary-barrel', + package: p, + rule: '${p.name} has no primary barrel $barrel', + fix: 'Add $barrel exporting the public API of ${p.name}.', + ), + ); + } + + final used = {}; + for (final f in p.dartFiles) { + for (final d in f.directives) { + final match = _packageUri.firstMatch(d.uri); + final name = match?.group(1); + final rest = match?.group(2) ?? d.uri; + + // A data barrel exporting its own data sources, DTOs, or mappers. + if ((match == null || name == p.name) && + p.layer == 'data' && + f.libRel == barrel && + d.kind == 'export' && + _dataInternals.hasMatch(rest)) { + out.add( + _Violation( + id: 'data-barrel-exports-internals', + package: p, + file: f, + rule: 'line ${d.line} exports ${d.uri} (a data barrel never ' + 'exposes its data sources, DTOs, or mappers)', + fix: 'Remove the export; the rest of the app only sees the ' + 'repository and the domain models it returns.', + ), + ); + } + if (match == null || name == p.name) continue; // within the package + + final target = ws.byName[name]; + if (target == null) continue; // external package + used.add(target.name); + + if (rest.contains('/')) { + out.add( + _Violation( + id: 'deep-import', + package: p, + file: f, + rule: 'line ${d.line} ${d.kind}s ${d.uri}, which reaches past the ' + 'barrels of ${target.name}', + fix: 'Import a barrel at the root of ${target.name}/lib/ instead, ' + 'and export what is missing from it.', + ), + ); + } else if (p.top == 'features' && + target.layer == 'presentation' && + target.feature != p.feature && + rest == '${target.name}.dart') { + out.add( + _Violation( + id: 'cross-feature-primary-barrel', + package: p, + file: f, + rule: 'line ${d.line} ${d.kind}s the primary barrel of ' + '${target.name} (another feature\'s presentation is imported ' + 'through a dedicated barrel)', + fix: + 'Add a subfeature barrel to ${target.name}/lib/ exporting only ' + 'the widget ${p.name} needs, and import that instead.', + ), + ); + } + } + } + + for (final depDir in p.runtimeDeps) { + final target = ws.byDir[depDir]; + if (target == null || used.contains(target.name)) continue; + out.add( + _Violation( + id: 'unused-dependency', + package: p, + rule: '${p.name} declares ${target.name} but never imports it in lib/ ' + 'or bin/', + fix: 'Remove ${target.name} from the ${p.name} pubspec, or import it ' + 'where it is needed.', + ), + ); + } + return out; +} + +final _packageUri = RegExp(r'^package:([^/]+)/(.+)$'); +final _dataInternals = RegExp(r'(^|/)(data_sources|dtos|mappers)/'); + _Violation _dependencyViolation(Package src, Package tgt) { final pair = '${src.name} depends on ${tgt.name}'; @@ -536,6 +706,7 @@ Workspace _discover(String root) { final parsed = _parsePubspec(entity); final package = _buildPackage(pkgDir, entity, parsed.name, top, root); package.suppressions.addAll(parsed.suppressions); + package.dartFiles.addAll(_readDartFiles(pkgDir, root)); package.flutterSdkDeps.addAll([ for (final dep in parsed.deps) if (dep.runtime && dep.sdk == 'flutter') dep.name, @@ -565,9 +736,45 @@ Workspace _discover(String root) { } final byDir = {for (final p in packages) p.dir: p}; - return Workspace(packages: packages, byDir: byDir); + return Workspace(packages: packages, byDir: byDir, byName: byName); } +/// Reads the imports, exports, and suppressions of every Dart file under the +/// package's lib/ and bin/. +List<_DartFile> _readDartFiles(String pkgDir, String root) { + final files = <_DartFile>[]; + for (final sub in ['lib', 'bin']) { + final dir = Directory('$pkgDir/$sub'); + if (!dir.existsSync()) continue; + final entities = dir.listSync(recursive: true, followLinks: false) + ..sort((a, b) => a.path.compareTo(b.path)); + for (final entity in entities) { + if (entity is! File || !entity.path.endsWith('.dart')) continue; + final path = _normalize(entity.absolute.path); + final relPath = path.substring(root.length + 1); + final lines = entity.readAsLinesSync(); + files.add( + _DartFile( + path: path, + relPath: relPath, + libRel: path.substring(pkgDir.length + 1), + directives: [ + for (var i = 0; i < lines.length; i++) + if (_directivePattern.firstMatch(lines[i]) case final m?) + _Directive(kind: m.group(1)!, uri: m.group(2)!, line: i + 1), + ], + suppressions: _parseSuppressions(lines, where: relPath), + ), + ); + } + } + return files; +} + +/// An `import` or `export` directive's URI. Directives always start a line in +/// formatted Dart, and the URI is the first string after the keyword. +final _directivePattern = RegExp(r'''^\s*(import|export)\s+['"]([^'"]+)['"]'''); + Package _buildPackage( String pkgDir, File pubspec, @@ -603,6 +810,7 @@ Package _buildPackage( runtimeDeps: [], flutterSdkDeps: [], suppressions: [], + dartFiles: [], ); } @@ -624,18 +832,10 @@ _ParsedPubspec _parsePubspec(File file) { final lines = file.readAsLinesSync(); // Suppressions are comments, so read them before comments are stripped. - final suppressions = <_Suppression>[]; - for (var i = 0; i < lines.length; i++) { - final match = _suppressionPattern.firstMatch(lines[i]); - if (match == null) continue; - suppressions.add( - _Suppression( - ruleId: match.group(1)!, - reason: match.group(2)!.replaceFirst(_reasonSeparator, '').trim(), - line: i + 1, - ), - ); - } + final suppressions = _parseSuppressions( + lines, + where: 'the ${_basename(_normalize(file.parent.absolute.path))} pubspec', + ); String? currentTop; for (var i = 0; i < lines.length; i++) { @@ -693,9 +893,29 @@ _ParsedPubspec _parsePubspec(File file) { return _ParsedPubspec(name: name, deps: deps, suppressions: suppressions); } -/// `# ffca-ignore: `, where the separator is an -/// em dash, an en dash, a hyphen, or a colon. -final _suppressionPattern = RegExp(r'#\s*ffca-ignore:\s*([A-Za-z0-9_-]+)(.*)$'); +/// Reads `ffca-ignore` comments from a pubspec or a Dart file. [where] names +/// the file in messages, for example `the cart_data pubspec`. +List<_Suppression> _parseSuppressions( + List lines, { + required String where, +}) => + [ + for (var i = 0; i < lines.length; i++) + if (_suppressionPattern.firstMatch(lines[i]) case final m?) + _Suppression( + ruleId: m.group(2)!, + reason: m.group(3)!.replaceFirst(_reasonSeparator, '').trim(), + marker: m.group(1)!, + where: 'line ${i + 1} of $where', + ), + ]; + +/// `# ffca-ignore: ` in a pubspec, or the same +/// after `//` in a Dart file, where the separator is an em dash, an en dash, a +/// hyphen, or a colon. +final _suppressionPattern = RegExp( + r'(#|//)\s*ffca-ignore:\s*([A-Za-z0-9_-]+)(.*)$', +); final _reasonSeparator = RegExp(r'^\s*[—–:-]+'); String _stripComment(String line) { @@ -727,10 +947,13 @@ void _report( ({List<_Violation> findings, int suppressed}) result, String root, ) { - // Group by the pubspec file the finding belongs to. + // Group by the file the finding is located in: a Dart file, or else the + // pubspec of the package it belongs to. final byPackage = >{}; for (final v in result.findings) { - byPackage.putIfAbsent(v.package.relPath, () => []).add(v); + byPackage + .putIfAbsent(v.file?.relPath ?? v.package.relPath, () => []) + .add(v); } final paths = byPackage.keys.toList()..sort(); for (final path in paths) { @@ -790,9 +1013,14 @@ String _normalize(String path) { // --------------------------------------------------------------------------- class Workspace { - Workspace({required this.packages, required this.byDir}); + Workspace({ + required this.packages, + required this.byDir, + required this.byName, + }); final List packages; final Map byDir; + final Map byName; } class Package { @@ -807,6 +1035,7 @@ class Package { required this.runtimeDeps, required this.flutterSdkDeps, required this.suppressions, + required this.dartFiles, }); final String dir; // normalized absolute package directory @@ -819,6 +1048,29 @@ class Package { final List runtimeDeps; // the subset declared under `dependencies` final List flutterSdkDeps; // runtime deps with `sdk: flutter` final List<_Suppression> suppressions; // `# ffca-ignore:` comments + final List<_DartFile> dartFiles; // Dart files under lib/ and bin/ +} + +class _DartFile { + _DartFile({ + required this.path, + required this.relPath, + required this.libRel, + required this.directives, + required this.suppressions, + }); + final String path; // normalized absolute path + final String relPath; // path relative to the workspace root + final String libRel; // path relative to the package, for example lib/a.dart + final List<_Directive> directives; + final List<_Suppression> suppressions; // `// ffca-ignore:` comments +} + +class _Directive { + _Directive({required this.kind, required this.uri, required this.line}); + final String kind; // import | export + final String uri; + final int line; // 1-based } class _ParsedPubspec { @@ -851,9 +1103,11 @@ class _Violation { required this.package, required this.rule, required this.fix, + this.file, }); final String id; // a key of ruleSeverities final Package package; + final _DartFile? file; // the Dart file the finding is located in, if any final String rule; final String fix; @@ -861,9 +1115,14 @@ class _Violation { } class _Suppression { - _Suppression( - {required this.ruleId, required this.reason, required this.line}); + _Suppression({ + required this.ruleId, + required this.reason, + required this.marker, + required this.where, + }); final String ruleId; final String reason; // empty when none was given - final int line; // 1-based line in the pubspec + final String marker; // `#` in a pubspec, `//` in a Dart file + final String where; // for example `line 2 of the cart_data pubspec` } diff --git a/skills/ffca-audit/SKILL.md b/skills/ffca-audit/SKILL.md index fd28bf4..8dc58e5 100644 --- a/skills/ffca-audit/SKILL.md +++ b/skills/ffca-audit/SKILL.md @@ -24,7 +24,7 @@ The audit itself is performed by the **`ffca-layer-auditor` agent**, so the work ## What to do 1. Confirm the repo is FFCA-shaped (a `features/` folder with `{feature}_{layer}` packages). If not, say so and stop. -2. Launch the `ffca-layer-auditor` agent (subagent type `ffca-layer-auditor`) against the repo. The agent runs `dart run scripts/validate_layers.dart --all` for the mechanical pass (naming, layer rules, cycles, feature ownership), then adds the source-level checks (declared-but-unused dependencies, barrel hygiene, DTO leakage, Command/Query necessity, module entry, misplaced packages, high fan-in) and reads `${CLAUDE_PLUGIN_ROOT}/references/ffca/` for the detail behind each. +2. Launch the `ffca-layer-auditor` agent (subagent type `ffca-layer-auditor`) against the repo. The agent runs `dart run scripts/validate_layers.dart --all` for the mechanical pass (naming, layer rules, cycles, feature ownership, imports and barrels), then adds the source-level checks (barrel scope, DTO leakage, Command/Query necessity, module entry, misplaced packages, high fan-in) and reads `${CLAUDE_PLUGIN_ROOT}/references/ffca/` for the detail behind each. 3. Relay the agent's output: the per-package verdict table and the severity-ordered findings list. Each finding states the rule and the fix. The agent is read-only and does not auto-fix. If the agent is unavailable for any reason, fall back to running `dart run ${CLAUDE_PLUGIN_ROOT}/scripts/validate_layers.dart --all` yourself and reporting its violations, then note that the qualitative source-level checks were skipped. From a52ebaca21801adc85140046fff208d5cac22854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Baudet?= Date: Thu, 15 Oct 2026 07:07:44 -0600 Subject: [PATCH 2/3] fix(validator): allow data barrels to export data sources data-barrel-exports-internals flagged every data-source export, which reported five false positives on mealify_feature_first: the app is the composition root and constructs the data sources to inject them into each repository, so the data barrels export them on purpose. The rule now covers DTOs and mappers only. The data page's "Neither is visible outside the package" contradicts mealify and is tracked in VGVentures/very_good_engineering#294. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq --- README.md | 4 ++-- agents/ffca-layer-auditor.md | 2 +- .../tau_remote_data_source.dart | 1 + .../features/tau/tau_data/lib/tau_data.dart | 4 +++- scripts/test/validate_layers_test.dart | 5 +++++ scripts/validate_layers.dart | 11 ++++++----- 6 files changed, 18 insertions(+), 9 deletions(-) create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/tau_remote_data_source.dart diff --git a/README.md b/README.md index 8d9fc43..ac8df45 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ A PostToolUse hook runs on every `Edit`, `MultiEdit`, or `Write` and ignores any | **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`MultiEdit`/`Write`) | Runs the FFCA validator incrementally: on the edited package and its direct dependents for a pubspec edit, and on the edited file alone for a Dart edit. Exits 2 on an error (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. Passes warnings to Claude as non-blocking context. Passes silently otherwise | | **Check VGV CLI** (`check_vgv_cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Auto-approves Very Good CLI MCP tool calls when the CLI is installed at 1.3.0 or newer, so they work in every run mode. Denies with an install or upgrade message when the CLI is missing or outdated. Stands aside for any other tool, or when the CLI version cannot be read | -The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK. In `lib/` and `bin/`, it checks that each feature package has a primary barrel `lib/{package}.dart`, that imports of other workspace packages go through a barrel at the root of their `lib/`, that another feature's presentation is imported through a dedicated barrel rather than its primary one, that a data barrel never exports data sources, DTOs, or mappers, and that every declared workspace dependency is imported. +The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK. In `lib/` and `bin/`, it checks that each feature package has a primary barrel `lib/{package}.dart`, that imports of other workspace packages go through a barrel at the root of their `lib/`, that another feature's presentation is imported through a dedicated barrel rather than its primary one, that a data barrel never exports DTOs or mappers, and that every declared workspace dependency is imported. The validator is also runnable directly for CI and audits, across the whole workspace: @@ -152,7 +152,7 @@ Every finding carries a rule ID in brackets. An **error** exits 2 and blocks the | `missing-primary-barrel` | error | A feature package has no `lib/{package}.dart` | | `deep-import` | error | A Dart file imports or exports another workspace package past the barrels at the root of its `lib/` | | `cross-feature-primary-barrel` | error | A feature imports another feature's primary presentation barrel instead of a dedicated one | -| `data-barrel-exports-internals` | error | A data package's primary barrel exports from `data_sources/`, `dtos/`, or `mappers/` | +| `data-barrel-exports-internals` | error | A data package's primary barrel exports from `dtos/` or `mappers/`. Data sources may be exported, since the app constructs them | | `unused-dependency` | warning | A package declares a workspace dependency under `dependencies` but never imports it in `lib/` or `bin/` | | `suppression-missing-reason` | error | An `ffca-ignore` comment has no reason | | `suppression-unknown-rule` | error | An `ffca-ignore` comment names a rule that does not exist or cannot be suppressed | diff --git a/agents/ffca-layer-auditor.md b/agents/ffca-layer-auditor.md index e3b7c07..094a5f1 100644 --- a/agents/ffca-layer-auditor.md +++ b/agents/ffca-layer-auditor.md @@ -39,7 +39,7 @@ From the repo root, run the validator in full-graph mode and treat every line it dart run ${CLAUDE_PLUGIN_ROOT}/scripts/validate_layers.dart --all ``` -This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK), and imports and barrels (each feature package has a primary barrel, no import reaches past another package's barrels, another feature's presentation is imported through a dedicated barrel, a data barrel never exports data sources, DTOs, or mappers, and every declared workspace dependency is imported). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding. +This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK), and imports and barrels (each feature package has a primary barrel, no import reaches past another package's barrels, another feature's presentation is imported through a dedicated barrel, a data barrel never exports DTOs or mappers, and every declared workspace dependency is imported). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding. ## Step 2: source-level checks (qualitative) diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/tau_remote_data_source.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/tau_remote_data_source.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/data_sources/tau_remote_data_source/tau_remote_data_source.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart index 0d46029..30f1982 100644 --- a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_data.dart @@ -1,6 +1,8 @@ import 'package:tau_domain/tau_domain.dart'; -// Exports its DTOs and mappers along with the repository. +// Exports its DTOs and mappers along with the repository and data source. export 'data_sources/tau_remote_data_source/dtos/tau_dto.dart'; export 'mappers/tau_mapper.dart'; export 'repositories/tau_repository.dart'; +// The app constructs the data source, so exporting it is allowed. +export 'data_sources/tau_remote_data_source/tau_remote_data_source.dart'; diff --git a/scripts/test/validate_layers_test.dart b/scripts/test/validate_layers_test.dart index 7eb0add..2cb43e6 100644 --- a/scripts/test/validate_layers_test.dart +++ b/scripts/test/validate_layers_test.dart @@ -246,6 +246,11 @@ void main() { 'mappers/tau_mapper.dart'), ); expect(err, isNot(contains('repositories/tau_repository.dart'))); + // The app constructs data sources, so a data barrel may export them. + expect( + err, + isNot(contains('tau_remote_data_source/tau_remote_data_source.dart')), + ); }); test('ignores Dart files outside lib/ and bin/', () { diff --git a/scripts/validate_layers.dart b/scripts/validate_layers.dart index 0eb4e33..73d8ed6 100644 --- a/scripts/validate_layers.dart +++ b/scripts/validate_layers.dart @@ -465,7 +465,8 @@ List<_Violation> _checkImports(Package p, Workspace ws) { final name = match?.group(1); final rest = match?.group(2) ?? d.uri; - // A data barrel exporting its own data sources, DTOs, or mappers. + // A data barrel exporting its own DTOs or mappers. Data sources are + // exported: the app constructs them to inject into the repository. if ((match == null || name == p.name) && p.layer == 'data' && f.libRel == barrel && @@ -477,9 +478,9 @@ List<_Violation> _checkImports(Package p, Workspace ws) { package: p, file: f, rule: 'line ${d.line} exports ${d.uri} (a data barrel never ' - 'exposes its data sources, DTOs, or mappers)', - fix: 'Remove the export; the rest of the app only sees the ' - 'repository and the domain models it returns.', + 'exposes its DTOs or mappers)', + fix: 'Remove the export; DTOs and mappers are implementation ' + 'details of the repository, which returns domain models.', ), ); } @@ -540,7 +541,7 @@ List<_Violation> _checkImports(Package p, Workspace ws) { } final _packageUri = RegExp(r'^package:([^/]+)/(.+)$'); -final _dataInternals = RegExp(r'(^|/)(data_sources|dtos|mappers)/'); +final _dataInternals = RegExp(r'(^|/)(dtos|mappers)/'); _Violation _dependencyViolation(Package src, Package tgt) { final pair = '${src.name} depends on ${tgt.name}'; From b9535e80d9d36666ffb141c206710baf710f1f16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Baudet?= Date: Thu, 15 Oct 2026 07:27:34 -0600 Subject: [PATCH 3/3] fix(validator): close review gaps in import and barrel checks From a review of #29, #30 and #31: - Check imports of undeclared workspace packages against the rules table. A Dart workspace resolves them, so `product_presentation` could import `cart_data` without either mode noticing. A forbidden layer reports that layer rule; an allowed one reports the new undeclared-dependency error. - On a pubspec edit, run import checks and suppression validation for the edited package only, so a clean edit never blocks on a dependent's existing Dart findings. - Skip missing-primary-barrel in incremental mode while a package has no lib/, so a new package can be scaffolded pubspec first. - Read directives with a scanner that stops at the first declaration, follows multi-line directives, and reads every URI, so imports inside strings are ignored and both sides of a conditional import are checked. - Check every barrel at the root of a data package's lib/, following relative exports one level, for exported DTOs and mappers. - Report a `//` suppression naming a pubspec-only rule, and group suppression findings under the file the comment is in. - Fix CLAUDE.md, which said dart only runs for pubspec edits, and cite the barrel rule's actual sources. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017Vxxv9fVFGupLXEnmx5bSq --- CLAUDE.md | 4 +- README.md | 7 +- agents/ffca-layer-auditor.md | 2 +- hooks/validate_layers_test.sh | 3 + .../phi/phi_presentation/bin/phi_tool.dart | 1 + .../lib/phi_presentation.dart | 3 + .../lib/psi_presentation.dart | 3 + .../psi/psi_presentation/pubspec.yaml | 6 + .../features/tau/tau_data/lib/tau_wire.dart | 1 + .../features/tau/tau_data/lib/wire/wire.dart | 1 + .../lib/upsilon_presentation.dart | 4 +- .../fixtures/invalid_workspace/pubspec.yaml | 1 + .../apps/mobile_app/lib/mobile_app.dart | 2 +- .../cart/cart_domain/lib/cart_domain.dart | 3 +- .../lib/product_presentation.dart | 5 + .../shared/api_client/lib/api_client.dart | 1 - .../kappa/kappa_domain/lib/kappa_domain.dart | 1 + .../kappa/kappa_domain/lib/models/kappa.dart | 1 + .../features/kappa/kappa_domain/pubspec.yaml | 4 + .../lib/kappa_presentation.dart | 1 + .../kappa/kappa_presentation/pubspec.yaml | 8 + .../fixtures/warning_workspace/pubspec.yaml | 2 + scripts/test/validate_layers_test.dart | 118 ++++++++- scripts/validate_layers.dart | 235 ++++++++++++++---- 24 files changed, 363 insertions(+), 54 deletions(-) create mode 100644 scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/bin/phi_tool.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/lib/psi_presentation.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/pubspec.yaml create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_wire.dart create mode 100644 scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/wire/wire.dart delete mode 100644 scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/kappa_domain.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/models/kappa.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/pubspec.yaml create mode 100644 scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/lib/kappa_presentation.dart create mode 100644 scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/pubspec.yaml diff --git a/CLAUDE.md b/CLAUDE.md index 8f140a9..cb643c6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -7,9 +7,9 @@ `hooks/hooks.json` defines one PostToolUse hook and one PreToolUse hook. -- `Edit|MultiEdit|Write` matcher → `validate_layers.sh`. The matcher is a filter, not a guarantee, so the hook reads `tool_name` from the payload and exits 0 unless it is `Edit`, `MultiEdit`, or `Write`. When the edited file is a `pubspec.yaml` or a Dart file inside an FFCA-shaped repo, meaning a `features/` folder exists above it, the hook runs `dart run scripts/validate_layers.dart --file `. For a pubspec, the validator checks the edited package and its direct dependents. For a Dart file under a package's `lib/` or `bin/`, it reports only the import and barrel findings located in that file, so a Dart edit never blocks on a violation elsewhere. Exit 2 means at least one error-severity finding. The hook prints the findings to stderr and exits 2, which blocks Claude until the dependency is fixed. When the validator exits 0 with output, every finding is a warning: the hook returns it as `additionalContext` JSON and exits 0, so Claude sees it without being blocked. A `# ffca-ignore: — ` comment in a pubspec suppresses that rule for that package, and `// ffca-ignore:` in a Dart file suppresses it for that file. +- `Edit|MultiEdit|Write` matcher → `validate_layers.sh`. The matcher is a filter, not a guarantee, so the hook reads `tool_name` from the payload and exits 0 unless it is `Edit`, `MultiEdit`, or `Write`. When the edited file is a `pubspec.yaml` or a Dart file inside an FFCA-shaped repo, meaning a `features/` folder exists above it, the hook runs `dart run scripts/validate_layers.dart --file `. For a pubspec, the validator checks the edited package and its direct dependents, running the import checks and suppression validation for the edited package only, and it does not require a barrel from a package with no `lib/` yet. For a Dart file under a package's `lib/` or `bin/`, it reports only the import and barrel findings located in that file, so a Dart edit never blocks on a violation elsewhere. Exit 2 means at least one error-severity finding. The hook prints the findings to stderr and exits 2, which blocks Claude until the dependency is fixed. When the validator exits 0 with output, every finding is a warning: the hook returns it as `additionalContext` JSON and exits 0, so Claude sees it without being blocked. A `# ffca-ignore: — ` comment in a pubspec suppresses that rule for that package, and `// ffca-ignore:` in a Dart file suppresses it for that file, except for rules reported on the pubspec, which a Dart file cannot suppress. - Any other file, or a repo with no `features/` folder, exits 0 silently. -- The hook skips with exit 0 when `jq` or `dart` is missing from `PATH`. The `dart` check runs only for FFCA pubspec edits. A validator failure other than exit 2 is reported to stderr but never blocks. +- The hook skips with exit 0 when `jq` or `dart` is missing from `PATH`. The `dart` check runs only for FFCA pubspec and Dart edits. A validator failure other than exit 2 is reported to stderr but never blocks. - `mcp__.*very-good-cli__.*` matcher → `check_vgv_cli.sh`. For a Very Good CLI MCP tool, it returns `allow` when `very_good --version` is 1.3.0 or newer and `deny` with an install or upgrade message when the CLI is missing or older. It stands aside with exit 0 for any other tool, when the version cannot be read, or when `jq` is missing. `allowed-tools` grants only last for the invoking turn, so this hook is what keeps the tools approved afterwards. diff --git a/README.md b/README.md index ac8df45..c0403ff 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,7 @@ A PostToolUse hook runs on every `Edit`, `MultiEdit`, or `Write` and ignores any | **Validate layers** (`validate_layers.sh`) | PostToolUse (`Edit`/`MultiEdit`/`Write`) | Runs the FFCA validator incrementally: on the edited package and its direct dependents for a pubspec edit, and on the edited file alone for a Dart edit. Exits 2 on an error (blocking: Claude must fix the dependency before continuing), printing the rule and the fix. Passes warnings to Claude as non-blocking context. Passes silently otherwise | | **Check VGV CLI** (`check_vgv_cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Auto-approves Very Good CLI MCP tool calls when the CLI is installed at 1.3.0 or newer, so they work in every run mode. Denies with an install or upgrade message when the CLI is missing or outdated. Stands aside for any other tool, or when the CLI version cannot be read | -The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK. In `lib/` and `bin/`, it checks that each feature package has a primary barrel `lib/{package}.dart`, that imports of other workspace packages go through a barrel at the root of their `lib/`, that another feature's presentation is imported through a dedicated barrel rather than its primary one, that a data barrel never exports DTOs or mappers, and that every declared workspace dependency is imported. +The validator checks package naming under `features/`, the layer dependency rules, that nothing depends on an app, and cycles. It also checks that each feature's packages fit together: a data package depends on its own `_domain` under `dependencies`, a data package never exists without a `_domain` in its feature, and a `_domain` package never depends on the Flutter SDK. In `lib/` and `bin/`, it checks that each feature package has a primary barrel `lib/{package}.dart`, that imports of other workspace packages go through a barrel at the root of their `lib/`, that another feature's presentation is imported through a dedicated barrel rather than its primary one, that a data barrel never exports DTOs or mappers, that every declared workspace dependency is imported, and that every imported workspace package is declared. An import of an undeclared package is checked against the same layer rules as a pubspec dependency, since a Dart workspace resolves it anyway. On a pubspec edit, a package with no `lib/` yet is not required to have a barrel, so a new package can be scaffolded pubspec first. The validator is also runnable directly for CI and audits, across the whole workspace: @@ -153,9 +153,10 @@ Every finding carries a rule ID in brackets. An **error** exits 2 and blocks the | `deep-import` | error | A Dart file imports or exports another workspace package past the barrels at the root of its `lib/` | | `cross-feature-primary-barrel` | error | A feature imports another feature's primary presentation barrel instead of a dedicated one | | `data-barrel-exports-internals` | error | A data package's primary barrel exports from `dtos/` or `mappers/`. Data sources may be exported, since the app constructs them | +| `undeclared-dependency` | error | A Dart file imports a workspace package its pubspec does not declare under `dependencies`. When the layer rules forbid that dependency, the layer rule's ID is reported instead | | `unused-dependency` | warning | A package declares a workspace dependency under `dependencies` but never imports it in `lib/` or `bin/` | | `suppression-missing-reason` | error | An `ffca-ignore` comment has no reason | -| `suppression-unknown-rule` | error | An `ffca-ignore` comment names a rule that does not exist or cannot be suppressed | +| `suppression-unknown-rule` | error | An `ffca-ignore` comment names a rule that does not exist or cannot be suppressed, or a `//` comment in a Dart file names a rule reported on the pubspec | | `unused-suppression` | warning | An `ffca-ignore` comment matches no finding (`--all` only) | To accept a finding as a deliberate exception, add a comment with the rule ID and a reason to the pubspec of the package it is reported on: @@ -172,7 +173,7 @@ A finding in a Dart file can also be suppressed in that file, with `//`: import 'package:orders_domain/models/order.dart'; ``` -A suppression in a pubspec covers every finding with that rule ID in that package, and one in a Dart file covers that file. The reason is required, so each exception explains itself in review. The last three rules cannot be suppressed. `--all` reports how many findings were suppressed, and flags suppressions that no longer match anything so they can be removed. +A suppression in a pubspec covers every finding with that rule ID in that package, and one in a Dart file covers that file. Rules reported on the pubspec, such as `unused-dependency` or `missing-primary-barrel`, can only be suppressed in the pubspec. The reason is required, so each exception explains itself in review. The last three rules cannot be suppressed. `--all` reports how many findings were suppressed, and flags suppressions that no longer match anything so they can be removed. ### Prerequisites diff --git a/agents/ffca-layer-auditor.md b/agents/ffca-layer-auditor.md index 094a5f1..66fcce9 100644 --- a/agents/ffca-layer-auditor.md +++ b/agents/ffca-layer-auditor.md @@ -39,7 +39,7 @@ From the repo root, run the validator in full-graph mode and treat every line it dart run ${CLAUDE_PLUGIN_ROOT}/scripts/validate_layers.dart --all ``` -This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK), and imports and barrels (each feature package has a primary barrel, no import reaches past another package's barrels, another feature's presentation is imported through a dedicated barrel, a data barrel never exports DTOs or mappers, and every declared workspace dependency is imported). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding. +This is the authoritative check for the pubspec dependency graph: package naming under `features/`, the layer dependency rules, the no-dependency-on-an-app rule, workspace-wide cycles, feature ownership (a data package depends on its own domain, a data package has a domain in its feature, a domain package never depends on the Flutter SDK), and imports and barrels (each feature package has a primary barrel, no import reaches past another package's barrels, another feature's presentation is imported through a dedicated barrel, a data barrel never exports DTOs or mappers, every declared workspace dependency is imported, and every imported workspace package is declared and allowed by the layer rules). It prints each finding with its rule ID, severity (`✗` error, `⚠` warning), and fix, then a summary line with the error, warning, and suppressed counts. It exits 2 if any error is found, and 0 when there are only warnings or nothing at all. An error makes the package `fail`; a warning makes it `warn`. A finding suppressed with an `# ffca-ignore:` comment is an accepted exception: do not report it as a violation, but list the suppression and its reason in the package's notes. Do not re-derive these checks by hand; the script is the source of truth for them. Read `references/ffca/project_structure.md`, sections *Dependency rules* and *Naming conventions*, only if you need to explain a finding. ## Step 2: source-level checks (qualitative) diff --git a/hooks/validate_layers_test.sh b/hooks/validate_layers_test.sh index fbda1fa..8157853 100755 --- a/hooks/validate_layers_test.sh +++ b/hooks/validate_layers_test.sh @@ -51,6 +51,8 @@ NOT_PUBSPEC="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/README.md" CLEAN_DART="$FIXTURES/invalid_workspace/features/alpha/alpha_domain/lib/alpha_domain.dart" VIOLATING_DART="$FIXTURES/invalid_workspace/features/sigma/sigma_presentation/lib/sigma_presentation.dart" SUPPRESSED_DART="$FIXTURES/warning_workspace/features/omega/omega_presentation/lib/omega_presentation.dart" +# A deep import in a Dart file outside lib/ and bin/, which the validator does not check. +OUTSIDE_LIB_DART="$FIXTURES/invalid_workspace/features/phi/phi_presentation/test/phi_helpers.dart" payload() { printf '{"tool_name":"%s","tool_input":{"file_path":"%s"}}' "$1" "$2"; } @@ -130,6 +132,7 @@ echo "--- FFCA Dart edits ---" assert_outcome block "Edit on a violating import" "$(payload Edit "$VIOLATING_DART")" assert_outcome pass "Edit on a clean file in a violating package" "$(payload Edit "$CLEAN_DART")" assert_outcome pass "Edit on an import suppressed in the file" "$(payload Edit "$SUPPRESSED_DART")" +assert_outcome pass "Edit on a Dart file outside lib/ and bin/" "$(payload Edit "$OUTSIDE_LIB_DART")" echo "" echo "--- Missing prerequisites (skip) ---" diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/bin/phi_tool.dart b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/bin/phi_tool.dart new file mode 100644 index 0000000..d76a099 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/bin/phi_tool.dart @@ -0,0 +1 @@ +import 'package:phi_domain/models/phi.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart index ce75e06..f0605db 100644 --- a/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart +++ b/scripts/test/fixtures/invalid_workspace/features/phi/phi_presentation/lib/phi_presentation.dart @@ -1,2 +1,5 @@ // Reaches past the phi_domain barrel. import 'package:phi_domain/models/phi.dart'; +export 'package:phi_domain/models/phi.dart'; +import 'package:phi_domain/phi_domain.dart' + if (dart.library.io) 'package:phi_domain/models/phi.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/lib/psi_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/lib/psi_presentation.dart new file mode 100644 index 0000000..aa2d66e --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/lib/psi_presentation.dart @@ -0,0 +1,3 @@ +// ffca-ignore: deep-import +// ffca-ignore: unused-dependency — Dart files cannot suppress pubspec rules +import 'package:phi_domain/models/phi.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/pubspec.yaml new file mode 100644 index 0000000..e1e7a2c --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/psi/psi_presentation/pubspec.yaml @@ -0,0 +1,6 @@ +name: psi_presentation +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + phi_domain: any diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_wire.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_wire.dart new file mode 100644 index 0000000..342b768 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/tau_wire.dart @@ -0,0 +1 @@ +export 'wire/wire.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/wire/wire.dart b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/wire/wire.dart new file mode 100644 index 0000000..750be25 --- /dev/null +++ b/scripts/test/fixtures/invalid_workspace/features/tau/tau_data/lib/wire/wire.dart @@ -0,0 +1 @@ +export '../mappers/tau_mapper.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart b/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart index ff5d578..d31af98 100644 --- a/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart +++ b/scripts/test/fixtures/invalid_workspace/features/upsilon/upsilon_presentation/lib/upsilon_presentation.dart @@ -1 +1,3 @@ -// Barrel of upsilon_presentation. +// Imports workspace packages its pubspec does not declare. +import 'package:delta_data/delta_data.dart'; +import 'package:gamma_domain/gamma_domain.dart'; diff --git a/scripts/test/fixtures/invalid_workspace/pubspec.yaml b/scripts/test/fixtures/invalid_workspace/pubspec.yaml index 4999f44..0a3c021 100644 --- a/scripts/test/fixtures/invalid_workspace/pubspec.yaml +++ b/scripts/test/fixtures/invalid_workspace/pubspec.yaml @@ -28,6 +28,7 @@ workspace: - features/phi/phi_domain - features/phi/phi_presentation - features/pi/pi_domain + - features/psi/psi_presentation - features/sigma/sigma_presentation - features/tau/tau_data - features/tau/tau_domain diff --git a/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart b/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart index 4744db2..05b52e2 100644 --- a/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart +++ b/scripts/test/fixtures/valid_workspace/apps/mobile_app/lib/mobile_app.dart @@ -1,5 +1,5 @@ import 'package:product_presentation/product_presentation.dart'; import 'package:cart_presentation/cart_presentation.dart'; -import 'package:ideas_presentation/ideas_presentation.dart'; +import "package:ideas_presentation/ideas_presentation.dart" deferred as ideas; import 'package:ui_kit/ui_kit.dart'; import 'package:api_client/api_client.dart'; diff --git a/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart b/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart index bb4f683..b9fad3f 100644 --- a/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart +++ b/scripts/test/fixtures/valid_workspace/features/cart/cart_domain/lib/cart_domain.dart @@ -1 +1,2 @@ -import 'package:product_domain/product_domain.dart'; +// A directive spanning two lines. +export 'package:product_domain/product_domain.dart' show Product; diff --git a/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart b/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart index 5a5d1ba..7bfad37 100644 --- a/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart +++ b/scripts/test/fixtures/valid_workspace/features/product/product_presentation/lib/product_presentation.dart @@ -1,2 +1,7 @@ import 'package:product_domain/product_domain.dart'; import 'package:ui_kit/ui_kit.dart'; + +// An import inside a string is not a directive. +const sample = ''' +import 'package:product_domain/models/product.dart'; +'''; diff --git a/scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart b/scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart deleted file mode 100644 index 642c57b..0000000 --- a/scripts/test/fixtures/valid_workspace/shared/api_client/lib/api_client.dart +++ /dev/null @@ -1 +0,0 @@ -// Barrel of api_client. diff --git a/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/kappa_domain.dart b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/kappa_domain.dart new file mode 100644 index 0000000..ef74a09 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/kappa_domain.dart @@ -0,0 +1 @@ +// Barrel of kappa_domain. diff --git a/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/models/kappa.dart b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/models/kappa.dart new file mode 100644 index 0000000..b335484 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/lib/models/kappa.dart @@ -0,0 +1 @@ +// Fixture source file. diff --git a/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/pubspec.yaml b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/pubspec.yaml new file mode 100644 index 0000000..2c02724 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_domain/pubspec.yaml @@ -0,0 +1,4 @@ +name: kappa_domain +resolution: workspace +environment: + sdk: ^3.11.0 diff --git a/scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/lib/kappa_presentation.dart b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/lib/kappa_presentation.dart new file mode 100644 index 0000000..0bb4750 --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/lib/kappa_presentation.dart @@ -0,0 +1 @@ +import 'package:kappa_domain/models/kappa.dart'; diff --git a/scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/pubspec.yaml b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/pubspec.yaml new file mode 100644 index 0000000..70f4d5e --- /dev/null +++ b/scripts/test/fixtures/warning_workspace/features/kappa/kappa_presentation/pubspec.yaml @@ -0,0 +1,8 @@ +# A pubspec suppression covers the deep import in lib/. +# ffca-ignore: deep-import — the model is not exported from kappa_domain yet +name: kappa_presentation +resolution: workspace +environment: + sdk: ^3.11.0 +dependencies: + kappa_domain: any diff --git a/scripts/test/fixtures/warning_workspace/pubspec.yaml b/scripts/test/fixtures/warning_workspace/pubspec.yaml index 7ab7427..73bc7f0 100644 --- a/scripts/test/fixtures/warning_workspace/pubspec.yaml +++ b/scripts/test/fixtures/warning_workspace/pubspec.yaml @@ -3,6 +3,8 @@ name: _warning_workspace environment: sdk: ^3.11.0 workspace: + - features/kappa/kappa_domain + - features/kappa/kappa_presentation - features/nu/nu_domain - features/nu/nu_presentation - features/omega/omega_data diff --git a/scripts/test/validate_layers_test.dart b/scripts/test/validate_layers_test.dart index c1249f3..09c146d 100644 --- a/scripts/test/validate_layers_test.dart +++ b/scripts/test/validate_layers_test.dart @@ -39,6 +39,14 @@ String _absScript(String? cwd) => String _pubspec(String workspaceRelPath, String pkgPath) => '$workspaceRelPath/$pkgPath/pubspec.yaml'; +/// The findings reported under [path], up to the next file's header. +String _group(String err, String path) { + final start = err.indexOf('FFCA findings in $path:'); + if (start == -1) return ''; + final next = err.indexOf('FFCA findings in ', start + 1); + return err.substring(start, next == -1 ? err.length : next); +} + void main() { group('valid workspace', () { test('passes in --all mode', () { @@ -253,6 +261,72 @@ void main() { ); }); + test('an undeclared import that breaks a layer rule', () { + expect( + err, + contains('[presentation-depends-on-data] line 2 imports ' + 'package:delta_data/delta_data.dart: upsilon_presentation depends ' + 'on delta_data'), + ); + }); + + test('an undeclared import of an allowed layer', () { + expect( + err, + contains('[undeclared-dependency] line 3 imports ' + 'package:gamma_domain/gamma_domain.dart, but gamma_domain is not ' + 'under dependencies'), + ); + }); + + test('a deep export and both URIs of a conditional import', () { + final phi = _group( + err, + 'features/phi/phi_presentation/lib/phi_presentation.dart', + ); + expect( + phi, + contains('line 3 exports package:phi_domain/models/phi.dart'), + ); + // The second URI, on the directive's continuation line. + expect( + phi, + contains('line 5 imports package:phi_domain/models/phi.dart'), + ); + }); + + test('scans bin/ as well as lib/', () { + expect( + _group(err, 'features/phi/phi_presentation/bin/phi_tool.dart'), + contains('[deep-import] line 1 imports'), + ); + }); + + test('a data barrel re-exporting a mapper through another file', () { + expect( + _group(err, 'features/tau/tau_data/lib/tau_wire.dart'), + contains('line 1 exports wire/wire.dart, which re-exports ' + 'lib/mappers/tau_mapper.dart'), + ); + }); + + test('invalid Dart-file suppressions, reported in that file', () { + final psi = _group( + err, + 'features/psi/psi_presentation/lib/psi_presentation.dart', + ); + expect( + psi, + contains('[suppression-missing-reason] ffca-ignore: ' + 'deep-import on line 1')); + expect( + psi, + contains('names "unused-dependency", which is reported on ' + 'the pubspec, so a Dart file cannot suppress it')); + // Neither comment suppresses the deep import. + expect(psi, contains('[deep-import] line 3 imports')); + }); + test('ignores Dart files outside lib/ and bin/', () { expect(err, isNot(contains('phi_presentation/test/'))); }); @@ -392,6 +466,29 @@ void main() { expect(r.err, contains('features/kappa/ has no kappa_domain')); }); + test("a pubspec edit ignores a dependent's Dart-file findings", () { + // phi_domain is clean; its dependent phi_presentation has deep imports, + // and psi_presentation has invalid suppressions. Neither changes with + // an edit to phi_domain's pubspec. + final r = _run([ + '--file', + _pubspec(_invalid, 'features/phi/phi_domain'), + ]); + expect(r.code, 0, reason: r.err); + expect(r.err, isEmpty); + }); + + test('a pubspec edit does not require a barrel before lib/ exists', () { + // chi_domain has a pubspec and no lib/ yet, as when a package is being + // scaffolded. --all still reports its missing barrel. + final r = _run([ + '--file', + _pubspec(_invalid, 'features/chi/chi_domain'), + ]); + expect(r.code, 0, reason: r.err); + expect(r.err, isEmpty); + }); + test('a pubspec edit reports Dart-file findings in the package', () { final r = _run([ '--file', @@ -432,7 +529,7 @@ void main() { test('--all exits 0 when every finding is a warning or suppressed', () { final r = _run(['--all', _warning]); expect(r.code, 0, reason: r.err); - expect(r.err, contains('FFCA: 0 errors, 3 warnings, 2 suppressed.')); + expect(r.err, contains('FFCA: 0 errors, 3 warnings, 3 suppressed.')); }); test('reports a warning with its rule ID', () { @@ -454,6 +551,11 @@ void main() { expect(r.err, isNot(contains('omega_presentation'))); }); + test('a pubspec suppression covers a finding in a Dart file', () { + final r = _run(['--all', _warning]); + expect(r.err, isNot(contains('kappa_presentation'))); + }); + test('warns about a dependency that is never imported', () { final r = _run(['--all', _warning]); expect( @@ -538,6 +640,20 @@ void main() { expect(r.err, isEmpty); }); + test('passes directives the scanner must read correctly', () { + // An import inside a multi-line string after the first declaration, an + // export spanning two lines, and a double-quoted deferred import. + for (final file in [ + 'features/product/product_presentation/lib/product_presentation.dart', + 'features/cart/cart_domain/lib/cart_domain.dart', + 'apps/mobile_app/lib/mobile_app.dart', + ]) { + final r = _run(['--file', '$_valid/$file']); + expect(r.code, 0, reason: '$file: ${r.err}'); + expect(r.err, isEmpty, reason: file); + } + }); + test('passes a cross-feature import of a dedicated barrel', () { final r = _run([ '--file', diff --git a/scripts/validate_layers.dart b/scripts/validate_layers.dart index a4dcb0b..8b8f676 100644 --- a/scripts/validate_layers.dart +++ b/scripts/validate_layers.dart @@ -83,11 +83,25 @@ const Map ruleSeverities = { 'cross-feature-primary-barrel': Severity.error, 'data-barrel-exports-internals': Severity.error, 'unused-dependency': Severity.warning, + 'undeclared-dependency': Severity.error, 'suppression-missing-reason': Severity.error, 'suppression-unknown-rule': Severity.error, 'unused-suppression': Severity.warning, }; +// Rules reported on a pubspec, never on a Dart file, so a `//` suppression in a +// Dart file cannot name them. +const _pubspecOnly = { + 'package-name', + 'dependency-cycle', + 'data-missing-own-domain', + 'data-without-domain', + 'domain-uses-flutter', + 'presentation-missing-own-domain', + 'missing-primary-barrel', + 'unused-dependency', +}; + const _unsuppressible = { 'suppression-missing-reason', 'suppression-unknown-rule', @@ -139,15 +153,19 @@ int _runIncremental(String rawPath) { if (p.deps.contains(edited.dir)) scope.add(p); // direct dependents } + // A pubspec edit cannot change another package's imports or suppressions, + // so dependents only get the layer and ownership checks. final violations = <_Violation>[]; for (final p in scope) { - violations.addAll(_checkPackage(p, workspace)); + violations.addAll( + _checkPackage(p, workspace, imports: p == edited, incremental: true), + ); } violations.addAll(_cycleViolations(workspace, from: edited)); final result = _applySuppressions( violations, - _declaredSuppressions(scope), + _declaredSuppressions([edited]), reportUnused: false, ); if (result.findings.isEmpty) return 0; // silent pass @@ -185,7 +203,7 @@ int _runDartFile(String rawPath) { final result = _applySuppressions( violations, [ - for (final s in dartFile.suppressions) (package, s), + for (final s in dartFile.suppressions) (package, s, dartFile), ], reportUnused: false); if (result.findings.isEmpty) return 0; @@ -241,14 +259,14 @@ int _exitCode(List<_Violation> findings) => // --------------------------------------------------------------------------- /// Every suppression declared in [packages], in their pubspecs and Dart files. -List<(Package, _Suppression)> _declaredSuppressions( +List<(Package, _Suppression, _DartFile?)> _declaredSuppressions( Iterable packages, ) => [ for (final p in packages) ...[ - for (final s in p.suppressions) (p, s), + for (final s in p.suppressions) (p, s, null), for (final f in p.dartFiles) - for (final s in f.suppressions) (p, s), + for (final s in f.suppressions) (p, s, f), ], ]; @@ -258,13 +276,14 @@ List<(Package, _Suppression)> _declaredSuppressions( /// full mode, where every finding has been computed. ({List<_Violation> findings, int suppressed}) _applySuppressions( List<_Violation> violations, - List<(Package, _Suppression)> declared, { + List<(Package, _Suppression, _DartFile?)> declared, { required bool reportUnused, }) { - bool valid(_Suppression s) => - s.reason.isNotEmpty && + bool canName(_Suppression s) => ruleSeverities.containsKey(s.ruleId) && - !_unsuppressible.contains(s.ruleId); + !_unsuppressible.contains(s.ruleId) && + !(s.marker == '//' && _pubspecOnly.contains(s.ruleId)); + bool valid(_Suppression s) => s.reason.isNotEmpty && canName(s); final used = <_Suppression>{}; final findings = <_Violation>[]; @@ -281,14 +300,26 @@ List<(Package, _Suppression)> _declaredSuppressions( } } - for (final (p, s) in declared) { + for (final (p, s, f) in declared) { final where = s.where; - if (!ruleSeverities.containsKey(s.ruleId) || - _unsuppressible.contains(s.ruleId)) { + if (_pubspecOnly.contains(s.ruleId) && s.marker == '//') { findings.add( _Violation( id: 'suppression-unknown-rule', package: p, + file: f, + rule: 'ffca-ignore on $where names "${s.ruleId}", which is ' + 'reported on the pubspec, so a Dart file cannot suppress it', + fix: 'Move the comment to the ${p.name} pubspec: ' + '# ffca-ignore: ${s.ruleId} — ', + ), + ); + } else if (!canName(s)) { + findings.add( + _Violation( + id: 'suppression-unknown-rule', + package: p, + file: f, rule: 'ffca-ignore on $where names "${s.ruleId}", which is not a ' 'rule that can be suppressed', fix: 'Use one of the rule IDs shown in brackets in the validator ' @@ -300,6 +331,7 @@ List<(Package, _Suppression)> _declaredSuppressions( _Violation( id: 'suppression-missing-reason', package: p, + file: f, rule: 'ffca-ignore: ${s.ruleId} on $where has no reason, so it ' 'suppresses nothing', fix: 'Add the reason after the rule ID: ' @@ -312,6 +344,7 @@ List<(Package, _Suppression)> _declaredSuppressions( _Violation( id: 'unused-suppression', package: p, + file: f, rule: 'ffca-ignore: ${s.ruleId} on $where matches no finding', fix: 'Remove the comment; the exception is no longer needed.', ), @@ -325,7 +358,12 @@ List<(Package, _Suppression)> _declaredSuppressions( // Checks // --------------------------------------------------------------------------- -List<_Violation> _checkPackage(Package p, Workspace ws) { +List<_Violation> _checkPackage( + Package p, + Workspace ws, { + bool imports = true, + bool incremental = false, +}) { final out = <_Violation>[]; // Naming: a package under features/{f}/ must be a valid FFCA package name. @@ -345,7 +383,7 @@ List<_Violation> _checkPackage(Package p, Workspace ws) { } out.addAll(_checkOwnership(p, ws)); - out.addAll(_checkImports(p, ws)); + if (imports) out.addAll(_checkImports(p, ws, incremental: incremental)); final allowed = rules[p.layer]; if (allowed == null) return out; // unknown source layer (e.g. app-less): skip @@ -439,15 +477,28 @@ List<_Violation> _checkOwnership(Package p, Workspace ws) { } /// Checks the package's barrels and its imports and exports of other -/// workspace packages. Implements "Layer subfolders" and the barrel rule in -/// references/ffca/project_structure.md, the dedicated-barrel condition in its -/// "Dependency rules", and "Widgets that own state" in -/// references/ffca/presentation.md. -List<_Violation> _checkImports(Package p, Workspace ws) { +/// workspace packages. Implements the barrels in "Folder layout" and "Layer +/// subfolders" and the dedicated-barrel condition in "Dependency rules" of +/// references/ffca/project_structure.md, and "Widgets that own state" in +/// references/ffca/presentation.md ("never the primary barrel"). The general +/// rule that packages import each other only through barrels is on the VGV +/// Engineering barrel files page, which the FFCA section does not state yet +/// (VGVentures/very_good_engineering#294). Imports go through the same rules +/// table as pubspec dependencies, since a workspace resolves an undeclared +/// import of another member. In [incremental] mode a package with no lib/ yet +/// is being scaffolded, so its missing barrel is not reported until --all. +List<_Violation> _checkImports( + Package p, + Workspace ws, { + bool incremental = false, +}) { final out = <_Violation>[]; final barrel = 'lib/${p.name}.dart'; + final scaffolding = incremental && !Directory('${p.dir}/lib').existsSync(); - if (p.top == 'features' && !p.dartFiles.any((f) => f.libRel == barrel)) { + if (p.top == 'features' && + !scaffolding && + !p.dartFiles.any((f) => f.libRel == barrel)) { out.add( _Violation( id: 'missing-primary-barrel', @@ -458,38 +509,99 @@ List<_Violation> _checkImports(Package p, Workspace ws) { ); } + // Files of this package by their path in it, to follow re-exports. + final byLibRel = {for (final f in p.dartFiles) f.libRel: f}; + String? ownPath(_DartFile from, String uri) { + final match = _packageUri.firstMatch(uri); + if (match != null) { + return match.group(1) == p.name ? 'lib/${match.group(2)}' : null; + } + if (uri.contains(':')) return null; // dart: and other schemes + final dir = from.libRel.substring(0, from.libRel.lastIndexOf('/')); + return _normalize('$dir/$uri'); + } + final used = {}; for (final f in p.dartFiles) { - for (final d in f.directives) { - final match = _packageUri.firstMatch(d.uri); - final name = match?.group(1); - final rest = match?.group(2) ?? d.uri; - - // A data barrel exporting its own DTOs or mappers. Data sources are - // exported: the app constructs them to inject into the repository. - if ((match == null || name == p.name) && - p.layer == 'data' && - f.libRel == barrel && - d.kind == 'export' && - _dataInternals.hasMatch(rest)) { + // Any barrel at the root of a data package's lib/ that exports its own + // DTOs or mappers, directly or through one re-exporting file. Data + // sources are exported: the app constructs them to inject them. + final rootBarrel = + f.libRel.startsWith('lib/') && !f.libRel.substring(4).contains('/'); + if (p.layer == 'data' && rootBarrel) { + for (final d in f.directives.where((d) => d.kind == 'export')) { + final target = ownPath(f, d.uri); + if (target == null) continue; + final via = byLibRel[target]; + final leaked = _dataInternals.hasMatch(target) + ? d.uri + : via?.directives + .where((e) => e.kind == 'export') + .map((e) => ownPath(via, e.uri)) + .whereType() + .where(_dataInternals.hasMatch) + .firstOrNull; + if (leaked == null) continue; out.add( _Violation( id: 'data-barrel-exports-internals', package: p, file: f, - rule: 'line ${d.line} exports ${d.uri} (a data barrel never ' - 'exposes its DTOs or mappers)', + rule: leaked == d.uri + ? 'line ${d.line} exports ${d.uri} (a data barrel never ' + 'exposes its DTOs or mappers)' + : 'line ${d.line} exports ${d.uri}, which re-exports ' + '$leaked (a data barrel never exposes its DTOs or ' + 'mappers)', fix: 'Remove the export; DTOs and mappers are implementation ' 'details of the repository, which returns domain models.', ), ); } + } + + for (final d in f.directives) { + final match = _packageUri.firstMatch(d.uri); + final name = match?.group(1); + final rest = match?.group(2) ?? d.uri; if (match == null || name == p.name) continue; // within the package final target = ws.byName[name]; if (target == null) continue; // external package used.add(target.name); + // An import of a workspace member that the pubspec does not declare. + // Check it against the rules table like a declared dependency. + if (!p.runtimeDeps.contains(target.dir)) { + final allowed = rules[p.layer]; + if (allowed != null && + target.layer != 'unknown' && + !allowed.contains(target.layer)) { + final layer = _dependencyViolation(p, target); + out.add( + _Violation( + id: layer.id, + package: p, + file: f, + rule: 'line ${d.line} ${d.kind}s ${d.uri}: ${layer.rule}', + fix: layer.fix, + ), + ); + } else { + out.add( + _Violation( + id: 'undeclared-dependency', + package: p, + file: f, + rule: 'line ${d.line} ${d.kind}s ${d.uri}, but ${target.name} ' + 'is not under dependencies in the ${p.name} pubspec', + fix: 'Add ${target.name} under dependencies in the ${p.name} ' + 'pubspec.', + ), + ); + } + } + if (rest.contains('/')) { out.add( _Violation( @@ -774,11 +886,7 @@ List<_DartFile> _readDartFiles(String pkgDir, String root) { path: path, relPath: relPath, libRel: path.substring(pkgDir.length + 1), - directives: [ - for (var i = 0; i < lines.length; i++) - if (_directivePattern.firstMatch(lines[i]) case final m?) - _Directive(kind: m.group(1)!, uri: m.group(2)!, line: i + 1), - ], + directives: _readDirectives(lines), suppressions: _parseSuppressions(lines, where: relPath), ), ); @@ -787,9 +895,50 @@ List<_DartFile> _readDartFiles(String pkgDir, String root) { return files; } -/// An `import` or `export` directive's URI. Directives always start a line in -/// formatted Dart, and the URI is the first string after the keyword. -final _directivePattern = RegExp(r'''^\s*(import|export)\s+['"]([^'"]+)['"]'''); +/// Reads the URIs of a file's `import` and `export` directives. Dart requires +/// directives before any declaration, so the scan stops at the first line that +/// is not a directive, a comment, an annotation, or blank: an `import` inside a +/// string or a doc comment further down is never read. A directive can span +/// lines (`show`, `hide`, conditional URIs), and every quoted URI in it is +/// read, so both sides of a conditional import are checked. +List<_Directive> _readDirectives(List lines) { + final directives = <_Directive>[]; + String? kind; // the directive being read, until its `;` + var inComment = false; + for (var i = 0; i < lines.length; i++) { + final line = lines[i].trim(); + if (inComment) { + inComment = !line.contains('*/'); + continue; + } + if (kind == null) { + if (line.isEmpty || + line.startsWith('//') || + line.startsWith('@') || + line.startsWith('#!')) { + continue; + } + if (line.startsWith('/*')) { + inComment = !line.contains('*/'); + continue; + } + final start = _directiveStart.firstMatch(line); + if (start == null) break; // first declaration: directives are over + kind = start.group(1); + } + if (kind == 'import' || kind == 'export') { + final code = line.split('//').first; + for (final m in _quotedUri.allMatches(code)) { + directives.add(_Directive(kind: kind!, uri: m.group(1)!, line: i + 1)); + } + } + if (line.contains(';')) kind = null; + } + return directives; +} + +final _directiveStart = RegExp(r'^(import|export|library|part)\b'); +final _quotedUri = RegExp(r'''['"]([^'"]+)['"]'''); Package _buildPackage( String pkgDir,