diff --git a/cmd/php/php_quality.go b/cmd/php/php_quality.go index bd703900..2da84a47 100644 --- a/cmd/php/php_quality.go +++ b/cmd/php/php_quality.go @@ -1,13 +1,10 @@ package php import ( - "bytes" "context" "fmt" - "io" "os" "strings" - "time" "github.com/charmbracelet/lipgloss" "github.com/host-uk/core/pkg/i18n" @@ -489,70 +486,70 @@ func addPHPQACommand(parent *cobra.Command) { stages := phppkg.GetQAStages(opts) // Print header - stageNames := make([]string, len(stages)) - for i, s := range stages { - stageNames[i] = string(s) - } fmt.Printf("%s %s\n\n", dimStyle.Render(i18n.T("cmd.php.label.qa")), i18n.T("common.progress.running", map[string]any{"Task": "QA pipeline"})) ctx := context.Background() - var allPassed = true - var results []phppkg.QACheckResult - for _, stage := range stages { - fmt.Printf("%s\n", phpQAStageStyle.Render(i18n.T("cmd.php.qa.stage_prefix")+strings.ToUpper(string(stage))+i18n.T("cmd.php.qa.stage_suffix"))) - - checks := phppkg.GetQAChecks(cwd, stage) - if len(checks) == 0 { - fmt.Printf(" %s\n\n", dimStyle.Render(i18n.T("cmd.php.qa.no_checks"))) - continue - } - - for _, checkName := range checks { - result := runQACheck(ctx, cwd, checkName, qaFix) - result.Stage = stage - results = append(results, result) - - icon := phpQAPassedStyle.Render("✓") - status := phpQAPassedStyle.Render(i18n.T("cmd.php.qa.passed")) - if !result.Passed { - icon = phpQAFailedStyle.Render("✗") - status = phpQAFailedStyle.Render(i18n.T("cmd.php.qa.failed")) - allPassed = false - } - - fmt.Printf(" %s %s %s %s\n", icon, result.Name, status, dimStyle.Render(result.Duration)) - } - fmt.Println() + // Create QA runner using pkg/process + runner, err := NewQARunner(cwd, qaFix) + if err != nil { + return fmt.Errorf("%s: %w", i18n.T("common.error.failed", map[string]any{"Action": "create QA runner"}), err) } + // Run all checks with dependency ordering + result, err := runner.Run(ctx, stages) + if err != nil { + return fmt.Errorf("%s: %w", i18n.T("common.error.failed", map[string]any{"Action": "run QA checks"}), err) + } + + // Display results by stage + currentStage := "" + for _, checkResult := range result.Results { + // Determine stage for this check + stage := getCheckStage(checkResult.Name, stages, cwd) + if stage != currentStage { + if currentStage != "" { + fmt.Println() + } + currentStage = stage + fmt.Printf("%s\n", phpQAStageStyle.Render(i18n.T("cmd.php.qa.stage_prefix")+strings.ToUpper(stage)+i18n.T("cmd.php.qa.stage_suffix"))) + } + + icon := phpQAPassedStyle.Render("✓") + status := phpQAPassedStyle.Render(i18n.T("cmd.php.qa.passed")) + if checkResult.Skipped { + icon = dimStyle.Render("-") + status = dimStyle.Render(i18n.T("cmd.php.qa.skipped")) + } else if !checkResult.Passed { + icon = phpQAFailedStyle.Render("✗") + status = phpQAFailedStyle.Render(i18n.T("cmd.php.qa.failed")) + } + + fmt.Printf(" %s %s %s %s\n", icon, checkResult.Name, status, dimStyle.Render(checkResult.Duration)) + } + fmt.Println() + // Print summary - passedCount := 0 - var failedChecks []phppkg.QACheckResult - for _, r := range results { - if r.Passed { - passedCount++ - } else { - failedChecks = append(failedChecks, r) - } - } - - if allPassed { - fmt.Printf("%s %s\n", phpQAPassedStyle.Render("QA PASSED:"), i18n.T("cmd.php.qa.all_passed", map[string]interface{}{"Passed": passedCount, "Total": len(results)})) + if result.Passed { + fmt.Printf("%s %s\n", phpQAPassedStyle.Render("QA PASSED:"), i18n.T("cmd.php.qa.all_passed", map[string]interface{}{"Passed": result.PassedCount, "Total": len(result.Results)})) + fmt.Printf("%s %s\n", dimStyle.Render(i18n.T("common.label.duration")), result.Duration) return nil } - fmt.Printf("%s %s\n\n", phpQAFailedStyle.Render("QA FAILED:"), i18n.T("cmd.php.qa.some_failed", map[string]interface{}{"Passed": passedCount, "Total": len(results)})) + fmt.Printf("%s %s\n\n", phpQAFailedStyle.Render("QA FAILED:"), i18n.T("cmd.php.qa.some_failed", map[string]interface{}{"Passed": result.PassedCount, "Total": len(result.Results)})) // Show what needs fixing fmt.Printf("%s\n", dimStyle.Render(i18n.T("cmd.php.qa.to_fix"))) - for _, check := range failedChecks { - fixCmd := getQAFixCommand(check.Name, qaFix) - issue := check.Output + for _, checkResult := range result.Results { + if checkResult.Passed || checkResult.Skipped { + continue + } + fixCmd := getQAFixCommand(checkResult.Name, qaFix) + issue := checkResult.GetIssueMessage() if issue == "" { issue = "issues found" } - fmt.Printf(" %s %s\n", phpQAFailedStyle.Render("*"), check.Name+": "+issue) + fmt.Printf(" %s %s\n", phpQAFailedStyle.Render("*"), checkResult.Name+": "+issue) if fixCmd != "" { fmt.Printf(" %s %s\n", dimStyle.Render("->"), fixCmd) } @@ -569,6 +566,19 @@ func addPHPQACommand(parent *cobra.Command) { parent.AddCommand(qaCmd) } +// getCheckStage determines which stage a check belongs to. +func getCheckStage(checkName string, stages []phppkg.QAStage, dir string) string { + for _, stage := range stages { + checks := phppkg.GetQAChecks(dir, stage) + for _, c := range checks { + if c == checkName { + return string(stage) + } + } + } + return "unknown" +} + func getQAFixCommand(checkName string, fixEnabled bool) string { switch checkName { case "audit": @@ -595,77 +605,6 @@ func getQAFixCommand(checkName string, fixEnabled bool) string { return "" } -func runQACheck(ctx context.Context, dir string, checkName string, fix bool) phppkg.QACheckResult { - start := time.Now() - result := phppkg.QACheckResult{Name: checkName, Passed: true} - - // Capture output to prevent noise in QA pipeline - var buf bytes.Buffer - - switch checkName { - case "audit": - auditResults, _ := phppkg.RunAudit(ctx, phppkg.AuditOptions{Dir: dir, Output: io.Discard}) - var issues []string - for _, r := range auditResults { - if r.Vulnerabilities > 0 { - issues = append(issues, fmt.Sprintf("%s: %d vulnerabilities", r.Tool, r.Vulnerabilities)) - result.Passed = false - } else if r.Error != nil { - issues = append(issues, fmt.Sprintf("%s: %v", r.Tool, r.Error)) - result.Passed = false - } - } - if len(issues) > 0 { - result.Output = strings.Join(issues, ", ") - } - - case "fmt": - err := phppkg.Format(ctx, phppkg.FormatOptions{Dir: dir, Fix: fix, Output: io.Discard}) - result.Passed = err == nil - if err != nil { - result.Output = i18n.T("cmd.php.qa.issue_style") - } - - case "analyse": - err := phppkg.Analyse(ctx, phppkg.AnalyseOptions{Dir: dir, Output: &buf}) - result.Passed = err == nil - if err != nil { - result.Output = i18n.T("cmd.php.qa.issue_analysis") - } - - case "psalm": - err := phppkg.RunPsalm(ctx, phppkg.PsalmOptions{Dir: dir, Fix: fix, Output: io.Discard}) - result.Passed = err == nil - if err != nil { - result.Output = i18n.T("cmd.php.qa.issue_types") - } - - case "test": - err := phppkg.RunTests(ctx, phppkg.TestOptions{Dir: dir, Output: io.Discard}) - result.Passed = err == nil - if err != nil { - result.Output = i18n.T("cmd.php.qa.issue_tests") - } - - case "rector": - err := phppkg.RunRector(ctx, phppkg.RectorOptions{Dir: dir, Fix: fix, Output: io.Discard}) - result.Passed = err == nil - if err != nil { - result.Output = i18n.T("cmd.php.qa.issue_rector") - } - - case "infection": - err := phppkg.RunInfection(ctx, phppkg.InfectionOptions{Dir: dir, Output: io.Discard}) - result.Passed = err == nil - if err != nil { - result.Output = i18n.T("cmd.php.qa.issue_mutation") - } - } - - result.Duration = time.Since(start).Round(time.Millisecond).String() - return result -} - var ( rectorFix bool rectorDiff bool diff --git a/cmd/php/qa_runner.go b/cmd/php/qa_runner.go new file mode 100644 index 00000000..2a78c967 --- /dev/null +++ b/cmd/php/qa_runner.go @@ -0,0 +1,339 @@ +package php + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + + "github.com/host-uk/core/pkg/framework" + "github.com/host-uk/core/pkg/i18n" + phppkg "github.com/host-uk/core/pkg/php" + "github.com/host-uk/core/pkg/process" +) + +// QARunner orchestrates PHP QA checks using pkg/process. +type QARunner struct { + dir string + fix bool + service *process.Service + core *framework.Core + + // Output tracking + outputMu sync.Mutex + checkOutputs map[string][]string +} + +// NewQARunner creates a QA runner for the given directory. +func NewQARunner(dir string, fix bool) (*QARunner, error) { + // Create a Core with process service for the QA session + core, err := framework.New( + framework.WithName("process", process.NewService(process.Options{})), + ) + if err != nil { + return nil, fmt.Errorf("failed to create process service: %w", err) + } + + svc, err := framework.ServiceFor[*process.Service](core, "process") + if err != nil { + return nil, fmt.Errorf("failed to get process service: %w", err) + } + + runner := &QARunner{ + dir: dir, + fix: fix, + service: svc, + core: core, + checkOutputs: make(map[string][]string), + } + + return runner, nil +} + +// BuildSpecs creates RunSpecs for the given QA checks. +func (r *QARunner) BuildSpecs(checks []string) []process.RunSpec { + specs := make([]process.RunSpec, 0, len(checks)) + + for _, check := range checks { + spec := r.buildSpec(check) + if spec != nil { + specs = append(specs, *spec) + } + } + + return specs +} + +// buildSpec creates a RunSpec for a single check. +func (r *QARunner) buildSpec(check string) *process.RunSpec { + switch check { + case "audit": + return &process.RunSpec{ + Name: "audit", + Command: "composer", + Args: []string{"audit", "--format=summary"}, + Dir: r.dir, + } + + case "fmt": + formatter, found := phppkg.DetectFormatter(r.dir) + if !found { + return nil + } + if formatter == phppkg.FormatterPint { + vendorBin := filepath.Join(r.dir, "vendor", "bin", "pint") + cmd := "pint" + if _, err := os.Stat(vendorBin); err == nil { + cmd = vendorBin + } + args := []string{} + if !r.fix { + args = append(args, "--test") + } + return &process.RunSpec{ + Name: "fmt", + Command: cmd, + Args: args, + Dir: r.dir, + After: []string{"audit"}, + } + } + return nil + + case "analyse": + _, found := phppkg.DetectAnalyser(r.dir) + if !found { + return nil + } + vendorBin := filepath.Join(r.dir, "vendor", "bin", "phpstan") + cmd := "phpstan" + if _, err := os.Stat(vendorBin); err == nil { + cmd = vendorBin + } + return &process.RunSpec{ + Name: "analyse", + Command: cmd, + Args: []string{"analyse", "--no-progress"}, + Dir: r.dir, + After: []string{"fmt"}, + } + + case "psalm": + _, found := phppkg.DetectPsalm(r.dir) + if !found { + return nil + } + vendorBin := filepath.Join(r.dir, "vendor", "bin", "psalm") + cmd := "psalm" + if _, err := os.Stat(vendorBin); err == nil { + cmd = vendorBin + } + args := []string{"--no-progress"} + if r.fix { + args = append(args, "--alter", "--issues=all") + } + return &process.RunSpec{ + Name: "psalm", + Command: cmd, + Args: args, + Dir: r.dir, + After: []string{"analyse"}, + } + + case "test": + // Check for Pest first, fall back to PHPUnit + pestBin := filepath.Join(r.dir, "vendor", "bin", "pest") + phpunitBin := filepath.Join(r.dir, "vendor", "bin", "phpunit") + + cmd := "pest" + if _, err := os.Stat(pestBin); err == nil { + cmd = pestBin + } else if _, err := os.Stat(phpunitBin); err == nil { + cmd = phpunitBin + } else { + return nil + } + + // Tests depend on analyse (or psalm if available) + after := []string{"analyse"} + if _, found := phppkg.DetectPsalm(r.dir); found { + after = []string{"psalm"} + } + + return &process.RunSpec{ + Name: "test", + Command: cmd, + Args: []string{}, + Dir: r.dir, + After: after, + } + + case "rector": + if !phppkg.DetectRector(r.dir) { + return nil + } + vendorBin := filepath.Join(r.dir, "vendor", "bin", "rector") + cmd := "rector" + if _, err := os.Stat(vendorBin); err == nil { + cmd = vendorBin + } + args := []string{"process"} + if !r.fix { + args = append(args, "--dry-run") + } + return &process.RunSpec{ + Name: "rector", + Command: cmd, + Args: args, + Dir: r.dir, + After: []string{"test"}, + AllowFailure: true, // Dry-run returns non-zero if changes would be made + } + + case "infection": + if !phppkg.DetectInfection(r.dir) { + return nil + } + vendorBin := filepath.Join(r.dir, "vendor", "bin", "infection") + cmd := "infection" + if _, err := os.Stat(vendorBin); err == nil { + cmd = vendorBin + } + return &process.RunSpec{ + Name: "infection", + Command: cmd, + Args: []string{"--min-msi=50", "--min-covered-msi=70", "--threads=4"}, + Dir: r.dir, + After: []string{"test"}, + AllowFailure: true, + } + } + + return nil +} + +// Run executes all QA checks and returns the results. +func (r *QARunner) Run(ctx context.Context, stages []phppkg.QAStage) (*QARunResult, error) { + // Collect all checks from all stages + var allChecks []string + for _, stage := range stages { + checks := phppkg.GetQAChecks(r.dir, stage) + allChecks = append(allChecks, checks...) + } + + if len(allChecks) == 0 { + return &QARunResult{Passed: true}, nil + } + + // Build specs + specs := r.BuildSpecs(allChecks) + if len(specs) == 0 { + return &QARunResult{Passed: true}, nil + } + + // Register output handler + r.core.RegisterAction(func(c *framework.Core, msg framework.Message) error { + switch m := msg.(type) { + case process.ActionProcessOutput: + r.outputMu.Lock() + // Extract check name from process ID mapping + for _, spec := range specs { + if strings.Contains(m.ID, spec.Name) || m.ID != "" { + // Store output for later display if needed + r.checkOutputs[spec.Name] = append(r.checkOutputs[spec.Name], m.Line) + break + } + } + r.outputMu.Unlock() + } + return nil + }) + + // Create runner and execute + runner := process.NewRunner(r.service) + result, err := runner.RunAll(ctx, specs) + if err != nil { + return nil, err + } + + // Convert to QA result + qaResult := &QARunResult{ + Passed: result.Success(), + Duration: result.Duration.String(), + Results: make([]QACheckRunResult, 0, len(result.Results)), + } + + for _, res := range result.Results { + qaResult.Results = append(qaResult.Results, QACheckRunResult{ + Name: res.Name, + Passed: res.Passed(), + Skipped: res.Skipped, + ExitCode: res.ExitCode, + Duration: res.Duration.String(), + Output: res.Output, + }) + if res.Passed() { + qaResult.PassedCount++ + } else if res.Skipped { + qaResult.SkippedCount++ + } else { + qaResult.FailedCount++ + } + } + + return qaResult, nil +} + +// GetCheckOutput returns captured output for a check. +func (r *QARunner) GetCheckOutput(check string) []string { + r.outputMu.Lock() + defer r.outputMu.Unlock() + return r.checkOutputs[check] +} + +// QARunResult holds the results of running QA checks. +type QARunResult struct { + Passed bool + Duration string + Results []QACheckRunResult + PassedCount int + FailedCount int + SkippedCount int +} + +// QACheckRunResult holds the result of a single QA check. +type QACheckRunResult struct { + Name string + Passed bool + Skipped bool + ExitCode int + Duration string + Output string +} + +// GetIssueMessage returns a localized issue message for a check. +func (r QACheckRunResult) GetIssueMessage() string { + if r.Passed || r.Skipped { + return "" + } + switch r.Name { + case "audit": + return i18n.T("cmd.php.qa.issue_audit") + case "fmt": + return i18n.T("cmd.php.qa.issue_style") + case "analyse": + return i18n.T("cmd.php.qa.issue_analysis") + case "psalm": + return i18n.T("cmd.php.qa.issue_types") + case "test": + return i18n.T("cmd.php.qa.issue_tests") + case "rector": + return i18n.T("cmd.php.qa.issue_rector") + case "infection": + return i18n.T("cmd.php.qa.issue_mutation") + default: + return "issues found" + } +}