Skip to content

Commit 17ca951

Browse files
LTSCommerceclaude
andcommitted
Merge feature/phparkitect: PHPArkitect integration + SSoT rule migration
Adds PHPArkitect as the on-by-default architecture-rules tool (generic-safe default tier + opt-in optional/symfony tiers), migrates structural type-suffix naming out of PHPStan into arkitect (SSoT), documents the PHPArkitect-vs-PHPStan placement decision, and lands all multi-lens audit fixes. Self-test green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 parents e701adf + e397905 commit 17ca951

42 files changed

Lines changed: 946 additions & 141 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/agents/php-qa-ci_phpstan-rule-creator.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ tools: Read, Edit, Glob, Grep, Write
99
You are a PHPStan rule creator agent. Your job is to create custom PHPStan rules that detect
1010
specific bug patterns at static analysis level, preventing entire classes of bugs from recurring.
1111

12+
## Precondition — confirm the engine first
13+
14+
Before authoring a PHPStan rule, confirm the pattern is NOT a structural naming /
15+
namespace-layering / dependency-direction convention — those belong in PHPArkitect
16+
(`qaConfig/phparkitect.php` or a shipped tier), never PHPStan. If it IS structural, STOP
17+
and tell the caller to use PHPArkitect instead; do not write the rule. Only proceed for
18+
method-level / semantic detection arkitect cannot express. See the README decision guide
19+
"Where does a rule belong — PHPArkitect or PHPStan?".
20+
1221
## Your Role
1322

1423
You create custom PHPStan rules as part of the "Defence Before Fix" strategy:

.claude/skills/defence-before-fix/SKILL.md

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,22 @@ This phase is primarily manual/guided. Help the user by:
108108

109109
## Phase 2: DETECT (Static Analysis)
110110

