diff --git a/README.md b/README.md index daed6ded..3d94686f 100644 --- a/README.md +++ b/README.md @@ -311,6 +311,7 @@ Run "mage gen:readme" to regenerate this section. | Plugin Name formatting / `pluginname` | Validates the plugin ID used conforms to our naming convention. | None | | Provenance attestation validation / `provenance` | Validates the provenance attestation if the plugin was built with a pipeline supporting provenance attestation (e.g Github Actions). | None | | Published / `published-plugin` | Detects whether any version of this plugin exists in the Grafana plugin catalog currently. | None | +| React 19 Compatibility / `reactcompat` | Detects usage of React APIs removed or deprecated in React 19 using @grafana/react-detect. | None | | Readme (exists) / `readme` | Ensures a `README.md` file exists within the zip file. | None | | Restrictive Dependency / `restrictivedep` | Specifies a valid range of Grafana versions that work with this version of the plugin. | None | | Safe Links / `safelinks` | Checks that links from `plugin.json` are safe. | None | diff --git a/pkg/analysis/passes/analysis.go b/pkg/analysis/passes/analysis.go index 874354cb..117843b9 100644 --- a/pkg/analysis/passes/analysis.go +++ b/pkg/analysis/passes/analysis.go @@ -38,6 +38,7 @@ import ( "github.com/grafana/plugin-validator/pkg/analysis/passes/pluginname" "github.com/grafana/plugin-validator/pkg/analysis/passes/provenance" "github.com/grafana/plugin-validator/pkg/analysis/passes/published" + "github.com/grafana/plugin-validator/pkg/analysis/passes/reactcompat" "github.com/grafana/plugin-validator/pkg/analysis/passes/readme" "github.com/grafana/plugin-validator/pkg/analysis/passes/restrictivedep" "github.com/grafana/plugin-validator/pkg/analysis/passes/safelinks" @@ -90,6 +91,7 @@ var Analyzers = []*analysis.Analyzer{ pluginname.Analyzer, provenance.Analyzer, published.Analyzer, + reactcompat.Analyzer, readme.Analyzer, restrictivedep.Analyzer, screenshots.Analyzer, diff --git a/pkg/analysis/passes/reactcompat/reactcompat.go b/pkg/analysis/passes/reactcompat/reactcompat.go new file mode 100644 index 00000000..74fcb2b3 --- /dev/null +++ b/pkg/analysis/passes/reactcompat/reactcompat.go @@ -0,0 +1,230 @@ +package reactcompat + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "time" + + "github.com/grafana/plugin-validator/pkg/analysis" + "github.com/grafana/plugin-validator/pkg/analysis/passes/archive" + "github.com/grafana/plugin-validator/pkg/logme" +) + +var ( + react19Issue = &analysis.Rule{ + Name: "react-19-issue", + Severity: analysis.Warning, + } + react19Compatible = &analysis.Rule{ + Name: "react-19-compatible", + Severity: analysis.OK, + } +) + +// Analyzer checks for React 19 compatibility issues in the plugin bundle by +// delegating to npx @grafana/react-detect. It silently skips if npx is not +// available in PATH. +var Analyzer = &analysis.Analyzer{ + Name: "reactcompat", + Requires: []*analysis.Analyzer{archive.Analyzer}, + Run: run, + Rules: []*analysis.Rule{react19Issue, react19Compatible}, + ReadmeInfo: analysis.ReadmeInfo{ + Name: "React 19 Compatibility", + Description: "Detects usage of React APIs removed or deprecated in React 19 using @grafana/react-detect.", + }, +} + +// reactDetectOutput is the top-level JSON structure emitted by @grafana/react-detect. +type reactDetectOutput struct { + SourceCodeIssues map[string][]sourceCodeIssue `json:"sourceCodeIssues"` + DependencyIssues []dependencyIssue `json:"dependencyIssues"` +} + +type sourceCodeIssue struct { + Pattern string `json:"pattern"` + Severity string `json:"severity"` + Location location `json:"location"` + Problem string `json:"problem"` + Fix fix `json:"fix"` + Link string `json:"link"` +} + +type location struct { + File string `json:"file"` + Line int `json:"line"` + Column int `json:"column"` +} + +type fix struct { + Description string `json:"description"` +} + +type dependencyIssue struct { + Pattern string `json:"pattern"` + Severity string `json:"severity"` + Problem string `json:"problem"` + Link string `json:"link"` + PackageNames []string `json:"packageNames"` +} + +func run(pass *analysis.Pass) (any, error) { + archiveDir, ok := pass.ResultOf[archive.Analyzer].(string) + if !ok || archiveDir == "" { + return nil, nil + } + + npxPath, err := exec.LookPath("npx") + if err != nil { + logme.DebugFln("npx not found in PATH, skipping react-detect") + return nil, nil + } + logme.DebugFln("npx path: %s", npxPath) + + tmpDir, cleanup, err := prepareTmpDir(archiveDir) + if err != nil { + logme.DebugFln("failed to prepare temp dir for react-detect: %v", err) + return nil, nil + } + defer cleanup() + + output, err := runReactDetect(npxPath, tmpDir) + if err != nil { + logme.DebugFln("react-detect failed: %v", err) + return nil, nil + } + + issueCount := reportIssues(pass, output) + + if issueCount == 0 && react19Compatible.ReportAll { + pass.ReportResult( + pass.AnalyzerName, + react19Compatible, + "Plugin is compatible with React 19", + "No React 19 compatibility issues were detected.", + ) + } + + return nil, nil +} + +// prepareTmpDir creates a temporary directory with a dist/ symlink pointing at +// archiveDir. The extracted archive has files at the root (module.js, plugin.json, +// etc.) but react-detect expects them under a dist/ subdirectory. +// The returned cleanup function removes the temp directory. +func prepareTmpDir(archiveDir string) (string, func(), error) { + tmpDir, err := os.MkdirTemp("", "reactcompat-*") + if err != nil { + return "", nil, fmt.Errorf("create temp dir: %w", err) + } + + cleanup := func() { os.RemoveAll(tmpDir) } + + distLink := filepath.Join(tmpDir, "dist") + if err := os.Symlink(archiveDir, distLink); err != nil { + cleanup() + return "", nil, fmt.Errorf("create dist symlink: %w", err) + } + + return tmpDir, cleanup, nil +} + +// runReactDetect shells out to react-detect and returns the parsed output. +func runReactDetect(npxPath, pluginRoot string) (*reactDetectOutput, error) { + ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) + defer cancel() + + // --json: machine-readable output. --skipBuildTooling: avoid running bundlers. + // --noErrorExitCode: always exit 0 so we can parse partial output on warnings. + // Dependency issues are intentionally included (no --skipDependencies). + args := []string{ + "-y", + "@grafana/react-detect@latest", + "--json", + "--pluginRoot", pluginRoot, + "--skipBuildTooling", + "--noErrorExitCode", + } + logme.DebugFln("running react-detect with args: %v", args) + + cmd := exec.CommandContext(ctx, npxPath, args...) + var stderr bytes.Buffer + cmd.Stderr = &stderr + out, err := cmd.Output() + if err != nil { + return nil, fmt.Errorf("react-detect exited with error: %w (stderr: %s)", err, stderr.String()) + } + + return parseResults(out) +} + +// parseResults unmarshals the raw JSON bytes from react-detect. +func parseResults(data []byte) (*reactDetectOutput, error) { + var output reactDetectOutput + if err := json.Unmarshal(data, &output); err != nil { + return nil, fmt.Errorf("parse react-detect output: %w", err) + } + return &output, nil +} + +// reportIssues translates the react-detect output into pass diagnostics and +// returns the total number of issues reported. +func reportIssues(pass *analysis.Pass, output *reactDetectOutput) int { + // react19Issue serves as the config gate for all dynamic react-19 rules. + if react19Issue.Disabled { + return 0 + } + + if output == nil { + return 0 + } + + count := 0 + + patterns := make([]string, 0, len(output.SourceCodeIssues)) + for p := range output.SourceCodeIssues { + patterns = append(patterns, p) + } + slices.Sort(patterns) + + for _, pattern := range patterns { + for _, issue := range output.SourceCodeIssues[pattern] { + rule := &analysis.Rule{ + Name: fmt.Sprintf("react-19-%s", issue.Pattern), + Severity: analysis.Warning, + } + detail := fmt.Sprintf( + "Detected in %s at line %d. %s See: %s Note: this may be a false positive.", + issue.Location.File, + issue.Location.Line, + issue.Fix.Description, + issue.Link, + ) + pass.ReportResult(pass.AnalyzerName, rule, issue.Problem, detail) + count++ + } + } + + for _, issue := range output.DependencyIssues { + rule := &analysis.Rule{ + Name: fmt.Sprintf("react-19-dep-%s", issue.Pattern), + Severity: analysis.Warning, + } + detail := fmt.Sprintf( + "Affected packages: %s. See: %s Note: this may be a false positive.", + strings.Join(issue.PackageNames, ", "), + issue.Link, + ) + pass.ReportResult(pass.AnalyzerName, rule, issue.Problem, detail) + count++ + } + + return count +} diff --git a/pkg/analysis/passes/reactcompat/reactcompat_test.go b/pkg/analysis/passes/reactcompat/reactcompat_test.go new file mode 100644 index 00000000..280190a6 --- /dev/null +++ b/pkg/analysis/passes/reactcompat/reactcompat_test.go @@ -0,0 +1,274 @@ +package reactcompat + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/plugin-validator/pkg/analysis" + "github.com/grafana/plugin-validator/pkg/analysis/passes/archive" + "github.com/grafana/plugin-validator/pkg/testpassinterceptor" +) + +func newPass(interceptor *testpassinterceptor.TestPassInterceptor, archiveDir string) *analysis.Pass { + return &analysis.Pass{ + AnalyzerName: "reactcompat", + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]interface{}{ + archive.Analyzer: archiveDir, + }, + Report: interceptor.ReportInterceptor(), + } +} + +// TestParseResults verifies that a valid JSON payload is correctly decoded and +// mapped to the expected diagnostics. +func TestParseResults(t *testing.T) { + jsonPayload := []byte(`{ + "sourceCodeIssues": { + "usePropTypes": [ + { + "pattern": "usePropTypes", + "severity": "critical", + "location": {"type": "source-map", "file": "module.js", "line": 42, "column": 10}, + "problem": "Uses deprecated propTypes", + "fix": {"description": "Remove propTypes usage."}, + "link": "https://react.dev/blog/2024/04/25/react-19-upgrade-guide" + } + ] + }, + "dependencyIssues": [ + { + "pattern": "oldReactDom", + "severity": "critical", + "problem": "Depends on old react-dom", + "link": "https://example.com", + "packageNames": ["react-dom", "react"] + } + ] + }`) + + output, err := parseResults(jsonPayload) + require.NoError(t, err) + require.Len(t, output.SourceCodeIssues, 1) + require.Len(t, output.SourceCodeIssues["usePropTypes"], 1) + require.Len(t, output.DependencyIssues, 1) + + sc := output.SourceCodeIssues["usePropTypes"][0] + require.Equal(t, "usePropTypes", sc.Pattern) + require.Equal(t, "module.js", sc.Location.File) + require.Equal(t, 42, sc.Location.Line) + require.Equal(t, "Uses deprecated propTypes", sc.Problem) + require.Equal(t, "Remove propTypes usage.", sc.Fix.Description) + require.Equal(t, "https://react.dev/blog/2024/04/25/react-19-upgrade-guide", sc.Link) + + dep := output.DependencyIssues[0] + require.Equal(t, "oldReactDom", dep.Pattern) + require.Equal(t, []string{"react-dom", "react"}, dep.PackageNames) +} + +// TestParseResultsEmpty verifies that a payload with no issues produces an +// empty but non-nil result. +func TestParseResultsEmpty(t *testing.T) { + jsonPayload := []byte(`{"sourceCodeIssues": {}, "dependencyIssues": []}`) + + output, err := parseResults(jsonPayload) + require.NoError(t, err) + require.NotNil(t, output) + require.Len(t, output.SourceCodeIssues, 0) + require.Len(t, output.DependencyIssues, 0) +} + +// TestParseResultsMalformed verifies that garbage input returns an error rather +// than a panic or silent zero value. +func TestParseResultsMalformed(t *testing.T) { + _, err := parseResults([]byte(`not valid json {{{`)) + require.Error(t, err) +} + +// TestReportIssuesSourceCode verifies correct diagnostic generation for source +// code issues. +func TestReportIssuesSourceCode(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := newPass(&interceptor, "/some/archive/dir") + + output := &reactDetectOutput{ + SourceCodeIssues: map[string][]sourceCodeIssue{ + "usePropTypes": { + { + Pattern: "usePropTypes", + Severity: "critical", + Location: location{File: "module.js", Line: 10, Column: 5}, + Problem: "Uses deprecated propTypes", + Fix: fix{Description: "Remove propTypes."}, + Link: "https://react.dev/upgrade", + }, + }, + }, + } + + count := reportIssues(pass, output) + require.Equal(t, 1, count) + require.Len(t, interceptor.Diagnostics, 1) + + d := interceptor.Diagnostics[0] + require.Equal(t, "react-19-usePropTypes", d.Name) + require.Equal(t, analysis.Warning, d.Severity) + require.Equal(t, "Uses deprecated propTypes", d.Title) + require.Contains(t, d.Detail, "module.js") + require.Contains(t, d.Detail, "10") + require.Contains(t, d.Detail, "Remove propTypes.") + require.Contains(t, d.Detail, "https://react.dev/upgrade") + require.Contains(t, d.Detail, "this may be a false positive") +} + +// TestReportIssuesDependency verifies correct diagnostic generation for +// dependency issues. +func TestReportIssuesDependency(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := newPass(&interceptor, "/some/archive/dir") + + output := &reactDetectOutput{ + DependencyIssues: []dependencyIssue{ + { + Pattern: "oldReactDom", + Severity: "critical", + Problem: "Depends on old react-dom", + Link: "https://example.com/fix", + PackageNames: []string{"react-dom", "react"}, + }, + }, + } + + count := reportIssues(pass, output) + require.Equal(t, 1, count) + require.Len(t, interceptor.Diagnostics, 1) + + d := interceptor.Diagnostics[0] + require.Equal(t, "react-19-dep-oldReactDom", d.Name) + require.Equal(t, analysis.Warning, d.Severity) + require.Equal(t, "Depends on old react-dom", d.Title) + require.Contains(t, d.Detail, "react-dom, react") + require.Contains(t, d.Detail, "https://example.com/fix") + require.Contains(t, d.Detail, "this may be a false positive") +} + +// TestReportIssuesNil verifies that a nil output produces no diagnostics. +func TestReportIssuesNil(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := newPass(&interceptor, "/some/archive/dir") + + count := reportIssues(pass, nil) + require.Equal(t, 0, count) + require.Len(t, interceptor.Diagnostics, 0) +} + +// TestPrepareTmpDir verifies that prepareTmpDir creates the expected symlink +// and that the cleanup function removes the directory. +func TestPrepareTmpDir(t *testing.T) { + archiveDir := t.TempDir() + + tmpDir, cleanup, err := prepareTmpDir(archiveDir) + require.NoError(t, err) + require.NotEmpty(t, tmpDir) + + distLink := filepath.Join(tmpDir, "dist") + target, err := os.Readlink(distLink) + require.NoError(t, err) + require.Equal(t, archiveDir, target) + + cleanup() + + _, statErr := os.Stat(tmpDir) + require.True(t, os.IsNotExist(statErr), "temp dir should be removed after cleanup") +} + +// TestNpxNotAvailable verifies that the analyzer silently skips (nil, nil) when +// npx is not found in PATH, producing no diagnostics. +func TestNpxNotAvailable(t *testing.T) { + if _, err := exec.LookPath("npx"); err == nil { + t.Skip("npx is available in this environment; skipping npx-not-found test") + } + + archiveDir := t.TempDir() + var interceptor testpassinterceptor.TestPassInterceptor + pass := newPass(&interceptor, archiveDir) + + result, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Nil(t, result) + require.Len(t, interceptor.Diagnostics, 0) +} + +// TestReportIssuesCombined verifies that multiple source code issue groups and a +// dependency issue are all counted and reported correctly in a single call. +func TestReportIssuesCombined(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := newPass(&interceptor, "/some/archive/dir") + + output := &reactDetectOutput{ + SourceCodeIssues: map[string][]sourceCodeIssue{ + "usePropTypes": { + { + Pattern: "usePropTypes", + Severity: "critical", + Location: location{File: "module.js", Line: 10, Column: 5}, + Problem: "Uses deprecated propTypes", + Fix: fix{Description: "Remove propTypes."}, + Link: "https://react.dev/upgrade", + }, + }, + "findDOMNode": { + { + Pattern: "findDOMNode", + Severity: "critical", + Location: location{File: "other.js", Line: 20, Column: 3}, + Problem: "Uses removed findDOMNode", + Fix: fix{Description: "Use a ref instead."}, + Link: "https://react.dev/upgrade#finddomnode", + }, + }, + }, + DependencyIssues: []dependencyIssue{ + { + Pattern: "oldReactDom", + Severity: "critical", + Problem: "Depends on old react-dom", + Link: "https://example.com/fix", + PackageNames: []string{"react-dom"}, + }, + }, + } + + count := reportIssues(pass, output) + require.Equal(t, 3, count) + require.Len(t, interceptor.Diagnostics, 3) + + ruleNames := make([]string, 0, 3) + for _, d := range interceptor.Diagnostics { + ruleNames = append(ruleNames, d.Name) + } + require.Contains(t, ruleNames, "react-19-usePropTypes") + require.Contains(t, ruleNames, "react-19-findDOMNode") + require.Contains(t, ruleNames, "react-19-dep-oldReactDom") +} + +// TestNoArchiveDir verifies that a missing archive result produces no diagnostics. +func TestNoArchiveDir(t *testing.T) { + var interceptor testpassinterceptor.TestPassInterceptor + pass := &analysis.Pass{ + RootDir: filepath.Join("./"), + ResultOf: map[*analysis.Analyzer]interface{}{ + archive.Analyzer: nil, + }, + Report: interceptor.ReportInterceptor(), + } + + result, err := Analyzer.Run(pass) + require.NoError(t, err) + require.Nil(t, result) + require.Len(t, interceptor.Diagnostics, 0) +}