-
Notifications
You must be signed in to change notification settings - Fork 0
feat: full quality gate v2 — PHPStan, Pint, Rector, Publish Guard #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
9f3f373
b3116c8
cc9a6de
832006e
507ef2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,72 @@ | ||||||||||||||||||||||
| <?php | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| declare(strict_types=1); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| namespace App\Checks; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| use App\Contracts\ProcessRunner; | ||||||||||||||||||||||
| use App\Services\SymfonyProcessRunner; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| final class PhpStanAnalyzer implements CheckInterface | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| public function __construct( | ||||||||||||||||||||||
| private readonly int $level = 5, | ||||||||||||||||||||||
| private readonly ProcessRunner $processRunner = new SymfonyProcessRunner, | ||||||||||||||||||||||
| ) {} | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function name(): string | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return 'PHPStan'; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public function run(string $workingDirectory): CheckResult | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| $binary = $workingDirectory.'/vendor/bin/phpstan'; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (! file_exists($binary)) { | ||||||||||||||||||||||
| return CheckResult::pass('PHPStan not installed — skipped'); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $result = $this->processRunner->run( | ||||||||||||||||||||||
| [$binary, 'analyse', '--no-progress', '--error-format=json', "--level={$this->level}", '--memory-limit=512M'], | ||||||||||||||||||||||
| $workingDirectory, | ||||||||||||||||||||||
| timeout: 300, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $data = json_decode($result->output, true); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if ($data === null) { | ||||||||||||||||||||||
| // Fall back to exit code if JSON parsing fails | ||||||||||||||||||||||
| if ($result->exitCode === 0) { | ||||||||||||||||||||||
| return CheckResult::pass('PHPStan passed (level '.$this->level.')'); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return CheckResult::fail( | ||||||||||||||||||||||
| 'PHPStan failed', | ||||||||||||||||||||||
| [substr($result->output, 0, 500)], | ||||||||||||||||||||||
| $result->output, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $errorCount = $data['totals']['errors'] ?? 0; | ||||||||||||||||||||||
| $fileErrors = $data['totals']['file_errors'] ?? 0; | ||||||||||||||||||||||
| $totalErrors = $errorCount + $fileErrors; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if ($totalErrors === 0) { | ||||||||||||||||||||||
| return CheckResult::pass('PHPStan passed (level '.$this->level.', 0 errors)'); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| $details = []; | ||||||||||||||||||||||
| foreach ($data['files'] ?? [] as $file => $fileData) { | ||||||||||||||||||||||
| foreach ($fileData['messages'] ?? [] as $msg) { | ||||||||||||||||||||||
| $details[] = basename($file).':'.$msg['line'].' — '.$msg['message']; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+60
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guard message fields before building detail lines. Line 62 assumes Suggested fix foreach ($data['files'] ?? [] as $file => $fileData) {
foreach ($fileData['messages'] ?? [] as $msg) {
- $details[] = basename($file).':'.$msg['line'].' — '.$msg['message'];
+ $line = $msg['line'] ?? '?';
+ $message = $msg['message'] ?? 'Unknown PHPStan error';
+ $details[] = basename($file).':'.$line.' — '.$message;
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return CheckResult::fail( | ||||||||||||||||||||||
| "PHPStan found {$totalErrors} error(s) at level {$this->level}", | ||||||||||||||||||||||
| array_slice($details, 0, 20), | ||||||||||||||||||||||
| $result->output, | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace App\Checks; | ||
|
|
||
| use App\Contracts\ProcessRunner; | ||
| use App\Services\SymfonyProcessRunner; | ||
|
|
||
| final class PintFormatter implements CheckInterface | ||
| { | ||
| public function __construct( | ||
| private readonly ProcessRunner $processRunner = new SymfonyProcessRunner, | ||
| ) {} | ||
|
|
||
| public function name(): string | ||
| { | ||
| return 'Pint Style'; | ||
| } | ||
|
|
||
| public function run(string $workingDirectory): CheckResult | ||
| { | ||
| $binary = $workingDirectory.'/vendor/bin/pint'; | ||
|
|
||
| if (! file_exists($binary)) { | ||
| return CheckResult::pass('Pint not installed — skipped'); | ||
| } | ||
|
|
||
| $result = $this->processRunner->run( | ||
| [$binary, '--test', '--format=json'], | ||
| $workingDirectory, | ||
| timeout: 120, | ||
| ); | ||
|
|
||
| $data = json_decode($result->output, true); | ||
|
|
||
| if ($data === null) { | ||
| if ($result->exitCode === 0) { | ||
| return CheckResult::pass('Code style is clean'); | ||
| } | ||
|
|
||
| return CheckResult::fail( | ||
| 'Pint check failed', | ||
| [substr($result->output, 0, 500)], | ||
| $result->output, | ||
| ); | ||
| } | ||
|
|
||
| if (($data['result'] ?? '') === 'pass') { | ||
| return CheckResult::pass('Code style is clean'); | ||
| } | ||
|
|
||
| $files = $data['files'] ?? []; | ||
| $details = []; | ||
|
|
||
| foreach (array_slice($files, 0, 20) as $f) { | ||
| if (is_array($f) && isset($f['path'])) { | ||
| $details[] = $f['path'].' ('.implode(', ', $f['fixers'] ?? []).')'; | ||
| } elseif (is_string($f)) { | ||
| $details[] = $f; | ||
| } | ||
| } | ||
|
|
||
| return CheckResult::fail( | ||
| count($files).' file(s) need formatting', | ||
| $details, | ||
| $result->output, | ||
| ); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace App\Checks; | ||
|
|
||
| use App\Contracts\ProcessRunner; | ||
| use App\Services\SymfonyProcessRunner; | ||
|
|
||
| final class PublishGuard implements CheckInterface | ||
| { | ||
| public function __construct( | ||
| private readonly ProcessRunner $processRunner = new SymfonyProcessRunner, | ||
| ) {} | ||
|
|
||
| public function name(): string | ||
| { | ||
| return 'Publish Guard'; | ||
| } | ||
|
|
||
| public function run(string $workingDirectory): CheckResult | ||
| { | ||
| $violations = []; | ||
|
|
||
| // Check for .env files (excluding .env.testing and .env.example) | ||
| $envResult = $this->processRunner->run( | ||
| ['git', 'ls-files', '*.env', '.env*'], | ||
| $workingDirectory, | ||
| timeout: 10, | ||
| ); | ||
|
|
||
| foreach (array_filter(explode("\n", trim($envResult->output))) as $file) { | ||
| if (! str_ends_with($file, '.env.testing') && ! str_ends_with($file, '.env.example')) { | ||
| $violations[] = "Tracked .env file: {$file}"; | ||
| } | ||
| } | ||
|
|
||
| // Check for source map files | ||
| $mapResult = $this->processRunner->run( | ||
| ['git', 'ls-files', '*.map'], | ||
| $workingDirectory, | ||
| timeout: 10, | ||
| ); | ||
|
|
||
| foreach (array_filter(explode("\n", trim($mapResult->output))) as $file) { | ||
| $violations[] = "Tracked .map file: {$file}"; | ||
| } | ||
|
|
||
| // Check for large files (> 1MB) | ||
| $lsResult = $this->processRunner->run( | ||
| ['git', 'ls-files'], | ||
| $workingDirectory, | ||
| timeout: 10, | ||
| ); | ||
|
Comment on lines
+26
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fail closed when Lines 26–54 parse command output without checking command success. A failed git call can bypass the guard and incorrectly report pass. Suggested fix $envResult = $this->processRunner->run(
['git', 'ls-files', '*.env', '.env*'],
$workingDirectory,
timeout: 10,
);
+ if (! $envResult->successful) {
+ return CheckResult::fail(
+ 'Publish Guard failed to inspect tracked .env files',
+ [trim($envResult->output)],
+ $envResult->output,
+ );
+ }
@@
$mapResult = $this->processRunner->run(
['git', 'ls-files', '*.map'],
$workingDirectory,
timeout: 10,
);
+ if (! $mapResult->successful) {
+ return CheckResult::fail(
+ 'Publish Guard failed to inspect tracked .map files',
+ [trim($mapResult->output)],
+ $mapResult->output,
+ );
+ }
@@
$lsResult = $this->processRunner->run(
['git', 'ls-files'],
$workingDirectory,
timeout: 10,
);
+ if (! $lsResult->successful) {
+ return CheckResult::fail(
+ 'Publish Guard failed to inspect tracked files',
+ [trim($lsResult->output)],
+ $lsResult->output,
+ );
+ }🤖 Prompt for AI Agents |
||
|
|
||
| foreach (array_filter(explode("\n", trim($lsResult->output))) as $file) { | ||
| $path = $workingDirectory.'/'.$file; | ||
| if (file_exists($path) && filesize($path) > 1_048_576) { | ||
| $size = round(filesize($path) / 1_048_576, 1); | ||
| $violations[] = "Large file ({$size}MB): {$file}"; | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+21
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Secrets check is missing from Publish Guard. This implementation enforces 🤖 Prompt for AI Agents |
||
| if ($violations === []) { | ||
| return CheckResult::pass('No disallowed artifacts found'); | ||
| } | ||
|
|
||
| return CheckResult::fail( | ||
| count($violations).' publish violation(s) found', | ||
| $violations, | ||
| implode("\n", $violations), | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||||||||||||||||||||||
| <?php | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| declare(strict_types=1); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| namespace App\Checks; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| use App\Contracts\ProcessRunner; | ||||||||||||||||||||||||||
| use App\Services\SymfonyProcessRunner; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| final class RectorAnalyzer implements CheckInterface | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| public function __construct( | ||||||||||||||||||||||||||
| private readonly ProcessRunner $processRunner = new SymfonyProcessRunner, | ||||||||||||||||||||||||||
| ) {} | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public function name(): string | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| return 'Rector'; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| public function run(string $workingDirectory): CheckResult | ||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||
| $binary = $workingDirectory.'/vendor/bin/rector'; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (! file_exists($binary)) { | ||||||||||||||||||||||||||
| return CheckResult::pass('Rector not installed — skipped'); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (! file_exists($workingDirectory.'/rector.php')) { | ||||||||||||||||||||||||||
| return CheckResult::pass('No rector.php config — skipped'); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| $result = $this->processRunner->run( | ||||||||||||||||||||||||||
| [$binary, 'process', '--dry-run', '--no-progress-bar'], | ||||||||||||||||||||||||||
| $workingDirectory, | ||||||||||||||||||||||||||
| timeout: 300, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if ($result->exitCode === 0) { | ||||||||||||||||||||||||||
| return CheckResult::pass('Rector found no issues'); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Count suggested changes from output | ||||||||||||||||||||||||||
| $lines = explode("\n", trim($result->output)); | ||||||||||||||||||||||||||
| $changeCount = 0; | ||||||||||||||||||||||||||
| $details = []; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| foreach ($lines as $line) { | ||||||||||||||||||||||||||
| if (str_contains($line, 'diff')) { | ||||||||||||||||||||||||||
| $changeCount++; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (str_starts_with(trim($line), '---') || str_starts_with(trim($line), '+++')) { | ||||||||||||||||||||||||||
| $details[] = trim($line); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| return CheckResult::fail( | ||||||||||||||||||||||||||
| 'Rector found '.max($changeCount, 1).' file(s) needing refactoring', | ||||||||||||||||||||||||||
| array_slice($details, 0, 20), | ||||||||||||||||||||||||||
|
Comment on lines
+57
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Failure count/message can be incorrect when no diffs are detected. Lines 57–59 force at least Suggested fix- return CheckResult::fail(
- 'Rector found '.max($changeCount, 1).' file(s) needing refactoring',
- array_slice($details, 0, 20),
- $result->output,
- );
+ $message = $changeCount > 0
+ ? "Rector found {$changeCount} file(s) needing refactoring"
+ : 'Rector failed to run cleanly; see raw output';
+
+ return CheckResult::fail(
+ $message,
+ array_slice($details, 0, 20),
+ $result->output,
+ );📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| $result->output, | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
phpstan-level=-1skip behavior is not implemented.The analyzer always runs and forwards the level, so a configured skip value still invokes PHPStan. Add an explicit skip branch and validate allowed range before execution.
Suggested fix
public function run(string $workingDirectory): CheckResult { + if ($this->level === -1) { + return CheckResult::pass('PHPStan skipped (level -1)'); + } + + if ($this->level < 0 || $this->level > 9) { + return CheckResult::fail( + "Invalid PHPStan level: {$this->level}. Expected -1 or 0-9." + ); + } + $binary = $workingDirectory.'/vendor/bin/phpstan';📝 Committable suggestion
🤖 Prompt for AI Agents