From 56d3d333002fa6c7ca1b33b7b5e32b33bdb80054 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Thu, 24 Sep 2026 15:34:44 +0200 Subject: [PATCH 01/14] fix: extension validate + fix + format should report what actually ran Assisted-by: OpenAI Codex --- cmd/extension/extension_validate.go | 138 +++++++++++++----- .../extension_validate_selection_test.go | 88 +++++++++++ internal/validation/reporter.go | 123 ++++++++++++++-- internal/validation/reporter_test.go | 117 +++++++++++++++ 4 files changed, 412 insertions(+), 54 deletions(-) create mode 100644 cmd/extension/extension_validate_selection_test.go diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index 01f66e96..b0e774a0 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -1,9 +1,12 @@ package extension import ( + "errors" "fmt" "os" "path/filepath" + "slices" + "strings" "time" "github.com/spf13/cobra" @@ -28,19 +31,15 @@ var extensionValidateCmd = &cobra.Command{ return err } checkAgainst, _ := cmd.Flags().GetString("check-against") - tmpDir, err := os.MkdirTemp(os.TempDir(), "analyse-extension-*") only, _ := cmd.Flags().GetString("only") exclude, _ := cmd.Flags().GetString("exclude") noCopy, _ := cmd.Flags().GetBool("no-copy") - // If the user does not want to run full validation, only run shopware-cli - if !isFull { - only = "sw-cli" - } - + tools, coverage, err := selectExtensionValidationTools(isFull, only, exclude) if err != nil { - return fmt.Errorf("cannot create temporary directory: %w", err) + return err } + needsTools := slices.ContainsFunc(tools, requiresToolSetup) path, err := filepath.Abs(args[0]) if err != nil { @@ -54,17 +53,14 @@ var extensionValidateCmd = &cobra.Command{ var toolCfg *verifier.ToolConfig if stat.IsDir() { + validationPath := path if noCopy { - tmpDir = path logging.FromContext(cmd.Context()).Debugf("Skipping copying extension files to temporary directory due to --no-copy flag") - } else if isFull { - beforeCopyTime := time.Now() - if err := system.CopyFiles(args[0], tmpDir); err != nil { - return err + } else if needsTools { + tmpDir, err := os.MkdirTemp(os.TempDir(), "analyse-extension-*") + if err != nil { + return fmt.Errorf("cannot create temporary directory: %w", err) } - - logging.FromContext(cmd.Context()).Debugf("Copied extension files to temporary directory in %s", time.Since(beforeCopyTime).String()) - defer func() { beforeDeleteTime := time.Now() if err := os.RemoveAll(tmpDir); err != nil { @@ -72,11 +68,17 @@ var extensionValidateCmd = &cobra.Command{ } logging.FromContext(cmd.Context()).Debugf("Removed temporary directory in %s", time.Since(beforeDeleteTime).String()) }() - } else if !isFull { - tmpDir = args[0] + + beforeCopyTime := time.Now() + if err := system.CopyFiles(path, tmpDir); err != nil { + return err + } + + logging.FromContext(cmd.Context()).Debugf("Copied extension files to temporary directory in %s", time.Since(beforeCopyTime).String()) + validationPath = tmpDir } - ext, err := extension.GetExtensionByFolder(cmd.Context(), tmpDir) + ext, err := extension.GetExtensionByFolder(cmd.Context(), validationPath) if err != nil { return err } @@ -109,20 +111,14 @@ var extensionValidateCmd = &cobra.Command{ result := verifier.NewCheck() result.SetSourceRoot(toolCfg.RootDir) - var gr errgroup.Group - - tools := verifier.GetTools() - - tools, err = tools.Only(only) - if err != nil { - return err - } - - tools, err = tools.Exclude(exclude) - if err != nil { - return err + if needsTools { + if err := verifier.SetupTools(cmd.Context(), cmd.Root().Version); err != nil { + return err + } + toolCfg.ToolDirectory = verifier.GetToolDirectory() } + var gr errgroup.Group for _, tool := range tools { tool := tool gr.Go(func() error { @@ -134,10 +130,78 @@ var extensionValidateCmd = &cobra.Command{ return err } - return validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat) + return validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, coverage...) }, } +// These tools have a real Check implementation; other verifier tools may only fix or format. +var extensionValidationToolNames = []string{"admin-twig", "eslint", "phpstan", "storefront-twig", "stylelint", "sw-cli"} + +func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList, []validation.CheckCoverage, error) { + requested := only + if requested == "" { + requested = "sw-cli" + if full { + requested = strings.Join(extensionValidationToolNames, ",") + } + } + selected, err := verifier.GetTools().Only(requested) + if err != nil { + return nil, nil, err + } + requestedNames := make(map[string]bool, len(selected)) + for _, tool := range selected { + name := tool.Name() + if !slices.Contains(extensionValidationToolNames, name) { + return nil, nil, fmt.Errorf("%s does not provide a validation check", name) + } + requestedNames[name] = true + } + selected, err = selected.Exclude(exclude) + if err != nil { + return nil, nil, err + } + if len(selected) == 0 { + return nil, nil, errors.New("no validation checks selected after applying --exclude") + } + + unique := make(verifier.ToolList, 0, len(selected)) + invoked := make(map[string]bool, len(selected)) + for _, tool := range selected { + if invoked[tool.Name()] { + continue + } + unique = append(unique, tool) + invoked[tool.Name()] = true + } + + coverage := make([]validation.CheckCoverage, 0, len(extensionValidationToolNames)) + for _, name := range extensionValidationToolNames { + check := validation.CheckCoverage{Name: name, Status: "skipped"} + switch { + case invoked[name]: + check.Status = "invoked" + case requestedNames[name]: + check.Reason = "excluded by --exclude" + case only != "": + check.Reason = "not selected by --only" + default: + check.Reason = "not selected; use --full or --only" + } + coverage = append(coverage, check) + } + return unique, coverage, nil +} + +func requiresToolSetup(tool verifier.Tool) bool { + switch tool.(type) { + case verifier.PhpStan, verifier.Eslint, verifier.StyleLint: + return true + default: + return false + } +} + func extensionValidationFormat(cmd *cobra.Command) (string, error) { format, _ := cmd.Flags().GetString("format") reporter, _ := cmd.Flags().GetString("reporter") @@ -153,12 +217,12 @@ func extensionValidationFormat(cmd *cobra.Command) (string, error) { func init() { extensionRootCmd.AddCommand(extensionValidateCmd) - extensionValidateCmd.PersistentFlags().Bool("full", false, "Run full validation including PHPStan, ESLint and Stylelint") + extensionValidateCmd.PersistentFlags().Bool("full", false, "Run all validation checks by default (minus --exclude selections)") extensionValidateCmd.PersistentFlags().Bool("store-compliance", false, "Run the Extension Store compliance checks") extensionValidateCmd.PersistentFlags().String("format", "", "Reporting format (summary, json, github, gitlab, junit, markdown)") extensionValidateCmd.PersistentFlags().String("reporter", "", "Reporting format (summary, json, github, gitlab, junit, markdown)") extensionValidateCmd.PersistentFlags().String("check-against", "highest", "Check against Shopware Version (highest, lowest)") - extensionValidateCmd.PersistentFlags().String("only", "", "Run only specific tools by name (comma-separated, e.g. phpstan,eslint)") + extensionValidateCmd.PersistentFlags().String("only", "", "Run only these validation checks, regardless of --full (comma-separated, e.g. phpstan,eslint)") extensionValidateCmd.PersistentFlags().String("exclude", "", "Exclude specific tools by name (comma-separated, e.g. phpstan,eslint)") extensionValidateCmd.PersistentFlags().Bool("no-copy", false, "Do not copy extension files to temporary directory") extensionValidateCmd.MarkFlagsMutuallyExclusive("format", "reporter") @@ -174,12 +238,6 @@ func init() { return fmt.Errorf("invalid --check-against value %q, allowed values: highest, lowest", mode) } - // Dont setup tools if we dont run full validation - full, _ := cmd.Flags().GetBool("full") - if !full { - return nil - } - - return verifier.SetupTools(cmd.Context(), cmd.Root().Version) + return nil } } diff --git a/cmd/extension/extension_validate_selection_test.go b/cmd/extension/extension_validate_selection_test.go new file mode 100644 index 00000000..a4a731be --- /dev/null +++ b/cmd/extension/extension_validate_selection_test.go @@ -0,0 +1,88 @@ +package extension + +import ( + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/shopware/shopware-cli/internal/validation" + "github.com/shopware/shopware-cli/internal/verifier" +) + +func coverageByName(t *testing.T, checks []validation.CheckCoverage, name string) validation.CheckCoverage { + t.Helper() + for _, check := range checks { + if check.Name == name { + return check + } + } + t.Fatalf("missing coverage for %s", name) + return validation.CheckCoverage{} +} + +func TestExtensionValidationSelection(t *testing.T) { + t.Run("default runs only sw-cli", func(t *testing.T) { + tools, checks, err := selectExtensionValidationTools(false, "", "") + require.NoError(t, err) + assert.Equal(t, []string{"sw-cli"}, toolNamesForValidation(tools)) + assert.False(t, slices.ContainsFunc(tools, requiresToolSetup)) + assert.Equal(t, "not selected; use --full or --only", coverageByName(t, checks, "phpstan").Reason) + }) + + t.Run("only phpstan works without full", func(t *testing.T) { + tools, checks, err := selectExtensionValidationTools(false, "phpstan", "") + require.NoError(t, err) + assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) + assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) + assert.Equal(t, "invoked", coverageByName(t, checks, "phpstan").Status) + assert.Equal(t, "not selected by --only", coverageByName(t, checks, "sw-cli").Reason) + }) + + t.Run("Twig validation needs no external tools", func(t *testing.T) { + tools, _, err := selectExtensionValidationTools(false, "admin-twig", "") + require.NoError(t, err) + assert.Equal(t, []string{"admin-twig"}, toolNamesForValidation(tools)) + assert.False(t, slices.ContainsFunc(tools, requiresToolSetup)) + }) + + t.Run("full selects all validation checks", func(t *testing.T) { + tools, checks, err := selectExtensionValidationTools(true, "", "") + require.NoError(t, err) + assert.Len(t, tools, 6) + assert.Len(t, checks, len(tools)) + assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) + }) + + t.Run("exclude applies after only", func(t *testing.T) { + tools, checks, err := selectExtensionValidationTools(false, "phpstan,sw-cli", "sw-cli") + require.NoError(t, err) + assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) + assert.Equal(t, "excluded by --exclude", coverageByName(t, checks, "sw-cli").Reason) + }) + + t.Run("duplicate only values run once", func(t *testing.T) { + tools, _, err := selectExtensionValidationTools(false, "sw-cli,sw-cli", "") + require.NoError(t, err) + assert.Equal(t, []string{"sw-cli"}, toolNamesForValidation(tools)) + }) + + t.Run("unsupported operation fails", func(t *testing.T) { + _, _, err := selectExtensionValidationTools(false, "prettier", "") + require.EqualError(t, err, "prettier does not provide a validation check") + }) + + t.Run("empty selection fails", func(t *testing.T) { + _, _, err := selectExtensionValidationTools(false, "sw-cli", "sw-cli") + require.EqualError(t, err, "no validation checks selected after applying --exclude") + }) +} + +func toolNamesForValidation(tools verifier.ToolList) []string { + names := make([]string, 0, len(tools)) + for _, tool := range tools { + names = append(names, tool.Name()) + } + return names +} diff --git a/internal/validation/reporter.go b/internal/validation/reporter.go index 133d6405..e62006fe 100644 --- a/internal/validation/reporter.go +++ b/internal/validation/reporter.go @@ -8,6 +8,7 @@ import ( "encoding/xml" "errors" "fmt" + "io" "os" "sort" "strings" @@ -38,34 +39,48 @@ func DetectDefaultReporter() string { return "summary" } -func DoCheckReport(result Check, reportingFormat string) error { +// CheckCoverage records whether a validation check was selected and invoked. +type CheckCoverage struct { + Name string `json:"name"` + Status string `json:"status"` + Reason string `json:"reason,omitempty"` +} + +// DoCheckReport reports findings and, when supplied, validation coverage. +func DoCheckReport(result Check, reportingFormat string, checks ...CheckCoverage) error { if err := ValidateReporter(reportingFormat); err != nil { return err } switch reportingFormat { case "summary": - if err := doSummaryReport(result); err != nil { + if err := doSummaryReport(result, checks...); err != nil { return err } case "json": - if err := doJSONReport(result); err != nil { + if err := doJSONReport(result, checks...); err != nil { return err } case "github": - if err := doGitHubReport(result); err != nil { + if err := doGitHubReport(result, checks...); err != nil { return err } case "gitlab": if err := doGitLabReport(result); err != nil { return err } + if err := printCoverage(os.Stderr, checks); err != nil { + return err + } case "markdown": - if err := doMarkdownReport(result); err != nil { + if err := doMarkdownReport(result, checks...); err != nil { return err } case "junit": - if err := doJUnitReport(result); err != nil { + if err := doJUnitReport(result, checks...); err != nil { + return err + } + if err := printCoverage(os.Stderr, checks); err != nil { return err } } @@ -77,7 +92,11 @@ func DoCheckReport(result Check, reportingFormat string) error { return nil } -func doSummaryReport(result Check) error { +func doSummaryReport(result Check, checks ...CheckCoverage) error { + if err := printCoverage(os.Stdout, checks); err != nil { + return err + } + // Group results by file fileGroups := make(map[string][]CheckResult) for _, r := range result.GetResults() { @@ -133,7 +152,11 @@ func doSummaryReport(result Check) error { } //nolint:forbidigo - fmt.Printf("\n%s\n", summaryLine(totalProblems, errorCount, warningCount)) + if checks != nil && !anyCheckInvoked(checks) { + fmt.Println("\nNo checks invoked; 0 problems reported") + } else { + fmt.Printf("\n%s\n", summaryLine(totalProblems, errorCount, warningCount)) + } return nil } @@ -155,17 +178,20 @@ func countNoun(n int, noun string) string { return fmt.Sprintf("%d %ss", n, noun) } -func doJSONReport(result Check) error { +func doJSONReport(result Check, checks ...CheckCoverage) error { data := map[string]interface{}{ "results": result.GetResults(), } + if checks != nil { + data["checks"] = checks + } encoder := json.NewEncoder(os.Stdout) encoder.SetIndent("", " ") return encoder.Encode(data) } -func doGitHubReport(result Check) error { +func doGitHubReport(result Check, checks ...CheckCoverage) error { // Print the human-readable summary first so the GitHub Actions log // shows file/line context, then emit annotations for PR inline display. // File paths and messages can contain `::` which the runner would parse @@ -178,7 +204,7 @@ func doGitHubReport(result Check) error { token := hex.EncodeToString(tokenBytes[:]) fmt.Printf("::stop-commands::%s\n", token) - if err := doSummaryReport(result); err != nil { + if err := doSummaryReport(result, checks...); err != nil { fmt.Printf("::%s::\n", token) return err } @@ -306,7 +332,7 @@ func doGitLabReport(result Check) error { return encoder.Encode(issues) } -func doMarkdownReport(result Check) error { +func doMarkdownReport(result Check, checks ...CheckCoverage) error { // Group results by file fileGroups := make(map[string][]CheckResult) for _, r := range result.GetResults() { @@ -341,6 +367,16 @@ func doMarkdownReport(result Check) error { fmt.Println("# Validation Report") fmt.Println() + if checks != nil { + fmt.Println("## Checks") + fmt.Println() + fmt.Println("```text") + for _, row := range coverageRows(checks) { + fmt.Println(row) + } + fmt.Println("```") + fmt.Println() + } totalProblems := 0 for _, path := range sortedPaths { @@ -370,19 +406,63 @@ func doMarkdownReport(result Check) error { fmt.Println() } - if totalProblems == 0 { + if checks != nil && !anyCheckInvoked(checks) { + fmt.Println("No checks invoked; 0 problems reported") + } else if totalProblems == 0 { fmt.Println("✅ No problems found") } return nil } +func printCoverage(w io.Writer, checks []CheckCoverage) error { + if checks == nil { + return nil + } + if _, err := fmt.Fprintln(w, "Checks:"); err != nil { + return err + } + for _, row := range coverageRows(checks) { + if _, err := fmt.Fprintln(w, " "+row); err != nil { + return err + } + } + return nil +} + +func coverageRows(checks []CheckCoverage) []string { + width := 0 + for _, check := range checks { + width = max(width, len(check.Name)) + } + + rows := make([]string, 0, len(checks)) + for _, check := range checks { + row := fmt.Sprintf("%-*s %-7s", width, check.Name, check.Status) + if check.Reason != "" { + row += " " + check.Reason + } + rows = append(rows, strings.TrimRight(row, " ")) + } + return rows +} + +func anyCheckInvoked(checks []CheckCoverage) bool { + for _, check := range checks { + if check.Status == "invoked" { + return true + } + } + return false +} + type JUnitTestSuite struct { XMLName xml.Name `xml:"testsuite"` Name string `xml:"name,attr"` Tests int `xml:"tests,attr"` Failures int `xml:"failures,attr"` Errors int `xml:"errors,attr"` + Skipped int `xml:"skipped,attr,omitempty"` TestCase []JUnitTestCase `xml:"testcase"` } @@ -391,6 +471,11 @@ type JUnitTestCase struct { ClassName string `xml:"classname,attr"` Failure *JUnitTestFailure `xml:"failure,omitempty"` Error *JUnitTestError `xml:"error,omitempty"` + Skipped *JUnitTestSkipped `xml:"skipped,omitempty"` +} + +type JUnitTestSkipped struct { + Message string `xml:"message,attr"` } type JUnitTestFailure struct { @@ -405,10 +490,11 @@ type JUnitTestError struct { Content string `xml:",chardata"` } -func doJUnitReport(result Check) error { +func doJUnitReport(result Check, checks ...CheckCoverage) error { var testCases []JUnitTestCase errors := 0 failures := 0 + skipped := 0 // Sort results for deterministic output results := result.GetResults() @@ -460,12 +546,21 @@ func doJUnitReport(result Check) error { testCases = append(testCases, testCase) } + for _, check := range checks { + testCase := JUnitTestCase{Name: check.Name, ClassName: "validation"} + if check.Status == "skipped" { + testCase.Skipped = &JUnitTestSkipped{Message: check.Reason} + skipped++ + } + testCases = append(testCases, testCase) + } suite := JUnitTestSuite{ Name: "shopware-cli-validation", Tests: len(testCases), Failures: failures, Errors: errors, + Skipped: skipped, TestCase: testCases, } diff --git a/internal/validation/reporter_test.go b/internal/validation/reporter_test.go index fd9aad7c..d10816c8 100644 --- a/internal/validation/reporter_test.go +++ b/internal/validation/reporter_test.go @@ -3,12 +3,14 @@ package validation import ( "bytes" "encoding/json" + "encoding/xml" "io" "os" "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestReportingOutputIsDeterministic(t *testing.T) { @@ -164,6 +166,102 @@ func TestErrorExistsSummary(t *testing.T) { assert.Error(t, DoCheckReport(check, "summary")) } +func TestCheckCoverageReports(t *testing.T) { + check := &testCheck{Results: []CheckResult{}} + checks := []CheckCoverage{ + {Name: "eslint", Status: "skipped", Reason: "no JavaScript source files"}, + {Name: "storefront-twig", Status: "skipped", Reason: "no storefront Twig templates"}, + } + rows := coverageRows(checks) + assert.Contains(t, rows[0], "eslint") + assert.Contains(t, rows[0], " skipped no JavaScript source files") + assert.NotContains(t, rows[0], "(") + + summary := captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "summary", checks...)) + }) + for _, row := range rows { + assert.Contains(t, summary, " "+row) + } + assert.Contains(t, summary, "No checks invoked; 0 problems reported") + assert.NotContains(t, summary, "No problems found") + + github := captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "github", checks...)) + }) + for _, row := range rows { + assert.Contains(t, github, " "+row) + } + + markdown := captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "markdown", checks...)) + }) + assert.Contains(t, markdown, "## Checks") + for _, row := range rows { + assert.Contains(t, markdown, row) + } + assert.Contains(t, markdown, "No checks invoked; 0 problems reported") + + jsonOutput := captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "json", checks...)) + }) + var report struct { + Results []CheckResult `json:"results"` + Checks []CheckCoverage `json:"checks"` + } + assert.NoError(t, json.Unmarshal([]byte(jsonOutput), &report)) + assert.Empty(t, report.Results) + assert.Equal(t, checks, report.Checks) + + checks[0] = CheckCoverage{Name: "eslint", Status: "invoked"} + summary = captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "summary", checks...)) + }) + assert.Contains(t, summary, "eslint") + assert.Contains(t, summary, " invoked") + assert.Contains(t, summary, "No problems found") +} + +func TestStructuredReportsKeepMachineOutputAndShowCoverage(t *testing.T) { + check := &testCheck{Results: []CheckResult{}} + checks := []CheckCoverage{ + {Name: "phpstan", Status: "invoked"}, + {Name: "sw-cli", Status: "skipped", Reason: "not selected by --only"}, + } + + var gitlabLog string + gitlab := captureOutput(func() { + gitlabLog = captureStderr(func() { + assert.NoError(t, DoCheckReport(check, "gitlab", checks...)) + }) + }) + var issues []GitLabCodeQualityIssue + assert.NoError(t, json.Unmarshal([]byte(gitlab), &issues)) + assert.Empty(t, issues) + for _, row := range coverageRows(checks) { + assert.Contains(t, gitlabLog, " "+row) + } + + var junitLog string + junit := captureOutput(func() { + junitLog = captureStderr(func() { + assert.NoError(t, DoCheckReport(check, "junit", checks...)) + }) + }) + var suite JUnitTestSuite + require.NoError(t, xml.Unmarshal([]byte(junit), &suite)) + assert.Equal(t, 2, suite.Tests) + assert.Equal(t, 1, suite.Skipped) + require.Len(t, suite.TestCase, 2) + assert.Equal(t, "phpstan", suite.TestCase[0].Name) + assert.Nil(t, suite.TestCase[0].Skipped) + require.NotNil(t, suite.TestCase[1].Skipped) + assert.Equal(t, "not selected by --only", suite.TestCase[1].Skipped.Message) + for _, row := range coverageRows(checks) { + assert.Contains(t, junitLog, " "+row) + } +} + func TestValidateReporter(t *testing.T) { for _, format := range []string{"summary", "json", "github", "gitlab", "junit", "markdown"} { assert.NoError(t, ValidateReporter(format)) @@ -541,6 +639,25 @@ func captureOutput(fn func()) string { return buf.String() } +func captureStderr(fn func()) string { + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + fn() + + if err := w.Close(); err != nil { + panic(err) + } + os.Stderr = oldStderr + + var buf bytes.Buffer + if _, err := io.Copy(&buf, r); err != nil { + panic(err) + } + return buf.String() +} + func TestSummaryLine(t *testing.T) { assert.Equal(t, "✓ No problems found", summaryLine(0, 0, 0)) assert.Equal(t, "✖ 1 problem (1 error, 0 warnings)", summaryLine(1, 1, 0)) From ab230c34fefaa4ea48cfcf44a418c102edc32d4c Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Thu, 24 Sep 2026 16:24:52 +0200 Subject: [PATCH 02/14] chore: refactor to interface marker --- cmd/extension/extension_validate.go | 22 ++++++++++++++-------- internal/verifier/admin_twig.go | 2 ++ internal/verifier/eslint.go | 2 ++ internal/verifier/phpstan.go | 2 ++ internal/verifier/storefront_twig.go | 2 ++ internal/verifier/stylelint.go | 2 ++ internal/verifier/sw_cli.go | 2 ++ internal/verifier/tool.go | 6 ++++++ 8 files changed, 32 insertions(+), 8 deletions(-) diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index b0e774a0..a3adacdb 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -6,7 +6,7 @@ import ( "os" "path/filepath" "slices" - "strings" + "sort" "time" "github.com/spf13/cobra" @@ -134,15 +134,19 @@ var extensionValidateCmd = &cobra.Command{ }, } -// These tools have a real Check implementation; other verifier tools may only fix or format. -var extensionValidationToolNames = []string{"admin-twig", "eslint", "phpstan", "storefront-twig", "stylelint", "sw-cli"} - func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList, []validation.CheckCoverage, error) { + validationTools := make(verifier.ToolList, 0) + for _, tool := range verifier.GetTools() { + if _, ok := tool.(verifier.ValidationTool); ok { + validationTools = append(validationTools, tool) + } + } + requested := only if requested == "" { requested = "sw-cli" if full { - requested = strings.Join(extensionValidationToolNames, ",") + requested = validationTools.PossibleString() } } selected, err := verifier.GetTools().Only(requested) @@ -152,7 +156,7 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T requestedNames := make(map[string]bool, len(selected)) for _, tool := range selected { name := tool.Name() - if !slices.Contains(extensionValidationToolNames, name) { + if _, ok := tool.(verifier.ValidationTool); !ok { return nil, nil, fmt.Errorf("%s does not provide a validation check", name) } requestedNames[name] = true @@ -175,8 +179,9 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T invoked[tool.Name()] = true } - coverage := make([]validation.CheckCoverage, 0, len(extensionValidationToolNames)) - for _, name := range extensionValidationToolNames { + coverage := make([]validation.CheckCoverage, 0, len(validationTools)) + for _, tool := range validationTools { + name := tool.Name() check := validation.CheckCoverage{Name: name, Status: "skipped"} switch { case invoked[name]: @@ -190,6 +195,7 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T } coverage = append(coverage, check) } + sort.Slice(coverage, func(i, j int) bool { return coverage[i].Name < coverage[j].Name }) return unique, coverage, nil } diff --git a/internal/verifier/admin_twig.go b/internal/verifier/admin_twig.go index cdc9704f..d122bdaa 100644 --- a/internal/verifier/admin_twig.go +++ b/internal/verifier/admin_twig.go @@ -22,6 +22,8 @@ func (a AdminTwigLinter) Name() string { return "admin-twig" } +func (a AdminTwigLinter) ValidationTool() {} + func (a AdminTwigLinter) Check(ctx context.Context, check *Check, config ToolConfig) error { fixers := twiglinter.GetAdministrationFixers(version.Must(version.NewVersion(config.MinShopwareVersion))) diff --git a/internal/verifier/eslint.go b/internal/verifier/eslint.go index d59ee5c4..f6418fed 100644 --- a/internal/verifier/eslint.go +++ b/internal/verifier/eslint.go @@ -46,6 +46,8 @@ func (e Eslint) Name() string { return "eslint" } +func (e Eslint) ValidationTool() {} + func (e Eslint) Check(ctx context.Context, check *Check, config ToolConfig) error { paths := append([]string{}, config.StorefrontDirectories...) paths = append(paths, config.AdminDirectories...) diff --git a/internal/verifier/phpstan.go b/internal/verifier/phpstan.go index 97cb1dcc..36513717 100644 --- a/internal/verifier/phpstan.go +++ b/internal/verifier/phpstan.go @@ -45,6 +45,8 @@ func (p PhpStan) Name() string { return "phpstan" } +func (p PhpStan) ValidationTool() {} + func (p PhpStan) configExists(pluginPath string) bool { for _, config := range possiblePHPStanConfigs { if _, err := os.Stat(path.Join(pluginPath, config)); err == nil { diff --git a/internal/verifier/storefront_twig.go b/internal/verifier/storefront_twig.go index a9dd48a7..6405363a 100644 --- a/internal/verifier/storefront_twig.go +++ b/internal/verifier/storefront_twig.go @@ -22,6 +22,8 @@ func (s StorefrontTwigLinter) Name() string { return "storefront-twig" } +func (s StorefrontTwigLinter) ValidationTool() {} + func (s StorefrontTwigLinter) Check(ctx context.Context, check *Check, config ToolConfig) error { fixers := twiglinter.GetStorefrontFixers(version.Must(version.NewVersion(config.MinShopwareVersion))) diff --git a/internal/verifier/stylelint.go b/internal/verifier/stylelint.go index f92d9ed5..fa039cc2 100644 --- a/internal/verifier/stylelint.go +++ b/internal/verifier/stylelint.go @@ -38,6 +38,8 @@ func (s StyleLint) Name() string { return "stylelint" } +func (s StyleLint) ValidationTool() {} + func (s StyleLint) Check(ctx context.Context, check *Check, config ToolConfig) error { paths := append([]string{}, config.StorefrontDirectories...) paths = append(paths, config.AdminDirectories...) diff --git a/internal/verifier/sw_cli.go b/internal/verifier/sw_cli.go index 9d4734d4..891c1058 100644 --- a/internal/verifier/sw_cli.go +++ b/internal/verifier/sw_cli.go @@ -13,6 +13,8 @@ func (s SWCLI) Name() string { return "sw-cli" } +func (s SWCLI) ValidationTool() {} + func (s SWCLI) Check(ctx context.Context, check *Check, config ToolConfig) error { if config.Extension == nil { return nil diff --git a/internal/verifier/tool.go b/internal/verifier/tool.go index b4678942..6933dc9c 100644 --- a/internal/verifier/tool.go +++ b/internal/verifier/tool.go @@ -54,6 +54,12 @@ type Tool interface { Format(ctx context.Context, config ToolConfig, dryRun bool) error } +// ValidationTool marks tools whose Check method performs validation. +type ValidationTool interface { + Tool + ValidationTool() +} + func (tl ToolList) Only(only string) (ToolList, error) { if only == "" { return tl, nil From eceaf07b46dead66a1e464b5e75dcdc51a7f0b1d Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Thu, 24 Sep 2026 16:42:30 +0200 Subject: [PATCH 03/14] chore: refactor to split tool interface into capabilities --- cmd/extension/extension_fix.go | 8 ++++---- cmd/extension/extension_format.go | 8 ++++---- cmd/extension/extension_validate.go | 13 ++++--------- cmd/project/project_fix.go | 8 ++++---- cmd/project/project_format.go | 8 ++++---- cmd/project/project_validate.go | 6 +++--- internal/verifier/admin_twig.go | 2 -- internal/verifier/eslint.go | 6 ------ internal/verifier/phpcsfixer.go | 8 -------- internal/verifier/phpstan.go | 10 ---------- internal/verifier/prettier.go | 8 -------- internal/verifier/rector.go | 8 -------- internal/verifier/storefront_twig.go | 10 ---------- internal/verifier/stylelint.go | 6 ------ internal/verifier/sw_cli.go | 10 ---------- internal/verifier/symfony_xml.go | 8 -------- internal/verifier/tool.go | 25 +++++++++++++++++++++---- internal/verifier/tool_test.go | 6 ++++++ 18 files changed, 50 insertions(+), 108 deletions(-) diff --git a/cmd/extension/extension_fix.go b/cmd/extension/extension_fix.go index f7c60f5f..76cfb14e 100644 --- a/cmd/extension/extension_fix.go +++ b/cmd/extension/extension_fix.go @@ -48,7 +48,7 @@ var extensionFixCmd = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetTools() + tools := verifier.GetToolsOf[verifier.FixTool]() only, _ := cmd.Flags().GetString("only") tools, err = tools.Only(only) @@ -57,9 +57,9 @@ var extensionFixCmd = &cobra.Command{ } for _, tool := range tools { - tool := tool + fixer := tool.(verifier.FixTool) gr.Go(func() error { - return tool.Fix(cmd.Context(), *toolCfg) + return fixer.Fix(cmd.Context(), *toolCfg) }) } @@ -73,6 +73,6 @@ var extensionFixCmd = &cobra.Command{ func init() { extensionRootCmd.AddCommand(extensionFixCmd) - extensionFixCmd.Flags().String("only", "", "Run only specific tools by name (comma-separated, e.g. phpstan,eslint)") + extensionFixCmd.Flags().String("only", "", "Run only specific fixers by name (comma-separated, e.g. eslint,rector)") extensionFixCmd.Flags().Bool("allow-non-git", false, "Allow running the fix command on non-git repositories") } diff --git a/cmd/extension/extension_format.go b/cmd/extension/extension_format.go index 307e64f6..31e2fbc1 100644 --- a/cmd/extension/extension_format.go +++ b/cmd/extension/extension_format.go @@ -41,7 +41,7 @@ var extensionFormat = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetTools() + tools := verifier.GetToolsOf[verifier.FormatTool]() only, _ := cmd.Flags().GetString("only") tools, err = tools.Only(only) @@ -50,9 +50,9 @@ var extensionFormat = &cobra.Command{ } for _, tool := range tools { - tool := tool + formatter := tool.(verifier.FormatTool) gr.Go(func() error { - return tool.Format(cmd.Context(), *toolCfg, dryRun) + return formatter.Format(cmd.Context(), *toolCfg, dryRun) }) } @@ -66,6 +66,6 @@ var extensionFormat = &cobra.Command{ func init() { extensionRootCmd.AddCommand(extensionFormat) - extensionFormat.Flags().String("only", "", "Run only specific tools by name (comma-separated, e.g. phpstan,eslint)") + extensionFormat.Flags().String("only", "", "Run only specific formatters by name (comma-separated, e.g. prettier,php-cs-fixer)") extensionFormat.Flags().Bool("dry-run", false, "Run in dry run mode") } diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index a3adacdb..94988bee 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -120,9 +120,9 @@ var extensionValidateCmd = &cobra.Command{ var gr errgroup.Group for _, tool := range tools { - tool := tool + checker := tool.(verifier.CheckTool) gr.Go(func() error { - return tool.Check(cmd.Context(), result, *toolCfg) + return checker.Check(cmd.Context(), result, *toolCfg) }) } @@ -135,12 +135,7 @@ var extensionValidateCmd = &cobra.Command{ } func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList, []validation.CheckCoverage, error) { - validationTools := make(verifier.ToolList, 0) - for _, tool := range verifier.GetTools() { - if _, ok := tool.(verifier.ValidationTool); ok { - validationTools = append(validationTools, tool) - } - } + validationTools := verifier.GetToolsOf[verifier.CheckTool]() requested := only if requested == "" { @@ -156,7 +151,7 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T requestedNames := make(map[string]bool, len(selected)) for _, tool := range selected { name := tool.Name() - if _, ok := tool.(verifier.ValidationTool); !ok { + if _, ok := tool.(verifier.CheckTool); !ok { return nil, nil, fmt.Errorf("%s does not provide a validation check", name) } requestedNames[name] = true diff --git a/cmd/project/project_fix.go b/cmd/project/project_fix.go index d5968b09..2cd678e4 100644 --- a/cmd/project/project_fix.go +++ b/cmd/project/project_fix.go @@ -54,7 +54,7 @@ var projectFixCmd = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetTools() + tools := verifier.GetToolsOf[verifier.FixTool]() tools, err = tools.Only(only) if err != nil { @@ -62,9 +62,9 @@ var projectFixCmd = &cobra.Command{ } for _, tool := range tools { - tool := tool + fixer := tool.(verifier.FixTool) gr.Go(func() error { - return tool.Fix(cmd.Context(), *toolCfg) + return fixer.Fix(cmd.Context(), *toolCfg) }) } @@ -74,6 +74,6 @@ var projectFixCmd = &cobra.Command{ func init() { projectRootCmd.AddCommand(projectFixCmd) - projectFixCmd.PersistentFlags().String("only", "", "Run only the specified tools (comma-separated, e.g. phpstan,eslint)") + projectFixCmd.PersistentFlags().String("only", "", "Run only the specified fixers (comma-separated, e.g. eslint,rector)") projectFixCmd.PersistentFlags().Bool("allow-non-git", false, "Allow fixes in projects without a Git repository") } diff --git a/cmd/project/project_format.go b/cmd/project/project_format.go index ff070c53..a692e1f4 100644 --- a/cmd/project/project_format.go +++ b/cmd/project/project_format.go @@ -46,7 +46,7 @@ var projectFormatCmd = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetTools() + tools := verifier.GetToolsOf[verifier.FormatTool]() tools, err = tools.Only(only) if err != nil { @@ -54,9 +54,9 @@ var projectFormatCmd = &cobra.Command{ } for _, tool := range tools { - tool := tool + formatter := tool.(verifier.FormatTool) gr.Go(func() error { - return tool.Format(cmd.Context(), *toolCfg, dryRun) + return formatter.Format(cmd.Context(), *toolCfg, dryRun) }) } @@ -66,6 +66,6 @@ var projectFormatCmd = &cobra.Command{ func init() { projectRootCmd.AddCommand(projectFormatCmd) - projectFormatCmd.PersistentFlags().String("only", "", "Run only the specified tools (comma-separated, e.g. phpstan,eslint)") + projectFormatCmd.PersistentFlags().String("only", "", "Run only the specified formatters (comma-separated, e.g. prettier,php-cs-fixer)") projectFormatCmd.PersistentFlags().Bool("dry-run", false, "Run formatters without changing files") } diff --git a/cmd/project/project_validate.go b/cmd/project/project_validate.go index 427e1fe1..d9956b92 100644 --- a/cmd/project/project_validate.go +++ b/cmd/project/project_validate.go @@ -79,7 +79,7 @@ var projectValidateCmd = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetTools() + tools := verifier.GetToolsOf[verifier.CheckTool]() tools, err = tools.Only(only) if err != nil { @@ -92,9 +92,9 @@ var projectValidateCmd = &cobra.Command{ } for _, tool := range tools { - tool := tool + checker := tool.(verifier.CheckTool) gr.Go(func() error { - return tool.Check(cmd.Context(), result, *toolCfg) + return checker.Check(cmd.Context(), result, *toolCfg) }) } diff --git a/internal/verifier/admin_twig.go b/internal/verifier/admin_twig.go index d122bdaa..cdc9704f 100644 --- a/internal/verifier/admin_twig.go +++ b/internal/verifier/admin_twig.go @@ -22,8 +22,6 @@ func (a AdminTwigLinter) Name() string { return "admin-twig" } -func (a AdminTwigLinter) ValidationTool() {} - func (a AdminTwigLinter) Check(ctx context.Context, check *Check, config ToolConfig) error { fixers := twiglinter.GetAdministrationFixers(version.Must(version.NewVersion(config.MinShopwareVersion))) diff --git a/internal/verifier/eslint.go b/internal/verifier/eslint.go index f6418fed..a26db960 100644 --- a/internal/verifier/eslint.go +++ b/internal/verifier/eslint.go @@ -46,8 +46,6 @@ func (e Eslint) Name() string { return "eslint" } -func (e Eslint) ValidationTool() {} - func (e Eslint) Check(ctx context.Context, check *Check, config ToolConfig) error { paths := append([]string{}, config.StorefrontDirectories...) paths = append(paths, config.AdminDirectories...) @@ -148,10 +146,6 @@ func (e Eslint) Fix(ctx context.Context, config ToolConfig) error { return gr.Wait() } -func (e Eslint) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - func init() { AddTool(Eslint{}) } diff --git a/internal/verifier/phpcsfixer.go b/internal/verifier/phpcsfixer.go index 6d49cebe..fd5b4f3c 100644 --- a/internal/verifier/phpcsfixer.go +++ b/internal/verifier/phpcsfixer.go @@ -15,14 +15,6 @@ func (p PHPCSFixer) Name() string { return "php-cs-fixer" } -func (p PHPCSFixer) Check(ctx context.Context, check *Check, config ToolConfig) error { - return nil -} - -func (p PHPCSFixer) Fix(ctx context.Context, config ToolConfig) error { - return nil -} - func (p PHPCSFixer) getConfigPath(toolDirectory, rootDir string) string { if _, err := os.Stat(path.Join(rootDir, ".php-cs-fixer.dist.php")); err == nil { return path.Join(rootDir, ".php-cs-fixer.dist.php") diff --git a/internal/verifier/phpstan.go b/internal/verifier/phpstan.go index 36513717..427f2cd7 100644 --- a/internal/verifier/phpstan.go +++ b/internal/verifier/phpstan.go @@ -45,8 +45,6 @@ func (p PhpStan) Name() string { return "phpstan" } -func (p PhpStan) ValidationTool() {} - func (p PhpStan) configExists(pluginPath string) bool { for _, config := range possiblePHPStanConfigs { if _, err := os.Stat(path.Join(pluginPath, config)); err == nil { @@ -150,14 +148,6 @@ func isPhpStanNoFilesOutput(output string) bool { return strings.Contains(output, "No files found to analyse") } -func (p PhpStan) Fix(ctx context.Context, config ToolConfig) error { - return nil -} - -func (p PhpStan) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - var tagPartRegex = regexp.MustCompile(`tag:v[0-9]+.[0-9]+.[0-9]+`) var parameterRemovedRegex = regexp.MustCompile("Parameter.*will be removed") diff --git a/internal/verifier/prettier.go b/internal/verifier/prettier.go index 7957071d..40bd693b 100644 --- a/internal/verifier/prettier.go +++ b/internal/verifier/prettier.go @@ -25,14 +25,6 @@ func (b Prettier) Name() string { return "prettier" } -func (b Prettier) Check(ctx context.Context, check *Check, config ToolConfig) error { - return nil -} - -func (b Prettier) Fix(ctx context.Context, config ToolConfig) error { - return nil -} - func (b Prettier) Format(ctx context.Context, config ToolConfig, dryRun bool) error { var gr errgroup.Group diff --git a/internal/verifier/rector.go b/internal/verifier/rector.go index a08b471a..00a3b4f2 100644 --- a/internal/verifier/rector.go +++ b/internal/verifier/rector.go @@ -14,10 +14,6 @@ func (r Rector) Name() string { return "rector" } -func (r Rector) Check(ctx context.Context, check *Check, config ToolConfig) error { - return nil -} - func (r Rector) Fix(ctx context.Context, config ToolConfig) error { if _, err := os.Stat(path.Join(config.RootDir, "composer.json")); err != nil { //nolint: nilerr @@ -93,10 +89,6 @@ func (r Rector) Fix(ctx context.Context, config ToolConfig) error { return nil } -func (r Rector) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - func init() { AddTool(Rector{}) } diff --git a/internal/verifier/storefront_twig.go b/internal/verifier/storefront_twig.go index 6405363a..fe9d0e93 100644 --- a/internal/verifier/storefront_twig.go +++ b/internal/verifier/storefront_twig.go @@ -22,8 +22,6 @@ func (s StorefrontTwigLinter) Name() string { return "storefront-twig" } -func (s StorefrontTwigLinter) ValidationTool() {} - func (s StorefrontTwigLinter) Check(ctx context.Context, check *Check, config ToolConfig) error { fixers := twiglinter.GetStorefrontFixers(version.Must(version.NewVersion(config.MinShopwareVersion))) @@ -93,14 +91,6 @@ func (s StorefrontTwigLinter) Check(ctx context.Context, check *Check, config To return nil } -func (s StorefrontTwigLinter) Fix(ctx context.Context, config ToolConfig) error { - return nil -} - -func (a StorefrontTwigLinter) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - func init() { AddTool(StorefrontTwigLinter{}) } diff --git a/internal/verifier/stylelint.go b/internal/verifier/stylelint.go index fa039cc2..4fd25079 100644 --- a/internal/verifier/stylelint.go +++ b/internal/verifier/stylelint.go @@ -38,8 +38,6 @@ func (s StyleLint) Name() string { return "stylelint" } -func (s StyleLint) ValidationTool() {} - func (s StyleLint) Check(ctx context.Context, check *Check, config ToolConfig) error { paths := append([]string{}, config.StorefrontDirectories...) paths = append(paths, config.AdminDirectories...) @@ -146,10 +144,6 @@ func (s StyleLint) Fix(ctx context.Context, config ToolConfig) error { return gr.Wait() } -func (s StyleLint) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - func init() { AddTool(StyleLint{}) } diff --git a/internal/verifier/sw_cli.go b/internal/verifier/sw_cli.go index 891c1058..1a7210ef 100644 --- a/internal/verifier/sw_cli.go +++ b/internal/verifier/sw_cli.go @@ -13,8 +13,6 @@ func (s SWCLI) Name() string { return "sw-cli" } -func (s SWCLI) ValidationTool() {} - func (s SWCLI) Check(ctx context.Context, check *Check, config ToolConfig) error { if config.Extension == nil { return nil @@ -46,14 +44,6 @@ func (s SWCLI) Check(ctx context.Context, check *Check, config ToolConfig) error return nil } -func (s SWCLI) Fix(ctx context.Context, config ToolConfig) error { - return nil -} - -func (s SWCLI) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - func init() { AddTool(SWCLI{}) } diff --git a/internal/verifier/symfony_xml.go b/internal/verifier/symfony_xml.go index d6ff0ad7..edede2b4 100644 --- a/internal/verifier/symfony_xml.go +++ b/internal/verifier/symfony_xml.go @@ -20,10 +20,6 @@ func (SymfonyXMLConverter) Name() string { return "symfony-xml" } -func (SymfonyXMLConverter) Check(ctx context.Context, check *Check, config ToolConfig) error { - return nil -} - func (s SymfonyXMLConverter) Fix(ctx context.Context, config ToolConfig) error { conversions := []struct { fileName string @@ -56,10 +52,6 @@ func (s SymfonyXMLConverter) Fix(ctx context.Context, config ToolConfig) error { return nil } -func (SymfonyXMLConverter) Format(ctx context.Context, config ToolConfig, dryRun bool) error { - return nil -} - // collectConfigDirs returns the Resources/config directories of all platform // plugins covered by the tool config. Apps cannot use the dependency injection // container and bundles may load their configuration files explicitly by diff --git a/internal/verifier/tool.go b/internal/verifier/tool.go index 6933dc9c..5f49e6d0 100644 --- a/internal/verifier/tool.go +++ b/internal/verifier/tool.go @@ -21,6 +21,17 @@ func GetTools() ToolList { return availableTools } +// GetToolsOf returns registered tools that implement the requested capability. +func GetToolsOf[T Tool]() ToolList { + var tools ToolList + for _, tool := range availableTools { + if _, ok := tool.(T); ok { + tools = append(tools, tool) + } + } + return tools +} + type ToolConfig struct { // Path to the tool directory ToolDirectory string @@ -49,15 +60,21 @@ type ToolConfig struct { type Tool interface { Name() string +} + +type CheckTool interface { + Tool Check(ctx context.Context, check *Check, config ToolConfig) error +} + +type FixTool interface { + Tool Fix(ctx context.Context, config ToolConfig) error - Format(ctx context.Context, config ToolConfig, dryRun bool) error } -// ValidationTool marks tools whose Check method performs validation. -type ValidationTool interface { +type FormatTool interface { Tool - ValidationTool() + Format(ctx context.Context, config ToolConfig, dryRun bool) error } func (tl ToolList) Only(only string) (ToolList, error) { diff --git a/internal/verifier/tool_test.go b/internal/verifier/tool_test.go index 36973ab8..c3ba05bc 100644 --- a/internal/verifier/tool_test.go +++ b/internal/verifier/tool_test.go @@ -22,6 +22,12 @@ func toolNames(list ToolList) []string { return out } +func TestToolsByCapability(t *testing.T) { + assert.ElementsMatch(t, []string{"admin-twig", "eslint", "phpstan", "storefront-twig", "stylelint", "sw-cli"}, toolNames(GetToolsOf[CheckTool]())) + assert.ElementsMatch(t, []string{"admin-twig", "eslint", "rector", "stylelint", "symfony-xml"}, toolNames(GetToolsOf[FixTool]())) + assert.ElementsMatch(t, []string{"admin-twig", "php-cs-fixer", "prettier"}, toolNames(GetToolsOf[FormatTool]())) +} + func TestExclude_EmptyString_NoChange(t *testing.T) { t.Parallel() base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} From e047882a51e40a30faca25a98847e460644f09b1 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Thu, 24 Sep 2026 17:08:32 +0200 Subject: [PATCH 04/14] fix: adjust extension fix + format as well and some refactoring --- cmd/extension/extension_fix.go | 11 +- cmd/extension/extension_format.go | 12 +- cmd/extension/extension_tool_invocation.go | 23 ++++ .../extension_tool_invocation_test.go | 26 ++++ cmd/extension/extension_validate.go | 24 ++-- .../extension_validate_selection_test.go | 30 ++--- internal/validation/reporter.go | 114 +++++++++--------- internal/validation/reporter_test.go | 78 ++++++++---- 8 files changed, 204 insertions(+), 114 deletions(-) create mode 100644 cmd/extension/extension_tool_invocation.go create mode 100644 cmd/extension/extension_tool_invocation_test.go diff --git a/cmd/extension/extension_fix.go b/cmd/extension/extension_fix.go index 76cfb14e..f116e79f 100644 --- a/cmd/extension/extension_fix.go +++ b/cmd/extension/extension_fix.go @@ -9,6 +9,7 @@ import ( "golang.org/x/sync/errgroup" "github.com/shopware/shopware-cli/internal/extension" + "github.com/shopware/shopware-cli/internal/validation" "github.com/shopware/shopware-cli/internal/verifier" "github.com/shopware/shopware-cli/logging" ) @@ -48,10 +49,10 @@ var extensionFixCmd = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetToolsOf[verifier.FixTool]() + allTools := verifier.GetToolsOf[verifier.FixTool]() only, _ := cmd.Flags().GetString("only") - tools, err = tools.Only(only) + tools, err := allTools.Only(only) if err != nil { return err } @@ -63,11 +64,11 @@ var extensionFixCmd = &cobra.Command{ }) } - if err := gr.Wait(); err != nil { + runErr := gr.Wait() + if err := validation.PrintToolInvocationTable(os.Stdout, "Fixers", extensionToolInvocationStatuses(allTools, tools)); err != nil { return err } - - return nil + return runErr }, } diff --git a/cmd/extension/extension_format.go b/cmd/extension/extension_format.go index 31e2fbc1..99a590dd 100644 --- a/cmd/extension/extension_format.go +++ b/cmd/extension/extension_format.go @@ -2,12 +2,14 @@ package extension import ( "fmt" + "os" "path/filepath" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" "github.com/shopware/shopware-cli/internal/extension" + "github.com/shopware/shopware-cli/internal/validation" "github.com/shopware/shopware-cli/internal/verifier" "github.com/shopware/shopware-cli/logging" ) @@ -41,10 +43,10 @@ var extensionFormat = &cobra.Command{ var gr errgroup.Group - tools := verifier.GetToolsOf[verifier.FormatTool]() + allTools := verifier.GetToolsOf[verifier.FormatTool]() only, _ := cmd.Flags().GetString("only") - tools, err = tools.Only(only) + tools, err := allTools.Only(only) if err != nil { return err } @@ -56,11 +58,11 @@ var extensionFormat = &cobra.Command{ }) } - if err := gr.Wait(); err != nil { + runErr := gr.Wait() + if err := validation.PrintToolInvocationTable(os.Stdout, "Formatters", extensionToolInvocationStatuses(allTools, tools)); err != nil { return err } - - return nil + return runErr }, } diff --git a/cmd/extension/extension_tool_invocation.go b/cmd/extension/extension_tool_invocation.go new file mode 100644 index 00000000..d573a37b --- /dev/null +++ b/cmd/extension/extension_tool_invocation.go @@ -0,0 +1,23 @@ +package extension + +import ( + "slices" + "sort" + + "github.com/shopware/shopware-cli/internal/validation" + "github.com/shopware/shopware-cli/internal/verifier" +) + +func extensionToolInvocationStatuses(all, selected verifier.ToolList) []validation.ToolInvocationStatus { + statuses := make([]validation.ToolInvocationStatus, 0, len(all)) + for _, tool := range all { + status := validation.ToolInvocationStatus{Name: tool.Name(), Status: "skipped", Reason: "not selected by --only"} + if slices.ContainsFunc(selected, func(selected verifier.Tool) bool { return selected.Name() == tool.Name() }) { + status.Status = "invoked" + status.Reason = "" + } + statuses = append(statuses, status) + } + sort.Slice(statuses, func(i, j int) bool { return statuses[i].Name < statuses[j].Name }) + return statuses +} diff --git a/cmd/extension/extension_tool_invocation_test.go b/cmd/extension/extension_tool_invocation_test.go new file mode 100644 index 00000000..e0ad72fb --- /dev/null +++ b/cmd/extension/extension_tool_invocation_test.go @@ -0,0 +1,26 @@ +package extension + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/shopware/shopware-cli/internal/verifier" +) + +func TestExtensionToolInvocationStatuses(t *testing.T) { + for _, all := range []verifier.ToolList{ + verifier.GetToolsOf[verifier.FixTool](), + verifier.GetToolsOf[verifier.FormatTool](), + } { + selected, err := all.Only(all[0].Name()) + require.NoError(t, err) + + statuses := extensionToolInvocationStatuses(all, selected) + assert.Len(t, statuses, len(all)) + assert.Equal(t, "invoked", toolStatusByName(t, statuses, all[0].Name()).Status) + assert.Equal(t, "skipped", toolStatusByName(t, statuses, all[1].Name()).Status) + assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, all[1].Name()).Reason) + } +} diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index 94988bee..88af888a 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -35,7 +35,7 @@ var extensionValidateCmd = &cobra.Command{ exclude, _ := cmd.Flags().GetString("exclude") noCopy, _ := cmd.Flags().GetBool("no-copy") - tools, coverage, err := selectExtensionValidationTools(isFull, only, exclude) + tools, statuses, err := selectExtensionValidationTools(isFull, only, exclude) if err != nil { return err } @@ -130,11 +130,11 @@ var extensionValidateCmd = &cobra.Command{ return err } - return validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, coverage...) + return validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, statuses...) }, } -func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList, []validation.CheckCoverage, error) { +func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList, []validation.ToolInvocationStatus, error) { validationTools := verifier.GetToolsOf[verifier.CheckTool]() requested := only @@ -174,24 +174,24 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T invoked[tool.Name()] = true } - coverage := make([]validation.CheckCoverage, 0, len(validationTools)) + statuses := make([]validation.ToolInvocationStatus, 0, len(validationTools)) for _, tool := range validationTools { name := tool.Name() - check := validation.CheckCoverage{Name: name, Status: "skipped"} + status := validation.ToolInvocationStatus{Name: name, Status: "skipped"} switch { case invoked[name]: - check.Status = "invoked" + status.Status = "invoked" case requestedNames[name]: - check.Reason = "excluded by --exclude" + status.Reason = "excluded by --exclude" case only != "": - check.Reason = "not selected by --only" + status.Reason = "not selected by --only" default: - check.Reason = "not selected; use --full or --only" + status.Reason = "not selected; use --full or --only" } - coverage = append(coverage, check) + statuses = append(statuses, status) } - sort.Slice(coverage, func(i, j int) bool { return coverage[i].Name < coverage[j].Name }) - return unique, coverage, nil + sort.Slice(statuses, func(i, j int) bool { return statuses[i].Name < statuses[j].Name }) + return unique, statuses, nil } func requiresToolSetup(tool verifier.Tool) bool { diff --git a/cmd/extension/extension_validate_selection_test.go b/cmd/extension/extension_validate_selection_test.go index a4a731be..71b50916 100644 --- a/cmd/extension/extension_validate_selection_test.go +++ b/cmd/extension/extension_validate_selection_test.go @@ -11,33 +11,33 @@ import ( "github.com/shopware/shopware-cli/internal/verifier" ) -func coverageByName(t *testing.T, checks []validation.CheckCoverage, name string) validation.CheckCoverage { +func toolStatusByName(t *testing.T, statuses []validation.ToolInvocationStatus, name string) validation.ToolInvocationStatus { t.Helper() - for _, check := range checks { - if check.Name == name { - return check + for _, status := range statuses { + if status.Name == name { + return status } } - t.Fatalf("missing coverage for %s", name) - return validation.CheckCoverage{} + t.Fatalf("missing tool status for %s", name) + return validation.ToolInvocationStatus{} } func TestExtensionValidationSelection(t *testing.T) { t.Run("default runs only sw-cli", func(t *testing.T) { - tools, checks, err := selectExtensionValidationTools(false, "", "") + tools, statuses, err := selectExtensionValidationTools(false, "", "") require.NoError(t, err) assert.Equal(t, []string{"sw-cli"}, toolNamesForValidation(tools)) assert.False(t, slices.ContainsFunc(tools, requiresToolSetup)) - assert.Equal(t, "not selected; use --full or --only", coverageByName(t, checks, "phpstan").Reason) + assert.Equal(t, "not selected; use --full or --only", toolStatusByName(t, statuses, "phpstan").Reason) }) t.Run("only phpstan works without full", func(t *testing.T) { - tools, checks, err := selectExtensionValidationTools(false, "phpstan", "") + tools, statuses, err := selectExtensionValidationTools(false, "phpstan", "") require.NoError(t, err) assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) - assert.Equal(t, "invoked", coverageByName(t, checks, "phpstan").Status) - assert.Equal(t, "not selected by --only", coverageByName(t, checks, "sw-cli").Reason) + assert.Equal(t, "invoked", toolStatusByName(t, statuses, "phpstan").Status) + assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, "sw-cli").Reason) }) t.Run("Twig validation needs no external tools", func(t *testing.T) { @@ -48,18 +48,18 @@ func TestExtensionValidationSelection(t *testing.T) { }) t.Run("full selects all validation checks", func(t *testing.T) { - tools, checks, err := selectExtensionValidationTools(true, "", "") + tools, statuses, err := selectExtensionValidationTools(true, "", "") require.NoError(t, err) assert.Len(t, tools, 6) - assert.Len(t, checks, len(tools)) + assert.Len(t, statuses, len(tools)) assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) }) t.Run("exclude applies after only", func(t *testing.T) { - tools, checks, err := selectExtensionValidationTools(false, "phpstan,sw-cli", "sw-cli") + tools, statuses, err := selectExtensionValidationTools(false, "phpstan,sw-cli", "sw-cli") require.NoError(t, err) assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) - assert.Equal(t, "excluded by --exclude", coverageByName(t, checks, "sw-cli").Reason) + assert.Equal(t, "excluded by --exclude", toolStatusByName(t, statuses, "sw-cli").Reason) }) t.Run("duplicate only values run once", func(t *testing.T) { diff --git a/internal/validation/reporter.go b/internal/validation/reporter.go index e62006fe..f9a89fc2 100644 --- a/internal/validation/reporter.go +++ b/internal/validation/reporter.go @@ -39,48 +39,48 @@ func DetectDefaultReporter() string { return "summary" } -// CheckCoverage records whether a validation check was selected and invoked. -type CheckCoverage struct { +// ToolInvocationStatus records whether a tool was invoked or skipped. +type ToolInvocationStatus struct { Name string `json:"name"` Status string `json:"status"` Reason string `json:"reason,omitempty"` } -// DoCheckReport reports findings and, when supplied, validation coverage. -func DoCheckReport(result Check, reportingFormat string, checks ...CheckCoverage) error { +// DoCheckReport reports findings and, when supplied, tool invocation statuses. +func DoCheckReport(result Check, reportingFormat string, tools ...ToolInvocationStatus) error { if err := ValidateReporter(reportingFormat); err != nil { return err } switch reportingFormat { case "summary": - if err := doSummaryReport(result, checks...); err != nil { + if err := doSummaryReport(result, tools...); err != nil { return err } case "json": - if err := doJSONReport(result, checks...); err != nil { + if err := doJSONReport(result, tools...); err != nil { return err } case "github": - if err := doGitHubReport(result, checks...); err != nil { + if err := doGitHubReport(result, tools...); err != nil { return err } case "gitlab": if err := doGitLabReport(result); err != nil { return err } - if err := printCoverage(os.Stderr, checks); err != nil { + if err := PrintToolInvocationTable(os.Stderr, "Tools", tools); err != nil { return err } case "markdown": - if err := doMarkdownReport(result, checks...); err != nil { + if err := doMarkdownReport(result, tools...); err != nil { return err } case "junit": - if err := doJUnitReport(result, checks...); err != nil { + if err := doJUnitReport(result, tools...); err != nil { return err } - if err := printCoverage(os.Stderr, checks); err != nil { + if err := PrintToolInvocationTable(os.Stderr, "Tools", tools); err != nil { return err } } @@ -92,11 +92,7 @@ func DoCheckReport(result Check, reportingFormat string, checks ...CheckCoverage return nil } -func doSummaryReport(result Check, checks ...CheckCoverage) error { - if err := printCoverage(os.Stdout, checks); err != nil { - return err - } - +func doSummaryReport(result Check, tools ...ToolInvocationStatus) error { // Group results by file fileGroups := make(map[string][]CheckResult) for _, r := range result.GetResults() { @@ -151,9 +147,13 @@ func doSummaryReport(result Check, checks ...CheckCoverage) error { } } + if err := PrintToolInvocationTable(os.Stdout, "Tools", tools); err != nil { + return err + } + //nolint:forbidigo - if checks != nil && !anyCheckInvoked(checks) { - fmt.Println("\nNo checks invoked; 0 problems reported") + if tools != nil && !anyToolInvoked(tools) { + fmt.Println("\nNo tools invoked; 0 problems reported") } else { fmt.Printf("\n%s\n", summaryLine(totalProblems, errorCount, warningCount)) } @@ -178,12 +178,12 @@ func countNoun(n int, noun string) string { return fmt.Sprintf("%d %ss", n, noun) } -func doJSONReport(result Check, checks ...CheckCoverage) error { +func doJSONReport(result Check, tools ...ToolInvocationStatus) error { data := map[string]interface{}{ "results": result.GetResults(), } - if checks != nil { - data["checks"] = checks + if tools != nil { + data["tools"] = tools } encoder := json.NewEncoder(os.Stdout) @@ -191,7 +191,7 @@ func doJSONReport(result Check, checks ...CheckCoverage) error { return encoder.Encode(data) } -func doGitHubReport(result Check, checks ...CheckCoverage) error { +func doGitHubReport(result Check, tools ...ToolInvocationStatus) error { // Print the human-readable summary first so the GitHub Actions log // shows file/line context, then emit annotations for PR inline display. // File paths and messages can contain `::` which the runner would parse @@ -204,7 +204,7 @@ func doGitHubReport(result Check, checks ...CheckCoverage) error { token := hex.EncodeToString(tokenBytes[:]) fmt.Printf("::stop-commands::%s\n", token) - if err := doSummaryReport(result, checks...); err != nil { + if err := doSummaryReport(result, tools...); err != nil { fmt.Printf("::%s::\n", token) return err } @@ -332,7 +332,7 @@ func doGitLabReport(result Check) error { return encoder.Encode(issues) } -func doMarkdownReport(result Check, checks ...CheckCoverage) error { +func doMarkdownReport(result Check, tools ...ToolInvocationStatus) error { // Group results by file fileGroups := make(map[string][]CheckResult) for _, r := range result.GetResults() { @@ -367,16 +367,6 @@ func doMarkdownReport(result Check, checks ...CheckCoverage) error { fmt.Println("# Validation Report") fmt.Println() - if checks != nil { - fmt.Println("## Checks") - fmt.Println() - fmt.Println("```text") - for _, row := range coverageRows(checks) { - fmt.Println(row) - } - fmt.Println("```") - fmt.Println() - } totalProblems := 0 for _, path := range sortedPaths { @@ -406,8 +396,19 @@ func doMarkdownReport(result Check, checks ...CheckCoverage) error { fmt.Println() } - if checks != nil && !anyCheckInvoked(checks) { - fmt.Println("No checks invoked; 0 problems reported") + if tools != nil { + fmt.Println("## Tools") + fmt.Println() + fmt.Println("```text") + for _, row := range toolInvocationRows(tools) { + fmt.Println(row) + } + fmt.Println("```") + fmt.Println() + } + + if tools != nil && !anyToolInvoked(tools) { + fmt.Println("No tools invoked; 0 problems reported") } else if totalProblems == 0 { fmt.Println("✅ No problems found") } @@ -415,14 +416,15 @@ func doMarkdownReport(result Check, checks ...CheckCoverage) error { return nil } -func printCoverage(w io.Writer, checks []CheckCoverage) error { - if checks == nil { +// PrintToolInvocationTable writes an aligned table of invoked and skipped tools. +func PrintToolInvocationTable(w io.Writer, title string, tools []ToolInvocationStatus) error { + if tools == nil { return nil } - if _, err := fmt.Fprintln(w, "Checks:"); err != nil { + if _, err := fmt.Fprintln(w, "\n"+title+":"); err != nil { return err } - for _, row := range coverageRows(checks) { + for _, row := range toolInvocationRows(tools) { if _, err := fmt.Fprintln(w, " "+row); err != nil { return err } @@ -430,26 +432,26 @@ func printCoverage(w io.Writer, checks []CheckCoverage) error { return nil } -func coverageRows(checks []CheckCoverage) []string { +func toolInvocationRows(tools []ToolInvocationStatus) []string { width := 0 - for _, check := range checks { - width = max(width, len(check.Name)) + for _, tool := range tools { + width = max(width, len(tool.Name)) } - rows := make([]string, 0, len(checks)) - for _, check := range checks { - row := fmt.Sprintf("%-*s %-7s", width, check.Name, check.Status) - if check.Reason != "" { - row += " " + check.Reason + rows := make([]string, 0, len(tools)) + for _, tool := range tools { + row := fmt.Sprintf("%-*s %-7s", width, tool.Name, tool.Status) + if tool.Reason != "" { + row += " " + tool.Reason } rows = append(rows, strings.TrimRight(row, " ")) } return rows } -func anyCheckInvoked(checks []CheckCoverage) bool { - for _, check := range checks { - if check.Status == "invoked" { +func anyToolInvoked(tools []ToolInvocationStatus) bool { + for _, tool := range tools { + if tool.Status == "invoked" { return true } } @@ -490,7 +492,7 @@ type JUnitTestError struct { Content string `xml:",chardata"` } -func doJUnitReport(result Check, checks ...CheckCoverage) error { +func doJUnitReport(result Check, tools ...ToolInvocationStatus) error { var testCases []JUnitTestCase errors := 0 failures := 0 @@ -546,10 +548,10 @@ func doJUnitReport(result Check, checks ...CheckCoverage) error { testCases = append(testCases, testCase) } - for _, check := range checks { - testCase := JUnitTestCase{Name: check.Name, ClassName: "validation"} - if check.Status == "skipped" { - testCase.Skipped = &JUnitTestSkipped{Message: check.Reason} + for _, tool := range tools { + testCase := JUnitTestCase{Name: tool.Name, ClassName: "tool"} + if tool.Status == "skipped" { + testCase.Skipped = &JUnitTestSkipped{Message: tool.Reason} skipped++ } testCases = append(testCases, testCase) diff --git a/internal/validation/reporter_test.go b/internal/validation/reporter_test.go index d10816c8..51e3100f 100644 --- a/internal/validation/reporter_test.go +++ b/internal/validation/reporter_test.go @@ -166,65 +166,98 @@ func TestErrorExistsSummary(t *testing.T) { assert.Error(t, DoCheckReport(check, "summary")) } -func TestCheckCoverageReports(t *testing.T) { +func TestToolInvocationReports(t *testing.T) { check := &testCheck{Results: []CheckResult{}} - checks := []CheckCoverage{ + tools := []ToolInvocationStatus{ {Name: "eslint", Status: "skipped", Reason: "no JavaScript source files"}, {Name: "storefront-twig", Status: "skipped", Reason: "no storefront Twig templates"}, } - rows := coverageRows(checks) + rows := toolInvocationRows(tools) assert.Contains(t, rows[0], "eslint") assert.Contains(t, rows[0], " skipped no JavaScript source files") assert.NotContains(t, rows[0], "(") summary := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "summary", checks...)) + assert.NoError(t, DoCheckReport(check, "summary", tools...)) }) for _, row := range rows { assert.Contains(t, summary, " "+row) } - assert.Contains(t, summary, "No checks invoked; 0 problems reported") + assert.Contains(t, summary, "Tools:") + assert.True(t, strings.HasPrefix(summary, "\nTools:\n")) + assert.Less(t, strings.Index(summary, "Tools:"), strings.Index(summary, "No tools invoked;")) + assert.Contains(t, summary, "No tools invoked; 0 problems reported") assert.NotContains(t, summary, "No problems found") github := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "github", checks...)) + assert.NoError(t, DoCheckReport(check, "github", tools...)) }) for _, row := range rows { assert.Contains(t, github, " "+row) } markdown := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "markdown", checks...)) + assert.NoError(t, DoCheckReport(check, "markdown", tools...)) }) - assert.Contains(t, markdown, "## Checks") + assert.Contains(t, markdown, "## Tools") + assert.Less(t, strings.Index(markdown, "## Tools"), strings.Index(markdown, "No tools invoked;")) for _, row := range rows { assert.Contains(t, markdown, row) } - assert.Contains(t, markdown, "No checks invoked; 0 problems reported") + assert.Contains(t, markdown, "No tools invoked; 0 problems reported") jsonOutput := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "json", checks...)) + assert.NoError(t, DoCheckReport(check, "json", tools...)) }) var report struct { - Results []CheckResult `json:"results"` - Checks []CheckCoverage `json:"checks"` + Results []CheckResult `json:"results"` + Tools []ToolInvocationStatus `json:"tools"` } assert.NoError(t, json.Unmarshal([]byte(jsonOutput), &report)) assert.Empty(t, report.Results) - assert.Equal(t, checks, report.Checks) + assert.Equal(t, tools, report.Tools) + assert.NotContains(t, jsonOutput, `"checks"`) - checks[0] = CheckCoverage{Name: "eslint", Status: "invoked"} + tools[0] = ToolInvocationStatus{Name: "eslint", Status: "invoked"} summary = captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "summary", checks...)) + assert.NoError(t, DoCheckReport(check, "summary", tools...)) }) assert.Contains(t, summary, "eslint") assert.Contains(t, summary, " invoked") assert.Contains(t, summary, "No problems found") } -func TestStructuredReportsKeepMachineOutputAndShowCoverage(t *testing.T) { +func TestPrintToolInvocationTableWithOperationTitle(t *testing.T) { + var output strings.Builder + tools := []ToolInvocationStatus{ + {Name: "eslint", Status: "invoked"}, + {Name: "rector", Status: "skipped", Reason: "not selected by --only"}, + } + assert.NoError(t, PrintToolInvocationTable(&output, "Fixers", tools)) + assert.Equal(t, "\nFixers:\n eslint invoked\n rector skipped not selected by --only\n", output.String()) +} + +func TestToolInvocationTableFollowsFindings(t *testing.T) { + check := &testCheck{Results: []CheckResult{{Path: "src/file.php", Line: 1, Message: "problem", Severity: SeverityWarning}}} + tools := []ToolInvocationStatus{{Name: "sw-cli", Status: "invoked"}} + + summary := captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "summary", tools...)) + }) + assert.Less(t, strings.Index(summary, "src/file.php"), strings.Index(summary, "Tools:")) + assert.Less(t, strings.Index(summary, "Tools:"), strings.Index(summary, "✖ 1 problem")) + assert.Contains(t, summary, "\n\nTools:\n") + assert.NotContains(t, summary, "\n\n\nTools:\n") + + markdown := captureOutput(func() { + assert.NoError(t, DoCheckReport(check, "markdown", tools...)) + }) + assert.Less(t, strings.Index(markdown, "## src/file.php"), strings.Index(markdown, "## Tools")) +} + +func TestStructuredReportsKeepMachineOutputAndShowToolStatuses(t *testing.T) { check := &testCheck{Results: []CheckResult{}} - checks := []CheckCoverage{ + tools := []ToolInvocationStatus{ {Name: "phpstan", Status: "invoked"}, {Name: "sw-cli", Status: "skipped", Reason: "not selected by --only"}, } @@ -232,20 +265,20 @@ func TestStructuredReportsKeepMachineOutputAndShowCoverage(t *testing.T) { var gitlabLog string gitlab := captureOutput(func() { gitlabLog = captureStderr(func() { - assert.NoError(t, DoCheckReport(check, "gitlab", checks...)) + assert.NoError(t, DoCheckReport(check, "gitlab", tools...)) }) }) var issues []GitLabCodeQualityIssue assert.NoError(t, json.Unmarshal([]byte(gitlab), &issues)) assert.Empty(t, issues) - for _, row := range coverageRows(checks) { + for _, row := range toolInvocationRows(tools) { assert.Contains(t, gitlabLog, " "+row) } var junitLog string junit := captureOutput(func() { junitLog = captureStderr(func() { - assert.NoError(t, DoCheckReport(check, "junit", checks...)) + assert.NoError(t, DoCheckReport(check, "junit", tools...)) }) }) var suite JUnitTestSuite @@ -254,10 +287,11 @@ func TestStructuredReportsKeepMachineOutputAndShowCoverage(t *testing.T) { assert.Equal(t, 1, suite.Skipped) require.Len(t, suite.TestCase, 2) assert.Equal(t, "phpstan", suite.TestCase[0].Name) + assert.Equal(t, "tool", suite.TestCase[0].ClassName) assert.Nil(t, suite.TestCase[0].Skipped) require.NotNil(t, suite.TestCase[1].Skipped) assert.Equal(t, "not selected by --only", suite.TestCase[1].Skipped.Message) - for _, row := range coverageRows(checks) { + for _, row := range toolInvocationRows(tools) { assert.Contains(t, junitLog, " "+row) } } @@ -539,6 +573,7 @@ func TestMarkdownReportWithTip(t *testing.T) { assert.Contains(t, output, "Method has no return type") assert.Contains(t, output, "*Tip: Add a return type declaration*") + assert.NotContains(t, output, "## Tools") } func TestJSONReportWithTip(t *testing.T) { @@ -565,6 +600,7 @@ func TestJSONReportWithTip(t *testing.T) { assert.NoError(t, err) assert.Len(t, result["results"], 1) assert.Equal(t, "Add a return type declaration", result["results"][0].Tip) + assert.NotContains(t, output, `"tools"`) } func TestJUnitReportWithTip(t *testing.T) { From e97759ea02082a050e346e236df5dbf2925efb1c Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Thu, 24 Sep 2026 17:33:25 +0200 Subject: [PATCH 05/14] fix: error on validate --only reports correct possible tools --- cmd/extension/extension_validate.go | 8 ++---- .../extension_validate_selection_test.go | 14 ++++++++-- internal/validation/reporter.go | 12 ++++----- internal/validation/reporter_test.go | 26 +++++++++---------- 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index 88af888a..e7a95d68 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -144,17 +144,13 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T requested = validationTools.PossibleString() } } - selected, err := verifier.GetTools().Only(requested) + selected, err := validationTools.Only(requested) if err != nil { return nil, nil, err } requestedNames := make(map[string]bool, len(selected)) for _, tool := range selected { - name := tool.Name() - if _, ok := tool.(verifier.CheckTool); !ok { - return nil, nil, fmt.Errorf("%s does not provide a validation check", name) - } - requestedNames[name] = true + requestedNames[tool.Name()] = true } selected, err = selected.Exclude(exclude) if err != nil { diff --git a/cmd/extension/extension_validate_selection_test.go b/cmd/extension/extension_validate_selection_test.go index 71b50916..ef08d8c4 100644 --- a/cmd/extension/extension_validate_selection_test.go +++ b/cmd/extension/extension_validate_selection_test.go @@ -68,9 +68,19 @@ func TestExtensionValidationSelection(t *testing.T) { assert.Equal(t, []string{"sw-cli"}, toolNamesForValidation(tools)) }) - t.Run("unsupported operation fails", func(t *testing.T) { + t.Run("unsupported operation lists checkers", func(t *testing.T) { _, _, err := selectExtensionValidationTools(false, "prettier", "") - require.EqualError(t, err, "prettier does not provide a validation check") + require.ErrorContains(t, err, `tool with name "prettier" not found, possible tools:`) + assert.NotContains(t, err.Error(), "prettier,") + assert.Contains(t, err.Error(), "phpstan") + }) + + t.Run("typo lists only checkers", func(t *testing.T) { + _, _, err := selectExtensionValidationTools(false, "phpsta", "") + require.ErrorContains(t, err, `tool with name "phpsta" not found, possible tools:`) + assert.Contains(t, err.Error(), "phpstan") + assert.NotContains(t, err.Error(), "prettier") + assert.NotContains(t, err.Error(), "rector") }) t.Run("empty selection fails", func(t *testing.T) { diff --git a/internal/validation/reporter.go b/internal/validation/reporter.go index f9a89fc2..ade46b28 100644 --- a/internal/validation/reporter.go +++ b/internal/validation/reporter.go @@ -69,7 +69,7 @@ func DoCheckReport(result Check, reportingFormat string, tools ...ToolInvocation if err := doGitLabReport(result); err != nil { return err } - if err := PrintToolInvocationTable(os.Stderr, "Tools", tools); err != nil { + if err := PrintToolInvocationTable(os.Stderr, "Checkers", tools); err != nil { return err } case "markdown": @@ -80,7 +80,7 @@ func DoCheckReport(result Check, reportingFormat string, tools ...ToolInvocation if err := doJUnitReport(result, tools...); err != nil { return err } - if err := PrintToolInvocationTable(os.Stderr, "Tools", tools); err != nil { + if err := PrintToolInvocationTable(os.Stderr, "Checkers", tools); err != nil { return err } } @@ -147,13 +147,13 @@ func doSummaryReport(result Check, tools ...ToolInvocationStatus) error { } } - if err := PrintToolInvocationTable(os.Stdout, "Tools", tools); err != nil { + if err := PrintToolInvocationTable(os.Stdout, "Checkers", tools); err != nil { return err } //nolint:forbidigo if tools != nil && !anyToolInvoked(tools) { - fmt.Println("\nNo tools invoked; 0 problems reported") + fmt.Println("\nNo checkers invoked; 0 problems reported") } else { fmt.Printf("\n%s\n", summaryLine(totalProblems, errorCount, warningCount)) } @@ -397,7 +397,7 @@ func doMarkdownReport(result Check, tools ...ToolInvocationStatus) error { } if tools != nil { - fmt.Println("## Tools") + fmt.Println("## Checkers") fmt.Println() fmt.Println("```text") for _, row := range toolInvocationRows(tools) { @@ -408,7 +408,7 @@ func doMarkdownReport(result Check, tools ...ToolInvocationStatus) error { } if tools != nil && !anyToolInvoked(tools) { - fmt.Println("No tools invoked; 0 problems reported") + fmt.Println("No checkers invoked; 0 problems reported") } else if totalProblems == 0 { fmt.Println("✅ No problems found") } diff --git a/internal/validation/reporter_test.go b/internal/validation/reporter_test.go index 51e3100f..749c9018 100644 --- a/internal/validation/reporter_test.go +++ b/internal/validation/reporter_test.go @@ -183,10 +183,10 @@ func TestToolInvocationReports(t *testing.T) { for _, row := range rows { assert.Contains(t, summary, " "+row) } - assert.Contains(t, summary, "Tools:") - assert.True(t, strings.HasPrefix(summary, "\nTools:\n")) - assert.Less(t, strings.Index(summary, "Tools:"), strings.Index(summary, "No tools invoked;")) - assert.Contains(t, summary, "No tools invoked; 0 problems reported") + assert.Contains(t, summary, "Checkers:") + assert.True(t, strings.HasPrefix(summary, "\nCheckers:\n")) + assert.Less(t, strings.Index(summary, "Checkers:"), strings.Index(summary, "No checkers invoked;")) + assert.Contains(t, summary, "No checkers invoked; 0 problems reported") assert.NotContains(t, summary, "No problems found") github := captureOutput(func() { @@ -199,12 +199,12 @@ func TestToolInvocationReports(t *testing.T) { markdown := captureOutput(func() { assert.NoError(t, DoCheckReport(check, "markdown", tools...)) }) - assert.Contains(t, markdown, "## Tools") - assert.Less(t, strings.Index(markdown, "## Tools"), strings.Index(markdown, "No tools invoked;")) + assert.Contains(t, markdown, "## Checkers") + assert.Less(t, strings.Index(markdown, "## Checkers"), strings.Index(markdown, "No checkers invoked;")) for _, row := range rows { assert.Contains(t, markdown, row) } - assert.Contains(t, markdown, "No tools invoked; 0 problems reported") + assert.Contains(t, markdown, "No checkers invoked; 0 problems reported") jsonOutput := captureOutput(func() { assert.NoError(t, DoCheckReport(check, "json", tools...)) @@ -244,15 +244,15 @@ func TestToolInvocationTableFollowsFindings(t *testing.T) { summary := captureOutput(func() { assert.NoError(t, DoCheckReport(check, "summary", tools...)) }) - assert.Less(t, strings.Index(summary, "src/file.php"), strings.Index(summary, "Tools:")) - assert.Less(t, strings.Index(summary, "Tools:"), strings.Index(summary, "✖ 1 problem")) - assert.Contains(t, summary, "\n\nTools:\n") - assert.NotContains(t, summary, "\n\n\nTools:\n") + assert.Less(t, strings.Index(summary, "src/file.php"), strings.Index(summary, "Checkers:")) + assert.Less(t, strings.Index(summary, "Checkers:"), strings.Index(summary, "✖ 1 problem")) + assert.Contains(t, summary, "\n\nCheckers:\n") + assert.NotContains(t, summary, "\n\n\nCheckers:\n") markdown := captureOutput(func() { assert.NoError(t, DoCheckReport(check, "markdown", tools...)) }) - assert.Less(t, strings.Index(markdown, "## src/file.php"), strings.Index(markdown, "## Tools")) + assert.Less(t, strings.Index(markdown, "## src/file.php"), strings.Index(markdown, "## Checkers")) } func TestStructuredReportsKeepMachineOutputAndShowToolStatuses(t *testing.T) { @@ -573,7 +573,7 @@ func TestMarkdownReportWithTip(t *testing.T) { assert.Contains(t, output, "Method has no return type") assert.Contains(t, output, "*Tip: Add a return type declaration*") - assert.NotContains(t, output, "## Tools") + assert.NotContains(t, output, "## Checkers") } func TestJSONReportWithTip(t *testing.T) { From 5dabd47c3f67d4e215b049edf91dafe05d3ff3b1 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 14:21:00 +0200 Subject: [PATCH 06/14] fix: ensure executed tool list is unique --- cmd/extension/extension_validate.go | 16 ++++++---------- internal/verifier/tool.go | 6 +++++- internal/verifier/tool_test.go | 11 +++++++++++ 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index e7a95d68..81b2ce97 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -126,11 +126,12 @@ var extensionValidateCmd = &cobra.Command{ }) } - if err := gr.Wait(); err != nil { - return err + runErr := gr.Wait() + reportErr := validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, statuses...) + if runErr != nil { + return runErr } - - return validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, statuses...) + return reportErr }, } @@ -160,13 +161,8 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T return nil, nil, errors.New("no validation checks selected after applying --exclude") } - unique := make(verifier.ToolList, 0, len(selected)) invoked := make(map[string]bool, len(selected)) for _, tool := range selected { - if invoked[tool.Name()] { - continue - } - unique = append(unique, tool) invoked[tool.Name()] = true } @@ -187,7 +183,7 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T statuses = append(statuses, status) } sort.Slice(statuses, func(i, j int) bool { return statuses[i].Name < statuses[j].Name }) - return unique, statuses, nil + return selected, statuses, nil } func requiresToolSetup(tool verifier.Tool) bool { diff --git a/internal/verifier/tool.go b/internal/verifier/tool.go index 5f49e6d0..9c5ff194 100644 --- a/internal/verifier/tool.go +++ b/internal/verifier/tool.go @@ -84,6 +84,7 @@ func (tl ToolList) Only(only string) (ToolList, error) { var filteredTools []Tool requestedTools := strings.Split(only, ",") + seen := make(map[string]bool, len(requestedTools)) for _, requestedTool := range requestedTools { requestedTool = strings.TrimSpace(requestedTool) @@ -91,7 +92,10 @@ func (tl ToolList) Only(only string) (ToolList, error) { for _, t := range tl { if t.Name() == requestedTool { - filteredTools = append(filteredTools, t) + if !seen[requestedTool] { + filteredTools = append(filteredTools, t) + seen[requestedTool] = true + } found = true break } diff --git a/internal/verifier/tool_test.go b/internal/verifier/tool_test.go index c3ba05bc..1503ed54 100644 --- a/internal/verifier/tool_test.go +++ b/internal/verifier/tool_test.go @@ -28,6 +28,17 @@ func TestToolsByCapability(t *testing.T) { assert.ElementsMatch(t, []string{"admin-twig", "php-cs-fixer", "prettier"}, toolNames(GetToolsOf[FormatTool]())) } +func TestOnly_DeduplicatesAndPreservesOrder(t *testing.T) { + t.Parallel() + base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} + res, err := base.Only("eslint, phpstan,eslint") + assert.NoError(t, err) + assert.Equal(t, []string{"eslint", "phpstan"}, toolNames(res)) + res, err = base.Only("eslint,eslint,unknown") + assert.ErrorContains(t, err, `tool with name "unknown" not found`) + assert.Nil(t, res) +} + func TestExclude_EmptyString_NoChange(t *testing.T) { t.Parallel() base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} From ba221e63ce236581f7662ccf9074be09b30da132 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 14:38:01 +0200 Subject: [PATCH 07/14] refactor: tool generics for more type safety --- cmd/extension/extension_fix.go | 3 +-- cmd/extension/extension_format.go | 3 +-- cmd/extension/extension_tool_invocation.go | 4 +-- .../extension_tool_invocation_test.go | 25 ++++++++++--------- cmd/extension/extension_validate.go | 7 +++--- .../extension_validate_selection_test.go | 2 +- cmd/project/project_fix.go | 3 +-- cmd/project/project_format.go | 3 +-- cmd/project/project_validate.go | 3 +-- internal/verifier/tool.go | 24 +++++++++--------- internal/verifier/tool_test.go | 16 ++++++------ 11 files changed, 44 insertions(+), 49 deletions(-) diff --git a/cmd/extension/extension_fix.go b/cmd/extension/extension_fix.go index f116e79f..0dec60fd 100644 --- a/cmd/extension/extension_fix.go +++ b/cmd/extension/extension_fix.go @@ -58,9 +58,8 @@ var extensionFixCmd = &cobra.Command{ } for _, tool := range tools { - fixer := tool.(verifier.FixTool) gr.Go(func() error { - return fixer.Fix(cmd.Context(), *toolCfg) + return tool.Fix(cmd.Context(), *toolCfg) }) } diff --git a/cmd/extension/extension_format.go b/cmd/extension/extension_format.go index 99a590dd..eb437666 100644 --- a/cmd/extension/extension_format.go +++ b/cmd/extension/extension_format.go @@ -52,9 +52,8 @@ var extensionFormat = &cobra.Command{ } for _, tool := range tools { - formatter := tool.(verifier.FormatTool) gr.Go(func() error { - return formatter.Format(cmd.Context(), *toolCfg, dryRun) + return tool.Format(cmd.Context(), *toolCfg, dryRun) }) } diff --git a/cmd/extension/extension_tool_invocation.go b/cmd/extension/extension_tool_invocation.go index d573a37b..d8def664 100644 --- a/cmd/extension/extension_tool_invocation.go +++ b/cmd/extension/extension_tool_invocation.go @@ -8,11 +8,11 @@ import ( "github.com/shopware/shopware-cli/internal/verifier" ) -func extensionToolInvocationStatuses(all, selected verifier.ToolList) []validation.ToolInvocationStatus { +func extensionToolInvocationStatuses[T verifier.Tool](all, selected verifier.ToolList[T]) []validation.ToolInvocationStatus { statuses := make([]validation.ToolInvocationStatus, 0, len(all)) for _, tool := range all { status := validation.ToolInvocationStatus{Name: tool.Name(), Status: "skipped", Reason: "not selected by --only"} - if slices.ContainsFunc(selected, func(selected verifier.Tool) bool { return selected.Name() == tool.Name() }) { + if slices.ContainsFunc(selected, func(selected T) bool { return selected.Name() == tool.Name() }) { status.Status = "invoked" status.Reason = "" } diff --git a/cmd/extension/extension_tool_invocation_test.go b/cmd/extension/extension_tool_invocation_test.go index e0ad72fb..d5b6474b 100644 --- a/cmd/extension/extension_tool_invocation_test.go +++ b/cmd/extension/extension_tool_invocation_test.go @@ -10,17 +10,18 @@ import ( ) func TestExtensionToolInvocationStatuses(t *testing.T) { - for _, all := range []verifier.ToolList{ - verifier.GetToolsOf[verifier.FixTool](), - verifier.GetToolsOf[verifier.FormatTool](), - } { - selected, err := all.Only(all[0].Name()) - require.NoError(t, err) + assertExtensionToolInvocationStatuses(t, verifier.GetToolsOf[verifier.FixTool]()) + assertExtensionToolInvocationStatuses(t, verifier.GetToolsOf[verifier.FormatTool]()) +} + +func assertExtensionToolInvocationStatuses[T verifier.Tool](t *testing.T, all verifier.ToolList[T]) { + t.Helper() + selected, err := all.Only(all[0].Name()) + require.NoError(t, err) - statuses := extensionToolInvocationStatuses(all, selected) - assert.Len(t, statuses, len(all)) - assert.Equal(t, "invoked", toolStatusByName(t, statuses, all[0].Name()).Status) - assert.Equal(t, "skipped", toolStatusByName(t, statuses, all[1].Name()).Status) - assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, all[1].Name()).Reason) - } + statuses := extensionToolInvocationStatuses(all, selected) + assert.Len(t, statuses, len(all)) + assert.Equal(t, "invoked", toolStatusByName(t, statuses, all[0].Name()).Status) + assert.Equal(t, "skipped", toolStatusByName(t, statuses, all[1].Name()).Status) + assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, all[1].Name()).Reason) } diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index 81b2ce97..f5d46ba7 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -120,9 +120,8 @@ var extensionValidateCmd = &cobra.Command{ var gr errgroup.Group for _, tool := range tools { - checker := tool.(verifier.CheckTool) gr.Go(func() error { - return checker.Check(cmd.Context(), result, *toolCfg) + return tool.Check(cmd.Context(), result, *toolCfg) }) } @@ -135,7 +134,7 @@ var extensionValidateCmd = &cobra.Command{ }, } -func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList, []validation.ToolInvocationStatus, error) { +func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList[verifier.CheckTool], []validation.ToolInvocationStatus, error) { validationTools := verifier.GetToolsOf[verifier.CheckTool]() requested := only @@ -186,7 +185,7 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T return selected, statuses, nil } -func requiresToolSetup(tool verifier.Tool) bool { +func requiresToolSetup(tool verifier.CheckTool) bool { switch tool.(type) { case verifier.PhpStan, verifier.Eslint, verifier.StyleLint: return true diff --git a/cmd/extension/extension_validate_selection_test.go b/cmd/extension/extension_validate_selection_test.go index ef08d8c4..302233e1 100644 --- a/cmd/extension/extension_validate_selection_test.go +++ b/cmd/extension/extension_validate_selection_test.go @@ -89,7 +89,7 @@ func TestExtensionValidationSelection(t *testing.T) { }) } -func toolNamesForValidation(tools verifier.ToolList) []string { +func toolNamesForValidation(tools verifier.ToolList[verifier.CheckTool]) []string { names := make([]string, 0, len(tools)) for _, tool := range tools { names = append(names, tool.Name()) diff --git a/cmd/project/project_fix.go b/cmd/project/project_fix.go index 2cd678e4..43d8c100 100644 --- a/cmd/project/project_fix.go +++ b/cmd/project/project_fix.go @@ -62,9 +62,8 @@ var projectFixCmd = &cobra.Command{ } for _, tool := range tools { - fixer := tool.(verifier.FixTool) gr.Go(func() error { - return fixer.Fix(cmd.Context(), *toolCfg) + return tool.Fix(cmd.Context(), *toolCfg) }) } diff --git a/cmd/project/project_format.go b/cmd/project/project_format.go index a692e1f4..95c92401 100644 --- a/cmd/project/project_format.go +++ b/cmd/project/project_format.go @@ -54,9 +54,8 @@ var projectFormatCmd = &cobra.Command{ } for _, tool := range tools { - formatter := tool.(verifier.FormatTool) gr.Go(func() error { - return formatter.Format(cmd.Context(), *toolCfg, dryRun) + return tool.Format(cmd.Context(), *toolCfg, dryRun) }) } diff --git a/cmd/project/project_validate.go b/cmd/project/project_validate.go index d9956b92..7ec6973e 100644 --- a/cmd/project/project_validate.go +++ b/cmd/project/project_validate.go @@ -92,9 +92,8 @@ var projectValidateCmd = &cobra.Command{ } for _, tool := range tools { - checker := tool.(verifier.CheckTool) gr.Go(func() error { - return checker.Check(cmd.Context(), result, *toolCfg) + return tool.Check(cmd.Context(), result, *toolCfg) }) } diff --git a/internal/verifier/tool.go b/internal/verifier/tool.go index 9c5ff194..0c208d80 100644 --- a/internal/verifier/tool.go +++ b/internal/verifier/tool.go @@ -9,24 +9,24 @@ import ( "github.com/shopware/shopware-cli/internal/validation" ) -type ToolList []Tool +type ToolList[T Tool] []T -var availableTools = ToolList{} +var availableTools = ToolList[Tool]{} func AddTool(tool Tool) { availableTools = append(availableTools, tool) } -func GetTools() ToolList { +func GetTools() ToolList[Tool] { return availableTools } // GetToolsOf returns registered tools that implement the requested capability. -func GetToolsOf[T Tool]() ToolList { - var tools ToolList +func GetToolsOf[T Tool]() ToolList[T] { + var tools ToolList[T] for _, tool := range availableTools { - if _, ok := tool.(T); ok { - tools = append(tools, tool) + if casted, ok := tool.(T); ok { + tools = append(tools, casted) } } return tools @@ -77,12 +77,12 @@ type FormatTool interface { Format(ctx context.Context, config ToolConfig, dryRun bool) error } -func (tl ToolList) Only(only string) (ToolList, error) { +func (tl ToolList[T]) Only(only string) (ToolList[T], error) { if only == "" { return tl, nil } - var filteredTools []Tool + var filteredTools ToolList[T] requestedTools := strings.Split(only, ",") seen := make(map[string]bool, len(requestedTools)) @@ -111,7 +111,7 @@ func (tl ToolList) Only(only string) (ToolList, error) { // Exclude filters out tools listed in the comma-separated exclude string. // Returns an error if any specified tool name does not exist in the current list. -func (tl ToolList) Exclude(exclude string) (ToolList, error) { +func (tl ToolList[T]) Exclude(exclude string) (ToolList[T], error) { if exclude == "" { return tl, nil } @@ -146,7 +146,7 @@ func (tl ToolList) Exclude(exclude string) (ToolList, error) { excludeSet[name] = struct{}{} } - var filtered ToolList + var filtered ToolList[T] for _, t := range tl { if _, ok := excludeSet[t.Name()]; ok { continue @@ -157,7 +157,7 @@ func (tl ToolList) Exclude(exclude string) (ToolList, error) { return filtered, nil } -func (tl ToolList) PossibleString() string { +func (tl ToolList[T]) PossibleString() string { var possibleTools []string for _, t := range tl { possibleTools = append(possibleTools, t.Name()) diff --git a/internal/verifier/tool_test.go b/internal/verifier/tool_test.go index 1503ed54..2262e25f 100644 --- a/internal/verifier/tool_test.go +++ b/internal/verifier/tool_test.go @@ -14,7 +14,7 @@ func (t testTool) Check(ctx context.Context, check *Check, config ToolConfig) er func (t testTool) Fix(ctx context.Context, config ToolConfig) error { return nil } func (t testTool) Format(ctx context.Context, config ToolConfig, dryRun bool) error { return nil } -func toolNames(list ToolList) []string { +func toolNames[T Tool](list ToolList[T]) []string { out := make([]string, 0, len(list)) for _, t := range list { out = append(out, t.Name()) @@ -30,7 +30,7 @@ func TestToolsByCapability(t *testing.T) { func TestOnly_DeduplicatesAndPreservesOrder(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} res, err := base.Only("eslint, phpstan,eslint") assert.NoError(t, err) assert.Equal(t, []string{"eslint", "phpstan"}, toolNames(res)) @@ -41,7 +41,7 @@ func TestOnly_DeduplicatesAndPreservesOrder(t *testing.T) { func TestExclude_EmptyString_NoChange(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} res, err := base.Exclude("") assert.NoError(t, err) assert.Equal(t, toolNames(base), toolNames(res)) @@ -49,7 +49,7 @@ func TestExclude_EmptyString_NoChange(t *testing.T) { func TestExclude_SingleTool(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} res, err := base.Exclude("eslint") assert.NoError(t, err) assert.Equal(t, []string{"phpstan", "sw-cli"}, toolNames(res)) @@ -57,7 +57,7 @@ func TestExclude_SingleTool(t *testing.T) { func TestExclude_MultipleTools(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}, testTool{"stylelint"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}, testTool{"stylelint"}} res, err := base.Exclude("eslint, stylelint") assert.NoError(t, err) assert.Equal(t, []string{"phpstan", "sw-cli"}, toolNames(res)) @@ -65,7 +65,7 @@ func TestExclude_MultipleTools(t *testing.T) { func TestExclude_AllTools_ReturnsEmpty(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}} res, err := base.Exclude("phpstan,eslint") assert.NoError(t, err) assert.Empty(t, res) @@ -73,7 +73,7 @@ func TestExclude_AllTools_ReturnsEmpty(t *testing.T) { func TestExclude_UnknownTool_Error(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}} res, err := base.Exclude("rector") assert.Error(t, err) assert.Nil(t, res) @@ -81,7 +81,7 @@ func TestExclude_UnknownTool_Error(t *testing.T) { func TestExclude_TrimsAndIgnoresDuplicates(t *testing.T) { t.Parallel() - base := ToolList{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} + base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} res, err := base.Exclude(" eslint , eslint , \teslint\t ") assert.NoError(t, err) assert.Equal(t, []string{"phpstan", "sw-cli"}, toolNames(res)) From 0e6d2325e01ed2efb934c49f941a54fdfceb59dc Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 14:55:14 +0200 Subject: [PATCH 08/14] chore: small code cleanup --- cmd/extension/extension_validate.go | 43 ++++++------------- .../extension_validate_selection_test.go | 14 ++++++ internal/verifier/tool.go | 32 +++----------- internal/verifier/tool_test.go | 2 +- 4 files changed, 34 insertions(+), 57 deletions(-) diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index f5d46ba7..c2ea204c 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "slices" - "sort" "time" "github.com/spf13/cobra" @@ -138,21 +137,14 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T validationTools := verifier.GetToolsOf[verifier.CheckTool]() requested := only - if requested == "" { + if requested == "" && !full { requested = "sw-cli" - if full { - requested = validationTools.PossibleString() - } } - selected, err := validationTools.Only(requested) + requestedTools, err := validationTools.Only(requested) if err != nil { return nil, nil, err } - requestedNames := make(map[string]bool, len(selected)) - for _, tool := range selected { - requestedNames[tool.Name()] = true - } - selected, err = selected.Exclude(exclude) + selected, err := requestedTools.Exclude(exclude) if err != nil { return nil, nil, err } @@ -160,28 +152,17 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T return nil, nil, errors.New("no validation checks selected after applying --exclude") } - invoked := make(map[string]bool, len(selected)) - for _, tool := range selected { - invoked[tool.Name()] = true - } - - statuses := make([]validation.ToolInvocationStatus, 0, len(validationTools)) - for _, tool := range validationTools { - name := tool.Name() - status := validation.ToolInvocationStatus{Name: name, Status: "skipped"} - switch { - case invoked[name]: - status.Status = "invoked" - case requestedNames[name]: - status.Reason = "excluded by --exclude" - case only != "": - status.Reason = "not selected by --only" - default: - status.Reason = "not selected; use --full or --only" + statuses := extensionToolInvocationStatuses(validationTools, selected) + for i := range statuses { + if statuses[i].Status == "invoked" { + continue + } + if slices.ContainsFunc(requestedTools, func(tool verifier.CheckTool) bool { return tool.Name() == statuses[i].Name }) { + statuses[i].Reason = "excluded by --exclude" + } else if only == "" { + statuses[i].Reason = "not selected; use --full or --only" } - statuses = append(statuses, status) } - sort.Slice(statuses, func(i, j int) bool { return statuses[i].Name < statuses[j].Name }) return selected, statuses, nil } diff --git a/cmd/extension/extension_validate_selection_test.go b/cmd/extension/extension_validate_selection_test.go index 302233e1..27092989 100644 --- a/cmd/extension/extension_validate_selection_test.go +++ b/cmd/extension/extension_validate_selection_test.go @@ -55,6 +55,20 @@ func TestExtensionValidationSelection(t *testing.T) { assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) }) + t.Run("only overrides full", func(t *testing.T) { + tools, statuses, err := selectExtensionValidationTools(true, "phpstan", "") + require.NoError(t, err) + assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) + assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, "sw-cli").Reason) + }) + + t.Run("full exclusion is reported", func(t *testing.T) { + tools, statuses, err := selectExtensionValidationTools(true, "", "phpstan") + require.NoError(t, err) + assert.NotContains(t, toolNamesForValidation(tools), "phpstan") + assert.Equal(t, "excluded by --exclude", toolStatusByName(t, statuses, "phpstan").Reason) + }) + t.Run("exclude applies after only", func(t *testing.T) { tools, statuses, err := selectExtensionValidationTools(false, "phpstan,sw-cli", "sw-cli") require.NoError(t, err) diff --git a/internal/verifier/tool.go b/internal/verifier/tool.go index 0c208d80..fb6a0444 100644 --- a/internal/verifier/tool.go +++ b/internal/verifier/tool.go @@ -3,6 +3,7 @@ package verifier import ( "context" "fmt" + "slices" "strings" "github.com/shopware/shopware-cli/internal/extension" @@ -116,42 +117,23 @@ func (tl ToolList[T]) Exclude(exclude string) (ToolList[T], error) { return tl, nil } - requested := strings.Split(exclude, ",") - - // Validate all requested excludes exist - for _, name := range requested { + names := strings.Split(exclude, ",") + for i, name := range names { name = strings.TrimSpace(name) + names[i] = name if name == "" { continue } - found := false - for _, t := range tl { - if t.Name() == name { - found = true - break - } - } - if !found { + if !slices.ContainsFunc(tl, func(tool T) bool { return tool.Name() == name }) { return nil, fmt.Errorf("tool with name %q not found, possible tools: %s", name, tl.PossibleString()) } } - // Build filtered list excluding requested names - excludeSet := map[string]struct{}{} - for _, name := range requested { - name = strings.TrimSpace(name) - if name == "" { - continue - } - excludeSet[name] = struct{}{} - } - var filtered ToolList[T] for _, t := range tl { - if _, ok := excludeSet[t.Name()]; ok { - continue + if !slices.Contains(names, t.Name()) { + filtered = append(filtered, t) } - filtered = append(filtered, t) } return filtered, nil diff --git a/internal/verifier/tool_test.go b/internal/verifier/tool_test.go index 2262e25f..f3a30115 100644 --- a/internal/verifier/tool_test.go +++ b/internal/verifier/tool_test.go @@ -82,7 +82,7 @@ func TestExclude_UnknownTool_Error(t *testing.T) { func TestExclude_TrimsAndIgnoresDuplicates(t *testing.T) { t.Parallel() base := ToolList[testTool]{testTool{"phpstan"}, testTool{"eslint"}, testTool{"sw-cli"}} - res, err := base.Exclude(" eslint , eslint , \teslint\t ") + res, err := base.Exclude(" , eslint , eslint , \teslint\t , ") assert.NoError(t, err) assert.Equal(t, []string{"phpstan", "sw-cli"}, toolNames(res)) } From c237c1bfb70a8d16efe9420bdffa664e3826ae84 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 14:57:15 +0200 Subject: [PATCH 09/14] fix: code formatting --- internal/extension/create_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/extension/create_test.go b/internal/extension/create_test.go index 91a23b30..595ea98a 100644 --- a/internal/extension/create_test.go +++ b/internal/extension/create_test.go @@ -141,7 +141,6 @@ func TestCreateErrors(t *testing.T) { }) } - func TestCreateGeneratesAnExtension(t *testing.T) { for _, extensionType := range []ExtensionType{Plugin, Theme} { for _, store := range []bool{false, true} { From bd46f774a215a7ef314fcc9ecd8d79d3e2d3225b Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 15:48:12 +0200 Subject: [PATCH 10/14] chore: adjust markdown docs --- AGENTS.md | 12 +++++------ architecture.md | 22 ++++++++++++++------ skills/shopware-cli-extension-store/SKILL.md | 3 ++- skills/shopware-cli/SKILL.md | 16 +++++++------- 4 files changed, 33 insertions(+), 20 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 28b7249e..d0bdd5b1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,10 +110,10 @@ shopware-cli project storefront-watch ## Code Quality Integration -The verifier system provides comprehensive code quality checks: -- **PHP**: PHPStan, PHP-CS-Fixer, Rector -- **JavaScript**: ESLint, Prettier, Stylelint -- **Twig**: Custom admin Twig linter with auto-fix capabilities -- **Composer**: Dependency validation +The verifier registers tools through the name-only `Tool` interface. `CheckTool`, `FixTool`, and `FormatTool` add capabilities; commands select the relevant capability before applying `--only` or `--exclude`. An unsupported tool name is an error, and `ToolList[T]` preserves the capability type through filtering. -Tools are configurable via JSON schemas and run automatically during builds. \ No newline at end of file +- **Checkers**: `sw-cli`, PHPStan, ESLint, Stylelint, Administration Twig, Storefront Twig +- **Fixers**: Rector, ESLint, Stylelint, Administration Twig, Symfony XML conversion +- **Formatters**: PHP-CS-Fixer, Prettier, Administration Twig + +`extension validate` defaults to `sw-cli`; `--full` selects all checkers, and an explicit `--only` overrides that default. The extension commands report whether each tool was invoked or skipped; invocation does not guarantee that files were analyzed or changed. diff --git a/architecture.md b/architecture.md index 2af7f5cb..0a2668cc 100644 --- a/architecture.md +++ b/architecture.md @@ -32,23 +32,33 @@ A new command is just a new file: drop `cmd//.go` (`cmd/root. ### 2.2 Verifier tools: provides reproducible pattern for implementing other capabilities -Each code-quality tool implements one small interface (name, check, fix, format) and adds itself to a shared list. Callers can then run them all, or filter to just some, in parallel. Currently these are code quality checkers: phpstan, eslint, stylelint, prettier, php-cs-fixer, rector, composer, admin-twig, storefront-twig, sw-cli. +Each verifier tool registers a name and implements only the capabilities it supports: checking, fixing, formatting, or a combination. Commands select tools by capability, then apply `--only` and (where available) `--exclude`. An unsupported `--only` name is an error that lists the tools available to that command. -**Decision**: will drop `dry run` and use Git. Why: Underlying tools do not support it. Under the hood, it uses eslint for js, rector for PHP. +`extension validate` runs only the built-in `sw-cli` checker by default; `--full` selects all checkers, while an explicit `--only` takes precedence over `--full`. The format commands support `--dry-run`; the fix commands do not. ```go -// internal/verifier/tool.go:50 type Tool interface { Name() string +} +type CheckTool interface { + Tool Check(ctx context.Context, check *Check, config ToolConfig) error +} +type FixTool interface { + Tool Fix(ctx context.Context, config ToolConfig) error +} +type FormatTool interface { + Tool Format(ctx context.Context, config ToolConfig, dryRun bool) error } ``` -Registration is `func init() { AddTool(PhpStan{}) }` into a global `availableTools`; consumers call `verifier.GetTools().Only(...)` / `.Exclude(...)`. +Registration is `func init() { AddTool(PhpStan{}) }` into a global `availableTools`. Consumers call `GetToolsOf[CheckTool]()` (or `FixTool` / `FormatTool`) to get a typed `ToolList[T]`; its `Only` and `Exclude` methods preserve that capability type. + +Current checkers are `sw-cli`, `phpstan`, `eslint`, `stylelint`, `admin-twig`, and `storefront-twig`. Fixers are `rector`, `admin-twig`, `eslint`, `stylelint`, and `symfony-xml`; formatters are `admin-twig`, `php-cs-fixer`, and `prettier`. `sw-cli` enforces built-in extension rules; it does not validate per-extension metadata in a project context. There is no separate Composer verifier tool. -Currently registered: phpstan, eslint, stylelint, prettier, php-cs-fixer, rector, composer, admin-twig, storefront-twig, sw-cli. The last one is a tool that enforces Shopware-specific validation rules the CLI implements itself; it runs through the same machinery as the external tools. +Extension commands report selected tools as `invoked` and others as `skipped`. This describes selection and invocation, not whether a tool found applicable files or changed them. ### 2.3 Extension types: simple interface @@ -103,7 +113,7 @@ In discussions we identified confusion opportunities around `doctor` vs. `valida ### 3.1 Where the CLI already does/runs things concurrently, for speed -- **Validate / format / fix:** runs all the code checkers at the same time via `errgroup.Group` (cmd/extension/extension_validate.go:112 and the project equivalents); if one fails, it stops the rest. +- **Validate / format / fix:** runs the selected capability-specific tools concurrently via `errgroup.Group` and waits for them; an execution error makes the command fail. - **npm installs:** installs dependencies for multiple extensions in parallel, with as many workers as you have CPU cores (`runtime.NumCPU()`, internal/extension/npm.go:70). - **Asset file hashing:** hashes files using a pool of eight workers at once (asset_config.go:231). - **DB dump:** dumps multiple database tables at once (`--parallel`), with a cap so it doesn't overload (internal/mysqldump/mysql.go). diff --git a/skills/shopware-cli-extension-store/SKILL.md b/skills/shopware-cli-extension-store/SKILL.md index fc9dd60c..51269ec4 100644 --- a/skills/shopware-cli-extension-store/SKILL.md +++ b/skills/shopware-cli-extension-store/SKILL.md @@ -24,7 +24,8 @@ shopware-cli extension validate . --store-compliance --format markdown - The **exit code** is the pass/fail signal (`0` = pass, non-zero = findings). The report goes to stdout; a usage block or error goes to stderr — do not read a validation failure as a usage error. - `--format markdown` gives a stable, quotable form. `--reporter` is a deprecated alias that prints a warning — use `--format`. - Treat the store-compliance run as a delta over the normal run: report only the lines it adds. -- Without `--full`, `extension validate` runs only the built-in `sw-cli` validator — **not** PHPStan/ESLint/Stylelint. (`sw-cli` is the name of that one check, as in `--only sw-cli`, not shorthand for the `shopware-cli` binary.) So report "the `sw-cli` checks passed", not "validation passed", unless `--full` was run. Source: `cmd/extension/extension_validate.go`, the `if !isFull { only = "sw-cli" }` branch. +- With neither `--full` nor `--only`, `extension validate` runs only the built-in `sw-cli` checker — **not** PHPStan/ESLint/Stylelint. (`sw-cli` is that checker's name, not shorthand for the binary.) An explicit `--only` selects its named checkers even without `--full`. For the two commands above, report "the `sw-cli` checks passed", not "full validation passed". Source: `cmd/extension/extension_validate.go`, `selectExtensionValidationTools`. +- The Markdown report includes a checker table. `invoked` means the checker was called; it does not prove that files were analyzed or that a check passed. `skipped` means it was not selected or was excluded. Classify only finding lines, not checker-status lines. - Use one `shopware-cli` binary throughout, and state its version. Never mix binaries mid-answer. - Each error line ends with its result identifier — that identifier is the row's Source, and `L0` catches any line the table does not name explicitly. The CLI currently prints a missing icon twice; count a repeated line once. diff --git a/skills/shopware-cli/SKILL.md b/skills/shopware-cli/SKILL.md index 8cd5343f..eecf448b 100644 --- a/skills/shopware-cli/SKILL.md +++ b/skills/shopware-cli/SKILL.md @@ -131,7 +131,7 @@ Validates the Shopware project against the checks implemented by the current CLI Common flags include: -- `--only ` — run only specific tools (comma-separated). +- `--only ` — run only specific checkers (comma-separated). - `--exclude ` — skip specific tools (comma-separated). - `--format ` — choose an output format supported by the current CLI (`--reporter` is a deprecated alias). - `--local-only` — limit extension discovery to plugins in `custom/*` (for the project toolset); does not add per-extension metadata validation. @@ -154,9 +154,9 @@ Normal extension validation runs the built-in checks implemented by the current Common flags include: -- `--only ` — run only specific tools (comma-separated). -- `--exclude ` — skip specific tools. -- `--full` — run additional/full validation tools such as PHPStan, ESLint, and Stylelint when supported/configured. +- `--only ` — run only the named checkers, independently of `--full` (comma-separated). +- `--exclude ` — remove checkers from the selected set. +- `--full` — select all checkers by default, including PHPStan, ESLint, and Stylelint; explicit `--only` takes precedence. - `--check-against ` — `highest` (default) or `lowest`: which supported Shopware version to check against. - `--store-compliance` — enable Store-compliance mode while the current CLI supports the flag. Prefer `validation.store_compliance: true` in `.shopware-extension.yml` for persistent Store intent. - `--format ` — choose an output format supported by the current CLI (`--reporter` is a deprecated alias). @@ -165,6 +165,8 @@ Common flags include: For Store-distribution workflows, use the `shopware-cli-extension-store` skill when available. +Each command selects only tools that support its operation. For example, `extension validate --only prettier` is an error because Prettier formats but does not check; the error lists available checkers. The extension command summaries show `invoked` or `skipped`: these describe selection and invocation, not whether files were applicable, findings were produced, or fixes were made. + ### Fresh results beat saved reports Saved outputs such as `validation.json`, JUnit XML, or markdown reports can become stale after files are fixed or changed. @@ -215,9 +217,9 @@ When `validate` produces unexpected results: - Full validation can depend on external tools such as PHPStan, ESLint, and Stylelint. - Verify relevant dependencies and configuration. -5. **Understand tool exclusions.** - - A tool may be skipped due to missing dependencies, unmet conditions, or configuration. - - Do not assume a skipped tool means validation passed. +5. **Understand tool statuses.** + - `skipped` means a tool was not selected or was excluded; the status note gives the reason. + - `invoked` means the tool was called, not that it analyzed files or succeeded. Use findings and the exit code for the validation result. 6. **Avoid ad hoc workarounds.** - Do not bypass validation with manual lower-level commands before understanding why the CLI behaved as it did. From 1127dc2b50c99bc627337b2f5c95e178d5928fcc Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 16:14:12 +0200 Subject: [PATCH 11/14] feat: add exclude flag to extension format and fix --- cmd/extension/extension_fix.go | 14 ++++++++++++-- cmd/extension/extension_format.go | 14 ++++++++++++-- cmd/extension/extension_tool_invocation.go | 7 +++++-- cmd/extension/extension_tool_invocation_test.go | 17 ++++++++++++++--- cmd/extension/extension_validate.go | 9 ++------- 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/cmd/extension/extension_fix.go b/cmd/extension/extension_fix.go index 0dec60fd..5e820e75 100644 --- a/cmd/extension/extension_fix.go +++ b/cmd/extension/extension_fix.go @@ -1,6 +1,7 @@ package extension import ( + "errors" "fmt" "os" "path/filepath" @@ -51,11 +52,19 @@ var extensionFixCmd = &cobra.Command{ allTools := verifier.GetToolsOf[verifier.FixTool]() only, _ := cmd.Flags().GetString("only") + exclude, _ := cmd.Flags().GetString("exclude") - tools, err := allTools.Only(only) + requestedTools, err := allTools.Only(only) if err != nil { return err } + tools, err := requestedTools.Exclude(exclude) + if err != nil { + return err + } + if len(tools) == 0 { + return errors.New("no fixers selected after applying --exclude") + } for _, tool := range tools { gr.Go(func() error { @@ -64,7 +73,7 @@ var extensionFixCmd = &cobra.Command{ } runErr := gr.Wait() - if err := validation.PrintToolInvocationTable(os.Stdout, "Fixers", extensionToolInvocationStatuses(allTools, tools)); err != nil { + if err := validation.PrintToolInvocationTable(os.Stdout, "Fixers", extensionToolInvocationStatuses(allTools, requestedTools, tools)); err != nil { return err } return runErr @@ -74,5 +83,6 @@ var extensionFixCmd = &cobra.Command{ func init() { extensionRootCmd.AddCommand(extensionFixCmd) extensionFixCmd.Flags().String("only", "", "Run only specific fixers by name (comma-separated, e.g. eslint,rector)") + extensionFixCmd.Flags().String("exclude", "", "Exclude fixers after applying --only (comma-separated, e.g. eslint,rector)") extensionFixCmd.Flags().Bool("allow-non-git", false, "Allow running the fix command on non-git repositories") } diff --git a/cmd/extension/extension_format.go b/cmd/extension/extension_format.go index eb437666..68ea7277 100644 --- a/cmd/extension/extension_format.go +++ b/cmd/extension/extension_format.go @@ -1,6 +1,7 @@ package extension import ( + "errors" "fmt" "os" "path/filepath" @@ -45,11 +46,19 @@ var extensionFormat = &cobra.Command{ allTools := verifier.GetToolsOf[verifier.FormatTool]() only, _ := cmd.Flags().GetString("only") + exclude, _ := cmd.Flags().GetString("exclude") - tools, err := allTools.Only(only) + requestedTools, err := allTools.Only(only) if err != nil { return err } + tools, err := requestedTools.Exclude(exclude) + if err != nil { + return err + } + if len(tools) == 0 { + return errors.New("no formatters selected after applying --exclude") + } for _, tool := range tools { gr.Go(func() error { @@ -58,7 +67,7 @@ var extensionFormat = &cobra.Command{ } runErr := gr.Wait() - if err := validation.PrintToolInvocationTable(os.Stdout, "Formatters", extensionToolInvocationStatuses(allTools, tools)); err != nil { + if err := validation.PrintToolInvocationTable(os.Stdout, "Formatters", extensionToolInvocationStatuses(allTools, requestedTools, tools)); err != nil { return err } return runErr @@ -68,5 +77,6 @@ var extensionFormat = &cobra.Command{ func init() { extensionRootCmd.AddCommand(extensionFormat) extensionFormat.Flags().String("only", "", "Run only specific formatters by name (comma-separated, e.g. prettier,php-cs-fixer)") + extensionFormat.Flags().String("exclude", "", "Exclude formatters after applying --only (comma-separated, e.g. prettier,php-cs-fixer)") extensionFormat.Flags().Bool("dry-run", false, "Run in dry run mode") } diff --git a/cmd/extension/extension_tool_invocation.go b/cmd/extension/extension_tool_invocation.go index d8def664..dcdcadb4 100644 --- a/cmd/extension/extension_tool_invocation.go +++ b/cmd/extension/extension_tool_invocation.go @@ -8,13 +8,16 @@ import ( "github.com/shopware/shopware-cli/internal/verifier" ) -func extensionToolInvocationStatuses[T verifier.Tool](all, selected verifier.ToolList[T]) []validation.ToolInvocationStatus { +func extensionToolInvocationStatuses[T verifier.Tool](all, requested, selected verifier.ToolList[T]) []validation.ToolInvocationStatus { statuses := make([]validation.ToolInvocationStatus, 0, len(all)) for _, tool := range all { status := validation.ToolInvocationStatus{Name: tool.Name(), Status: "skipped", Reason: "not selected by --only"} - if slices.ContainsFunc(selected, func(selected T) bool { return selected.Name() == tool.Name() }) { + switch { + case slices.ContainsFunc(selected, func(selected T) bool { return selected.Name() == tool.Name() }): status.Status = "invoked" status.Reason = "" + case slices.ContainsFunc(requested, func(requested T) bool { return requested.Name() == tool.Name() }): + status.Reason = "excluded by --exclude" } statuses = append(statuses, status) } diff --git a/cmd/extension/extension_tool_invocation_test.go b/cmd/extension/extension_tool_invocation_test.go index d5b6474b..b724ef76 100644 --- a/cmd/extension/extension_tool_invocation_test.go +++ b/cmd/extension/extension_tool_invocation_test.go @@ -16,12 +16,23 @@ func TestExtensionToolInvocationStatuses(t *testing.T) { func assertExtensionToolInvocationStatuses[T verifier.Tool](t *testing.T, all verifier.ToolList[T]) { t.Helper() - selected, err := all.Only(all[0].Name()) + requested, err := all.Only(all[0].Name() + "," + all[1].Name()) + require.NoError(t, err) + selected, err := requested.Exclude(all[1].Name()) require.NoError(t, err) - statuses := extensionToolInvocationStatuses(all, selected) + statuses := extensionToolInvocationStatuses(all, requested, selected) assert.Len(t, statuses, len(all)) assert.Equal(t, "invoked", toolStatusByName(t, statuses, all[0].Name()).Status) assert.Equal(t, "skipped", toolStatusByName(t, statuses, all[1].Name()).Status) - assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, all[1].Name()).Reason) + assert.Equal(t, "excluded by --exclude", toolStatusByName(t, statuses, all[1].Name()).Reason) + assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, all[2].Name()).Reason) + + _, err = requested.Exclude(all[2].Name()) + require.ErrorContains(t, err, "not found") +} + +func TestExtensionFixAndFormatHaveExcludeFlag(t *testing.T) { + assert.NotNil(t, extensionFixCmd.Flags().Lookup("exclude")) + assert.NotNil(t, extensionFormat.Flags().Lookup("exclude")) } diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index c2ea204c..d746cee2 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -152,14 +152,9 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T return nil, nil, errors.New("no validation checks selected after applying --exclude") } - statuses := extensionToolInvocationStatuses(validationTools, selected) + statuses := extensionToolInvocationStatuses(validationTools, requestedTools, selected) for i := range statuses { - if statuses[i].Status == "invoked" { - continue - } - if slices.ContainsFunc(requestedTools, func(tool verifier.CheckTool) bool { return tool.Name() == statuses[i].Name }) { - statuses[i].Reason = "excluded by --exclude" - } else if only == "" { + if only == "" && statuses[i].Reason == "not selected by --only" { statuses[i].Reason = "not selected; use --full or --only" } } From 4bd33b2645e31dd0c31cb2c48bb98cfa579e4a18 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 16:39:34 +0200 Subject: [PATCH 12/14] feat!: deprecate full and make it default for extension validate BREAKING CHANGE: this changes the behaviour of extension validate --- AGENTS.md | 2 +- architecture.md | 2 +- cmd/extension/extension_validate.go | 24 +++------ .../extension_validate_selection_test.go | 52 ++++++++----------- skills/shopware-cli-extension-store/SKILL.md | 10 ++-- skills/shopware-cli/SKILL.md | 4 +- 6 files changed, 38 insertions(+), 56 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d0bdd5b1..832b5757 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -116,4 +116,4 @@ The verifier registers tools through the name-only `Tool` interface. `CheckTool` - **Fixers**: Rector, ESLint, Stylelint, Administration Twig, Symfony XML conversion - **Formatters**: PHP-CS-Fixer, Prettier, Administration Twig -`extension validate` defaults to `sw-cli`; `--full` selects all checkers, and an explicit `--only` overrides that default. The extension commands report whether each tool was invoked or skipped; invocation does not guarantee that files were analyzed or changed. +`extension validate` runs all checkers by default. The deprecated `--full` flag remains accepted but has no effect; use `--only` or `--exclude` to select checkers. The extension commands report whether each tool was invoked or skipped; invocation does not guarantee that files were analyzed or changed. diff --git a/architecture.md b/architecture.md index 0a2668cc..68ae2a6d 100644 --- a/architecture.md +++ b/architecture.md @@ -34,7 +34,7 @@ A new command is just a new file: drop `cmd//.go` (`cmd/root. Each verifier tool registers a name and implements only the capabilities it supports: checking, fixing, formatting, or a combination. Commands select tools by capability, then apply `--only` and (where available) `--exclude`. An unsupported `--only` name is an error that lists the tools available to that command. -`extension validate` runs only the built-in `sw-cli` checker by default; `--full` selects all checkers, while an explicit `--only` takes precedence over `--full`. The format commands support `--dry-run`; the fix commands do not. +`extension validate` runs all checkers by default. The deprecated `--full` flag remains accepted but has no effect; use `--only` or `--exclude` to select checkers. The format commands support `--dry-run`; the fix commands do not. ```go type Tool interface { diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index d746cee2..4c67db01 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -23,7 +23,6 @@ var extensionValidateCmd = &cobra.Command{ Short: "Validate extension metadata, assets, and code quality", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - isFull, _ := cmd.Flags().GetBool("full") storeCompliance, _ := cmd.Flags().GetBool("store-compliance") reportingFormat, err := extensionValidationFormat(cmd) if err != nil { @@ -34,7 +33,7 @@ var extensionValidateCmd = &cobra.Command{ exclude, _ := cmd.Flags().GetString("exclude") noCopy, _ := cmd.Flags().GetBool("no-copy") - tools, statuses, err := selectExtensionValidationTools(isFull, only, exclude) + tools, statuses, err := selectExtensionValidationTools(only, exclude) if err != nil { return err } @@ -133,14 +132,10 @@ var extensionValidateCmd = &cobra.Command{ }, } -func selectExtensionValidationTools(full bool, only, exclude string) (verifier.ToolList[verifier.CheckTool], []validation.ToolInvocationStatus, error) { +func selectExtensionValidationTools(only, exclude string) (verifier.ToolList[verifier.CheckTool], []validation.ToolInvocationStatus, error) { validationTools := verifier.GetToolsOf[verifier.CheckTool]() - requested := only - if requested == "" && !full { - requested = "sw-cli" - } - requestedTools, err := validationTools.Only(requested) + requestedTools, err := validationTools.Only(only) if err != nil { return nil, nil, err } @@ -152,13 +147,7 @@ func selectExtensionValidationTools(full bool, only, exclude string) (verifier.T return nil, nil, errors.New("no validation checks selected after applying --exclude") } - statuses := extensionToolInvocationStatuses(validationTools, requestedTools, selected) - for i := range statuses { - if only == "" && statuses[i].Reason == "not selected by --only" { - statuses[i].Reason = "not selected; use --full or --only" - } - } - return selected, statuses, nil + return selected, extensionToolInvocationStatuses(validationTools, requestedTools, selected), nil } func requiresToolSetup(tool verifier.CheckTool) bool { @@ -185,15 +174,16 @@ func extensionValidationFormat(cmd *cobra.Command) (string, error) { func init() { extensionRootCmd.AddCommand(extensionValidateCmd) - extensionValidateCmd.PersistentFlags().Bool("full", false, "Run all validation checks by default (minus --exclude selections)") + extensionValidateCmd.PersistentFlags().Bool("full", false, "Run all validation checks") extensionValidateCmd.PersistentFlags().Bool("store-compliance", false, "Run the Extension Store compliance checks") extensionValidateCmd.PersistentFlags().String("format", "", "Reporting format (summary, json, github, gitlab, junit, markdown)") extensionValidateCmd.PersistentFlags().String("reporter", "", "Reporting format (summary, json, github, gitlab, junit, markdown)") extensionValidateCmd.PersistentFlags().String("check-against", "highest", "Check against Shopware Version (highest, lowest)") - extensionValidateCmd.PersistentFlags().String("only", "", "Run only these validation checks, regardless of --full (comma-separated, e.g. phpstan,eslint)") + extensionValidateCmd.PersistentFlags().String("only", "", "Run only these validation checks (comma-separated, e.g. phpstan,eslint)") extensionValidateCmd.PersistentFlags().String("exclude", "", "Exclude specific tools by name (comma-separated, e.g. phpstan,eslint)") extensionValidateCmd.PersistentFlags().Bool("no-copy", false, "Do not copy extension files to temporary directory") extensionValidateCmd.MarkFlagsMutuallyExclusive("format", "reporter") + _ = extensionValidateCmd.PersistentFlags().MarkDeprecated("full", "all validation checks now run by default; omit --full; to restore old behaviour use --only sw-cli") _ = extensionValidateCmd.PersistentFlags().MarkDeprecated("reporter", "use --format instead") _ = extensionValidateCmd.PersistentFlags().MarkHidden("reporter") extensionValidateCmd.PreRunE = func(cmd *cobra.Command, args []string) error { diff --git a/cmd/extension/extension_validate_selection_test.go b/cmd/extension/extension_validate_selection_test.go index 27092989..a7dc8a6d 100644 --- a/cmd/extension/extension_validate_selection_test.go +++ b/cmd/extension/extension_validate_selection_test.go @@ -23,16 +23,17 @@ func toolStatusByName(t *testing.T, statuses []validation.ToolInvocationStatus, } func TestExtensionValidationSelection(t *testing.T) { - t.Run("default runs only sw-cli", func(t *testing.T) { - tools, statuses, err := selectExtensionValidationTools(false, "", "") + t.Run("default runs all checkers", func(t *testing.T) { + tools, statuses, err := selectExtensionValidationTools("", "") require.NoError(t, err) - assert.Equal(t, []string{"sw-cli"}, toolNamesForValidation(tools)) - assert.False(t, slices.ContainsFunc(tools, requiresToolSetup)) - assert.Equal(t, "not selected; use --full or --only", toolStatusByName(t, statuses, "phpstan").Reason) + assert.Len(t, tools, 6) + assert.Len(t, statuses, len(tools)) + assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) + assert.Equal(t, "invoked", toolStatusByName(t, statuses, "phpstan").Status) }) - t.Run("only phpstan works without full", func(t *testing.T) { - tools, statuses, err := selectExtensionValidationTools(false, "phpstan", "") + t.Run("only phpstan", func(t *testing.T) { + tools, statuses, err := selectExtensionValidationTools("phpstan", "") require.NoError(t, err) assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) @@ -41,56 +42,41 @@ func TestExtensionValidationSelection(t *testing.T) { }) t.Run("Twig validation needs no external tools", func(t *testing.T) { - tools, _, err := selectExtensionValidationTools(false, "admin-twig", "") + tools, _, err := selectExtensionValidationTools("admin-twig", "") require.NoError(t, err) assert.Equal(t, []string{"admin-twig"}, toolNamesForValidation(tools)) assert.False(t, slices.ContainsFunc(tools, requiresToolSetup)) }) - t.Run("full selects all validation checks", func(t *testing.T) { - tools, statuses, err := selectExtensionValidationTools(true, "", "") - require.NoError(t, err) - assert.Len(t, tools, 6) - assert.Len(t, statuses, len(tools)) - assert.True(t, slices.ContainsFunc(tools, requiresToolSetup)) - }) - - t.Run("only overrides full", func(t *testing.T) { - tools, statuses, err := selectExtensionValidationTools(true, "phpstan", "") - require.NoError(t, err) - assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) - assert.Equal(t, "not selected by --only", toolStatusByName(t, statuses, "sw-cli").Reason) - }) - - t.Run("full exclusion is reported", func(t *testing.T) { - tools, statuses, err := selectExtensionValidationTools(true, "", "phpstan") + t.Run("default exclusion is reported", func(t *testing.T) { + tools, statuses, err := selectExtensionValidationTools("", "phpstan") require.NoError(t, err) assert.NotContains(t, toolNamesForValidation(tools), "phpstan") assert.Equal(t, "excluded by --exclude", toolStatusByName(t, statuses, "phpstan").Reason) }) t.Run("exclude applies after only", func(t *testing.T) { - tools, statuses, err := selectExtensionValidationTools(false, "phpstan,sw-cli", "sw-cli") + tools, statuses, err := selectExtensionValidationTools("phpstan,sw-cli", "sw-cli") require.NoError(t, err) assert.Equal(t, []string{"phpstan"}, toolNamesForValidation(tools)) assert.Equal(t, "excluded by --exclude", toolStatusByName(t, statuses, "sw-cli").Reason) }) t.Run("duplicate only values run once", func(t *testing.T) { - tools, _, err := selectExtensionValidationTools(false, "sw-cli,sw-cli", "") + tools, _, err := selectExtensionValidationTools("sw-cli,sw-cli", "") require.NoError(t, err) assert.Equal(t, []string{"sw-cli"}, toolNamesForValidation(tools)) }) t.Run("unsupported operation lists checkers", func(t *testing.T) { - _, _, err := selectExtensionValidationTools(false, "prettier", "") + _, _, err := selectExtensionValidationTools("prettier", "") require.ErrorContains(t, err, `tool with name "prettier" not found, possible tools:`) assert.NotContains(t, err.Error(), "prettier,") assert.Contains(t, err.Error(), "phpstan") }) t.Run("typo lists only checkers", func(t *testing.T) { - _, _, err := selectExtensionValidationTools(false, "phpsta", "") + _, _, err := selectExtensionValidationTools("phpsta", "") require.ErrorContains(t, err, `tool with name "phpsta" not found, possible tools:`) assert.Contains(t, err.Error(), "phpstan") assert.NotContains(t, err.Error(), "prettier") @@ -98,11 +84,17 @@ func TestExtensionValidationSelection(t *testing.T) { }) t.Run("empty selection fails", func(t *testing.T) { - _, _, err := selectExtensionValidationTools(false, "sw-cli", "sw-cli") + _, _, err := selectExtensionValidationTools("sw-cli", "sw-cli") require.EqualError(t, err, "no validation checks selected after applying --exclude") }) } +func TestExtensionValidateFullFlagDeprecated(t *testing.T) { + flag := extensionValidateCmd.PersistentFlags().Lookup("full") + require.NotNil(t, flag) + assert.NotEmpty(t, flag.Deprecated) +} + func toolNamesForValidation(tools verifier.ToolList[verifier.CheckTool]) []string { names := make([]string, 0, len(tools)) for _, tool := range tools { diff --git a/skills/shopware-cli-extension-store/SKILL.md b/skills/shopware-cli-extension-store/SKILL.md index 51269ec4..e94c3f34 100644 --- a/skills/shopware-cli-extension-store/SKILL.md +++ b/skills/shopware-cli-extension-store/SKILL.md @@ -17,14 +17,14 @@ Run both validations from the extension root and capture the full output **and** ```bash shopware-cli --version -shopware-cli extension validate . --format markdown -shopware-cli extension validate . --store-compliance --format markdown +shopware-cli extension validate . --only sw-cli --format markdown +shopware-cli extension validate . --only sw-cli --store-compliance --format markdown ``` - The **exit code** is the pass/fail signal (`0` = pass, non-zero = findings). The report goes to stdout; a usage block or error goes to stderr — do not read a validation failure as a usage error. - `--format markdown` gives a stable, quotable form. `--reporter` is a deprecated alias that prints a warning — use `--format`. - Treat the store-compliance run as a delta over the normal run: report only the lines it adds. -- With neither `--full` nor `--only`, `extension validate` runs only the built-in `sw-cli` checker — **not** PHPStan/ESLint/Stylelint. (`sw-cli` is that checker's name, not shorthand for the binary.) An explicit `--only` selects its named checkers even without `--full`. For the two commands above, report "the `sw-cli` checks passed", not "full validation passed". Source: `cmd/extension/extension_validate.go`, `selectExtensionValidationTools`. +- `extension validate` now runs all checkers by default. The two commands above explicitly select only the built-in `sw-cli` checker, not PHPStan/ESLint/Stylelint. (`sw-cli` is that checker's name, not shorthand for the binary.) Report "the `sw-cli` checks passed", not "full validation passed". Source: `cmd/extension/extension_validate.go`, `selectExtensionValidationTools`. - The Markdown report includes a checker table. `invoked` means the checker was called; it does not prove that files were analyzed or that a check passed. `skipped` means it was not selected or was excluded. Classify only finding lines, not checker-status lines. - Use one `shopware-cli` binary throughout, and state its version. Never mix binaries mid-answer. - Each error line ends with its result identifier — that identifier is the row's Source, and `L0` catches any line the table does not name explicitly. The CLI currently prints a missing icon twice; count a repeated line once. @@ -123,7 +123,7 @@ Eight invariants. Check each finding against all eight before writing it. | [`store-review-errors`](https://developer.shopware.com/docs/guides/development/testing/store/store-review-errors.html) | Common reasons reviewers reject a submission | user asks why a submission failed, or wants rejection risks | | [`not-allowed-store-behaviors`](https://developer.shopware.com/docs/guides/development/testing/store/not-allowed-store-behaviors.html) | Prohibited patterns | extension touches core internals, filesystem, or DB directly | | [`functionality-integration`](https://developer.shopware.com/docs/guides/development/testing/store/functionality-integration.html) | Correct integration with core, persistence, public APIs | extension has subscribers, entities, or API endpoints | -| [`code-quality`](https://developer.shopware.com/docs/guides/development/testing/store/code-quality.html) | Code standards reviewers apply | user asks about code quality, or `--full` validation was run | +| [`code-quality`](https://developer.shopware.com/docs/guides/development/testing/store/code-quality.html) | Code standards reviewers apply | user asks about code quality, or code-quality checkers were invoked | | [`installation-and-cleanup`](https://developer.shopware.com/docs/guides/development/testing/store/installation-and-cleanup.html) | Install/update/uninstall and data removal | extension implements lifecycle methods or creates tables | | [`cookies-and-privacy`](https://developer.shopware.com/docs/guides/development/testing/store/cookies-and-privacy.html) | Cookie registration, GDPR, subprocessors | extension sets cookies, tracks, or sends data to third parties | | [`seo-and-structured-data`](https://developer.shopware.com/docs/guides/development/testing/store/seo-and-structured-data.html) | SEO output and structured data | extension changes storefront markup, URLs, or meta tags | @@ -139,7 +139,7 @@ Preconditions here work like §2's: a page you had no trigger to read produces n - CLI binary and version - inspection timestamp -- sw-cli checks: pass/fail + exit code (state whether `--full` was run) +- sw-cli checks: pass/fail + exit code (state that `--only sw-cli` was used) - store-compliance checks: pass/fail + exit code - remote Store listing: inspected / not inspected - files modified: no diff --git a/skills/shopware-cli/SKILL.md b/skills/shopware-cli/SKILL.md index eecf448b..cf05add0 100644 --- a/skills/shopware-cli/SKILL.md +++ b/skills/shopware-cli/SKILL.md @@ -154,9 +154,9 @@ Normal extension validation runs the built-in checks implemented by the current Common flags include: -- `--only ` — run only the named checkers, independently of `--full` (comma-separated). +- `--only ` — run only the named checkers (comma-separated). - `--exclude ` — remove checkers from the selected set. -- `--full` — select all checkers by default, including PHPStan, ESLint, and Stylelint; explicit `--only` takes precedence. +- All checkers run by default, including PHPStan, ESLint, and Stylelint. `--full` is deprecated and has no effect. To do a quick validation use `--only sw-cli` - `--check-against ` — `highest` (default) or `lowest`: which supported Shopware version to check against. - `--store-compliance` — enable Store-compliance mode while the current CLI supports the flag. Prefer `validation.store_compliance: true` in `.shopware-extension.yml` for persistent Store intent. - `--format ` — choose an output format supported by the current CLI (`--reporter` is a deprecated alias). From 5af72c438765252d3fb54451e8e0d6c7726a4f01 Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Fri, 25 Sep 2026 17:02:12 +0200 Subject: [PATCH 13/14] fix: smoke-test CI --- .github/workflows/smoke-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/smoke-test.yml b/.github/workflows/smoke-test.yml index 7695e69d..9389e530 100644 --- a/.github/workflows/smoke-test.yml +++ b/.github/workflows/smoke-test.yml @@ -55,7 +55,7 @@ jobs: run: shopware-cli extension package plugin --disable-git --release - name: Validate Plugin - run: shopware-cli extension validate FroshTools.zip + run: shopware-cli extension validate FroshTools.zip --only sw-cli - name: Get Changelog run: shopware-cli extension get-changelog FroshTools.zip From dab26c7b0c8363bde602a21425e380970d2e5b4e Mon Sep 17 00:00:00 2001 From: Malte Janz Date: Mon, 28 Sep 2026 14:33:12 +0200 Subject: [PATCH 14/14] fix: merge issues --- cmd/extension/extension_validate.go | 2 +- internal/validation/reporter_test.go | 130 --------------------------- 2 files changed, 1 insertion(+), 131 deletions(-) diff --git a/cmd/extension/extension_validate.go b/cmd/extension/extension_validate.go index 4c67db01..0a769a6e 100644 --- a/cmd/extension/extension_validate.go +++ b/cmd/extension/extension_validate.go @@ -124,7 +124,7 @@ var extensionValidateCmd = &cobra.Command{ } runErr := gr.Wait() - reportErr := validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, statuses...) + reportErr := validation.DoCheckReport(result.RemoveByIdentifier(toolCfg.ValidationIgnores), reportingFormat, runErr != nil, statuses...) if runErr != nil { return runErr } diff --git a/internal/validation/reporter_test.go b/internal/validation/reporter_test.go index 3f944afd..a4365487 100644 --- a/internal/validation/reporter_test.go +++ b/internal/validation/reporter_test.go @@ -324,136 +324,6 @@ func TestStructuredReportsKeepMachineOutputAndShowToolStatuses(t *testing.T) { } } -func TestToolInvocationReports(t *testing.T) { - check := &testCheck{Results: []CheckResult{}} - tools := []ToolInvocationStatus{ - {Name: "eslint", Status: "skipped", Reason: "no JavaScript source files"}, - {Name: "storefront-twig", Status: "skipped", Reason: "no storefront Twig templates"}, - } - rows := toolInvocationRows(tools) - assert.Contains(t, rows[0], "eslint") - assert.Contains(t, rows[0], " skipped no JavaScript source files") - assert.NotContains(t, rows[0], "(") - - summary := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "summary", tools...)) - }) - for _, row := range rows { - assert.Contains(t, summary, " "+row) - } - assert.Contains(t, summary, "Checkers:") - assert.True(t, strings.HasPrefix(summary, "\nCheckers:\n")) - assert.Less(t, strings.Index(summary, "Checkers:"), strings.Index(summary, "No checkers invoked;")) - assert.Contains(t, summary, "No checkers invoked; 0 problems reported") - assert.NotContains(t, summary, "No problems found") - - github := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "github", tools...)) - }) - for _, row := range rows { - assert.Contains(t, github, " "+row) - } - - markdown := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "markdown", tools...)) - }) - assert.Contains(t, markdown, "## Checkers") - assert.Less(t, strings.Index(markdown, "## Checkers"), strings.Index(markdown, "No checkers invoked;")) - for _, row := range rows { - assert.Contains(t, markdown, row) - } - assert.Contains(t, markdown, "No checkers invoked; 0 problems reported") - - jsonOutput := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "json", tools...)) - }) - var report struct { - Results []CheckResult `json:"results"` - Tools []ToolInvocationStatus `json:"tools"` - } - assert.NoError(t, json.Unmarshal([]byte(jsonOutput), &report)) - assert.Empty(t, report.Results) - assert.Equal(t, tools, report.Tools) - assert.NotContains(t, jsonOutput, `"checks"`) - - tools[0] = ToolInvocationStatus{Name: "eslint", Status: "invoked"} - summary = captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "summary", tools...)) - }) - assert.Contains(t, summary, "eslint") - assert.Contains(t, summary, " invoked") - assert.Contains(t, summary, "No problems found") -} - -func TestPrintToolInvocationTableWithOperationTitle(t *testing.T) { - var output strings.Builder - tools := []ToolInvocationStatus{ - {Name: "eslint", Status: "invoked"}, - {Name: "rector", Status: "skipped", Reason: "not selected by --only"}, - } - assert.NoError(t, PrintToolInvocationTable(&output, "Fixers", tools)) - assert.Equal(t, "\nFixers:\n eslint invoked\n rector skipped not selected by --only\n", output.String()) -} - -func TestToolInvocationTableFollowsFindings(t *testing.T) { - check := &testCheck{Results: []CheckResult{{Path: "src/file.php", Line: 1, Message: "problem", Severity: SeverityWarning}}} - tools := []ToolInvocationStatus{{Name: "sw-cli", Status: "invoked"}} - - summary := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "summary", tools...)) - }) - assert.Less(t, strings.Index(summary, "src/file.php"), strings.Index(summary, "Checkers:")) - assert.Less(t, strings.Index(summary, "Checkers:"), strings.Index(summary, "✖ 1 problem")) - assert.Contains(t, summary, "\n\nCheckers:\n") - assert.NotContains(t, summary, "\n\n\nCheckers:\n") - - markdown := captureOutput(func() { - assert.NoError(t, DoCheckReport(check, "markdown", tools...)) - }) - assert.Less(t, strings.Index(markdown, "## src/file.php"), strings.Index(markdown, "## Checkers")) -} - -func TestStructuredReportsKeepMachineOutputAndShowToolStatuses(t *testing.T) { - check := &testCheck{Results: []CheckResult{}} - tools := []ToolInvocationStatus{ - {Name: "phpstan", Status: "invoked"}, - {Name: "sw-cli", Status: "skipped", Reason: "not selected by --only"}, - } - - var gitlabLog string - gitlab := captureOutput(func() { - gitlabLog = captureStderr(func() { - assert.NoError(t, DoCheckReport(check, "gitlab", tools...)) - }) - }) - var issues []GitLabCodeQualityIssue - assert.NoError(t, json.Unmarshal([]byte(gitlab), &issues)) - assert.Empty(t, issues) - for _, row := range toolInvocationRows(tools) { - assert.Contains(t, gitlabLog, " "+row) - } - - var junitLog string - junit := captureOutput(func() { - junitLog = captureStderr(func() { - assert.NoError(t, DoCheckReport(check, "junit", tools...)) - }) - }) - var suite JUnitTestSuite - require.NoError(t, xml.Unmarshal([]byte(junit), &suite)) - assert.Equal(t, 2, suite.Tests) - assert.Equal(t, 1, suite.Skipped) - require.Len(t, suite.TestCase, 2) - assert.Equal(t, "phpstan", suite.TestCase[0].Name) - assert.Equal(t, "tool", suite.TestCase[0].ClassName) - assert.Nil(t, suite.TestCase[0].Skipped) - require.NotNil(t, suite.TestCase[1].Skipped) - assert.Equal(t, "not selected by --only", suite.TestCase[1].Skipped.Message) - for _, row := range toolInvocationRows(tools) { - assert.Contains(t, junitLog, " "+row) - } -} - func TestValidateReporter(t *testing.T) { for _, format := range []string{"summary", "json", "github", "gitlab", "junit", "markdown"} { assert.NoError(t, ValidateReporter(format))