Conversation
Replace the namespace+suffix lookup (broken by `array_merge_recursive` for framework presets) with an ordered list of glob LayerRule (first match wins). This fixes the headline false positive on Laravel: `App\Actions\*` was previously misclassified as Controller, generating ~12 spurious "Infrastructure → Controller" violations on a typical app. Add a new `Wiring` layer for ServiceProviders, container extensions, DI bindings — exempt from layer-violation reporting because by nature they must reference concrete classes from any layer. Detection by glob pattern + extends Illuminate\Support\ServiceProvider / Symfony bundles. Extend ProjectTypeInterface with three new methods (defaults in AbstractProjectType): getLayerRules(), getWiringPatterns(), getDipIgnoreList(). LaravelProjectType and SymfonyProjectType ship real opinionated presets so `--type=laravel` / `--type=symfony` is no longer a near-no-op. ArchitectureAnalyzer is rewired to consume these via project type and (in upcoming commits) ProjectConfig. Tests: LayerDetectorTest extended with anti-regression cases covering the Action→Application reclassification, ServiceProvider→Wiring, glob matching semantics, and project-supplied custom rules. Refs: A1, A2 of the diagnostic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DIP detection no longer counts every non-`use` dependency. It is now
restricted to truly *injected* dependencies: type_hint_param,
type_hint_return, type_hint_property. Traits, extends, new, static_call,
const, instanceof, catch, implements, attributes — none can be substituted
via DI, so they are excluded.
This alone eliminates the pathological case of OrganizationController
flagged with 104/104 concrete dependencies (most of which were OpenAPI
attributes and use statements). On the Partoo Laravel project the count
falls to a defensible 10/10 actually-injected dependencies.
DependencyVisitor stores granular kinds (type_hint_param/return/property)
instead of a single 'type_hint'. SolidAnalyzer dedupes by FQN (a class
type-hinting `Foo` in three params counts once, not three times).
Framework-supplied concretes are excluded via getDipIgnoreList():
- Laravel: Eloquent Model / Relations / Builder, Carbon, Closure,
facades, queue/bus traits (Queueable, Dispatchable, Serializes…),
Http\Request / Response, OpenAPI attributes, App\Models\**, App\Data\**.
- Symfony: Doctrine ORM, HttpFoundation, Form, Validator, Security,
Messenger, Routing, Serializer, DI, Twig.
Wiring classes (Providers, container extensions) are exempt from DIP
scoring as well — binding concretes is their entire purpose.
Bug also fixed: a `$fqn` shadowing in detectDipViolations() caused the
violation's `className` field to point to the last dependency seen
instead of the analysed class. Renamed to `$depFqn`.
Tests: SolidAnalyzerTest extended with anti-regression cases for
Queueable/Dispatchable jobs, Eloquent type-hints, FQN dedup, wiring
exemption, and the DIP scope reduction itself.
Refs: B1, B2 of the diagnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a project-level configuration file at the root of the analyzed
project. Schema (all keys optional):
{
"layers": { "rules": [{ "match": "App\\Foo\\**", "layer": "Application" }] },
"wiring": { "patterns": ["**ServiceProvider"] },
"abstractionRatio": { "ignore": ["App\\Models\\**"] },
"ignore": { "violations": ["solid.dip:App\\Foo\\Bar"] },
"baseline": "phpquality.baseline.json"
}
Merge semantics with the framework preset:
- layers.rules: project rules are PREPENDED (first match wins)
- wiring.patterns: union with framework patterns
- abstractionRatio.ignore: union with framework whitelist
- ignore.violations: pre-canonicalised "code:FQN" pairs
- baseline: relative path resolved against the source dir
ProjectAnalyzer gains setConfigPath(), setBaselinePath(),
setGenerateBaselinePath() and now requires ProjectConfigLoader +
BaselineManager (see next commit) via Symfony autowiring. The CLI
exposes --config / --baseline / --generate-baseline.
This makes `--type=laravel` truly a preset that the project can extend
without forking the bundle.
Tests: ProjectConfigLoaderTest covers fallback to framework defaults,
prepend / union merge semantics, baseline path resolution (relative vs
absolute), invalid JSON, and explicit --config override.
Refs: D2 of the diagnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two complementary suppression mechanisms — both make the tool usable
on existing projects without rewriting them.
== @phpquality-ignore docblock annotation ==
Parsed by IgnoreAnnotationParser from the docblock of any class /
interface during AST traversal. Recognised codes: solid.srp, solid.dip,
solid.isp, architecture.layer. Comma-separated values supported,
end-of-line "— rationale" trailing text is ignored.
/**
* @phpquality-ignore solid.dip — wiring intentionnel
*/
final class FooService { /* … */ }
ArchitectureAnalyzer collects per-class ignores from DependencyVisitor
output and the project config (`ignore.violations` keys) before invoking
SOLID and layer-violation checks.
== Baseline ==
BaselineManager generates a stable hash per violation
(filePath | line | code | source→target / className) and serialises a
sorted, deduplicated list. The hash strips the path prefix up to /src/
or /app/ so the file is comparable across machines.
phpquality:analyze … --generate-baseline=phpquality.baseline.json
phpquality:analyze … --baseline=phpquality.baseline.json
The latter filters violations whose hash is in the baseline; the
analyser exposes summary.suppressedByBaseline. Entries that match no
current violation are reported as obsolete (regenerate-please warning).
`--generate-baseline` is exclusive of `--fail-on-violation` (by design,
running it accepts the current state).
Tests: IgnoreAnnotationParserTest (8 cases: case-insensitive tag,
comma-separated, dedup, "—" terminator, multi-line); BaselineManagerTest
(round-trip, partial application, obsolete-entry detection, path
normalisation, malformed-file handling).
Refs: D1 of the diagnostic.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end validation of the four PR2.0 changes (A1, A2, B1, B2, D1, D2)
on a real Laravel project (Partoo storeloc-saas, 136 files, 110 classes):
Layer violations: 12+ → 1 (the one remaining is a real
Domain → Infrastructure violation)
SOLID violations: 100+ → 6 (all legitimate; biggest controller is
10/10 instead of 104/104)
AppServiceProvider: flagged → exempt (Wiring layer)
Architecture score: B (73)
Add an integration test under tests/Integration/ that pins those
behaviours on a self-contained laravel-mini fixture (Action, Job,
Provider, Controller, Repository, Model, ignore-annotated Service):
- testActionIsApplicationNotController (A1)
- testServiceProviderIsWiring (A2)
- testNoLayerViolationsOnCommonLaravelStructure (A2)
- testJobUsingQueueableHasNoDipViolation (B1+B2)
- testIgnoreAnnotationSuppressesBillingServiceDip (D1)
- testBaselineRoundTripSuppressesAllRemainingViolations (D1)
A standalone PHP smoke runner is also included (run-partoo-smoke.php)
for ad-hoc execution against any local Laravel/Symfony project, no
Symfony kernel required.
Documentation:
- CHANGELOG.md gets a 2.0.0 section listing every breaking change
with rationale and a "Migrating from 1.x" subsection.
- README.md adds a "Migrating from 1.x" preamble and documents the
new options (--config, --baseline, --generate-baseline, --wizard),
the phpquality.json schema, the @phpquality-ignore annotation,
and a baseline-based CI workflow.
- DOCKERHUB.md mirrors the new options + a "What's new in 2.0"
blurb pointing at the changelog.
- composer.json branch-alias bumped to 2.0.x-dev.
Total test count: 510 → 544. All green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
End-to-end refactor that turns
--type=laravelinto a real, opinionatedpreset. On a real Laravel project (Partoo, 136 files, 110 classes):
Domain → Infrastructure)OrganizationControllerDIPWhat's in this PR
feat(arch): a98f3c6)LayerDetectornow consumes orderedLayerRuleglob patterns (first match wins) instead ofarray_merge_recursiveof namespace + suffix lookups.Wiringlayer forServiceProvider/ DI extensions, exempt from layer-violation reporting because by nature they reference concrete classes from any layer.LaravelProjectTypeandSymfonyProjectTypeship full opinionated rules (App\Actions\**→ Application,App\Providers\**→ Wiring, etc.).feat(solid): 6c56690)type_hint_param,type_hint_return,type_hint_property). Traits,extends,new, static calls, attributes, instanceof, catches, implements — none are counted anymore.$fqnshadowing indetectDipViolationswas making the violationclassNamepoint to the last seen dependency instead of the analysed class.phpquality.jsonproject configuration (feat(config): 42a549d)--config.@phpquality-ignoreannotations + baseline (feat(config): 7e14434)solid.srp,solid.dip,solid.isp,architecture.layer.BaselineManagerproduces a stable hash per violation. New CLI:--generate-baseline,--baseline. Report exposessummary.suppressedByBaselineand warns about obsolete entries.release: 701479b)tests/Integration/pinning the six anti-regression assertions on a self-contained Laravel-mini fixture.CHANGELOG.md2.0.0 section with full breaking-change list and "Migrating from 1.x".README.mdpreamble + new options +phpquality.jsonschema + baseline workflow.DOCKERHUB.mdmirrored.composer.jsonbranch-alias bumped to2.0.x-dev.Breaking changes
*Actionis no longer Controller in Laravel — it maps toApplication.Wiringis a new layer;ServiceProviderno longer generates layer violations.ProjectTypeInterfacegains 3 methods (defaults provided inAbstractProjectType).ProjectAnalyzerconstructor now requiresProjectConfigLoader+BaselineManager(Symfony autowiring picks them up).SolidAnalyzer::analyze()andArchitectureAnalyzer::analyze()signatures gained optional context parameters.Test plan
vendor/bin/phpunit— 544 / 544 green (was 510 before this PR; +34 new tests acrossLayerDetectorTest,SolidAnalyzerTest,ProjectConfigLoaderTest,BaselineManagerTest,IgnoreAnnotationParserTest, and the newLaravelEndToEndTest).~/src/partoo/storeloc-saas/appviatests/Integration/run-partoo-smoke.php— confirms the headline numbers above.v2.0.0to trigger the Docker Hub workflow (amoifr13/phpquality:2.0.0/2.0/2/latest) and Packagist update.Migration
ProjectTypeInterfaceimplementations: extendAbstractProjectType(gets defaults for free) or implementgetLayerRules()/getDipIgnoreList()/getWiringPatterns().phpquality:analyze ... --generate-baseline=phpquality.baseline.json # subsequent runs: phpquality:analyze ... --baseline=phpquality.baseline.json --fail-on-violation🤖 Generated with Claude Code