111-
**Goal:** Create a PHPStan rule that catches ALL instances of the bug pattern.
111+
**Goal:** Create a rule that catches ALL instances of the bug pattern.
112+
113+
### Step 0: Choose the engine first
114+
115+
Before writing a PHPStan rule, decide which engine owns the convention:
116+
117+
- **Structural conventions** — class/interface/enum/trait **NAMING**, namespace
118+
**LAYERING**, **DEPENDENCY** direction — belong in **PHPArkitect** (extend a tier or
119+
the project's `qaConfig/phparkitect.php`), NOT PHPStan.
120+
- Only **finer-grained / method-level / semantic** patterns (which PHPArkitect cannot
121+
express) become PHPStan rules.
122+
- **NEVER enforce one convention in both engines** — migrate, don't duplicate.
123+
124+
Decision guide (SSoT): [README "Where does a rule belong"](../../../README.md#where-does-a-rule-belong--phparkitect-or-phpstan).
125+
If the pattern is structural, do the rest of this phase against arkitect; only continue to
126+
the PHPStan rule creator below for patterns arkitect cannot express.
112127

113128
### Step 1: Launch the Rule Creator Agent
114129

CLAUDE.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,8 @@ The pipeline runs tools in 4 distinct phases:
9292
### Phase 3: Static Analysis Tools
9393

9494
10. **PHPStan** (`phpstan`) - Static analysis tool
95-
11. **SensitiveParameter Usage** (`sensitiveParameterUsage`) - Always-on security baseline: fails if `#[\SensitiveParameter]` is used nowhere in `src/`. Opt out per-project with `export useSensitiveParameterCheck=0`.
95+
11. **PHPArkitect** (`phpArkitect`) - Architecture rules (class naming, namespace layering, dependency direction). On by default; applies a generic-safe baseline and is composable/overridable per project. Opt out with `export useArkitect=0`. See the [PHPArkitect section in README.md](README.md#phparkitect-architecture-rules).
96+
12. **SensitiveParameter Usage** (`sensitiveParameterUsage`) - Always-on security baseline: fails if `#[\SensitiveParameter]` is used nowhere in `src/`. Opt out per-project with `export useSensitiveParameterCheck=0`.
9697

9798
### Phase 4: Testing Tools
9899

@@ -514,6 +515,18 @@ cp vendor/lts/php-qa-ci/configDefaults/generic/php_cs.php qaConfig/
514515
- Extensible with custom rules
515516
- Understands PHPDoc annotations
516517

518+
### PHPArkitect
519+
520+
- **Purpose**: Enforce architectural/structural rules — class-naming conventions, namespace layering, dependency direction — that PHPStan expresses awkwardly
521+
- **Tool**: [@includes/generic/phpArkitect.inc.bash](includes/generic/phpArkitect.inc.bash)
522+
- **PHAR**: `vendor-phar/phparkitect.phar` (PHIVE, key `47CD54B6398FE21B3709D0A4D9C905CED1932CA2`, short id `D9C905CED1932CA2`)
523+
- **Entry config (default)**: [@configDefaults/generic/phparkitect.php](configDefaults/generic/phparkitect.php) — applies the default tier to the detected source dir when a project has no `qaConfig/phparkitect.php`
524+
- **Rule tiers**: `phparkitect-rules-default.php` (on by default), `phparkitect-rules-optional.php` + `phparkitect-rules-optional-symfony.php` (opt-in) under [@configDefaults/generic](configDefaults/generic)
525+
- **Project template**: [@templates/qaConfig-phparkitect.php](templates/qaConfig-phparkitect.php)
526+
- **How it works**: parses each class into an AST and matches expressions (naming, dependencies); rules and the paths to scan are defined inside the config (so `-p` does not apply). The pipeline passes `--autoload` and exports the tier paths + detected `srcDir` as env vars
527+
- **Where a rule belongs (PHPArkitect vs PHPStan)**: arkitect by default for structural rules; upgrade to a PHPStan rule only for finer-grained / method-level / semantic detection arkitect cannot express. **Never enforce one convention in both engines** — migrate, don't duplicate (SSoT). Full decision guide: [README.md "Where does a rule belong"](README.md#where-does-a-rule-belong--phparkitect-or-phpstan)
528+
- **Full usage** (tiers, extend/replace/customise, disable): see the [PHPArkitect section in README.md](README.md#phparkitect-architecture-rules)
529+
517530
### PHPUnit
518531

519532
- **Purpose**: Unit testing framework

README.md

Lines changed: 79 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,11 @@ PHP-QA-CI orchestrates multiple PHP quality tools across four phases:
7171

7272
**Phase 3 -- Static Analysis:**
7373
9\. PHPStan (level max)
74+
10\. PHPArkitect (architecture rules; on by default, `useArkitect=0` to disable)
7475

7576
**Phase 4 -- Testing:**
76-
10\. PHPUnit
77-
11\. Infection (mutation testing, optional, requires Xdebug)
77+
11\. PHPUnit
78+
12\. Infection (mutation testing, optional, requires Xdebug)
7879

7980
**Post-Success:** PHPLoc (stats only, cannot fail)
8081

@@ -84,12 +85,87 @@ See [Pipeline Architecture](./docs/pipeline.md) for full details.
8485

8586
PHP-QA-CI uses a hybrid approach to tool delivery:
8687

87-
- **PHARs** (via [PHIVE](https://phar.io/)): PHPStan, PHP CS Fixer, Infection, Composer Require Checker -- delivered in `vendor-phar/`
88+
- **PHARs** (via [PHIVE](https://phar.io/)): PHPStan, PHP CS Fixer, Infection, Composer Require Checker, PHPArkitect (PHIVE key `D9C905CED1932CA2` — the trailing 16 chars of the full fingerprint `47CD54B6398FE21B3709D0A4D9C905CED1932CA2`, which is what `tool-install.bash` pins) -- delivered in `vendor-phar/`
8889
- **Composer dependencies**: PHPUnit, phpstan-strict-rules, phpstan-phpunit, parallel-lint
8990
- **Isolated Composer project**: Rector -- in `tools/rector/` with its own `composer.json` to prevent dependency conflicts
9091

9192
The `phpstan/phpstan` package is in the `replace` section of `composer.json` since PHPStan is provided via PHAR. This prevents version conflicts when consuming projects also require PHPStan extensions.
9293

94+
## PHPArkitect (architecture rules)
95+
96+
[PHPArkitect](https://github.com/phparkitect/arkitect) enforces *structural*
97+
rules that PHPStan expresses awkwardly: class-naming conventions, namespace
98+
layering, and dependency direction. It runs in Phase 3 and is **on by default**.
99+
100+
### Where does a rule belong — PHPArkitect or PHPStan?
101+
102+
**Default to PHPArkitect for structural rules. Upgrade to a PHPStan rule only when
103+
you need finer-grained, method-level, or semantic detection that arkitect cannot
104+
express.**
105+
106+
- **PHPArkitect (the default)** reasons about a class's *identity*: its kind
107+
(interface / enum / trait / class), its name, the namespace it sits in, and its
108+
ancestry. Reach for it for naming conventions, namespace layering, and
109+
dependency direction.
110+
- **PHPStan (the upgrade)** reasons about *code*. Move up to a PHPStan rule only
111+
when the check needs something arkitect cannot see or say:
112+
- a **method-level** predicate — e.g. "the class has a public `__invoke`";
113+
- **"any of N name patterns, except an allow-list"** — arkitect's
114+
`HaveNameMatching` is a single glob with no OR / except composite;
115+
- a **type-kind carve-out** in a dependency rule — e.g. allow generated *enums*
116+
but forbid generated *objects*; `NotDependsOnTheseNamespaces` has no type-kind
117+
awareness;
118+
- any **behavioural / semantic** check — type bans, call-site shape,
119+
docblock-driven rules, loose comparison, nested ternary.
120+
121+
**Single Source of Truth — never enforce one convention in both engines.** Adding
122+
arkitect is *not* purely additive: when a structural convention already lives in a
123+
PHPStan rule, **migrate** it to arkitect (and delete the PHPStan rule) rather than
124+
running both. Two engines enforcing one rule is a defect — duplicated failure
125+
messages, drift between them, and double maintenance. (The shipped Interface / Enum /
126+
Trait suffix convention was migrated exactly this way: it used to be the PHPStan
127+
`RequireTypeSuffixRule` and is now owned solely by the default arkitect tier.)
128+
129+
Rules are organised in tiers (mirroring the `rules-default` / `rules-optional`
130+
PHPStan neon split). php-qa-ci ships each as a file returning a list of arkitect
131+
`ArchRule` objects, and the pipeline exports the resolved path of each so a
132+
project config can compose them without knowing the vendor layout:
133+
134+
| Tier | Env var | Default | Contents |
135+
| ------------------------------------ | ----------------------------------------- | --------------------- | --------------------------------------- |
136+
| `phparkitect-rules-default` | `PHPQACI_ARKITECT_RULES_DEFAULT` | **on**, every project | Interface / Enum / Trait name suffixes |
137+
| `phparkitect-rules-optional` | `PHPQACI_ARKITECT_RULES_OPTIONAL` | opt-in | `*Exception` suffix, `Abstract*` prefix |
138+
| `phparkitect-rules-optional-symfony` | `PHPQACI_ARKITECT_RULES_OPTIONAL_SYMFONY` | opt-in | `*Command`, `*Subscriber` |
139+
140+
The default tier matches on AST node *kind*, so it never forces arkitect to
141+
resolve class ancestry — that keeps it safe for any project. Ancestry-resolving
142+
rules (`IsA`/`Extend`/`Implement`, e.g. the `*Exception` convention) need a
143+
complete autoloader, so they live in the optional tier.
144+
145+
### Troubleshooting: optional/symfony tiers need a complete autoloader
146+
147+
The optional and symfony tiers use ancestry rules (`IsA`) that resolve a class's
148+
parents by **reflecting** it — so the analysed classes must be autoloadable. The
149+
pipeline runs arkitect with `--autoload=vendor/autoload.php`, so this is normally
150+
fine. But if you opt into these tiers and your autoloader is incomplete, `IsA`
151+
rules **silently match nothing** — arkitect reports "No violations" (a false
152+
green) rather than failing. (A genuine crash — exit > 1 — instead means a broken
153+
config or an unparseable file.) If an opted-in `*Exception`/`*Command`/`*Subscriber`
154+
rule never seems to fire, run `composer dump-autoload` and confirm your classes
155+
load.
156+
157+
**Project usage.** With no project config, the default tier is applied to the
158+
detected source dir automatically. To go further, add `qaConfig/phparkitect.php`
159+
(copy `templates/qaConfig-phparkitect.php`) where you can:
160+
161+
- **extend** the default tier (`require getenv('PHPQACI_ARKITECT_RULES_DEFAULT')`),
162+
- **opt in** to the optional / symfony tiers (their env vars),
163+
- **add** project-bespoke rules,
164+
- **replace** a tier wholesale by dropping your own `qaConfig/phparkitect-rules-*.php` (resolved ahead of the shipped copy by `configPath`).
165+
166+
Disable arkitect for a project with `export useArkitect=0` in
167+
`qaConfig/qaConfig.inc.bash`. Run it alone with `vendor/bin/qa -t arch`.
168+
93169
## Custom PHPStan Rules
94170

95171
### Always-on rules (auto-loaded)
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* php-qa-ci PHPArkitect — GENERIC DEFAULT ruleset (the "rules-default" tier).
7+
*
8+
* Generic, safe-everywhere naming conventions. This tier is applied
9+
* BY DEFAULT to every consuming project (the shipped configDefaults entry
10+
* config `phparkitect.php` adds it to the detected source dir), exactly the way
11+
* rules-default.neon is the PHPStan baseline. Keep it conservative — anything
12+
* that is not safe across every project belongs in the optional tiers
13+
* (phparkitect-rules-optional.php, phparkitect-rules-optional-symfony.php).
14+
*
15+
* This file returns a list of ArchRule objects; it is NOT a runnable config (no
16+
* ClassSet). Projects compose it from qaConfig/phparkitect.php:
17+
*
18+
* $default = require getenv('PHPQACI_ARKITECT_RULES_DEFAULT');
19+
* $config->add($classSet, ...$default, ...$projectRules); // EXTEND
20+
*
21+
* To CUSTOMISE the baseline itself, copy this file to
22+
* qaConfig/phparkitect-rules-default.php and edit it — configPath resolves the
23+
* project copy first, and both the entry config and the
24+
* PHPQACI_ARKITECT_RULES_DEFAULT env var follow it.
25+
*
26+
* @return list<\Arkitect\Rules\ArchRule>
27+
*/
28+
29+
use Arkitect\Expression\ForClasses\HaveNameMatching;
30+
use Arkitect\Expression\ForClasses\IsEnum;
31+
use Arkitect\Expression\ForClasses\IsInterface;
32+
use Arkitect\Expression\ForClasses\IsTrait;
33+
use Arkitect\Rules\Rule;
34+
35+
// These rules match on AST node KIND (interface/enum/trait keyword) and so do
36+
// NOT force PHPArkitect to resolve class ancestry — that keeps them robust even
37+
// when a project references a base type that is not in the autoloader. Rules
38+
// that DO resolve ancestry (IsA/Extend/Implement, e.g. the *Exception
39+
// convention) live in the optional tier, because they require a complete
40+
// autoloader and are therefore not safe-everywhere.
41+
return [
42+
// Interfaces / enums / traits carry their kind in the suffix.
43+
// This tier is the SINGLE SOURCE OF TRUTH for type-suffix naming: the former
44+
// PHPStan RequireTypeSuffixRule was migrated here (structural naming belongs
45+
// in PHPArkitect, not PHPStan), so do not re-introduce a PHPStan equivalent.
46+
Rule::allClasses()
47+
->that(new IsInterface())
48+
->should(new HaveNameMatching('*Interface'))
49+
->because('an Interface suffix makes the symbol kind obvious at every use site'),
50+
Rule::allClasses()
51+
->that(new IsEnum())
52+
->should(new HaveNameMatching('*Enum'))
53+
->because('an Enum suffix makes the symbol kind obvious at every use site'),
54+
Rule::allClasses()
55+
->that(new IsTrait())
56+
->should(new HaveNameMatching('*Trait'))
57+
->because('a Trait suffix makes the symbol kind obvious at every use site'),
58+
];
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* php-qa-ci PHPArkitect — OPTIONAL Symfony ruleset (the
7+
* "rules-optional-symfony" tier).
8+
*
9+
* Symfony-specific naming conventions, OPT-IN (mirrors
10+
* rules-optional-symfony.neon for PHPStan). A Symfony project composes it into
11+
* its qaConfig/phparkitect.php:
12+
*
13+
* $default = require getenv('PHPQACI_ARKITECT_RULES_DEFAULT');
14+
* $symfony = require getenv('PHPQACI_ARKITECT_RULES_OPTIONAL_SYMFONY');
15+
* $config->add($classSet, ...$default, ...$symfony, ...$projectRules);
16+
*
17+
* Rules key off framework base types (resolved via the project's --autoload),
18+
* so they stay namespace-agnostic and work in any Symfony project.
19+
*
20+
* @return list<\Arkitect\Rules\ArchRule>
21+
*/
22+
23+
use Arkitect\Expression\ForClasses\HaveNameMatching;
24+
use Arkitect\Expression\ForClasses\IsA;
25+
use Arkitect\Rules\Rule;
26+
27+
return [
28+
// Console commands are suffixed *Command.
29+
Rule::allClasses()
30+
->that(new IsA('Symfony\\Component\\Console\\Command\\Command'))
31+
->should(new HaveNameMatching('*Command'))
32+
->because('a Command suffix makes console commands obvious and consistent'),
33+
34+
// Event subscribers are suffixed *Subscriber.
35+
Rule::allClasses()
36+
->that(new IsA('Symfony\\Component\\EventDispatcher\\EventSubscriberInterface'))
37+
->should(new HaveNameMatching('*Subscriber'))
38+
->because('a Subscriber suffix makes event subscribers obvious and consistent'),
39+
];
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* php-qa-ci PHPArkitect — OPTIONAL generic ruleset (the "rules-optional" tier).
7+
*
8+
* Stricter, opinionated naming conventions that are NOT safe to force on every
9+
* project, so they are OPT-IN — a project enables them by composing this file
10+
* into its qaConfig/phparkitect.php (mirrors rules-optional.neon for PHPStan):
11+
*
12+
* $default = require getenv('PHPQACI_ARKITECT_RULES_DEFAULT');
13+
* $optional = require getenv('PHPQACI_ARKITECT_RULES_OPTIONAL');
14+
* $config->add($classSet, ...$default, ...$optional, ...$projectRules);
15+
*
16+
* This is a curated set meant to grow. Keep each rule generic (no project
17+
* namespaces); project-specific rules belong in the project's own config.
18+
*
19+
* NOTE: the *Exception rule below keys off `IsA(\Throwable)`, which makes
20+
* PHPArkitect resolve each class's ancestry — so it requires a COMPLETE
21+
* autoloader (a class implementing/extending a type missing from
22+
* vendor/autoload.php aborts the run). That fragility is exactly why it is
23+
* opt-in here rather than in the safe-everywhere default tier.
24+
*
25+
* @return list<\Arkitect\Rules\ArchRule>
26+
*/
27+
28+
use Arkitect\Expression\ForClasses\HaveNameMatching;
29+
use Arkitect\Expression\ForClasses\IsA;
30+
use Arkitect\Expression\ForClasses\IsAbstract;
31+
use Arkitect\Rules\Rule;
32+
33+
return [
34+
// Every \Throwable is suffixed *Exception, so throw/catch sites and
35+
// signatures are unambiguous. (Ancestry-resolving — see NOTE above.)
36+
Rule::allClasses()
37+
->that(new IsA(\Throwable::class))
38+
->should(new HaveNameMatching('*Exception'))
39+
->because('a consistent Exception suffix makes throw/catch sites and signatures unambiguous'),
40+
41+
// Abstract classes are prefixed Abstract*, so the abstract-ness of a base
42+
// type is obvious at the use site without opening the file.
43+
Rule::allClasses()
44+
->that(new IsAbstract())
45+
->should(new HaveNameMatching('Abstract*'))
46+
->because('an Abstract prefix signals a non-instantiable base type at every use site'),
47+
];
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* php-qa-ci PHPArkitect — DEFAULT ENTRY CONFIG (on by default).
7+
*
8+
* configPath resolves this file whenever a project has NOT supplied its own
9+
* qaConfig/phparkitect.php, so every consuming project gets the generic-safe
10+
* default baseline (phparkitect-rules-default.php) applied to its detected
11+
* source directory — no per-project setup required. This is the arkitect
12+
* equivalent of the rules-default PHPStan baseline.
13+
*
14+
* A project takes over by adding qaConfig/phparkitect.php (use the template at
15+
* templates/qaConfig-phparkitect.php), where it can extend the defaults, opt
16+
* into the optional/symfony tiers, and add bespoke rules. To turn arkitect off
17+
* for a project entirely, set `export useArkitect=0` in qaConfig/qaConfig.inc.bash.
18+
*
19+
* The wrapper exports PHPQACI_ARKITECT_SRC_DIR (the pipeline's detected srcDir)
20+
* and PHPQACI_ARKITECT_RULES_DEFAULT (the resolved default ruleset path).
21+
*/
22+
23+
use Arkitect\ClassSet;
24+
use Arkitect\CLI\Config;
25+
26+
return static function (Config $config): void {
27+
$srcDir = getenv('PHPQACI_ARKITECT_SRC_DIR') ?: (getcwd() . '/src');
28+
29+
// Nothing to analyse (e.g. a project without a conventional src/) — pass cleanly.
30+
if (!\is_dir($srcDir)) {
31+
return;
32+
}
33+
34+
$defaultRulesFile = getenv('PHPQACI_ARKITECT_RULES_DEFAULT')
35+
?: __DIR__ . '/phparkitect-rules-default.php';
36+
$defaultRules = \is_file($defaultRulesFile) ? require $defaultRulesFile : [];
37+
38+
if ([] === $defaultRules) {
39+
fwrite(STDERR, "PHPArkitect: default ruleset resolved empty (looked at {$defaultRulesFile}) — NO rules applied.\n");
40+
41+
return;
42+
}
43+
44+
// Generated code is regenerated and cannot be renamed — never check it.
45+
$config->add(ClassSet::fromDir($srcDir)->excludePath('Generated'), ...$defaultRules);
46+
};

includes/functions.inc.bash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ function findTestsDir() {
277277

278278
function findSrcDir() {
279279
srcDir="$projectRoot/src"
280-
if [[ "" == "$srcDir" ]]; then
280+
if [[ ! -d "$srcDir" ]]; then
281281
echo "
282282
283283

includes/generic/allStaticAnalysisTools.inc.bash

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,13 @@ fi
1919

2020
echo "
2121
22+
Running PHPArkitect (architecture rules)
23+
----------------------------------------
24+
"
25+
runTool phpArkitect
26+
27+
echo "
28+
2229
Checking SensitiveParameter Usage
2330
---------------------------------
2431
"

0 commit comments

Comments
 (0)